[PATCH] D106624: [DWARF] Don't process .debug_info relocations for DWO Context

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:390
 
+  enum class ContextType { ALL, DWO, MONOLITHIC };
+
----------------
This name might be a bit vague - and I don't think LLVM usually uses all-upper for enum constants. So maybe "SplitOrObjectSections" with "{Both, DWO, Object}"? I'm not really sure - open to better names.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1742-1746
+          HandleError(createError(
+              "Creating DWO Context with relocation section for " + Name));
+        else if (Type == DWARFContext::ContextType::ALL &&
+                 Name.contains(".dwo"))
+          HandleWarning(createError("Relocation " + Name + " exists"));
----------------
Any thoughts/details on why one is an error and the other is a warning? (since they're about the same content, I think?)

Hmm, also it might not be invalid to encounter relocation sections in a dwo file in general - that's the split-dwarf=single case, isn't it? (ie: in split-dwarf=single, we'd be in this codiepath for ContextType::DWO, but the dwo file is really the .o file, so it'll have relocations for the non-.dwo sections - though they can/should be silently ignored, but they aren't an error/problem)


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-header.s:24
         .section .debug_str.dwo,"MSe", at progbits,1
-dwo_TU_5:
+.dwo_TU_5:
         .asciz "V5_split_type_unit"
----------------
The changes to this file could probably be committed separately in a standalone patch before this relocation handling patch, yeah? If so, please do that and rebase this patch on top.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-rela-dwo.s:1
+# Test objective to verify warning is printed if DWO secton has relocations.
+#
----------------
This test seems a bit long (I see it's got structure types? That seems unnecessary for this purpose) - maybe a simple/single file with "extern int i; int i = 3;".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106624/new/

https://reviews.llvm.org/D106624