[PATCH] D71732: Move the sysroot attribute from DIModule to DICompileUnit
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 14 14:17:35 PST 2020
dblaikie added inline comments.
================
Comment at: clang/test/CodeGen/debug-info-sysroot.c:6
+// RUN: %s -isysroot /CLANG_SYSROOT -emit-llvm -o - \
+// RUN: -debugger-tuning=gdb | FileCheck %s --check-prefix=GDB
+
----------------
side note: debugger tuning is not in the IR at all - so it probably doesn't work during LTO without passing it to the backend compile(s)/link step, somehow (not sure if any command line syntax supports that, though - maybe -mllvm somehow). At some point I'm guessing it should be added as module level metadata.
================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1421
case bitc::METADATA_MODULE: {
- if (Record.size() != 6)
+ if (Record.size() != 6 && Record.size() != 5)
return error("Invalid record");
----------------
Probably might as well phrase this as bounds - size < 5 || size > 6 - then any changes can modify the upper bound rather than rephrasing this then.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:850-851
NewCU.addString(Die, dwarf::DW_AT_name, FN);
+ if (!SysRoot.empty())
+ NewCU.addString(Die, dwarf::DW_AT_LLVM_sysroot, SysRoot);
----------------
Yeah, shouldn't this use the parameter DIUnit's SysRoot? Rather than needing/using a member of DwarfDebug?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:901
CompilationDir = DIUnit->getDirectory();
+ SysRoot = DIUnit->getSysRoot();
----------------
This looks suspicious - if SysRoot can vary per DICompileUnit, what's the e meaning of using one of those DICompileUnit's SysRoot on DwarfDebug which is cross-CU?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71732/new/
https://reviews.llvm.org/D71732
More information about the llvm-commits
mailing list