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

Enabling Full Info Points on AArch64 causes compile failures #4542

Closed
adinn opened this issue May 5, 2022 · 6 comments
Closed

Enabling Full Info Points on AArch64 causes compile failures #4542

adinn opened this issue May 5, 2022 · 6 comments
Assignees

Comments

@adinn
Copy link
Collaborator

adinn commented May 5, 2022

** Describe the Problem**

The latest patch for debug info (PR #4505) modifies the GraphBuilderConfiguration to enable full info point generation.

class SVMHost {
  ...
@Override
public GraphBuilderConfiguration updateGraphBuilderConfiguration(GraphBuilderConfiguration config, AnalysisMethod method) {
    return config.withRetainLocalVariables(retainLocalVariables())
                    .withUnresolvedIsError(linkAtBuildTimeSupport.linkAtBuildTime(method.getDeclaringClass()))
                    .withFullInfopoints(SubstrateOptions.GenerateDebugInfo.getValue() > 0);
...

This is not a problem on x86_64. However, it leads to multiple compile failures on AArch64. They all occur because LocationMarkerPhase.processState() has been passed a FullInfopointNode referencing a live value in the link register lr aka r30.

java.lang.NullPointerException
at com.oracle.svm.core.CalleeSavedRegisters.getOffsetInFrame(CalleeSavedRegisters.java:108)
at com.oracle.svm.core.heap.SubstrateReferenceMapBuilder.addLiveValue(SubstrateReferenceMapBuilder.java:65)
at jdk.internal.vm.compiler/org.graalvm.compiler.lir.dfa.RegStackValueSet$1.visitValue(RegStackValueSet.java:155)
at jdk.internal.vm.compiler/org.graalvm.compiler.lir.ValueConsumer.visitValue(ValueConsumer.java:51)
at jdk.internal.vm.compiler/org.graalvm.compiler.lir.util.IndexedValueMap.visitEach(IndexedValueMap.java:455)
at jdk.internal.vm.compiler/org.graalvm.compiler.lir.dfa.RegStackValueSet.addLiveValues(RegStackValueSet.java:158)
at jdk.internal.vm.compiler/org.graalvm.compiler.lir.dfa.LocationMarkerPhase$Marker.processState(LocationMarkerPhase.java:97)
    ...

The reference map builder assumes that lr is a callee saved register which is why it ends up calling CalleeSavedRegisters.getOffsetInFrame. Unfortunately, when the call to getOffsetInFrame looks up lr in the map stored in field offsetsInSaveArea the result is null leading to a NPE.

Steps to reproduce the issue

Path 1:
1 Clone the repo from #4505
git clone git://github.com/adinn/graal debug_location_info
2 Build on Linux/AArch64
mx build
3 Run the debug info test
mx debuginfotest

Path 2:
1 Clone the Oracle master
2) Modify class SVMHost.updateGraphBuilderConfiguration to always enable full infopoints

  public GraphBuilderConfiguration updateGraphBuilderConfiguration(GraphBuilderConfiguration config, AnalysisMethod method) {
      return config.withRetainLocalVariables(retainLocalVariables())
                    .withUnresolvedIsError(linkAtBuildTimeSupport.linkAtBuildTime(method.getDeclaringClass()))
                    .withFullInfopoints(true);
  ...
  1. Build on Linux/AArch64
    mx build
    4 Run the debug info test
    mx debuginfotest

Describe GraalVM and your environment:

@olpaw
Copy link
Member

olpaw commented May 5, 2022

The reference map builder assumes that lr is a callee saved register which is why it ends up calling CalleeSavedRegisters.getOffsetInFrame. Unfortunately, when the call to getOffsetInFrame looks up lr in the map stored in field offsetsInSaveArea the result is null leading to a NPE.

@teshull, sounds like SubstrateReferenceMapBuilder#addLiveValue needs to handle AArch64 differently. Do you have any hints / ideas on how to fix this?

@teshull
Copy link
Member

teshull commented May 5, 2022

hmm. I'm off the rest of this week, but I'll look into it the beginning of next week

@olpaw
Copy link
Member

olpaw commented May 5, 2022

hmm. I'm off the rest of this week, but I'll look into it the beginning of next week

Thanks a lot!

@adinn
Copy link
Collaborator Author

adinn commented May 5, 2022

Hi Tom, Thanks for taking a look at this.

@adinn
Copy link
Collaborator Author

adinn commented May 10, 2022

@teshull I discussed a possible fix here #4505 (comment)

@teshull
Copy link
Member

teshull commented May 14, 2022

fixed by #4565

@teshull teshull closed this as completed May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants