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

Check array-shadow symbol and DataType for refineArrayAliasing #7119

Merged

Conversation

rmnattas
Copy link
Contributor

@rmnattas rmnattas commented Sep 20, 2023

This is to check that the array access is an array-shadow symbol and that the symbol DataType matches the node DataType before performing TR_LoopUnroller::refineArrayAliasing on that list of accesses.

This prevents array-accesses that are not array-shadow (e.g. <unsafe shadow sym>) or cases where the symbol DataType and the node DataType don't match (e.g. loading from Byte[] String.value using a short-load node) from being refined, and incorrectly changed to an array-shadow or to the wrong DataType, that might cause a crash during compilation.

DLT stderr _ZN3OMR20SymbolReferenceTable33createRefinedArrayShadowSymbolRefEN2TR8DataTypeE+0x2c (0x000077BF124D5C3C [libj9jit29.so+0x6b5c3c])
DLT stderr _ZN15TR_LoopUnroller19refineArrayAliasingEv+0x144 (0x000077BF125DACD4 [libj9jit29.so+0x7bacd4])
DLT stderr _ZN15TR_LoopUnroller6unrollEP18TR_RegionStructureP24TR_StructureSubGraphNode+0x17c (0x000077BF125E668C [libj9jit29.so+0x7c668c])
DLT stderr _ZN15TR_LoopUnroller6unrollEPN2TR11CompilationEP18TR_RegionStructureP27TR_PrimaryInductionVariableNS_10UnrollKindEiiPNS0_9OptimizerE.part.265+0x814 (0x000077BF125E70B4 [libj9jit29.so+0x7c70b4])
DLT stderr _ZN22TR_GeneralLoopUnroller7performEv+0x54c (0x000077BF125E801C [libj9jit29.so+0x7c801c])
DLT stderr _ZN3OMR9Optimizer19performOptimizationEPK20OptimizationStrategyiii+0x1814 (0x000077BF1276E6F4 [libj9jit29.so+0x94e6f4])
DLT stderr _ZN3OMR9Optimizer8optimizeEv+0x204 (0x000077BF1276F3F4 [libj9jit29.so+0x94f3f4])
DLT stderr _ZN3OMR11Compilation20performOptimizationsEv+0x3c (0x000077BF124DDD9C [libj9jit29.so+0x6bdd9c])

This is to check that the array access is an array-shadow symbol
and that the symbol DataType matches the node DataType before
performing `TR_LoopUnroller::refineArrayAliasing` on that list
of accesses.

Signed-off-by: Abdulrahman Alattas <rmnattas@gmail.com>
@rmnattas
Copy link
Contributor Author

fyi: @vijaysun-omr @jdmpapin

@jdmpapin
Copy link
Contributor

I've been quite involved in this fix, so it would be best for someone else to review

@vijaysun-omr vijaysun-omr self-assigned this Sep 20, 2023
@vijaysun-omr
Copy link
Contributor

I will review this and shepherd to merging. Thanks

@vijaysun-omr
Copy link
Contributor

jenkins build all

@vijaysun-omr
Copy link
Contributor

vijaysun-omr commented Sep 22, 2023

While the change itself looks safe/conservative, I think it would be good if the PR mentioned briefly (in a sentence or two) what could go wrong if this change was not done. i.e. what happens if the sym ref is not an array shadow sym ref of a consistent type ?

@rmnattas
Copy link
Contributor Author

Updated PR 👍

@vijaysun-omr
Copy link
Contributor

jenkins build win

@vijaysun-omr
Copy link
Contributor

Thanks, checks have passed. Merging.

@vijaysun-omr vijaysun-omr merged commit a25bcb0 into eclipse-omr:master Sep 22, 2023
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