[PATCH] D16292: Improved Macro emission in dwarf

[Amjad Aboud via llvm-commits]

aaboud added a comment.

Thanks David for the comments.
I uploaded a new patch.

Please, let me know if this looks right now.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:627
@@ +626,3 @@
+      U.addLabelDelta(U.getUnitDie(), dwarf::DW_AT_macro_info, CurrLabel,
+                      PrevLabel, dwarf::DW_FORM_sec_offset);
+    PrevLabel = CurrLabel;
----------------
dblaikie wrote:
> This seems wrong - a delta isn't a section offset. Once this DWARF is relocated (eg: linked with another object file containing DWARF) these deltas would become incorrect.
> 
> A label delta is constant, not a dynamic/link time relocation - representing the distance between two labels. That doesn't change when the debug info is linked and multiple macinfo sections are concatenated together.
> 
> But DWARF requires the sec_offset to represent the final offset within the section of the linked debug info. So I assume, much like DW_AT_stmt_list and DW_AT_ranges. Like those two attributes, I assume this one should be emitted using "addSectionOffset" to ensure it has the right kind of relocation, etc. (& on MachO it doesn't use a reloc - addSectionOffset handles that case too)
> 
> 
I should check more for similar cases before highjack the addLabelDelta. Thanks for pointing that out.

Also, I had a bug there, because I wanted to calculate the offset from the beginning of the section and not from the previous section!

I reverted the change in addLabelDelta function and now is using addSectionDelta function, hope that it is the right one.

do you agree?


http://reviews.llvm.org/D16292