On big endian architecture IntegerType based values are incorrectly decoded #1104
Conversation
matthiasblaesing
force-pushed
the
direct-invoke-integer-big-endian
branch
2 times, most recently
from
8564e24
to
6509b73
Compare
| @@ -32,31 +32,31 @@ | |||
| * and result in deterministic behaviour and not in a JVM crash. | |||
| */ | |||
| public class ArgumentsMarshalNullableTest extends TestCase { | |||
| public static class Int32Integer extends IntegerType { | |||
| public static class Int16Integer extends IntegerType { | |||
There was a problem hiding this comment.
Did you mean to entirely replace the 32-bit tests with 16-bit tests? Unless you have a compelling reason to remove them, the 32-bit tests should remain in place.
There was a problem hiding this comment.
Yes this was intentional. Some background: I added the test when I implemented nullable IntegerTypes in the direct invoke path in 1a2a94f. I used the mapped function because they used the fixed size int32_t. Reduzing the size should expose more systems, that show problematic behavior. If sizeof(targettype) >= sizeof(ffi_arg) we will see no problem, so reducing the size of the argument makes it more likely to hit the problematic path. I see no benefit keeping both variants.
matthiasblaesing
force-pushed
the
direct-invoke-integer-big-endian
branch
from
6509b73
to
64bafab
Compare
|
Do you know why s390x is unable to do the Crash Protection test? Is it something I can help with? |
matthiasblaesing
force-pushed
the
direct-invoke-integer-big-endian
branch
4 times, most recently
from
6221435
to
689273c
Compare
…ecoded in the direct invocation path For IntegerType base values, the member number is read by a direct field read from C. This results in a jlong. If the target site expects a datatype smaller than a jlong only parts of that value are read. On little endian architectures the "correct" part of the long value is read, but on big endian architectures, unused high bytes are read instead of the required low bytes. This can be worked around by bitshifting the value by the size difference between the target type and the real type.
matthiasblaesing
force-pushed
the
direct-invoke-integer-big-endian
branch
from
689273c
to
21da80a
Compare
|
The crash protection on non-windows systems is not reliable. If I'm asked, I strongly advise not to use the crash protection, as the non-windows systems depend on signal handling and signals are not reliably delivered into the right thread and if I remember correctly I have seen strange behavior with it in place. For s390x it seems not so much a problem of the architecture, but an implementation detail. I normally use a JVM with hotspot, but on your installation it is OpenJ9 that is used as the VM and it seems J9 handles the signals itself: At that point, the testsuite stops. |
|
Thanks. I may have a word to the J9 developers to let them know.
I had skipped this test myself when I was building it for use with Cassandra.
|
|
I don't think this is a bug in J9, in fact the output looks reasonable, it is just a different behavior than Hotspot. Given that signal handling is not part of the platform differences are ok. Raising an exception is a good way in fact to report a segfault. |
No description provided.