[PATCH] D72828: [DWARF5] Added support for emission of debug_macro section.

Mon Feb 10 05:43:57 PST 2020

ikudrin added inline comments.

================
Comment at: llvm/include/llvm/BinaryFormat/Dwarf.def:830
+// DWARF v5 Macro header flags
+HANDLE_MACRO_FLAGS(0x00, OFFSET_SIZE_32)
+HANDLE_MACRO_FLAGS(0x01, OFFSET_SIZE_64)
----------------
Remove `OFFSET_SIZE_32`, as this value does not define any real flag.

================
Comment at: llvm/include/llvm/BinaryFormat/Dwarf.def:831
+HANDLE_MACRO_FLAGS(0x00, OFFSET_SIZE_32)
+HANDLE_MACRO_FLAGS(0x01, OFFSET_SIZE_64)
+HANDLE_MACRO_FLAGS(0x02, DEBUG_LINE_OFFSET)
----------------
This should be just `OFFSET_SIZE` to correspond to the other flags.

================
Comment at: llvm/include/llvm/BinaryFormat/Dwarf.def:833
+HANDLE_MACRO_FLAGS(0x02, DEBUG_LINE_OFFSET)
+HANDLE_MACRO_FLAGS(0x03, OPCODE_OPERANDS_TABLE)

----------------
The value for `OPCODE_OPERANDS_TABLE` should be `0x04`. It looks like @dblaikie already said that for one of previous versions of the patch.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2810
+  Asm->emitInt16(5);
+  assert(DwarfParams.getDwarfOffsetByteSize() == 4 && "DWARF64 not supported!");
+  // We are setting Offset and line offset flags unconditionally here,
----------------
This crashes on a Debug build, probably because `DwarfParams` is not initialized. 

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

https://reviews.llvm.org/D72828