[llvm-commits] [PATCH] PR9493, Fix for DW_AT_location

Nick Lewycky nlewycky at google.com
Tue Jun 19 15:06:03 PDT 2012


--- test/DebugInfo/X86/DW_AT_location-reference.ll      (revision 0)
+++ test/DebugInfo/X86/DW_AT_location-reference.ll      (revision 0)
@@ -0,0 +1,108 @@
+; RUN: llc -O1 -mtriple=x86_64-apple-darwin < %s | FileCheck
-check-prefix=DARWIN %s
+; RUN: llc -O1 -mtriple=x86_64-pc-linux-gnu < %s | FileCheck
-check-prefix=LINUX %s
+;Bug 9493
+;Adapted from the original test case in r127757.

It's standard to say "; PR9493" instead of "Bug 9493". There used to be
code in the test runner that would parse it and emit which PR's had
regressed, but I think that code is gone?

I'm also concerned about the fact that this test runs llc -O1. It looks
like this will work even if we change the register allocator only because
we don't actually test the contents in the DWARF location anyhow. I don't
know how easy this is to do, but it'd be great if you could simplify the
.ll file or not run it with -O1. It's acceptable as-is if you can't
simplify it, but I'm not looking forward to debugging why this test fails
some day in the future.

Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===================================================================
--- lib/CodeGen/AsmPrinter/DwarfDebug.cpp       (revision 158734)
+++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp       (working copy)
@@ -1637,10 +1637,14 @@
       break;
     }
     case dwarf::DW_AT_location: {
-      if (DIELabel *L = dyn_cast<DIELabel>(Values[i]))
-        Asm->EmitLabelDifference(L->getValue(), DwarfDebugLocSectionSym,
4);
-      else
+      if (DIELabel *L = dyn_cast<DIELabel>(Values[i])) {
+        if (Asm->MAI->doesDwarfUseLabelOffsetForRanges())
+          Asm->EmitReference(L->getValue(), dwarf::DW_EH_PE_udata4);
+        else
+          Asm->EmitLabelDifference(L->getValue(), DwarfDebugLocSectionSym,
4);
+      } else {
         Values[i]->EmitValue(Asm, Form);
+      }
       break;
     }

Why do we ignore "Form" when doesDwarfUseLabelOffsetForRanges() is true?
EH_PE_udata4 can't possibly be right, it's prefixed with EH (exception
handling) for starters. In DWARF2 which is (still) the only version of
dwarf we support, your choices for form here are any of
DW_FORM_data{1,2,4,8} or DW_FORM_block{1,2}. If the problem is
GetSizeOfEncodedValue(), please extend that.

Feel free to rename to DwarfUsesRelocationsAcrossSections. Thanks!

Nick

On 19 June 2012 12:19, Robinson, Paul <Paul.Robinson at am.sony.com> wrote:

>  Ping, now that people should be recovered from that big conference last
> week.
>
> --paulr
>
>
>  ------------------------------
> *From:* llvm-commits-bounces at cs.uiuc.edu [llvm-commits-bounces at cs.uiuc.edu]
> on behalf of Robinson, Paul [Paul.Robinson at am.sony.com]
> *Sent:* Thursday, June 07, 2012 2:13 PM
> *To:* llvm-commits at cs.uiuc.edu
> *Subject:* [llvm-commits] [PATCH] PR9493, Fix for DW_AT_location
>
>   Ping, with improved subject line.
>
> --paulr
>
>
>
>
> My first patch attempt, here goes!
>
>
>
> Fix for bug 9493.  On non-Darwin platforms, references from DW_AT_location
> to the .debug_loc section need to be relocatable.  This patch improves on
> r127757, which didn't do the right thing for Darwin (and was reverted
> pretty quickly).
>
>
>
> The original bug showed up on ARM, but because of the Darwin/non-Darwin
> difference I revised the test to target x86_64, where it was easy to try
> both cases.  Also there isn't an ARM subdirectory under test/DebugInfo and
> I didn't want to get too ambitious on my first try.  The problem isn't
> really architecture-specific but it would be easy to create an ARM
> equivalent for the non-Darwin version of the test, if anybody thought that
> was useful.
>
>
>
> One question, I did not change the name of
> doesDwarfUseLabelOffsetForRanges() or its underlying data member
> DwarfUsesLabelOffsetForRanges. These used to apply only to the DW_AT_ranges
> attribute, but now they apply to DW_AT_location as well. Change the names,
> or not?  It seemed excessive to add a second function/flag because these
> DWARF attributes will always want to be handled the same way.  If renaming
> is better, maybe DwarfUsesRelocationsAcrossSections would be more accurate.
>
> Thanks,
>
> Paul Robinson
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120619/a40ac5c0/attachment.html>


More information about the llvm-commits mailing list