[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