[GR-39210] Upgrade CE debug info feature to provide information about Java types for Windows/PECOFF

[stooke]

This PR adds type information to the CodeView 4 debug information (See PR #2396). It is related to issue #2986, but is targeted to Windows and Visual Studio.

This PR is based off (and highly dependant on) the work in PR #3046.

With this patch, memory can be examined from the Visual Studio or WinDbg command line and cast to Java types as laid out by the Graal compiler. Static member variables can be referenced directly.

Caveats:

• Pointer types for instance members in the presence of isolates are not yet implemented, as there appears to be no way to express a pointer that is an offset from r14 in CodeView. I will continue to research this, but at this time, this implementation works best if isolates are disabled.
• As there is no local variable information added by this PR, users must manually cast registers to pointers; for example (java_lang_String*)(rdx).

I recommend @adinn as a first reviewer for this PR, as it is based on his debug type PR.

[olpaw]

@adinn looks like github does not allow me to put you on this one as reviewer :disappointed:

[adinn]

@olpaw No worry. I'll review it anyway :-)

[adinn]

I believe this is going to need to be modified slightly when rebased over head in order to take account of the changes made to support inline info. Specifically PrimaryEntry.getSubranges() is still being called from CVLinerecordBuilder. That has been dropped in head in favour of calling leafRangeIterator(). I don't think there us anything else needs changing though.

@stooke The above tweak turns out to be unneeded. The upstream code was updated to use the correct iterator when the gdb inline methods patch went in. Once this PR is rebased it all fits together without any conflict. So, it appears to be ready for Oracle review and merging.

@pejovica Could you please review this and help get it through the Oracle gate?

[pejovica]

@adinn Sure, I should be able to start with that after next week.

[adinn]

@pejovica This PR is still waiting to be reviewed. will you be able to do that soon or do we need to find someone else to review it?

[olpaw]

• [ ] Please consolidate the commits. E.g. combine subsequent style related commits into one (or even better, fold them into the commit that is the origin of the style issue). Also start commit messages with uppercase.

[stooke]

• [x] Please consolidate the commits. E.g. combine subsequent style related commits into one (or even better, fold them into the commit that is the origin of the style issue). Also start commit messages with uppercase.

Hello @olpaw , and thank you for your review! I've started to address your concerns, and squashed many of the commits, changing the messages to start with uppercase. Please let me know if I should squash further.

[olpaw]

I've started to address your concerns, and squashed many of the commits, changing the messages to start with uppercase. Please let me know if I should squash further.

Looks much better now. Thanks!

[adinn]

@olpaw is this still waiting on a review from @pejovica or is it ok for it to be pushed through the gate?

[olpaw]

@olpaw is this still waiting on a review from @pejovica or is it ok for it to be pushed through the gate?

@pejovica is currently reviewing the PR (and testing it thoroughly on his Windows machine). Since the PR is quite big this might take some more time. Sorry for the inconvenience.

[adinn]

@olpaw Sure, thanks for the update and no inconvenience or problem about @pejovica taking his time. I'm very glad to hear he is giving it a proper workout.

[stooke]

@pejovica thank you for the extensive review! I'll be responding to the issues you've raised over the next few days.

[stooke]

@pejovica , thanks again for the review. I'm made extensive changes in line with your suggested improvements. Some conversations I left unresolved as I might have a question or two. At this point, I'd like to see if there are further changes you'd like to see before I merge the latest Graal head into this branch.

[pejovica]

@stooke You can update right away as I'd be doing testing on the latest master anyway.

And I'll do another review round in the next few days.

[stooke]

@pejovica, I've updated, but (like many other PRs) I can't pass the debugInfo gates because of issue #4018. Please let me know if there's anything I can address in the meantime.

[pejovica]

@stooke Sorry for the delay. There were a few unrelated issues that I had to take care of, so unfortunately I didn't have enough time to do a proper review. I should be able to continue reviewing this PR next week.

[adinn]

@pejovica This issue is still waiting for a review after from before Christmas. I believ the debuginfotets gate problem is no longer stopping it progressing. Are you likely to get time to review it soon?

[pejovica]

@adinn Yes, and I've actually already started, so I hope to finish in the next few days.

[stooke]

Hello @pejovica , and thanks for the review - I have incorporated the changes suggested into the prereq PR #4333, and will update this PR shortly.

[pejovica]

@stooke Excellent, thanks!

[stooke]

@pejovica , I have updated this PR to reflect the changes from the prereq PR.
While doing so, I've simplified CVTypeSectionImpl, and refactored CVTypeSectionBuilder to separate out utility functions (getIndexForPointerorPrimitive() for example) into a utility class.

[adinn]

@fniephaus @pejovica @olpaw It is getting very close to cutoff for the next release and we would very much like to see this patch go in. Simon believe he has provided a suitable response to all outstanding review requests. Can we push it as is, leaving any further pending changes to follow-up PRs or, failing that, can we work quickly to resolve any outstanding problems so we can make the release?

[stooke]

@pejovica , thanks for this review cycle, and especially the catch of the reversal of typeindex and vtable in one of the type records. As far as I know, I have addressed this latest round.

[adinn]

@fniephaus @pejovica @olpaw We would very much like to see this patch go in. @stooke believes he has provided a suitable response to all outstanding review requests. Can we push it as is, leaving any further pending changes to follow-up PRs or, failing that, can we work quickly to resolve any outstanding problems?

[fniephaus]

I will leave the reviewing to @pejovica and @olpaw. It's not going to be merged in time for the 22.2 release but we should be able to merge it right after the branch-off. In the meantime, I'm running our internal gates and will report any issues that I run into.

[fniephaus]

Ok so it seems most gates are passing except for unfortunately some that build the EE version of gu.exe:

Fatal error: java.lang.RuntimeException: There was an error linking the native image: Linker command exited with 2
...
gu.obj : fatal error LNK1103: debugging information corrupt; recompile module
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.image.NativeImageViaCC.handleLinkerFailure(NativeImageViaCC.java:188)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.image.NativeImageViaCC.write(NativeImageViaCC.java:135)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:702)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:519)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:407)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:585)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:128)

I'm not sure how we could provide a reproducer but maybe @pejovica or @olpaw have an idea how to fix this.

[stooke]

Ok so it seems most gates are passing except for unfortunately some that build the EE version of gu.exe:

Fatal error: java.lang.RuntimeException: There was an error linking the native image: Linker command exited with 2
...
gu.obj : fatal error LNK1103: debugging information corrupt; recompile module
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.image.NativeImageViaCC.handleLinkerFailure(NativeImageViaCC.java:188)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.image.NativeImageViaCC.write(NativeImageViaCC.java:135)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:702)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:519)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:407)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:585)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:128)

I'm not sure how we could provide a reproducer but maybe @pejovica or @olpaw have an idea how to fix this.

If I can get hold of the object file, I might be able to see what's out of place.

[fniephaus]

If I can get hold of the object file, I might be able to see what's out of place.

Sounds good. Maybe we can provide that.

Anyway, I just realized that we don't use the the CE debuginfo in EE yet, which may be causing this issue. I'll check with @olpaw next week.

[stooke]

If I can get hold of the object file, I might be able to see what's out of place.

Sounds good. Maybe we can provide that.

Anyway, I just realized that we don't use the the CE debuginfo in EE yet, which may be causing this issue. I'll check with @olpaw next week.

@fniephaus , any word on this? As this is an issue that shows up only in EE, and I don't have access to the EE code, and the EE version doesn't support Windows debugging (as I understand; please correct me if I'm mistaken), I suggest moving forward with this at this time.

[fniephaus]

any word on this?

As you may know, we're in a feature freeze atm and the team is busy preparing the upcoming release. Unfortunately, creating a reproducer for you turned out harder than expected, which is why we had to deprioritize this. I hope we can follow up very soon.