Debug info prototype #2088

adinn · 2020-01-22T10:51:13Z

This PR is for an initial prototype for the native image debug info implementation. It is really intended for discussion rather than to be pulled into the main repo. However, it does actually implement support for gdb on Linux/x86_64 (see DEBUGINFO.md for details of how to generate and use debug info and what features are supported).

The patch contains 3 main commits of which the second is the actual code that generates the DWARF debug records. The first patch defines a set of Graal-neutral interfaces defining an API that allow the Native Image Generator to communicate the necessary details of compiled code to an object file. The third patch modifies the Native Image Generator to drive generation of debug info using that API.

Work on a Windows implementation is progressing and should be able to rely on employing the same Graal-neutral interface API. AArch64 support still needs testing (it will probably require only a little bit of tweaking to get stack backtraces working).

I'd be very grateful for feedback on all aspects of the implementation but clearly the most important things to look at form the point of view of the rest of the project are patches 1 and 3.

n.b. the interface API includes place holders for notifying details of types and heap data elements to the debug generator. Fleshing out this API and implementing associated debug info generation is the next task in hand.

adinn · 2020-01-22T12:21:13Z

Pushed a fix for the check-style errors modulo a problem including a (c) Red Hat Inc line below the (c) Oracle one. Previously this did not cause a problem but it seems to be rejected now? Any ideas?

If I could only remember the eclipses style check command I'd fix that too ;-)

adinn · 2020-01-22T15:29:53Z

Ok, I think I have fixed the eclipse style issues too :-]

adinn · 2020-01-22T16:13:05Z

hurrah! one more game of whackamole and I believe the style dragon has been slain :-)

olpaw · 2020-01-23T08:54:07Z

Hi @adinn
Thanks for your prototype PR. I should be able to take a deeper look somewhere next week.

adinn · 2020-01-23T09:37:42Z

Thanks Paul. I may even have fixed all the style check issues by then!

dougxc · 2020-01-23T09:48:46Z

@adinn I wouldn't worry too much about the style fixes yet (unless they block more substantial testing). They can be fixes once more substantiative review has happened.

...tratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java

adinn · 2020-01-23T09:53:19Z

@dougxc yes, of course you are right. I was merely enjoying the fight. A-and it does seem that I have not broken any tests ... yet.

adinn · 2020-01-24T15:39:59Z

I appear to have broken something when re-assembling this patch against the recent head version. My info record lo and high pc values are now coming out as 0. I think it is to do with applying relocs as I am posting the correct offsets for insertion into the debug_info reloc section. I'll update it as soon as I find the culprit.

adinn · 2020-01-24T16:04:40Z

Ok, I found the problem. I omitted a change I had to make to ELFRelocationSection. I had to add an explicit CONTENT dependency from each DWARF section to its associated reloc section. This is not needed for the standard sections because they are all created before writing the sections commences and they get inserted into the to_be_processed list in the right order.
The DWARF sections are added later after generation of the text section but that means there is no guarantee that sections and their relocs will be processed in the correct dependency order.

olpaw

If haven't already, please join our public slack channel:
https://www.graalvm.org/slack-invitation

It might be easier to discuss debuginfo releated stuff in real-time.

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFObjectFile.java

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/Range.java

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeBootImage.java

olpaw · 2020-01-29T12:57:22Z

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

+        }
+
+        protected void debug(String format, Object... args) {
+            if (debug) {


We have org.graalvm.compiler.debug.DebugContext for logging. See e.g. com.oracle.svm.hosted.ResourcesFeature#scanDirectory

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

olpaw · 2020-01-29T13:07:35Z

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

+
+        public int putAsciiStringBytes(String s, int startChar, byte[] buffer, int p) {
+            int pos = p;
+            for (int l = startChar; l < s.length(); l++) {


Maybe Charset.forName("US-ASCII").newEncoder().canEncode ?

olpaw · 2020-01-29T13:10:31Z

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

+        @Override
+        public void createContent() {
+            int pos = 0;
+            // an abbrev table contains abbrev entries for one or


CUDOs for documenting the binary representations. Please use

/* * This style for code comments wherever possible. * */

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

olpaw · 2020-01-29T13:14:38Z

substratevm/DEBUGINFO.md

+To add debug info to a generated native image add flag
+-H:+TrackNodeSourcePosition to the native image command line.
+
+  mx native-image -H:+TrackNodeSourcePosition Hello.java


Nah ... instead add HostedOption GenerateDebugInfo and whenever it is > 0 make sure TrackNodeSourcePosition gets set.

Yes, I wanted to do something like that but I was not sure exactly how to implement it. Having looked around at the code I think I worked out how to do it and have pushed a further commit which I believe does it correctly.

GenerateDebugInfo is now a HostedOption in SubstrateOptions. It uses an onValueUpdate override to test TrackNodeSourcePosition and force it to true if it is not set. Hope that is ok.

Looks good now. Thanks!

olpaw · 2020-01-29T13:21:54Z

substratevm/DEBUGINFO.md

+methods, Graal methods and JDK runtime methods you need to provide gdb
+with a list of source root dirs a 'set directories' command:
+
+    (gdb) set directories /home/adinn/hello/src:/home/adinn/graal/sdk/src/org/graalvm.word/src:/home/adinn/graal/sdk/src/org.graalvm.options/src:...


Instead of having a bash script that will get very complicated over time as part of building debuginfo produce a sources folder next to the binary (subdir of Path HostedOption) where you collect all the source files that the generated debuginfo refers to. This way the debugger only needs to know about a single source dir. Also keep in mind that in the Java ecosystem jar-files often have their corresponding zip or jar file that contain the sources. In Java its easy to aggregate all these files from source bundles into a single common directory location.

... also for the Windows version you would need to provide a *.bat version of that script. Don't go down that path ...

Yeah, I'll get onto that after I sort out using Path instead of String.

I have just pushed a change to populate a sources tree in the working directory using source file lookups based on whatever sources can be found in $JAVA_HOME/lib/src/zip for JDK classes ($JAVA_HOME/lib/src/zip for jdk11) or via the classpath for GraalVM & application classes. The cache manager triages classes based on package prefix and uses a lookup scheme appropriate to the identified target type: jdk, graalvm or application.

JDK classes are looked up in src.zip, prepending a module name to the lookup path if necessary

GraalVM classes are looked up relative to the classpath. If foo.jar is found on the class path then if foo.src.zip is found it is assumed to contain graalvm sources and added to the list of search roots. If a directory foo is found on the class path and foo/src or foo/src_gen exist then they are added to the list of search roots.

Application sources are looked up relative to the classpath. If foo.jar is found on the class path then if foo-sources.jar is found it is assumed to contain application sources and added to the list of search roots. If a directory foo/classes or foo/target/classes is found on the class path and foo/src exists it is added to the list of search roots. The current working directory is also added to the list of application search roots.

Files are cached in local dir sources under 3 distinct subdirectories jdk, graalvm and src (for JDK, GraalVM and application sources, respectively). So, in order to make this work with gdb that requires adding 3 entries to the source search path

(gdb) set directories /path/to/pwd/sources/jdk:/path/to/pwd/sources/graalvm:/path/to/pwd/sources/src

The file names and paths used in the debug info are normally derived using the package name, substituted with the local file system separator, suffixed with the classfile source name. If the source name is missing then the unqualified class name is used. In the latter case for inner classes the name before the $ is used. Class names which start with $ (e.g. anon classes etc) are not mapped to files and so cannot have debug entries. Of course, paths for JDK classes are prefixed with the name of the module that the class belongs to.

There is one problem with this. Compiler classes which are referenced from NodeSourcePosition records are not present in jars on the classpath (well, not when running with jdk11). For example, class org.graalvm.compiler.replacements.amd64.AMD64StringLatin1Substitutions is referenced from a simple HelloWorld app but the corresponding source is not found. That's because these classes are made available via the platform classloader as jmod files. In consequence there is no way to locate a source for these classes.

There are actually sources for classes like this in src.zip. However, 1) they are almost always going to be out of date wrt the source used by the native image generator and 2) they are bundled into the src.zip with both under a path which includes not just their module name but also a secondary directory based on some initial segment of their package prefix.

I am not really sure how to make these classes available. It appears that all the source files needed are available under compiler/mxbuild/dists/jdk11/ i.e. the directory that contains the bundled jmod files (they seem to be mostly in graal.src.zip). Is there any way for the native image generator to identify the location of this dir from the build config info and search it for extra roots to add to the GraalVM search path?

@olpaw Forgot to tag this for your attention

That sounds great!
I should be able to take a closer look somewhere in the next two days or so.

Thanks. I'm just updating the DEBUGINFO.md readme which will clarify the latest status quo.

substratevm/DEBUGINFO.md

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DirEntry.java

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/dwarf/DwarfSections.java

Tweak SourceCacheType enum Clean up SourceCache initialization Memoize source cache checks

adinn · 2020-04-03T14:13:36Z

@olpaw I have squashed the commits down into a small number representing groups of related changes and adjusted the comments to be more precise and minimal. Do I need to do anythign further?

n.b. Simon Tooke is currently preparing an updated version of his Windows debug info PR rebased on this PR. I think he still needs 1) to adopt the use of a DebugContext for logging details of the PECOFF section generation and 2) to modify his code to use DebugInfoBase, the common base class I factored out of the DWARF code. His code uses its own version of that class that is almost identical so this should not require a lot of work. Note that the latter update will also ensure that data passed in via the DebugCodeInfo API gets logged using a DebugContext.

olpaw · 2020-04-03T15:16:06Z

@olpaw I have squashed the commits down into a small number representing groups of related changes and adjusted the comments to be more precise and minimal. Do I need to do anythign further?

Thanks! That's all for now :-)

n.b. Simon Tooke is currently preparing an updated version of his Windows debug info PR rebased on this PR. I think he still needs 1) to adopt the use of a DebugContext for logging details

Sound like this will take a bit more time. So I will not also wait for Simons changes. I will proceed with merging your PR to master on Monday. With a bit of luck your PR will be on master mid-next week.

Thanks for all you patience!

adinn · 2020-04-04T13:13:10Z

Sound like this will take a bit more time. So I will not also wait for Simons changes. I will proceed with merging your PR to master on Monday. With a bit of luck your PR will be on master mid-next week.

Excellent news!

Thanks for all you patience!

I am very aware that this is a two way street. So thank you very much in return for your patience!

olpaw · 2020-04-06T09:33:24Z

Last chance to fix the typo in commit message of b9572ad

…o DebugInfoBase Rename subclass DwarfSections to DwarfDebugInfo

adinn · 2020-04-06T09:46:58Z

Fixed two typos!

olpaw

This PR is now in shape for getting merged to master.

graalvmbot added the oca-signed label Jan 22, 2020

dougxc requested review from olpaw and christianwimmer January 22, 2020 11:56

olpaw self-assigned this Jan 23, 2020

dougxc reviewed Jan 23, 2020

View reviewed changes

...tratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debuginfo/DebugInfoProvider.java Outdated Show resolved Hide resolved

dougxc assigned adinn Jan 23, 2020