The VM should only assume it should/will crash if it triggers an
e.g. EXCEPTION_ACCESS_VIOLATION itself. In other words, if some
non-VM code triggers such an event, the VM should let that code handle
it as appropriate. Avian already does this properly, but it also
generates crash dumps for each event encountered if avian.crash.dir is
set, regardless if whether the VM triggered it or not. This can be a
problem if such events are generated frequently -- a full memory dump
takes a long time to write and a lot of disk space.
Ideally, we'd have some reliable way to determine whether the faulting
instruction pointer belongs to the VM or not, but the closest thing I
could come up with was to compare the module that owns the address
with the module representing the main executable. This can lead to
both false positives (e.g. application code in the main executable
which is not part of the VM) and false negatives (e.g. when the VM is
loaded from a library instead of the main executable), but it's still
an improvement over the status quo.
Even better would be to ensure that the VM's exception handler is the
very last one to run, so that we know for sure that the error is
unrecoverable and thus it is appropriate to do a full memory dump.
If an JNI_OnLoad implementation calls FindClass when using the OpenJDK
class library, the calling method on the Java stack will be
ClassLoader.loadLibrary. However, we must use the class loader of the
class attempting to load the library in this case, not the system
classloader.
Therefore, we now maintain a stack such that the latest class to load
a library in the current thread is at the top, and we use that class
whenever FindClass is called by ClassLoader.loadLibrary (via
JNI_OnLoad).
Note that this patch does not attempt to address the same problem for
the Avian or Android class libraries, but the same strategy should
work for them as well.
There's no need to enter IdleState (and incur synchronization
overhead) unless another thread is waiting to enter ExclusiveState.
This change improves the performance of the MemoryRamp test by a
factor of about 100.
The problem (which we've only been able to reproduce consistently with
the openjdk-src process=interpret build on Linux virtual machines) was
a race condition during VM shutdown. Thread "A" would exit, see there
were other threads still running and thus enter ZombieState, which
involves acquiring and releasing a lock using RawMonitorResource.
Then the last thread (thread "B") would exit, wait for thread "A" to
release the lock, then shut down the VM, freeing all memory. However,
thread "A" writes to its Thread object one last time after releasing
the lock (in ~Resource, the destructor of the superclass of
RawMonitorResource, which sets Thread::resource). If thread "B" frees
that Thread before ~Resource runs, we end up writing to freed memory.
Thus, we need to update Thread::resource before releasing the lock.
Apparently C++ destructors run in order from most derived to least
derived, which is not what we want here. My solution to split
Resource into two classes, one that has no destructor and another that
extends it (called AutoResource) which does hafe a destructor. Now
all the classes which used to extend Resource extend AutoResource,
except for RawMonitorResource, which extends Resource directly so it
can control the order of operations.
This allows them to be shared between the Avian and Android class
library builds.
This commit also disables the URL test in Misc.java on Android, since
it's known to fail, and we still want to know whether the other tests
pass.
This was causing crashes on 32-bit OS X continuations=true builds.
There were two important differences between vmInvoke and
vmJumpAndInvoke: (1) vmInvoke expects its stack to be aligned on
entry, modulo the return address whereas the stack argument to
vmJumpAndInvoke is aligned without allowing for the return address,
and (2) vmInvoke pushes EBP before doing its frame allocation, whereas
vmJumpAndInvoke did not take that into account. So in order for
vmJumpAndInvoke to allocate the exact same frame size that vmInvoke
would have when calling the same method, it needed to add an extra two
words beyond what it was already allocating.
Aside from alignment concerns, the code is not particularly sensitive
to vmJumpAndInvoke allocating a different frame size than vmInvoke,
since we store the frame pointer in a "thread local" variable:
// remember this stack position, since we won't be able to rely on
// %rbp being restored when the call returns
movl 8(%ebp),%eax
movl %esp,TARGET_THREAD_SCRATCH(%eax)
...
GLOBAL(vmInvoke_returnAddress):
// restore stack pointer
movl TARGET_THREAD_SCRATCH(%ebx),%esp
My original patch makes an equivalent change for the 64-bit changes,
but I'll leave that for after we release 1.0 since we're in
bugfix-only mode right now
This was a bug, wherein upon throwing an exception, we would try to
allocate memory for the message - all while holding a critical
reference to the jbyteArray representing the exception string. This
caused an expect to fail in allocate3.
This ensures that all tests pass when Avian is built with an
openjdk=$path option such that $path points to either OpenJDK 7 or 8.
Note that I have not yet tried using the openjdk-src option with
OpenJDK 8. I'll work on that next.
617bd85 broke the Android build by creating an unresolvable
order-of-operations bug in classpath-android.cpp's
MyClasspath::preBoot method.
The problem is that, while JNIEnv::FindClass is supposed to initialize
the class that it finds, this causes JniConstants::init to indirectly
invoke native methods which are not registered until JNI_OnLoad is
called (which happens after JniConstants::init is called). However,
if we call JNI_OnLoad first, that causes methods to be invoked which
rely on JniConstants::init having already been run.
I haven't checked to see how Dalvik handles this, but I don't see any
way around the problem besides disabling initialization by
JNIEnv::FindClass until the preBoot phase is complete. Moreover, it's
dangerous to allow Java code to be invoked so early anyway, since the
VM is not yet fully initialized.
For some reason, running Avian under the SVN version of Valgrind
caused mmap to fail, which caused tryAllocateExecutable to return a
null pointer, which led to a non-obvious crash later on. Adding an
expect to check the result immediately will at least make it obvious
what went wrong.
bb86500 was a step in the right direction, but there was a bug that
caused Type_pad fields to be inserted between every other field in for
a derived class when type-maps.cpp was generated, and this led to
miscompilation of e.g. Android's
java.lang.reflect.Constructor.getModifiers.
We should define EXPORT to be __declspec(dllexport) on Windows
regardless of architecture, not just non-x86_64 arches. This fixes
errors to to embedded JAVA_HOME files not being found in openjdk-src
builds, e.g. lib/currency.data.
Although the JNI reference documentation does not mention it,
FindClass should initialize the class before it returns it. That's
what HotSpot does, and that's what we have to do too.
In particular, OpenJDK's
Java_java_net_Inet6AddressImpl_lookupAllHostAddr relies on
Inet6Address's static initializer being run when it is resolved using
FindClass, or else it will crash.
* Unsafe.arrayIndexScale was always returning the native word size,
due to a thinko on my part
* Unsafe.getLongVolatile and putLongVolatile did not work for array
elements on 32-bit systems
Turns out Function can do the jobs of both CallbackReceiver and
FunctionReceiver, so I've removed the latter two.
Also, shift and reset should work with a combination of types, not
just a single type, so I've expanded their generic signatures.
There's more work to do to derive all the properties of a given class
from its code source (e.g. JAR file), but this at least ensures that
ClassLoader.getPackage will actually return something non-null when
appropriate.
classpath-common.h's getDeclaringClass was trying to look up
non-existing classes, which led to an abort, and I don't even know
what Class.getDeclaredClasses was trying to do, but it was ugly and
wrong.
The various Architecture::nextFrame implementations were not walking
the stack correctly when a StackOverflowError was thrown. The
throwStackOverflow thunk is called before the frame of the most
recently called method has been fully created, and because tails=true
builds use a different calling convention, we need to treat this
situation carefully when building a stack trace or unwinding.
Otherwise, we will skip past all the java frames to the next native
frame, which is what was happening.
Tail call and dead code optimizations can cause code after a throw to
be eliminated, which confuses findUnwindTarget because it doesn't know
what code is throwing the exception. So we need at least one
instruction to follow the call to the throw_ thunk. Previously, we
only added such an instruction when we knew the throw was the last
instruction in the bytecode, but it turns out there are other cases
where it is needed, including certain try/finally situations.
This is the simplest possible ConcurrentHashMap I could come up with
that works and is actually concurrent in the way one would expect.
It's pretty unconventional, being based on a persistent red-black
tree, and not particularly memory-efficient or cache-friendly. I
think this is a good place to start, though, and it should perform
reasonably well for most workloads. Patches for a more efficient
implementation are welcome!
I also implemented AtomicReferenceArray, since I was using it in my
first, naive attempt to implement ConcurrentHashMap.
I had to do a bit of refactoring, including moving some non-standard
stuff from java.util.Collections to avian.Data so I could make it
available to code outside the java.util package, which is why I had to
modify several unrelated files.
getDeclaredMethods was returning methods which were inherited from
interfaces but not (re)declared in the class itself, due to the VM's
internal use of VMClass.methodTable differing from its role in
reflection. For reflection, we must only include the declared
methods, not the inherited but un-redeclared ones.
Previously, we saved the original method table in
ClassAddendum.methodTable before creating a new one which contains
both declared and inherited methods. That wasted space, so this patch
replaces ClassAddendum.methodTable with
ClassAddendum.declaredMethodCount, which specifies how many of the
methods in VMClass.methodTable were declared in that class.
Alternatively, we could ensure that undeclared methods always have
their VMMethod.class_ field set to the declaring class instead of the
inheriting class. I tried this, but it led to subtle crashes in
interface method lookup. The rest of the VM relies not only on
VMClass.methodTable containing all inherited interface methods but
also that those methods point to the inheriting class, not the
declaring class. Changing those assumptions would be a much bigger
(and more dangerous in terms of regression potential) effort than I
care to take on right now. The solution I chose is a bit ugly, but
it's safe.
An inner class has two sets of modifier flags: one is declared in the
usual place in the class file and the other is part of the
InnerClasses attribute. Not only is that redundant, but they can
contradict, and the VM can't just pick one and roll with it. Instead,
Class.getModifiers must return the InnerClasses version, whereas
reflection must check the top-level version. So even if
Class.getModifiers says the class is protected, it might still be
public for the purpose of reflection depending on what the
InnerClasses attribute says. Crazy? Yes.
This makes them available in all class libraries, not just the OpenJDK
library. Note that I've also removed the unecessary idle statements,
per ab4adef.
Android's class library uses this to find out whether the VM supports
compareAndSwapLong natively. Avian does on all platforms, so we just
return true.
There's a small optimization in compileDirectInvoke which tries to
avoid generating calls to empty methods. However, this causes
problems for code which uses such a call to ensure a class is
initialized -- if we omit that call, the class may not be
initialized and any side effects of that initialization may not
happen when the program expects them to.
This commit ensures that the compiler only omits empty method calls
when the target class does not need initialization. It also removes
commented-out code in classpath-openjdk.cpp which was responsible for
loading libmawt proactively; that was a hack to get JogAmp to work
before we understood what the real problem was.
Previously, we used "lzma:", which worked fine on Windows (where the
path separator is ";") but not on Unix-style OSes (where the path
separator is ":"). In the latter case, the VM would parse
"[lzma:bootJar]" as a path containing two elements, "[lzma" and
"bootJar]", which is not what was intended. So now we use "lzma." as
the prefix, which works on all OSes.
armv7 and later provide weaker cache coherency models than armv6 and
earlier, so we cannot just implement memory barriers as no-ops. This
patch uses the DMB instruction (or the equivalent OS-provided barrier
function) to implement barriers. This should fix concurrency issues
on newer chips such as the Apple A6 and A7.
If you still need to support ARMv6 devices, you should pass
"armv6=true" to make when building Avian. Ideally, the VM would
detect what kind of CPU it was executing on at runtime and direct the
JIT compiler accordingly, but I don't know how to do that on ARM.
Patches are welcome, though!
When calculating field offsets in the bootimage generator, we failed
to consider alignment at inheritence boundaries (i.e. the last field
inherited by from a superclass should be followed by enough padding to
align the first non-inherited field at a machine word boundary). This
led to a mismatch between native code and Java code in terms of class
layouts, including that of java.lang.reflect.Method.
The former just defers to the latter for now, since it provides
strictly weaker guarantees. Thus it's correct to use full
volatile-style barriers, though not as efficient as it could be on
some architectures.
Clang was complaining that newIp might be used uninitialized at the
bottom of our giant, unstructured compile loop, so I initialized it
with a bogus value, which means it will at least fail consistently if
Clang is right and there really is a path by which that code is
reached without otherwise initializing newIp.
It looks like the iOS 7 SDK doesn't have GCC anymore, so we need to
use clang instead. Also, thread_act.h and thread_status.h have moved,
so I updated arm.h accordingly. That might break the build for older
SDKs, but I don't have one available at the moment. If it does break,
I'll fix it.
Unsafe.compareAndSwapLong was moved from classpath-openjdk.cpp to
builtin.cpp, but the fieldForOffset helper function was not, which
only caused problems when I tried to build for ARM. This commit moves
said helper function, along with Unsafe.getVolatileLong, which also
uses it.
Most of these regressions were simply due to testing a lot more stuff,
esp. annotations and reflection, revealing holes in the Android
compatibility code. There are still some holes, but at least the
suite is passing (except for a fragile test in Serialize.java which I
will open an issue for).
Sorry this is such a big commit; there was more to address than I
initially expected.
Method.invoke should initialize its class before invoking the method,
throwing an ExceptionInInitializerError if it fails, without wrapping
said error in an InvocationTargetException.
Also, we must initialize ExceptionInInitializerError.exception when
throwing instances from the VM, since OpenJDK's
ExceptionInInitializerError.getCause uses the exception field, not the
cause field.
This is by no means a complete support for the deserialization compliant
to the Java Language Specification, but it is better to add the support
incrementally, for better readability of the commits.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Previously, we initialized it to the boot class loader, but that is
inconsistent with Java; if compiling against OpenJDK's class library,
the context class loader is therefore initialized to the app class
loader, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
