Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assert dstObj in arraycopyEval is not a dataAddrPtr in P/AArch64 #7500

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

rmnattas
Copy link
Contributor

@rmnattas rmnattas commented Oct 24, 2024

For OffHeap, the arraycopy transformations uses the correct base object as the dstObj node, instead of the dataAddrPtr load.
To ensure correctness this PR adds an asserts that checks that the dstObj is not a dataAddrPtr.

Valid:
n60n        arraycopy  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#405  unresolved notAccessed static Method] [flags 0x400 0x0 ] (Unsigned forwardArrayCopy noArra>
n58n          aload  <parm 0 [Ljava/lang/Object;>[#403  Parm]                          
n59n          aload  <parm 1 [Ljava/lang/Object;>[#404  Parm]                            
n54n          aloadi  <contiguousArrayDataAddrFieldSymbol>[#352  final Shadow +8] [flags 0x20604 0x0 ] (internalPtr )  [    0x78f0fb0e00a0] bci=[-1,0,20] rc=1 vc=33 vn=- li=- udi=- nc=1 fl>
n55n            aload  <parm 0 [Ljava/lang/Object;>[#403  Parm]                
n56n          aloadi  <contiguousArrayDataAddrFieldSymbol>[#352  final Shadow +8] [flags 0x20604 0x0 ] (internalPtr )  [    0x78f0fb0e0140] bci=[-1,2,20] rc=1 vc=33 vn=- li=- udi=- nc=1 fl>
n57n            aload  <parm 1 [Ljava/lang/Object;>[#404  Parm]                         
n53n          lconst 8 (highWordZero X!=0 X>=0 cannotOverflow )             

Not-Valid:
n60n        arraycopy  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#405  unresolved notAccessed static Method] [flags 0x400 0x0 ] (Unsigned forwardArrayCopy noArra>
n58n          aload  <parm 0 [Ljava/lang/Object;>[#403  Parm]       
n62n          aloadi  <contiguousArrayDataAddrFieldSymbol>[#352  final Shadow +8] [flags 0x20604 0x0 ] (internalPtr )  [    0x78f0fb0e00a0] bci=[-1,0,20] rc=1 vc=33 vn=- li=- udi=- nc=1 fl>                   
n59n            aload  <parm 1 [Ljava/lang/Object;>[#404  Parm]                            
n54n          aloadi  <contiguousArrayDataAddrFieldSymbol>[#352  final Shadow +8] [flags 0x20604 0x0 ] (internalPtr )  [    0x78f0fb0e00a0] bci=[-1,0,20] rc=1 vc=33 vn=- li=- udi=- nc=1 fl>
n55n            aload  <parm 0 [Ljava/lang/Object;>[#403  Parm]                
n56n          aloadi  <contiguousArrayDataAddrFieldSymbol>[#352  final Shadow +8] [flags 0x20604 0x0 ] (internalPtr )  [    0x78f0fb0e0140] bci=[-1,2,20] rc=1 vc=33 vn=- li=- udi=- nc=1 fl>
n57n            aload  <parm 1 [Ljava/lang/Object;>[#404  Parm]                         
n53n          lconst 8 (highWordZero X!=0 X>=0 cannotOverflow )      

Reason being the writeBarrier code assumes the dstObj node being the base object and uses that for card-dirtying address calculation.

Following: eclipse-openj9/openj9#20264
X/Z in OpenJ9: eclipse-openj9/openj9#20480

@rmnattas
Copy link
Contributor Author

fyi @zl-wang

@rmnattas rmnattas changed the title Add not dataAddrPtr assert in arraycopyEval for dstObj Assert dstObj in arraycopyEval is not a dataAddrPtr in P/AArch64 Dec 11, 2024
@rmnattas
Copy link
Contributor Author

Both Openj9/OMR are ready for review and merge, no need for coordination.

Copy link
Contributor

@zl-wang zl-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zl-wang
Copy link
Contributor

zl-wang commented Dec 11, 2024

@dsouzai please review/approve/merge

@dsouzai
Copy link
Contributor

dsouzai commented Dec 11, 2024

jenkins build aarch64,amac,aix,plinux

@dsouzai dsouzai merged commit 2b0e765 into eclipse-omr:master Dec 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants