[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > I think this can be solved by setting the line number to the line number of, > e.g., the `class` / `struct` keyword or the line number of the class name, or > whatever is easily available Possibly, though I'm not sure if we should. We don't put a line number on an implicit ctor, for instance - I think it's fine to leave off the line number & write off the one time change in metrics. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
vogelsgesang wrote: Afaict, the issue is not that the address gets optimized out, but rather that we are setting `LineNo` to 0: ``` DBuilder.createGlobalVariableExpression( TheCU, SymbolName, VTable->getName(), Unit, /*LineNo=*/0, getOrCreateType(VoidPtr, Unit), VTable->hasLocalLinkage(), /*isDefined=*/true, nullptr, DT, /*TemplateParameters=*/nullptr, PAlign); ``` I think this can be solved by setting the line number to the line number of, e.g., the `class` / `struct` keyword or the line number of the class name, or whatever is easily available https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
jmorse wrote: Interesting -- I suppose the new variable recording where vtables are count as a source-variable, and in some circumstances the vtable address must be optimised away? (Whole program devirtualisation for example). I'd suggest that this isn't a concerning reduction in coverage (because it's new coverage we're adding, but not 100% of the time). Is the 100% source-location-coverage for google a target that you want to maintain, or is it alright to accept the new lower coverage measurement? We could try to claw back some of the coverage (perhaps by not emitting vtable locations if they're likely to be optimised away?), but it'd be an uphill battle. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
asmok-g wrote: Heads-up: I don't know if this is expected but our (google's) debug metrics report for some targets show decreased dwarfdump coverage as follows: `#vars_with_source_location/#total_var` dropped from 1 to 0.988 or to 9.85. One example target is the loader target [from Tensorflow](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/summary/BUILD). https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @dwblaikie @labath @pogo59 @jmorse @Michael137 @tromey Many thanks for all your valuable feedback. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso closed https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + if (!CGM.getTarget().getCXXABI().isItaniumFamily()) +return; + + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + StringRef SymbolName = "_vtable$"; + const DeclContext *DC = static_cast(RD); CarlosAlbertoEnciso wrote: You are correct. Removed the redundant cast. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,57 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + if (!CGM.getTarget().getCXXABI().isItaniumFamily()) +return; + + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; Michael137 wrote: ```suggestion ``` unused? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + if (!CGM.getTarget().getCXXABI().isItaniumFamily()) +return; + + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + StringRef SymbolName = "_vtable$"; + const DeclContext *DC = static_cast(RD); Michael137 wrote: I *think* this cast is redundant. You should be able to just pass `RD` into `getContextDescriptor` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @labath @pogo59 @jmorse @Michael137 @tromey Is there any additional points about the current patch that you would like to address? Thanks https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @tromey You added a note about a missing test for the case where a class is defined inside a function. That case is handled by `clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > Could you remove the LLVM tests? They don't add coverage since tnhis isn't an > LLVM feature - as far as LLVM is concerned, this is another static member > variable like any other. Removed all the LLVM tests. > > Why do some of the test cases use distinct input files/headers? I'd have > thought these could all be written as standalone files with classes all > written in a single file? (easier to read the test case in isolation, no > implication that the header-ness matters (like this shouldn't be behaving > differently because some content exists in a header V not, right?)) Moved all the code into a single file. The performed checks are the same. Uploaded updated patch. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/dwblaikie commented: Could you remove the LLVM tests? They don't add coverage since tnhis isn't an LLVM feature - as far as LLVM is concerned, this is another static member variable like any other. Why do some of the test cases use distinct input files/headers? I'd have thought these could all be written as standalone files with classes all written in a single file? (easier to read the test case in isolation, no implication that the header-ness matters (like this shouldn't be behaving differently because some content exists in a header V not, right?)) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @dwblaikie I have used bloaty to get some debug info size changes. The VTable work is based on this specific revision c02019141. I built Clang using `DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang`: - at that specific revision c02019141 (original Clang) --> **original**. - and that revision with the current patch applied --> **modified**. Using those builds I built Clang twice with debug info, using `DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS=clang`: Using **original** --> **reference** Using **modified** --> **vtables** These are the sizes reported by `bloaty` ``` $ [...]./bloaty [...]/vtables/bin/clang++ -- [...]/reference/bin/clang++ FILE SIZEVM SIZE -- -- +0.2% +436Ki [ = ] 0.debug_str +0.1% +361Ki [ = ] 0.debug_info +3.2% +300Ki [ = ] 0.debug_abbrev +0.3% +122Ki [ = ] 0.debug_addr +0.1% +69.9Ki [ = ] 0.debug_str_offsets +0.0% +9.75Ki [ = ] 0.debug_line +0.0% +2.51Ki [ = ] 0.debug_rnglists +0.0% +21 [ = ] 0.debug_line_str +9.9% +15 [ = ] 0.comment +0.0% +1 [ = ] 0.debug_loclists +0.1% +1.27Mi [ = ] 0TOTAL $ ls -Ll [...]/vtables/bin/clang++ [...]/reference/bin/clang++ -rwxrwxr-x 1 [...] 1678577192 May 20 10:53 [...]/vtables/bin/clang++ -rwxrwxr-x 1 [...] 1677243032 May 20 10:35 [...]/reference/bin/clang++ ``` ``` $ [...]./bloaty [...]/vtables/bin/llc -- [...]/reference/bin/llc FILE SIZEVM SIZE -- -- +0.2% +291Ki [ = ] 0.debug_str +0.1% +283Ki [ = ] 0.debug_info +3.0% +202Ki [ = ] 0.debug_abbrev +0.3% +71.1Ki [ = ] 0.debug_addr +0.1% +42.0Ki [ = ] 0.debug_str_offsets +0.0% +1.94Ki [ = ] 0.debug_rnglists +0.0%+486 [ = ] 0.debug_line +0.0% +18 [ = ] 0.debug_line_str +9.9% +15 [ = ] 0.comment +0.0% +1 [ = ] 0.debug_loclists +0.1% +892Ki [ = ] 0TOTAL ``` ``` $ [...]./bloaty [...]/vtables/bin/opt -- [...]/reference/bin/opt FILE SIZEVM SIZE -- -- +0.2% +293Ki [ = ] 0.debug_str +0.1% +283Ki [ = ] 0.debug_info +3.0% +202Ki [ = ] 0.debug_abbrev +0.3% +71.3Ki [ = ] 0.debug_addr +0.1% +42.1Ki [ = ] 0.debug_str_offsets +0.0% +1.93Ki [ = ] 0.debug_rnglists +0.0%+486 [ = ] 0.debug_line +0.0% +18 [ = ] 0.debug_line_str +9.9% +15 [ = ] 0.comment +0.0% +1 [ = ] 0.debug_loclists +0.1% +895Ki [ = ] 0TOTAL ``` ``` $ [...]./bloaty [...]/vtables/bin/llvm-as -- [...]/reference/bin/llvm-as FILE SIZEVM SIZE -- -- +0.1% +59.6Ki [ = ] 0.debug_info +0.1% +34.6Ki [ = ] 0.debug_str +2.7% +29.9Ki [ = ] 0.debug_abbrev +0.3% +11.2Ki [ = ] 0.debug_addr +0.1% +6.51Ki [ = ] 0.debug_str_offsets +0.0%+288 [ = ] 0.debug_rnglists +0.0% +18 [ = ] 0.debug_line_str +9.9% +15 [ = ] 0.comment -0.0% -22 [ = ] 0.debug_line +0.1% +142Ki [ = ] 0TOTAL ``` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > Got measurements on debug info size growth or any other metrics we should be > considering? I will prepare some measurements on debug info size. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/dwblaikie commented: We can skip the llvm testing here, I think - there are no LLVM changes to test. (as far as LLVM is concerned, this is just another static member variable) Got measurements on debug info size growth or any other metrics we should be considering? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: Updated patch. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -1816,6 +1816,15 @@ class CodeGenModule : public CodeGenTypeCache { void requireVectorDestructorDefinition(const CXXRecordDecl *RD); bool classNeedsVectorDestructor(const CXXRecordDecl *RD); + // Helper to get the alignment for a variable. + unsigned getGlobalVarAlignment(const VarDecl *D = nullptr) { +LangAS AS = GetGlobalVarAddressSpace(D); +unsigned PAlign = getItaniumVTableContext().isRelativeLayout() + ? 32 + : getTarget().getPointerAlign(AS); +return PAlign; + } + CarlosAlbertoEnciso wrote: Changed function name to `getVtableGlobalVarAlignment`. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > Apologies if I missed it, but one thing I didn't see in the patch is a test > for the case where a class is defined inside a function. > > Given the discussion > [here](https://discourse.llvm.org/t/rfc-dwarfdebug-fix-and-improve-handling-imported-entities-types-and-static-local-in-subprogram-and-lexical-block-scopes/68544), > I guess this might not fully work correctly; but it seems to me that > checking that the vtable symbol is global could be done and might provide > some future-proofing. Added 2 test cases to cover when a class is define inside a function: Using `CBase` and `CDerived` from the previous test cases: - `CBase` defined at global scope and `CDerived` defined at function scope. - `CBase` and `CDerived` both defined at function scope. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso updated https://github.com/llvm/llvm-project/pull/130255 Rate limit · GitHub body { background-color: #f6f8fa; color: #24292e; font-family: -apple-system,BlinkMacSystemFont,Segoe UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol; font-size: 14px; line-height: 1.5; margin: 0; } .container { margin: 50px auto; max-width: 600px; text-align: center; padding: 0 24px; } a { color: #0366d6; text-decoration: none; } a:hover { text-decoration: underline; } h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; text-shadow: 0 1px 0 #fff; } p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; } ul { list-style: none; margin: 25px 0; padding: 0; } li { display: table-cell; font-weight: bold; width: 1%; } .logo { display: inline-block; margin-top: 35px; } .logo-img-2x { display: none; } @media only screen and (-webkit-min-device-pixel-ratio: 2), only screen and ( min--moz-device-pixel-ratio: 2), only screen and ( -o-min-device-pixel-ratio: 2/1), only screen and (min-device-pixel-ratio: 2), only screen and (min-resolution: 192dpi), only screen and (min-resolution: 2dppx) { .logo-img-1x { display: none; } .logo-img-2x { display: inline-block; } } #suggestions { margin-top: 35px; color: #ccc; } #suggestions a { color: #66; font-weight: 200; font-size: 14px; margin: 0 10px; } Whoa there! You have exceeded a secondary rate limit. Please wait a few minutes before you try again; in some cases this may take up to an hour. https://support.github.com/contact";>Contact Support — https://githubstatus.com";>GitHub Status — https://twitter.com/githubstatus";>@githubstatus ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: Apologies if I missed it, but one thing I didn't see in the patch is a test for the case where a class is defined inside a function. Given the discussion [here](https://discourse.llvm.org/t/rfc-dwarfdebug-fix-and-improve-handling-imported-entities-types-and-static-local-in-subprogram-and-lexical-block-scopes/68544), I guess this might not fully work correctly; but it seems to me that checking that the vtable symbol is global could be done and might provide some future-proofing. Thanks. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/jmorse commented: On the whole this looks fine to me with a final nit https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/jmorse edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -1816,6 +1816,15 @@ class CodeGenModule : public CodeGenTypeCache { void requireVectorDestructorDefinition(const CXXRecordDecl *RD); bool classNeedsVectorDestructor(const CXXRecordDecl *RD); + // Helper to get the alignment for a variable. + unsigned getGlobalVarAlignment(const VarDecl *D = nullptr) { +LangAS AS = GetGlobalVarAddressSpace(D); +unsigned PAlign = getItaniumVTableContext().isRelativeLayout() + ? 32 + : getTarget().getPointerAlign(AS); +return PAlign; + } + jmorse wrote: It makes sense to refactor this out; I feel the name of the function should contain "vtable" somewhere though, it's fundamentally tied to producing vtable information as there's a call to `getItaniumVTableContext`, yes? There's a small risk that someone uses it for a different purpose, which we can fix by putting "vtable" in the name. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso updated https://github.com/llvm/llvm-project/pull/130255 Rate limit · GitHub body { background-color: #f6f8fa; color: #24292e; font-family: -apple-system,BlinkMacSystemFont,Segoe UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol; font-size: 14px; line-height: 1.5; margin: 0; } .container { margin: 50px auto; max-width: 600px; text-align: center; padding: 0 24px; } a { color: #0366d6; text-decoration: none; } a:hover { text-decoration: underline; } h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; text-shadow: 0 1px 0 #fff; } p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; } ul { list-style: none; margin: 25px 0; padding: 0; } li { display: table-cell; font-weight: bold; width: 1%; } .logo { display: inline-block; margin-top: 35px; } .logo-img-2x { display: none; } @media only screen and (-webkit-min-device-pixel-ratio: 2), only screen and ( min--moz-device-pixel-ratio: 2), only screen and ( -o-min-device-pixel-ratio: 2/1), only screen and (min-device-pixel-ratio: 2), only screen and (min-resolution: 192dpi), only screen and (min-resolution: 2dppx) { .logo-img-1x { display: none; } .logo-img-2x { display: inline-block; } } #suggestions { margin-top: 35px; color: #ccc; } #suggestions a { color: #66; font-weight: 200; font-size: 14px; margin: 0 10px; } Whoa there! You have exceeded a secondary rate limit. Please wait a few minutes before you try again; in some cases this may take up to an hour. https://support.github.com/contact";>Contact Support — https://githubstatus.com";>GitHub Status — https://twitter.com/githubstatus";>@githubstatus ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
jmorse wrote: It sounds like there's agreement that the "before" approach was better/acceptable, i.e. having a CU-level variable that refers by `DW_AT_specification` to a variable in the class type. Doing so would also avoid the customisation for vtable-addresses in the latest patch with the `createGlobalVariableVTableDIE` method, which'd be neater. With that in mind, we'll head back in that direction. It's also worth noting that this has spawned some DWARF issues such as https://dwarfstd.org/issues/250506.2.html , but I feel that's "future work". https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > As a note - when you say "at the CU scope" do you mean a direct child of the > CU, or anything outside a function or class definition? (ie: could be inside > a namespace) - Clang puts definitions, I think, in the namespace nearest the > declaration for the definition - compare these: > https://godbolt.org/z/EoK4noe7o Outside of function bodies is probably good enough. For me conceptually the vtable is an artificial global, but I could understand wanting it to be in a namespace or whatever. And really if one were going that route, having the vtable object be a function-scoped static would also make sense. It's just that this incurs a new cost on the debuginfo reader -- but not for any deep source-related reason, because these aren't source-accessible objects anyway. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > > My intent (haven't checked the patch) is that it'd be modeled as a static > > member variable - so there'd be a declaration in the class, but a > > definition DIE outside the class that'd be indexed by gdb OK, I'd have > > thought? (it'd go in .debug_names, and gdb_index, I think - figure gdb > > would parse/index the definition DIE?) > > I think this would be fine. The crucial thing, I think, is that there's some > indication at the CU scope. This way the initial scan can take note of the > global and its address; then fully read the CU if the class type is needed at > some point. As a note - when you say "at the CU scope" do you mean a direct child of the CU, or anything outside a function or class definition? (ie: could be inside a namespace) - Clang puts definitions, I think, in the namespace nearest the declaration for the definition - compare these: https://godbolt.org/z/EoK4noe7o https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
pogo59 wrote: I like modeling it as an artificial static member, which I *think* is the "before the patch" version from https://github.com/llvm/llvm-project/pull/130255#issuecomment-2866460040 The CU-level variable definition has a DW_AT_specification pointing to its declaration within the class type (which is using DW_AT_specification correctly), letting you find the class type from the variable. The declaration within the class type has the vtable's linkage name, which lets you find the vtable from the class type. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > My intent (haven't checked the patch) is that it'd be modeled as a static > member variable - so there'd be a declaration in the class, but a definition > DIE outside the class that'd be indexed by gdb OK, I'd have thought? (it'd go > in .debug_names, and gdb_index, I think - figure gdb would parse/index the > definition DIE?) I think this would be fine. The crucial thing, I think, is that there's some indication at the CU scope. This way the initial scan can take note of the global and its address; then fully read the CU if the class type is needed at some point. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: (I'm open to being overruled by other folks/perspectives if the straight up global variable is preferred - other folks who are in support of that?) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: Ah, yeah, I see the example from https://github.com/llvm/llvm-project/pull/130255#issuecomment-2866460040 isn't consistent with what I had in mind (in the example there the member DIE is a definition - I don't think many consumers will be ready to handle that, since it's not how even inline-defined static member variables tend to be rendered in DWARF today) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > > Uploaded a patch that eliminates the global variable and it moves the > > vtable information into the static member; in that way, a consumer always > > will have access to the vtable information, just by having the object > > instance or the object definition. > > IIUC this means that to see the vtable for a class, the debugger has to scan > the class declaration itself -- the vtable isn't a separate CU-level global > variable but is instead a static member of the class. > > This approach won't work well for gdb. gdb tries not to scan DIEs that it > does not need, in order to improve startup times. In particular it tries not > to scan function bodies until necessary. My intent (haven't checked the patch) is that it'd be modeled as a static member variable - so there'd be a declaration in the class, but a definition DIE outside the class that'd be indexed by gdb OK, I'd have thought? (it'd go in .debug_names, and gdb_index, I think - figure gdb would parse/index the definition DIE?) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > Uploaded a patch that eliminates the global variable and it moves the vtable > information into the static member; in that way, a consumer always will have > access to the vtable information, just by having the object instance or the > object definition. IIUC this means that to see the vtable for a class, the debugger has to scan the class declaration itself -- the vtable isn't a separate CU-level global variable but is instead a static member of the class. This approach won't work well for gdb. gdb tries not to scan DIEs that it does not need, in order to improve startup times. In particular it tries not to scan function bodies until necessary. OTOH having a separate global variable representing the vtable itself is reasonably easy to handle. And, it would solve the "function-local class" problem for gdb. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: Uploaded a patch that eliminates the global variable and it moves the vtable information into the static member; in that way, a consumer always will have access to the vtable information, just by having an object instance or the object definition. The patch has been tested by the (SCE) debugger team. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.cpp clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.h clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.cpp clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.h clang/test/CodeGenCXX/vtable-debug-info-inheritance-diamond.cpp clang/test/CodeGenCXX/vtable-debug-info-inheritance-multiple.cpp clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp clang/test/CodeGenCXX/vtable-debug-info-inheritance-virtual.cpp clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX/debug-info-class.cpp clang/test/CodeGenCXX/debug-info-template-member.cpp clang/test/Modules/ExtDebugInfo.cpp llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h `` View the diff from clang-format here. ``diff diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp index 3a2654f18..47f011ae5 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp @@ -228,7 +228,7 @@ void DwarfCompileUnit::createGlobalVariableVTableDIE( // attribute and assume that they'll be a specification DIE somewhere // else that refers to it. Skip the DW_AT_declaration generation. DIE *VariableSpecDIE = - getOrCreateStaticMemberDIE(SDMDecl, /*IsDeclaration*/false); + getOrCreateStaticMemberDIE(SDMDecl, /*IsDeclaration*/ false); if (uint32_t AlignInBytes = GV->getAlignInBytes()) addUInt(*VariableSpecDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata, `` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso updated https://github.com/llvm/llvm-project/pull/130255 >From 4bd0c48e12114301d8b81e9abe59e538684a6f71 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Tue, 25 Feb 2025 09:23:24 + Subject: [PATCH 1/4] [DebugInfo] Add symbol for debugger with VTable information. The IR now includes a global variable for the debugger that holds the address of the vtable. Now every class that contains virtual functions, has a static member (marked as artificial) that identifies where that vtable is loaded in memory. The unmangled name is '_vtable$'. This new symbol will allow a debugger to easily associate classes with the physical location of their VTables using only the DWARF information. Previously, this had to be done by searching for ELF symbols with matching names; something that was time-consuming and error-prone in certain edge cases. --- clang/lib/CodeGen/CGDebugInfo.cpp | 53 + clang/lib/CodeGen/CGDebugInfo.h | 3 + clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 + ...ble-debug-info-inheritance-simple-base.cpp | 14 ++ ...table-debug-info-inheritance-simple-base.h | 15 ++ ...-debug-info-inheritance-simple-derived.cpp | 13 ++ ...le-debug-info-inheritance-simple-derived.h | 14 ++ clang/test/CodeGenCXX/debug-info-class.cpp| 26 ++- .../CodeGenCXX/debug-info-template-member.cpp | 52 ++--- .../vtable-debug-info-inheritance-diamond.cpp | 87 ...vtable-debug-info-inheritance-multiple.cpp | 72 ++ ...ble-debug-info-inheritance-simple-main.cpp | 87 .../vtable-debug-info-inheritance-simple.cpp | 55 + .../vtable-debug-info-inheritance-virtual.cpp | 87 clang/test/Modules/ExtDebugInfo.cpp | 10 +- .../vtable-debug-info-inheritance-simple.ll | 206 ++ 16 files changed, 755 insertions(+), 43 deletions(-) create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.h create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.h create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-diamond.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-multiple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-virtual.cpp create mode 100644 llvm/test/DebugInfo/X86/vtable-debug-info-inheritance-simple.ll diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0e6daa42ee7bf..9cadeadc54111 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBu
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso updated https://github.com/llvm/llvm-project/pull/130255 Rate limit · GitHub body { background-color: #f6f8fa; color: #24292e; font-family: -apple-system,BlinkMacSystemFont,Segoe UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol; font-size: 14px; line-height: 1.5; margin: 0; } .container { margin: 50px auto; max-width: 600px; text-align: center; padding: 0 24px; } a { color: #0366d6; text-decoration: none; } a:hover { text-decoration: underline; } h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; text-shadow: 0 1px 0 #fff; } p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; } ul { list-style: none; margin: 25px 0; padding: 0; } li { display: table-cell; font-weight: bold; width: 1%; } .logo { display: inline-block; margin-top: 35px; } .logo-img-2x { display: none; } @media only screen and (-webkit-min-device-pixel-ratio: 2), only screen and ( min--moz-device-pixel-ratio: 2), only screen and ( -o-min-device-pixel-ratio: 2/1), only screen and (min-device-pixel-ratio: 2), only screen and (min-resolution: 192dpi), only screen and (min-resolution: 2dppx) { .logo-img-1x { display: none; } .logo-img-2x { display: inline-block; } } #suggestions { margin-top: 35px; color: #ccc; } #suggestions a { color: #66; font-weight: 200; font-size: 14px; margin: 0 10px; } Whoa there! You have exceeded a secondary rate limit. Please wait a few minutes before you try again; in some cases this may take up to an hour. https://support.github.com/contact";>Contact Support — https://githubstatus.com";>GitHub Status — https://twitter.com/githubstatus";>@githubstatus ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
clayborg wrote: FYI: There is already VTable support in our lldb::SBValue class and it is part of the public API in LLDB and doesn't require any of this: ``` /// If this value represents a C++ class that has a vtable, return an value /// that represents the virtual function table. /// /// SBValue::GetError() will be in the success state if this value represents /// a C++ class with a vtable, or an appropriate error describing that the /// object isn't a C++ class with a vtable or not a C++ class. /// /// SBValue::GetName() will be the demangled symbol name for the virtual /// function table like "vtable for ". /// /// SBValue::GetValue() will be the address of the first vtable entry if the /// current SBValue is a class with a vtable, or nothing the current SBValue /// is not a C++ class or not a C++ class that has a vtable. /// /// SBValue::GetValueAtUnsigned(...) will return the address of the first /// vtable entry. /// /// SBValue::GetLoadAddress() will return the address of the vtable pointer /// found in the parent SBValue. /// /// SBValue::GetNumChildren() will return the number of virtual function /// pointers in the vtable, or zero on error. /// /// SBValue::GetChildAtIndex(...) will return each virtual function pointer /// as a SBValue object. /// /// The child SBValue objects will have the following values: /// /// SBValue::GetError() will indicate success if the vtable entry was /// successfully read from memory, or an error if not. /// /// SBValue::GetName() will be the vtable function index in the form "[%u]" /// where %u is the index. /// /// SBValue::GetValue() will be the virtual function pointer value as a /// string. /// /// SBValue::GetValueAtUnsigned(...) will return the virtual function /// pointer value. /// /// SBValue::GetLoadAddress() will return the address of the virtual function /// pointer. /// /// SBValue::GetNumChildren() returns 0 lldb::SBValue lldb::SBValue::GetVTable(); ``` So you can do this: ``` $ cat main.cpp 1#include 2 3class Foo { 4public: 5 virtual ~Foo() = default; 6 virtual void Dump() { 7puts(__PRETTY_FUNCTION__); 8 } 9}; 10 11 int main(int argc, const char **argv) { 12 Foo f; 13 f.Dump(); 14 return 0; 15 } ``` Then when you debug: ``` (lldb) script Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D. >>> v = lldb.frame.FindVariable('f') >>> v.GetVTable() vtable for Foo = 0x00014030 { [0] = 0x00013ea4 a.out`Foo::~Foo() at main.cpp:5 [1] = 0x00013ef4 a.out`Foo::~Foo() at main.cpp:5 [2] = 0x00013e7c a.out`Foo::Dump() at main.cpp:6 } ``` Doesn't require any debug info. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > > Removing the vtable global variable and moving the "location info" into the > > static within the class, will work for the SCE debugger. > > I was thinking about this last night and wondering if the vtable will appear > as a class member even if the class is local to a function? > > If so then it seems like this would be hard for gdb to find (can't speak for > other debuggers). The issue being that gdb tends not to read DIEs that it > thinks are uninteresting, and this means function bodies in general are > skipped. > > If the vtable were a global-but-artificial object, then it would readily be > found by the initial scan. Hmm, so I think the idea was that there'd still be an out-of-line definition (like for an inline static class member variable with storage - the DWARF contains a definition DIE that's outside the class, eg: https://godbolt.org/z/z3Kz8Eorn ) - though since static class members can't exist in function-local classes we can't quite extrapolate from there. I'd say we can extrapolate from function-local types member function definitions, which appear outside the function the type is local to: https://godbolt.org/z/zvf15YKhE https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > Removing the vtable global variable and moving the "location info" into the > static within the class, will work for the SCE debugger. I was thinking about this last night and wondering if the vtable will appear as a class member even if the class is local to a function? If so then it seems like this would be hard for gdb to find (can't speak for other debuggers). The issue being that gdb tends not to read DIEs that it thinks are uninteresting, and this means function bodies in general are skipped. If the vtable were a global-but-artificial object, then it would readily be found by the initial scan. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @dwblaikie: > Not sure why it'd be necessary to make that vtable global variable "global" > rather than static within the class? Is that for debug_names lookup? (I think > static members are still in the index, right?) If it's a class member you can > still do bidirectional lookup, right? IF you find the variable, you can find > its parent to see which class it applies to, and if you have the class you > can find the vtable variable inside it? ``` 0x004c: DW_TAG_structure_type ("CDerived") ... 0x005c: DW_TAG_variable DW_AT_name ("_vtable$") DW_AT_type (0x0041 "void *") DW_AT_external (true) DW_AT_artificial (true) DW_AT_accessibility (DW_ACCESS_private) DW_AT_location (DW_OP_addrx 0x1) ... .debug_addr contents: Addrs: [ 0x 0x <- DW_OP_addrx 0x1 0x ] ``` Removing the vtable global variable and moving the "location info" into the static within the class, will work for the SCE debugger. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > FYI: There is already VTable support in our lldb::SBValue class and it is > part of the public API in LLDB and doesn't require any of this: > ... > Doesn't require any debug info. Does this/can this be used to determine the type of an object that points to that vtable, though? If so, how does lldb resolve the ambiguities discussed earlier in this PR? (it'd be great to find out there's a way to do this, but it's not clear if/how that can be done - so more details about how lldb does it, if it does, would be super interesting!) Looks like it fails under ambiguity: `test.h` ``` struct base { virtual ~base() { } int i = 3; }; base* f1(); base* f2(); ``` `test.cpp` ``` #include "test.h" namespace { struct derived: base { float f = 3.14; }; } base* f1() { return new derived(); }; void breakpoint() { } int main() { base* b1 = f1(); base* b2 = f2(); breakpoint(); } ``` `test2.cpp` ``` #include "test.h" namespace { struct derived: base { long l = 42; }; } base* f2() { return new derived(); } ``` ``` $ lldb ./a.out (lldb) b breakpoint (lldb) r (lldb) up (lldb) v *b1 (derived) *b1 = { base = (i = 3) f = 3.141 } (lldb) v *b2 (derived) *b2 = { base = (i = 3) f = 0 } ``` The latter should have a member named `l`, not `f` (with a different value too, but I assume that's just from interpreting the long bitpattern as a float). gdb fails in the same way: ``` (gdb) p *b1 $1 = ((anonymous namespace)::derived) { = { _vptr$base = 0x7d20 , i = 3 }, members of (anonymous namespace)::derived: f = 3.141 } (gdb) p *b2 $2 = ((anonymous namespace)::derived) { = { _vptr$base = 0x7d88 , i = 3 }, members of (anonymous namespace)::derived: f = 0 } ``` If you actually debug into the derived type (add a virtual function, call it in main, step into it from there in the debugger) lldb still thinks the type of derived is the float version, even when you're in the long version - gdb at least shows you the long version from inside, even though it continues to show you the float version from outside. == Unrelated to this discussion on vtables, I was curious about some related local-type-naming-collision things: if you remove the virtuality from it, lldb always thinks "derived" names the type in the main file, even when you're in the context of test2.cpp. lldb correctly follows type links (so if you print a local variable of "derived*" in test2.cpp, it prints the long version), but not naming the type in the expression evaluator, it seems. ``` (lldb) n Process 2958937 stopped * thread #1, name = 'a.out', stop reason = step over frame #0: 0x52d4 a.out`s2(b=0x5556b2d0) at test2.cpp:12:3 9} 10 void s2(base* b) { 11 derived* d = (derived*)b; -> 12 breakpoint(); 13 } (lldb) p *d (derived) { base = (i = 3) l = 42 } (lldb) p *(derived*)b; (derived) { base = (i = 3) f = 0 } ``` (whereas gdb does get this ^ right, naming the type finds the appropriate local one - and gdb rejects using the name in a context it doesn't apply, whereas lldb will resolve the name in files that don't have this file-local name (seems to resolve it to whichever instance of the name comes first in the DWARF)) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > Though I haven't thought seriously about the representation - truly off the > cuff, take with a grain of salt, etc, the static member that is the vtable > seems sort of reasonable to me. > > Not sure why it'd be necessary to make that vtable global variable "global" > rather than static within the class? Is that for debug_names lookup? (I think > static members are still in the index, right?) If it's a class member you can > still do bidirectional lookup, right? IF you find the variable, you can find > its parent to see which class it applies to, and if you have the class you > can find the vtable variable inside it? We used the global variable approach to give the debugger (SCE) a similar mechanism to an existing one to identify symbols that represent vtables. I think using just the static member will work and have the additional benefit of smaller debug info. I will check with the debugger team. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > > The link from the class to the specific vtable even seems mildly incorrect, > > in that during object construction the vtable will go through several > > different values, not just one. > > Not sure I follow this - the object is only of the type, in some sense, when > it is pointing to that particular vtable. Yeah, I agree, sorry about that. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @clayborg Thanks very much for the extra information. > FYI: There is already VTable support in our lldb::SBValue class and it is > part of the public API in LLDB and doesn't require any of this: > > ``` > $ cat main.cpp >1 #include >2 >3 class Foo { >4 public: >5virtual ~Foo() = default; >6virtual void Dump() { >7 puts(__PRETTY_FUNCTION__); >8} >9 }; >10 >11 int main(int argc, const char **argv) { >12 Foo f; >13 f.Dump(); >14 return 0; >15 } > ``` > > Then when you debug: > > ``` > (lldb) script > Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D. > >>> v = lldb.frame.FindVariable('f') > >>> v.GetVTable() > vtable for Foo = 0x00014030 { > [0] = 0x00013ea4 a.out`Foo::~Foo() at main.cpp:5 > [1] = 0x00013ef4 a.out`Foo::~Foo() at main.cpp:5 > [2] = 0x00013e7c a.out`Foo::Dump() at main.cpp:6 > } > ``` > > Doesn't require any debug info. Just a question: Can that functionality be used before the object is constructed? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > The link from the class to the specific vtable even seems mildly incorrect, > in that during object construction the vtable will go through several > different values, not just one. Not sure I follow this - the object is only of the type, in some sense, when it is pointing to that particular vtable. When the base subobject is constructed - it's only that, the base subobject (Or on destruction - once the most derived destruction has run, and the vtable is set to the base type, all the object is, in some sense, at that point, is the base subobject) Though I haven't thought seriously about the representation - truly off the cuff, take with a grain of salt, etc, the static member that is the vtable seems sort of reasonable to me. Not sure why it'd be necessary to make that vtable global variable "global" rather than static within the class? Is that for debug_names lookup? (I think static members are still in the index, right?) If it's a class member you can still do bidirectional lookup, right? IF you find the variable, you can find its parent to see which class it applies to, and if you have the class you can find the vtable variable inside it? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: `DW_AT_specification` has a fairly specific meaning in DWARF. I don't really understand why you want to link from the class type to the vtable (the reverse seems more useful to me), but I would suggest a new attribute, considering it is a new capability. The link from the class to the specific vtable even seems mildly incorrect, in that during object construction the vtable will go through several different values, not just one. Also linking from the vtable object to a member of the class seems less useful than the `DW_AT_containing_type` approach, where the link is explicitly to the type and not some member. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > > Given the _vtable$ artificial member: use the DW_AT_containing_type to find > > the vtable global variable. > > It seems to me that this attribute should refer to a type and not a variable. Good point. Another option is to use `DW_AT_specification`. ``` 0x0042: DW_TAG_variable DW_AT_specification (0x005c "_vtable$") DW_AT_linkage_name ("_ZTV8CDerived") ... 0x004c: DW_TAG_structure_type ("CDerived") ... 0x005c: DW_TAG_variable DW_AT_name ("_vtable$") DW_AT_artificial (true) DW_AT_specification <0x0042> --> VTable global variable ... ``` But potentially consumers can go into a loop if they follow both `DW_AT_specification`, without having additional knowledge about the `artificial` DIE. ``` 0x0042: DW_TAG_variable DW_AT_linkage_name ("_ZTV8CDerived") ... 0x004c: DW_TAG_structure_type ("CDerived") ... 0x005c: DW_TAG_variable DW_AT_name ("_vtable$") DW_AT_artificial (true) DW_AT_specification <0x0042> --> VTable global variable ... ``` If we remove the first `DW_AT_specification`, we lost the ability to traverse the global variables (vtable's) and find their associated compound type. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > Given the _vtable$ artificial member: use the DW_AT_containing_type to find > the vtable global variable. It seems to me that this attribute should refer to a type and not a variable. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @Michael137, @dwblaikie, @labath, @tromey Thanks very much for your valuable feedback. Is there any additional points about the current patch that you would like to address? Thanks https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: As pointed out by @Michael137 > If we're given the `_vtable$` artificial member, there's nothing useful we > can do with it right? ``` 0x0042: DW_TAG_variable DW_AT_specification (0x005c "_vtable$") DW_AT_alignment (8) DW_AT_location (DW_OP_addrx 0x1) DW_AT_linkage_name ("_ZTV8CDerived") 0x004c: DW_TAG_structure_type ("CDerived") ... 0x005c: DW_TAG_variable DW_AT_name ("_vtable$") DW_AT_type (0x0041 "void *") DW_AT_external(true) DW_AT_declaration (true) DW_AT_artificial (true) DW_AT_accessibility (DW_ACCESS_private) DW_AT_containing_type <0x0042> --> VTable global variable ... .debug_addr contents: Addrs: [ 0x 0x <- DW_OP_addrx 0x1 0x ] ``` And taking a variation of what @tromey described, maybe we can add a `DW_AT_containing_type` pointing back to the global variable, providing 2 ways to use the new vtable information entry: - Given the `_vtable$` artificial member: use the `DW_AT_containing_type` to find the vtable global variable. - Given the vtable global variable: use the `DW_AT_specification` to find the object definition. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > > > To work out which class A this refers to would involve parsing the > > > template parameter correctly and matching to the correct anonymous > > > namespace. While this technically isn’t impossible > > > > > > Are you sure about that? Anonymous types are confined to a single CU > > statically, but their values can definitely leak out at runtime. So if I'm > > stopped in a random CU and I see am object whose dynamic type is > > `(anonymous namespace)::X`, I don't see how one could determine which type > > (out of possibly many) is that vtable referring to. > > @labath I will double check with our debugger team. >From our debugger team: "You could in theory look at the ELF File symbol for the VTable symbol to work out which CU the anonymous namespace refers to, which is why we say it’s *technically* possible. You’d have to transfer that information to the debugger during loading though which we don’t currently do." https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
tromey wrote: > But, yeah, I wouldn't mind hearing more about lldb's > needs/preferences/hopes/dreams for this feature so we might get a design > that's more portable at least between SCE and LLDB. (bonus points if anyone's > got GDB's needs in mind - perhaps @tromey might be able to lend us some > insight as to how GDB does things and what they might be inclined to > use/support to improve this feature area) For C++, GDB knows the details of the Itanium ABI. When `set print object` is enabled, it uses these to find the runtime type of an object. It's somewhat buggy, though, since it too does not understand types local to a function. For Rust, we added a feature to LLVM to enable this. In Rust, a "trait object" has a vtable pointer and a pointer to the underlying real object. To discover the type of this real object, the vtable is emitted like: ``` <1><2a>: Abbrev Number: 2 (DW_TAG_variable) <2b> DW_AT_name: (indirect string, offset: 0xda): as core::ops::function::Fn<()>>::{vtable} <2f> DW_AT_type: <0x3d> <33> DW_AT_location: 9 byte block: 3 68 5e 5 0 0 0 0 0 (DW_OP_addr: 55e68) <1><3d>: Abbrev Number: 3 (DW_TAG_structure_type) <3e> DW_AT_containing_type: <0xb5> <42> DW_AT_name: (indirect string, offset: 0x178): as core::ops::function::Fn<()>>::{vtable_type} <46> DW_AT_byte_size : 48 <47> DW_AT_alignment : 8 ... members ... ``` That is, the vtable is emitted as a global variable. It's type describes the vtable itself (of course). But the vtable type has a `DW_AT_containing_type` that points to the runtime type corresponding to this particular vtable. I tend to think C++ should do something like this as well. The reason for this approach is that it makes it simple to go from some C++ object in memory to the runtime type: fetch the vtable pointer, look through the DWARF for the object at this address (which can sometimes be problematic as pointed out earlier), then examine the "containing type" to find the DWARF for the real type. Existing code for Rust is [here](https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp#L1039-L1045). https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: > It would be better if we could go from the vtable pointer straight to the > right type DIE, but I don't think we can do that without inventing a new sort > of an accelerator table (which I guess we don't have an appetite for). yeah, data address lookup (as opposed to code address lookup) is somewhat of a gap in DWARF at the moment. In /theory/ aranges held data and code addresses, but GCC only produced code addresses (LLVM produced data and code addresses, but didn't produce aranges by default because they were redundant (at least when ignoring data) with ranges)... We could revisit that in some way - it (ranges or aranges) is not a lookup table, but it does at least give quick per-CU ranges. For DWARFv5 output, if you know only indexed addresses are being used (either becuase it's Split DWARF, or by scanning the abbrevs, maybe?) maybe you can still get a per-CU "which CU covers this vtable" query that's pretty quick (not sure if DWARF compression tools like dwz would make that more difficult because the indexed addr entry might not point straight to the start of the vtable - an offset to the vtable might be used somehow (like the new addr+offset form in DWARFv6)?) But, yeah, I wouldn't mind hearing more about lldb's needs/preferences/hopes/dreams for this feature so we might get a design that's more portable at least between SCE and LLDB. (bonus points if anyone's got GDB's needs in mind - perhaps @tromey might be able to lend us some insight as to how GDB does things and what they might be inclined to use/support to improve this feature area) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); CarlosAlbertoEnciso wrote: Changed to: `const DeclContext *DC = static_cast(RD);` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso updated https://github.com/llvm/llvm-project/pull/130255 >From 4bd0c48e12114301d8b81e9abe59e538684a6f71 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Tue, 25 Feb 2025 09:23:24 + Subject: [PATCH 1/2] [DebugInfo] Add symbol for debugger with VTable information. The IR now includes a global variable for the debugger that holds the address of the vtable. Now every class that contains virtual functions, has a static member (marked as artificial) that identifies where that vtable is loaded in memory. The unmangled name is '_vtable$'. This new symbol will allow a debugger to easily associate classes with the physical location of their VTables using only the DWARF information. Previously, this had to be done by searching for ELF symbols with matching names; something that was time-consuming and error-prone in certain edge cases. --- clang/lib/CodeGen/CGDebugInfo.cpp | 53 + clang/lib/CodeGen/CGDebugInfo.h | 3 + clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 + ...ble-debug-info-inheritance-simple-base.cpp | 14 ++ ...table-debug-info-inheritance-simple-base.h | 15 ++ ...-debug-info-inheritance-simple-derived.cpp | 13 ++ ...le-debug-info-inheritance-simple-derived.h | 14 ++ clang/test/CodeGenCXX/debug-info-class.cpp| 26 ++- .../CodeGenCXX/debug-info-template-member.cpp | 52 ++--- .../vtable-debug-info-inheritance-diamond.cpp | 87 ...vtable-debug-info-inheritance-multiple.cpp | 72 ++ ...ble-debug-info-inheritance-simple-main.cpp | 87 .../vtable-debug-info-inheritance-simple.cpp | 55 + .../vtable-debug-info-inheritance-virtual.cpp | 87 clang/test/Modules/ExtDebugInfo.cpp | 10 +- .../vtable-debug-info-inheritance-simple.ll | 206 ++ 16 files changed, 755 insertions(+), 43 deletions(-) create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.h create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.h create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-diamond.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-multiple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-virtual.cpp create mode 100644 llvm/test/DebugInfo/X86/vtable-debug-info-inheritance-simple.ll diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0e6daa42ee7bf..9cadeadc54111 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBu
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > > To work out which class A this refers to would involve parsing the template > > parameter correctly and matching to the correct anonymous namespace. While > > this technically isn’t impossible > > Are you sure about that? Anonymous types are confined to a single CU > statically, but their values can definitely leak out at runtime. So if I'm > stopped in a random CU and I see am object whose dynamic type is `(anonymous > namespace)::X`, I don't see how one could determine which type (out of > possibly many) is that vtable referring to. @labath I will double check with our debugger team. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); + + // Use the same vtable pointer to global alignment for the symbol. + LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr); + unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout() +? 32 +: CGM.getTarget().getPointerAlign(AS); CarlosAlbertoEnciso wrote: The whole function `emitVTableSymbol` now is guarded. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); + + // Use the same vtable pointer to global alignment for the symbol. + LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr); + unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout() +? 32 +: CGM.getTarget().getPointerAlign(AS); CarlosAlbertoEnciso wrote: Created a helper function just to calculate the alignment. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
labath wrote: IIUC, your debugger parses all of debug info upfront and builds up a vtable pointer->class DIE map. That's not something we would want to do in lldb, but I think we could still make use of this by first searching for the type using the name from the vtable (like we do now) and then confirming the match using the new information. It would be better if we could go from the vtable pointer straight to the right type DIE, but I don't think we can do that without inventing a new sort of an accelerator table (which I guess we don't have an appetite for). > To work out which class A this refers to would involve parsing the template > parameter correctly and matching to the correct anonymous namespace. While > this technically isn’t impossible Are you sure about that? Anonymous types are confined to a single CU statically, but their values can definitely leak out at runtime. So if I'm stopped in a random CU and I see am object whose dynamic type is `(anonymous namespace)::X`, I don't see how one could determine which type (out of possibly many) is that vtable referring to. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > The DWARF currently provides access to the vtable location for /instances/ of > the class, so curious what the distinction/need is for doing this from the > class, without instances? > The need to be done for the class is to give the debugger extra information about the vtables during the debug information loading before any code is executed. We are using it to construct a map of `vtable pointer` => `Class definition` to enable the type promotion. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > The DWARF currently provides access to the vtable location for /instances/ of > the class, so curious what the distinction/need is for doing this from the > class, without instances? > > > Previously, this had to be done by searching for ELF symbols with matching > > names; something that was time-consuming and error-prone in certain edge > > cases. > > (I can appreciate that, if we are at the point of searching the symbol table, > it's not a great one - but could you talk more about the edge > cases/error-prone situations?) The simplest example of an edge-case is classes with VTables inside functions. This is because the demangled name of the VTable’s ELF symbol (e.g., `vtable for Func()::ClassA` ) is not a searchable name in the global context (in the SCE debugger at least). You could argue that it should be, but it is further complicated by things like template parameters, function overloading, anonymous namespaces etc. Another example, the demangled ELF symbol: `vtable for int (anonymous namespace)::Func<((anonymous namespace)::E)0>(int)::A`. To work out which class `A` this refers to would involve parsing the template parameter correctly and matching to the correct anonymous namespace. While this technically isn’t impossible, it would involve the debugger keeping a lot of extra information around to disambiguate these rare cases, something we’re unlikely to be able to justify. Implementing such demangling-and-interpretation would be error-prone, and the whole point of DWARF is to present information in a structured manner, so making it easy to access + interpret is part of its purpose. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: > What sort of features are you picturing building with this? > Automatic type promotion: when displaying an object through a base pointer the debugger wants to still be able to show the object’s state. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
CarlosAlbertoEnciso wrote: @Michael137, @dwblaikie Thanks for your feedback. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); CarlosAlbertoEnciso wrote: Very good point. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); CarlosAlbertoEnciso wrote: You are right. Changed to be local. https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
dwblaikie wrote: I wouldn't mind a few more details here on the motivation. > This new symbol will allow a debugger to easily associate classes with the > physical location of their VTables using only the DWARF information. What sort of features are you picturing building with this? The DWARF currently provides access to the vtable location for /instances/ of the class, so curious what the distinction/need is for doing this from the class, without instances? > Previously, this had to be done by searching for ELF symbols with matching > names; something that was time-consuming and error-prone in certain edge > cases. (I can appreciate that, if we are at the point of searching the symbol table, it's not a great one - but could you talk more about the edge cases/error-prone situations?) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. Michael137 wrote: ```suggestion // placed inside the scope of the C++ class/structure. ``` https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); + + // Use the same vtable pointer to global alignment for the symbol. + LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr); + unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout() +? 32 +: CGM.getTarget().getPointerAlign(AS); Michael137 wrote: Probably also need to guard this behind `CGM.getTarget().getCXXABI().isItaniumFamily()` or something (or even just short-circuit this entire function if we're not generating for Itanium?) https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); Michael137 wrote: Instead of calling `createArtificialType` could we just add `llvm::DINode::FlagArtificial` to the `Flags` parameter we pass to `createStaticMemberType`? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 commented: Would be great to have a simpler of getting to the vtable info! I left some nits Don't see an issue with the the general approach, but i'll let others chime in https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); + + // Use the same vtable pointer to global alignment for the symbol. + LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr); + unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout() +? 32 +: CGM.getTarget().getPointerAlign(AS); Michael137 wrote: Looks like this is how `ItaniumCXXABI::getAddrOfVTable` does it? Might be worth splitting this out into a common helper that can be shared between the two? (there's a couple more copies of this around Clang). https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); Michael137 wrote: Don't think we need the call to `internString` here? AFAIU it's just used when we require the copy of a string and don't want to heap allocate? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
@@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); Michael137 wrote: Why do we need to cast away const here? https://github.com/llvm/llvm-project/pull/130255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
https://github.com/CarlosAlbertoEnciso created https://github.com/llvm/llvm-project/pull/130255 The IR now includes a global variable for the debugger that holds the address of the vtable. Now every class that contains virtual functions, has a static member (marked as artificial) that identifies where that vtable is loaded in memory. The unmangled name is '_vtable$'. This new symbol will allow a debugger to easily associate classes with the physical location of their VTables using only the DWARF information. Previously, this had to be done by searching for ELF symbols with matching names; something that was time-consuming and error-prone in certain edge cases. >From 4bd0c48e12114301d8b81e9abe59e538684a6f71 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Tue, 25 Feb 2025 09:23:24 + Subject: [PATCH] [DebugInfo] Add symbol for debugger with VTable information. The IR now includes a global variable for the debugger that holds the address of the vtable. Now every class that contains virtual functions, has a static member (marked as artificial) that identifies where that vtable is loaded in memory. The unmangled name is '_vtable$'. This new symbol will allow a debugger to easily associate classes with the physical location of their VTables using only the DWARF information. Previously, this had to be done by searching for ELF symbols with matching names; something that was time-consuming and error-prone in certain edge cases. --- clang/lib/CodeGen/CGDebugInfo.cpp | 53 + clang/lib/CodeGen/CGDebugInfo.h | 3 + clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 + ...ble-debug-info-inheritance-simple-base.cpp | 14 ++ ...table-debug-info-inheritance-simple-base.h | 15 ++ ...-debug-info-inheritance-simple-derived.cpp | 13 ++ ...le-debug-info-inheritance-simple-derived.h | 14 ++ clang/test/CodeGenCXX/debug-info-class.cpp| 26 ++- .../CodeGenCXX/debug-info-template-member.cpp | 52 ++--- .../vtable-debug-info-inheritance-diamond.cpp | 87 ...vtable-debug-info-inheritance-multiple.cpp | 72 ++ ...ble-debug-info-inheritance-simple-main.cpp | 87 .../vtable-debug-info-inheritance-simple.cpp | 55 + .../vtable-debug-info-inheritance-virtual.cpp | 87 clang/test/Modules/ExtDebugInfo.cpp | 10 +- .../vtable-debug-info-inheritance-simple.ll | 206 ++ 16 files changed, 755 insertions(+), 43 deletions(-) create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.h create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.cpp create mode 100644 clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.h create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-diamond.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-multiple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp create mode 100644 clang/test/CodeGenCXX/vtable-debug-info-inheritance-virtual.cpp create mode 100644 llvm/test/DebugInfo/X86/vtable-debug-info-inheritance-simple.ll diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0e6daa42ee7bf..9cadeadc54111 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); +
[clang] [llvm] [clang][DebugInfo] Add symbol for debugger with VTable information. (PR #130255)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Carlos Alberto Enciso (CarlosAlbertoEnciso) Changes The IR now includes a global variable for the debugger that holds the address of the vtable. Now every class that contains virtual functions, has a static member (marked as artificial) that identifies where that vtable is loaded in memory. The unmangled name is '_vtable$'. This new symbol will allow a debugger to easily associate classes with the physical location of their VTables using only the DWARF information. Previously, this had to be done by searching for ELF symbols with matching names; something that was time-consuming and error-prone in certain edge cases. --- Patch is 44.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130255.diff 16 Files Affected: - (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+53) - (modified) clang/lib/CodeGen/CGDebugInfo.h (+3) - (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+4) - (added) clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.cpp (+14) - (added) clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-base.h (+15) - (added) clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.cpp (+13) - (added) clang/test/CodeGenCXX/Inputs/vtable-debug-info-inheritance-simple-derived.h (+14) - (modified) clang/test/CodeGenCXX/debug-info-class.cpp (+14-12) - (modified) clang/test/CodeGenCXX/debug-info-template-member.cpp (+26-26) - (added) clang/test/CodeGenCXX/vtable-debug-info-inheritance-diamond.cpp (+87) - (added) clang/test/CodeGenCXX/vtable-debug-info-inheritance-multiple.cpp (+72) - (added) clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple-main.cpp (+87) - (added) clang/test/CodeGenCXX/vtable-debug-info-inheritance-simple.cpp (+55) - (added) clang/test/CodeGenCXX/vtable-debug-info-inheritance-virtual.cpp (+87) - (modified) clang/test/Modules/ExtDebugInfo.cpp (+5-5) - (added) llvm/test/DebugInfo/X86/vtable-debug-info-inheritance-simple.ll (+206) ``diff diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0e6daa42ee7bf..9cadeadc54111 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2518,6 +2518,59 @@ StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) { return internString("_vptr$", RD->getNameAsString()); } +// Emit symbol for the debugger that points to the vtable address for +// the given class. The symbol is named as '_vtable$'. +// The debugger does not need to know any details about the contents of the +// vtable as it can work this out using its knowledge of the ABI and the +// existing information in the DWARF. The type is assumed to be 'void *'. +void CGDebugInfo::emitVTableSymbol(llvm::GlobalVariable *VTable, + const CXXRecordDecl *RD) { + ASTContext &Context = CGM.getContext(); + SmallString<64> Buffer; + Twine SymbolName = internString("_vtable$"); + StringRef SymbolNameRef = SymbolName.toStringRef(Buffer); + DeclContext *DC = static_cast(const_cast(RD)); + SourceLocation Loc; + QualType VoidPtr = Context.getPointerType(Context.VoidTy); + + // We deal with two different contexts: + // - The type for the variable, which is part of the class that has the + // vtable, is placed in the context of the DICompositeType metadata. + // - The DIGlobalVariable for the vtable is put in the DICompileUnitScope. + + // The created non-member should be mark as 'artificial'. It will be + // placed it inside the scope of the C++ class/structure. + llvm::DIScope *DContext = getContextDescriptor(cast(DC), TheCU); + auto *Ctxt = cast(DContext); + llvm::DIFile *Unit = getOrCreateFile(Loc); + llvm::DIType *VTy = getOrCreateType(VoidPtr, Unit); + llvm::DINode::DIFlags Flags = getAccessFlag(AccessSpecifier::AS_private, RD); + auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5 + ? llvm::dwarf::DW_TAG_variable + : llvm::dwarf::DW_TAG_member; + llvm::DIDerivedType *OldDT = DBuilder.createStaticMemberType( + Ctxt, SymbolNameRef, Unit, /*LineNumber=*/0, VTy, Flags, + /*Val=*/nullptr, Tag); + llvm::DIDerivedType *DT = + static_cast(DBuilder.createArtificialType(OldDT)); + + // Use the same vtable pointer to global alignment for the symbol. + LangAS AS = CGM.GetGlobalVarAddressSpace(nullptr); + unsigned PAlign = CGM.getItaniumVTableContext().isRelativeLayout() +? 32 +: CGM.getTarget().getPointerAlign(AS); + + // The global variable is in the CU scope, and links back to the type it's + // "within" via the declaration field. + llvm::DIGlobalVariableExpression *GVE = + DBuilder.createGlobalVariableExpression( + TheCU, SymbolNameRef, VTable->getName(), Unit, /*LineNo=*/0, + getOrCreateType(VoidPtr, Unit), VTable->hasLocalLinkage(), + /*isDefined=*/true, nullp