Correct debug info test script for new object layout and to bypass latest gdb bug

minor code cleanups

style fixes

fix comment text

further fixes after latest review

Author: adinn

substratevm/DebugInfo.md

@@ -106,7 +106,7 @@ compiled method.
 
 ### Currently Missing Features
 
-  - reference by name to values boudn to parameter and local vars
+  - reference by name to values bound to parameter and local vars
 
 This feature is scheduled for inclusion in a later release.
 

substratevm/mx.substratevm/testhello.py

@@ -42,6 +42,7 @@
 
 import re
 import sys
+import os
 
 # A helper class which checks that a sequence of lines of output
 # from a gdb command matches a sequence of per-line regular
@@ -139,10 +140,16 @@ def test():
     # n.b. can only get back here with one match
     match = matches[0]
     major = int(match.group(1))
-    # may need this if 8.x and 9.x get patched
-    # minor = int(match.group(2))
-    # printing object data requires gdb 10+
-    can_print_data = major > 9
+    minor = int(match.group(2))
+    # printing object data requires a patched gdb
+    # once the patch is in we can check for a suitable
+    # range of major.minor versions
+    # for now we use an env setting
+    print("Found gdb version %s.%s"%(major, minor))
+    # can_print_data = major > 10 or (major == 10 and minor > 1)
+    can_print_data = False
+    if os.environ.get('GDB_CAN_PRINT', '') == 'True':
+        can_print_data = True
 
     if not can_print_data:
         print("Warning: cannot test printing of objects!")
@@ -182,12 +189,13 @@ def test():
         exec_string = execute("print /x *(('java.lang.String[]' *)$rdi)")
         checker = Checker("print String[] args",
                           [r"%s = {"%(wildcard_pattern),
-                           r"%s<_arrhdrA> = {"%(spaces_pattern),
+                           r"%s<java.lang.Object> = {"%(spaces_pattern),
+                           r"%s<_objhdr> = {"%(spaces_pattern),
                            r"%shub = %s,"%(spaces_pattern, address_pattern),
-                           r"%sidHash = %s,"%(spaces_pattern, address_pattern),
-                           r"%slen = 0x0"%(spaces_pattern),
-                           r"%s},"%(spaces_pattern),
+                           r"%sidHash = %s"%(spaces_pattern, address_pattern),
+                           r"%s}, <No data fields>}, "%(spaces_pattern),
                            r"%smembers of java\.lang\.String\[\]:"%(spaces_pattern),
+                           r"%slen = 0x0,"%(spaces_pattern),
                            r"%sdata = %s"%(spaces_pattern, address_pattern),
                            "}"])
 
@@ -225,7 +233,7 @@ def test():
         # ensure we can dereference static fields
         exec_string = execute("print 'java.math.BigDecimal'::BIG_TEN_POWERS_TABLE->data[3]->mag->data[0]")
         checker = Checker("print static field value contents",
-                          r"%s = 1000\$"%(wildcard_pattern))
+                          r"%s = 1000"%(wildcard_pattern))
         checker.check(exec_string, skip_fails=False)
 
     # look up PrintStream.println methods
@@ -308,13 +316,12 @@ def test():
     checker = Checker('ptype _objhdr', rexp)
     checker.check(exec_string, skip_fails=True)
 
-    exec_string = execute("ptype _arrhdrA")
-    rexp = [r"type = struct _arrhdrA {",
-            r"%sjava\.lang\.Class \*hub;"%(spaces_pattern),
-            r"%sint idHash;"%(spaces_pattern),
+    exec_string = execute("ptype 'java.lang.String[]'")
+    rexp = [r"type = class java.lang.String\[\] : public java.lang.Object {",
             r"%sint len;"%(spaces_pattern),
+            r"%sjava\.lang\.String \*data\[0\];"%(spaces_pattern),
             r"}"]
-    checker = Checker('ptype _objhdr', rexp)
+    checker = Checker('ptype String[]', rexp)
     checker.check(exec_string, skip_fails=True)
 
     # run a backtrace

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ClassEntry.java

@@ -74,7 +74,7 @@ public class ClassEntry extends StructureTypeEntry {
      */
     private Map<FileEntry, Integer> localFilesIndex;
     /**
-     * a list of the same files.
+     * A list of the same files.
      */
     private LinkedList<FileEntry> localFiles;
     /**
@@ -86,7 +86,7 @@ public class ClassEntry extends StructureTypeEntry {
      */
     private LinkedList<DirEntry> localDirs;
     /**
-     * true iff the entry includes methods that are deopt targets.
+     * This flag is true iff the entry includes methods that are deopt targets.
      */
     private boolean includesDeoptTarget;
 
@@ -289,18 +289,20 @@ protected void processMethod(DebugMethodInfo debugMethodInfo, DebugInfoBase debu
         String fileName = debugMethodInfo.fileName();
         Path filePath = debugMethodInfo.filePath();
         Path cachePath = debugMethodInfo.cachePath();
-        // n.b. the method file may differ from the owning class file when the method is a
-        // substitution
+        /*
+         * n.b. the method file may differ from the owning class file when the method is a
+         * substitution
+         */
         FileEntry methodFileEntry = debugInfoBase.ensureFileEntry(fileName, filePath, cachePath);
         methods.add(new MethodEntry(methodFileEntry, methodName, this, resultType, paramTypeArray, paramNameArray, modifiers));
     }
 
     @Override
     protected FieldEntry addField(DebugInfoProvider.DebugFieldInfo debugFieldInfo, DebugInfoBase debugInfoBase, DebugContext debugContext) {
         FieldEntry fieldEntry = super.addField(debugFieldInfo, debugInfoBase, debugContext);
-        FileEntry fileEntry = fieldEntry.getFileEntry();
-        if (fileEntry != null) {
-            indexLocalFileEntry(fileEntry);
+        FileEntry fieldFileEntry = fieldEntry.getFileEntry();
+        if (fieldFileEntry != null) {
+            indexLocalFileEntry(fieldFileEntry);
         }
         return fieldEntry;
     }
@@ -344,13 +346,13 @@ public Range makePrimaryRange(String methodName, String symbolName, String param
              */
             for (MethodEntry methodEntry : methods) {
                 if (methodEntry.match(methodName, paramSignature, returnTypeName)) {
-                    // maybe the method's file entry
+                    /* maybe the method's file entry */
                     fileEntryToUse = methodEntry.getFileEntry();
                     break;
                 }
             }
             if (fileEntryToUse == null) {
-                /* last chance is the class's file entry */
+                /* Last chance is the class's file entry. */
                 fileEntryToUse = this.fileEntry;
             }
         }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java

@@ -160,18 +160,18 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
          */
 
         /*
-         * track whether we need to use a heap base regsiter
+         * Track whether we need to use a heap base register.
          */
         useHeapBase = debugInfoProvider.useHeapBase();
 
         /*
-         * save count of low order tag bits that may appear in references
+         * Save count of low order tag bits that may appear in references.
          */
         int oopTagsMask = debugInfoProvider.oopTagsMask();
 
-        /* tag bits must be between 1 and 32 for us to emit as DW_OP_lit<n> */
+        /* Tag bits must be between 1 and 32 for us to emit as DW_OP_lit<n>. */
         assert oopTagsMask > 0 && oopTagsMask < 32;
-        /* mask must be contiguous from bit 0 */
+        /* Mask must be contiguous from bit 0. */
         assert ((oopTagsMask + 1) & oopTagsMask) == 0;
 
         oopTagsCount = Integer.bitCount(oopTagsMask);
@@ -185,13 +185,13 @@ public void installDebugInfo(DebugInfoProvider debugInfoProvider) {
         /* Save number of bytes in a reference field. */
         oopReferenceSize = debugInfoProvider.oopReferenceSize();
 
-        /* Save alignment of a reference */
+        /* Save alignment of a reference. */
         oopAlignment = debugInfoProvider.oopAlignment();
 
-        /* Save alignment of a reference */
+        /* Save alignment of a reference. */
         oopAlignShift = Integer.bitCount(oopAlignment - 1);
 
-        /* reference alignment must be 8 bytes */
+        /* Reference alignment must be 8 bytes. */
         assert oopAlignment == 8;
 
         /* Ensure we have a null string in the string section. */
@@ -499,7 +499,8 @@ public boolean isHubClassEntry(ClassEntry classEntry) {
     public int classLayoutAbbrevCode(ClassEntry classEntry) {
         if (useHeapBase & isHubClassEntry(classEntry)) {
             /*
-             * this layout adds special logic to remove tag bits from indirect pointers to this type
+             * This layout adds special logic to remove tag bits from indirect pointers to this
+             * type.
              */
             return DwarfDebugInfo.DW_ABBREV_CODE_class_layout2;
         }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/InterfaceClassEntry.java

@@ -74,10 +74,10 @@ public int getSize() {
          */
         int maxSize = super.size;
         for (ClassEntry implementor : implementors) {
-            int size = implementor.getSize();
+            int nextSize = implementor.getSize();
 
-            if (size > maxSize) {
-                maxSize = size;
+            if (nextSize > maxSize) {
+                maxSize = nextSize;
             }
         }
         return maxSize;

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/StructureTypeEntry.java

@@ -71,8 +71,10 @@ protected FieldEntry addField(DebugFieldInfo debugFieldInfo, DebugInfoBase debug
         String fileName = debugFieldInfo.fileName();
         Path filePath = debugFieldInfo.filePath();
         Path cachePath = debugFieldInfo.cachePath();
-        // n.b. the field file may differ from the owning class file when the field is a
-        // substitution
+        /*
+         * n.b. the field file may differ from the owning class file when the field is a
+         * substitution
+         */
         FileEntry fileEntry = debugInfoBase.ensureFileEntry(fileName, filePath, cachePath);
         FieldEntry fieldEntry = new FieldEntry(fileEntry, fieldName, this, valueType, fieldSize, fieldoffset, fieldModifiers);
         fields.add(fieldEntry);

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

@@ -41,22 +41,22 @@ public interface DebugInfoProvider {
     boolean useHeapBase();
 
     /**
-     * number of bits oops are left shifted by when using compressed oops
+     * Number of bits oops are left shifted by when using compressed oops.
      */
     int oopCompressShift();
 
     /**
-     * mask delecting low order bits used for tagging oops
+     * Mask delecting low order bits used for tagging oops.
      */
     int oopTagsMask();
 
     /**
-     * number of bytes used to store an oop reference
+     * Number of bytes used to store an oop reference.
      */
     int oopReferenceSize();
 
     /**
-     * alignment of object memory area (and, therefore, of any oop) in bytes
+     * Alignment of object memory area (and, therefore, of any oop) in bytes.
      */
     int oopAlignment();
 

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

@@ -948,7 +948,7 @@ private int writeMethodDeclarationAbbrev(@SuppressWarnings("unused") DebugContex
         /* We don't (yet?) have a proper start line for the method itself */
         // pos = writeAttrType(DwarfDebugInfo.DW_AT_decl_line, buffer, pos);
         // pos = writeAttrForm(DwarfDebugInfo.DW_FORM_data2, buffer, pos);
-        /* This probably needs to use the symbol name -- n.b. it is not in DWARF2 */
+        /* This probably needs to use the symbol name */
         // pos = writeAttrType(DwarfDebugInfo.DW_AT_linkage_name, buffer, pos);
         // pos = writeAttrForm(DwarfDebugInfo.DW_FORM_strp, buffer, pos);
         pos = writeAttrType(DwarfDebugInfo.DW_AT_type, buffer, pos);
@@ -1187,7 +1187,7 @@ private int writeStaticFieldLocationAbbrev(@SuppressWarnings("unused") DebugCont
         pos = writeFlag(DwarfDebugInfo.DW_CHILDREN_no, buffer, pos);
         pos = writeAttrType(DwarfDebugInfo.DW_AT_specification, buffer, pos);
         pos = writeAttrForm(DwarfDebugInfo.DW_FORM_ref_addr, buffer, pos);
-        /* Do we have a symbol name to use here? n.b. this is not in DWARF2. */
+        /* Do we have a symbol name to use here? */
         // pos = writeAttrType(DwarfDebugInfo.DW_AT_linkage_name, buffer, pos);
         // pos = writeAttrForm(DwarfDebugInfo.DW_FORM_strp, buffer, pos);
         pos = writeAttrType(DwarfDebugInfo.DW_AT_location, buffer, pos);
@@ -1209,11 +1209,9 @@ private int writeSuperReferenceAbbrev(@SuppressWarnings("unused") DebugContext c
         pos = writeAttrType(DwarfDebugInfo.DW_AT_type, buffer, pos);
         pos = writeAttrForm(DwarfDebugInfo.DW_FORM_ref_addr, buffer, pos);
         pos = writeAttrType(DwarfDebugInfo.DW_AT_data_member_location, buffer, pos);
-        pos = writeAttrForm(DwarfDebugInfo.DW_FORM_data1, buffer, pos); // = offset? in which
-                                                                        // segment though?
+        pos = writeAttrForm(DwarfDebugInfo.DW_FORM_data1, buffer, pos);
         pos = writeAttrType(DwarfDebugInfo.DW_AT_accessibility, buffer, pos);
-        pos = writeAttrForm(DwarfDebugInfo.DW_FORM_data1, buffer, pos); // = offset? in which
-                                                                        // segment though?
+        pos = writeAttrForm(DwarfDebugInfo.DW_FORM_data1, buffer, pos);
         /*
          * Now terminate.
          */

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

@@ -255,11 +255,14 @@ public class DwarfDebugInfo extends DebugInfoBase {
     public static final byte rthread_x86 = (byte) 15;
 
     /*
-     * prefix used to label indirect types used to ensure gdb performs oop reference --> raw address
-     * translation
+     * A prefix used to label indirect types used to ensure gdb performs oop reference --> raw
+     * address translation
      */
     public static final String INDIRECT_PREFIX = "_z_.";
-    /* name of type for hub field which needs special case processing to remove tag bits */
+    /*
+     * The name of the type for header field hub which needs special case processing to remove tag
+     * bits
+     */
     public static final String HUB_TYPE_NAME = "java.lang.Class";
 
     private DwarfStrSectionImpl dwarfStrSection;
@@ -343,11 +346,11 @@ public byte getThreadRegister() {
      */
     static class DwarfTypeProperties {
         /**
-         * index in debug_info section of type declaration for this class.
+         * Index in debug_info section of type declaration for this class.
          */
         private int typeInfoIndex;
         /**
-         * index in debug_info section of indirect type declaration for this class.
+         * Index in debug_info section of indirect type declaration for this class.
          *
          * this is normally just the same as the index of the normal type declaration, however, when
          * oops are stored in static and instance fields as offsets from the heapbase register gdb