SIGFPE for flattened array of value type that has no fields - #17994
Conversation
There was a problem hiding this comment.
please:
- add extra brackets to not rely on order of operations
- add space after "if"
There was a problem hiding this comment.
please add extra brackets around (size / stride) == numberOfElements
There was a problem hiding this comment.
please create another variable for J9ARRAYCLASS_GET_STRIDE(arrayClass) and use it instead of calling macro twice
There was a problem hiding this comment.
please add spaces after "if" and before "{"
There was a problem hiding this comment.
We need comment before this assertion explained (_dataSize == 0) for InlineContiguous is possible for 0-stride elements of Flattened Array
There was a problem hiding this comment.
also please put constant first (0 == _dataSize)
hangshao0
added
project:valhalla
There was a problem hiding this comment.
@TobiAjila This function is used for Primitive arrays only and supports element sizes 1, 2, 4, 8 bytes. Can Flattened array be declared Primitive and have different element size (except 0-stride we are adding now)? If so, what size of chunk we should use to copy? We can postpone the implementation and add it in different PR. Currently this function triggers assertion if size of the element is not supported.
There was a problem hiding this comment.
If a flattened array is declared as primitive it will only have size 1, 2, 4 or 8.
There was a problem hiding this comment.
Long term, it would be nice if we can deal with zero payload arrays. Where we have a header and no body.
There was a problem hiding this comment.
I think this change is not necessary due we don't support any 32-bit JVM for Java Next. However code is here and it is reasonable to fix it. So, keep this change.
|
@kangyining Please update comment and squash commits (except you want to add something else to the change). |
Update gc code to support 0-stride flattened array and get rid of division by 0 exceptions This is done by adding manually comparison against 0, might find better alternatives in the future. Related: eclipse-openj9#14027 Signed-off-by: Frank Kang <frank.kang@ibm.com>
d0eb34f to
4f81482
Compare
dmitripivkine
changed the title
|
Jenkins test sanity,extended xlinuxval jdknext |
|
Will the test case be added ? It can be done in a separate PR though. |
Yes, I think test can be added as a separate submission. We can use existed issue #14027 to track further work. |
I will open a PR to add a test case. |
5 participants
J9ARRAYCLASS_GET_STRIDE() returns the element size, and could return 0 when flattened is enabled and the class is empty.
We should fix this somehow. Currently we hard set the stride to 1 inside division when it is 0.