[GR-46599] Foreign debuginfo types #6625
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
Current ProgressThe current PR implements all of steps 1 2 and 3. Step 4 is complete for Linux. Step 4 for Windows still requires the PECOFF generator to be updated to generate an appropriate encoding for foreign types. n.b. as a minimum the PECOFF generator could simply continue to treat these types as though they were Java classes or interfaces. However, it would be better if it were able to use an equivalent encoding to the DWARF one. ExamplesNotable ChangesForeign types appear as interfaces in the Java type system i.e. as object references. However, they manifest (for the most part) as raw pointers at runtime, whether that be to pointers to foreign primitive values, structs, other pointers or simply undifferentiated (except, perhaps, as signed or unsigned) foreign words. The DWARF encoding uses the Java type name as a typdef to label the relevant pointer types rather than the data referenced by the pointer. So, for example, the foreign types used to model foreign pointers of type n.b. As an aside, appropriate at this point, I'll mention that I have deliberately chosen to use 'int8_t' as the name for the target type rather than This naming convention differs from the approach used with proper Java types where the Java name is used to label the underlying struct. It makes sense because a Note that this different naming convention has required a corresponding modification to the mangled name encoding employed when generating local or global symbols for the foreign types. |
adinn
force-pushed
the
foreign_debuginfo_types
branch
3 times, most recently
from
c566571 to
8d45c39
Compare
|
@olpaw @christianhaeubl You might want to have a preliminary look at this and comment on the choices I have made regarding the DWARF encoding for foreign types and how that appears when viewed via gdb. @christianhaeubl It would be very helpful if you could direct me at a simple example program that employs foreign data (whether explicitly or implicitly via, say, the GC) that I can use to verify the corrcetness of the encoding and its clarity/utility. @stooke I would welcome your input on what might be done to ensure the Windows debug info is complete now that some of the Java types appear in the generic debug model via |
adinn
force-pushed
the
foreign_debuginfo_types
branch
2 times, most recently
from
bc83d12 to
50bec16
Compare
substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/TypeEntry.java
Outdated
Show resolved
Hide resolved
|
@adinn I've had a chance to (quickly) review your changes as they impact the Windows PE/COFF code. Your fix to CVTypeSectionBuilder.java should prevent the code from dying a death the first time a foreign type is encountered. As the Windows code maps java primitive types to C types, unfortunately I don't think I can have the debugger differentiate between java char and c++ uint8_t. |
Ok, fingers crossed. Let me know if there are problems that need I need to address in the stages that feed the PECOFF generator.
Contrariwise, if that is already a lost cause then introducing foreign type info the PECOFF encoding cannot compound the damage :-) |
There was a problem hiding this comment.
The changes look good to me.
@christianhaeubl It would be very helpful if you could direct me at a simple example program that employs foreign data (whether explicitly or implicitly via, say, the GC) that I can use to verify the corrcetness of the encoding and its clarity/utility.
+1
I have deliberately chosen to use 'int8_t' as the name for the target type rather than char because of the obvious name clash with the Java type.
Is char8_t (available since C++20) too new to be used instead of int8_t?
substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/DebugInfoBase.java
Outdated
Show resolved
Hide resolved
No, I don't think it is too new but it may be unadvisable. At the moment all the generator knows is that some foreign POINTER types point to integral values with known byte size and signedness. So, I generate names accordingly like uint8_t, int32_t, etc. Much as I could swap char for int when the bit count is 8, I think I prefer the above uniformity to consistency with the slightly weird norms of C nomenclature. The original distinction between char for single byte integral values vs various, possibly short or long, flavours of int for multi byte integral values is a somewhat unfortunate artefact of C pre-history. It has eventually landed us with the nonsense of char8_t and int8_t both having the same sense in terms of the underlying integral representation yet being distinguished as a sop to different intended usage (we won't even consider the merits of long64_t and int64_t). That distinction also foreshadows the need for char16_t (wide chars) vs, say, short in or int16_t. I think I'd rather avoid the whole can of worms. |
|
@adinn: here are a few examples for
|
|
Running in internal CI to find potential issues in internal gates ... ⌛ |
|
I'll take it from here. Small adjustments to get this merged will be on #6774 |
|
@adinn unfortunately I do get NPEs in EE gates. They fail with: I think it has to do with compressed references. I will keep you updated. |
|
@olpaw Hmm, it would be interesting to know what the struct and struct field are when this blows up. The assumption at the point where the NPE happens is that an ADDRESS struct field will be typed by dereferencing a foreign pointer type to a known foreign target type. The pointer type is the derived from return type of the ADDRESS getter. Which means the filed itself must either be a value of the target type or an array of such values. That explains why the call to getPointerTo() is unchecked (it should, of course, have been asserted non-null). It might well be that in this specific case we have a struct field whose ADDRESS getter return a pointer to a foreign primitive type (or maybe a Word type?). I may not have correctly catered for that case. If you have a problem debugging this and can provide a reproducer I will be happy to help investigate. |
|
@olpaw n.b. when I say a pointer type I mean a ForeignTypeEntry derived from a ResolvedJavaType which 1) is a subclass of PointerBase and 2) has attached element info with kind |
|
@olpaw I think I may understand where the problem is coming from here. It is quite possible for ForeignTypeEntry::isPointer() to return true and ForeignTypeEntry::getPointerTo() to return false. Indeed that case is catered for when writing the info for the type by generating a So, when defining the type for an embedded (ADDRESS getter) struct element/element array field this case needs to be detected and handled correctly. The current code assumes that the 'isPointer()' Unfortunately, when this return value is null because I have encoded the pointer type as |
@adinn the struct in question is I can only reproduce with |
|
Adding diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ForeignTypeEntry.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ForeignTypeEntry.java
index 825984255b8..05c543da943 100644
--- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ForeignTypeEntry.java
+++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/debugentry/ForeignTypeEntry.java
@@ -78,6 +78,8 @@ public class ForeignTypeEntry extends ClassEntry {
ResolvedJavaType referent = debugForeignTypeInfo.pointerTo();
if (referent != null) {
pointerTo = debugInfoBase.lookupTypeEntry(referent);
+ } else {
+ System.out.println("FLAG_POINTER without pointerTo: " + debugForeignTypeInfo.typeName() + '(' + debugForeignTypeInfo.typedefName() + ')');
}
} else if (debugForeignTypeInfo.isIntegral()) {
flags = FLAG_INTEGRAL;shows me the following problematic cases: |
|
@olpaw I believe the assumption that an ADDRESS-only struct field has a known foreign type is actually valid. What is missing -- apart from the asserts that I have just pushed a patch which adds the secondary lookup. It also asserts that |
|
There is one more aarch64 specific issue. On EE I see on some gates. I assume I just need to increase |
Yes, I guess that is all that will be needed. It would probably be best to eyeball the instruction sequence just to be sure that the EE build is still terminating the prologue with an instruction that matches |
The issue was something else entirely. |
|
PR is now in the merge queue and should land on master today. |
|
Merged as #6774 |
This PR provides a fix for issue #6142.
It involves 4 sets of changes:
DebugForeignTypeInfo, information about Java interfaces (and a small number of classes) derived fromWordBaseandPointerBasewhose purpose is to model foreign data (or, in the case of classes, foreign, raw pointers to Java data).Modification of
NativeImageDebugInfoProviderto identify the relevant types and advertise them via the new API.Modification of the generic debug info model in package
com.oracle.objectfile.debugentryto accomodate foreign type info.Modification of the Linux (and Windows) info generator to output a DWARF (or PECOFF) encoding of the foreign types that is intelligible to the relevant debugger.