kamleshbhalui added a comment. In D103131#2791881 <https://reviews.llvm.org/D103131#2791881>, @dblaikie wrote:
> In D103131#2789493 <https://reviews.llvm.org/D103131#2789493>, @probinson > wrote: > >>> Mixed feelings - somewhat in favor of "do the thing that's probably already >>> fairly tested/known to work" (GCC's thing). But open to the idea that that >>> approach has problems, for sure. >> >> "Known to work" for whom? gdb, sure, because gcc/gdb cooperate on many >> things. > > Yep. Given the diversity of ways of expressing things in DWARF, no consumer > will handle everything that could be produced in the way a producer might > intend - so we do sort of have a defacto standard of "what would GCC do/what > does GDB understand". > > But also GCC/GDB has had more implementation experience with DWARF in > general, and with any feature we haven't implemented yet in particular - that > I wouldn't throw out their representational choice too quickly - there may be > important tradeoffs that were considered, implemented, and discarded due to > limitations. > >> I'll have to check with my debugger guys whether this would work for us; and >> I'll just reiterate that it's certainly not what the DWARF spec suggests >> should be done. > > I don't agree that it's "certainly not what the DWARF spec suggests should be > done" - the spec's pretty generous and exactly how a C or ELF level alias > should be rendered in DWARF seems pretty unclear to me. For instance an alias > is a symbol in the ELF file, arguably different from a source level alias > like a typedef or using declaration that DW_TAG_imported_declaration seems to > be promoted for. > > I doubt gdb would have trouble with DW_TAG_imported_declaration > > In D103131#2791373 <https://reviews.llvm.org/D103131#2791373>, @probinson > wrote: > >> In D103131#2789493 <https://reviews.llvm.org/D103131#2789493>, @probinson >> wrote: >> >>>> Mixed feelings - somewhat in favor of "do the thing that's probably >>>> already fairly tested/known to work" (GCC's thing). But open to the idea >>>> that that approach has problems, for sure. >>> >>> "Known to work" for whom? gdb, sure, because gcc/gdb cooperate on many >>> things. I'll have to check with my debugger guys whether this would work >>> for us; and I'll just reiterate that it's certainly not what the DWARF spec >>> suggests should be done. >> >> The Sony debugger will not be able to figure out the address of "newname" >> given the DWARF as emitted by gcc (because it looks like an external >> declaration with no address). We'd much rather have the >> DW_TAG_imported_declaration that the original patch had. > > Good to know - any interest in the debugger supporting declarations without > addresses? I guess there's no need for GCC compatibility? Clang might emit > them under some circumstances... let's see... Hmm, can't find a case now. But > there are cases where it would be desirable (such as when compiling some code > at -g0 and some with debug info - when you only have the declaration on the > debug info side, and no debug info on the definition side (eg: GCC produces a > declaration of 'i' in this code: `extern int i; int main() { return i; }`)) > > @kamleshbhalui - perhaps you could run the gdb and lldb test suites without > either change, then with both the variable declaration and imported > declaration versions to see how the results vary? (Well, I guess the lldb > test suite probably won't show any changes - but maybe worth running anyway) > - along with the manual test you've described in the patch description, to > demonstrate that both solutions address that? @dblaikie I have ran gdb and lldb regression here is details.Please let me know if we should go to original fix(imported decl). **case 1) Without any change in clang:** | === gdb Summary === | | 1. of expected passes 75531 2. of unexpected failures 695 3. of expected failures 125 4. of known failures 74 5. of unresolved testcases 7 6. of untested testcases 98 7. of unsupported tests 326 8. of duplicate test names 202 | ===lldb summary=== | | Failed Tests (3): lldb-shell :: Breakpoint/jit-loader_jitlink_elf.test lldb-shell :: Breakpoint/jit-loader_rtdyld_elf.test lldb-shell :: Reproducer/TestDiscard.test Testing Time: 60.57s Unsupported: 1095 Passed : 1202 Failed : 3 | | **case 2) with imported decl change in clang** | === gdb Summary === | | 1. of expected passes 75531 2. of unexpected failures 695 3. of expected failures 125 4. of known failures 74 5. of unresolved testcases 7 6. of untested testcases 98 7. of unsupported tests 326 8. of duplicate test names 202 | ===lldb summary=== | | Failed Tests (3): lldb-shell :: Breakpoint/jit-loader_jitlink_elf.test lldb-shell :: Breakpoint/jit-loader_rtdyld_elf.test lldb-shell :: Reproducer/TestDiscard.test Testing Time: 60.57s Unsupported: 1095 Passed : 1202 Failed : 3 | | **case 2) with current change(same change as in this review) in clang** | === gdb Summary === | | 1. of expected passes 75531 2. of unexpected failures 695 3. of expected failures 125 4. of known failures 74 5. of unresolved testcases 7 6. of untested testcases 98 7. of unsupported tests 326 8. of duplicate test names 202 | ===lldb summary=== | | Failed Tests (3): lldb-shell :: Breakpoint/jit-loader_jitlink_elf.test lldb-shell :: Breakpoint/jit-loader_rtdyld_elf.test lldb-shell :: Reproducer/TestDiscard.test Testing Time: 60.57s Unsupported: 1095 Passed : 1202 Failed : 3 | Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103131/new/ https://reviews.llvm.org/D103131 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
