<div dir="ltr">Jonas, I reverted this change in r355255 as it caused <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/30050/steps/check-llvm%20ubsan/logs/stdio">UBSan failures</a>:<div><pre style="font-family:"Courier New",courier,monotype,monospace;font-size:medium"><span class="inbox-inbox-stdout">[ RUN      ] LineTableTestParams/DebugLineParameterisedFixture.GetOrParseLineTableValidTable/3
/b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:218:42: runtime error: reference binding to null pointer of type 'const llvm::DWARFUnit'
    #0 0x1792018 in parseV5DirFileTables /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:217:38
    #1 0x1792018 in llvm::DWARFDebugLine::Prologue::parse(llvm::DWARFDataExtractor const&, unsigned int*, llvm::DWARFContext const&, llvm::DWARFUnit const*) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:323
    #2 0x179357b in llvm::DWARFDebugLine::LineTable::parse(llvm::DWARFDataExtractor&, unsigned int*, llvm::DWARFContext const&, llvm::DWARFUnit const*, std::function<void (llvm::Error)>, llvm::raw_ostream*) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:482:32
    #3 0x17931ea in llvm::DWARFDebugLine::getOrParseLineTable(llvm::DWARFDataExtractor&, unsigned int, llvm::DWARFContext const&, llvm::DWARFUnit const*, std::function<void (llvm::Error)>) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:467:17
    #4 0x4ecbfa in (anonymous namespace)::DebugLineParameterisedFixture_GetOrParseLineTableValidTable_Test::TestBody() /b/sanitizer-x86_64-linux-fast/build/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp:247:33
    #5 0x1993db6 in testing::Test::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474:5
    #6 0x1994ef3 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #7 0x19958d2 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #8 0x199d2c2 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #9 0x199cd06 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
    #10 0x198c5b7 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #11 0x198c5b7 in main /b/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
    #12 0x7f315bb802e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #13 0x4474f9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/unittests/DebugInfo/DWARF/DebugInfoDWARFTests+0x4474f9)</span></pre></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 1, 2019 at 2:25 PM Jonas Devlieghere via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jdevlieghere<br>
Date: Fri Mar  1 14:14:24 2019<br>
New Revision: 355233<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=355233&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=355233&view=rev</a><br>
Log:<br>
[DWARFFormValue] Cleanup DWARFFormValue interface. (2/2) (NFC)<br>
<br>
Continues the work started in r354941. Changes (all but one) uses of the<br>
extractValue to static createFromData.<br>
<br>
Modified:<br>
    llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h<br>
    llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp<br>
    llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
    llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp<br>
    llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp<br>
    llvm/trunk/tools/dsymutil/DwarfLinker.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFFormValue.h Fri Mar  1 14:14:24 2019<br>
@@ -61,6 +61,10 @@ private:<br>
<br>
   DWARFFormValue(dwarf::Form F, ValueType V) : Form(F), Value(V) {}<br>
<br>
+  bool extractValue(const DWARFDataExtractor &Data, uint32_t *OffsetPtr,<br>
+                    dwarf::FormParams FormParams, const DWARFUnit *Unit,<br>
+                    const DWARFContext *Ctx);<br>
+<br>
 public:<br>
   DWARFFormValue(dwarf::Form F = dwarf::Form(0)) : Form(F) {}<br>
<br>
@@ -69,8 +73,13 @@ public:<br>
   static DWARFFormValue createFromPValue(dwarf::Form F, const char *V);<br>
   static DWARFFormValue createFromBlockValue(dwarf::Form F,<br>
                                              ArrayRef<uint8_t> D);<br>
-  static DWARFFormValue createFromUnit(dwarf::Form F, const DWARFUnit *Unit,<br>
-                                       uint32_t *OffsetPtr);<br>
+<br>
+  /// Creates a from value from the given data. The DWARF context form the unit<br>
+  /// is used, unless one is provided explicitly.<br>
+  static DWARFFormValue<br>
+  createFromData(dwarf::Form F, dwarf::FormParams FormParams,<br>
+                 const DWARFUnit &U, const DWARFDataExtractor &Data,<br>
+                 uint32_t *OffsetPtr, const DWARFContext *Ctx = nullptr);<br>
<br>
   dwarf::Form getForm() const { return Form; }<br>
   uint64_t getRawUValue() const { return Value.uval; }<br>
@@ -83,18 +92,10 @@ public:<br>
   static void dumpAddressSection(const DWARFObject &Obj, raw_ostream &OS,<br>
                                  DIDumpOptions DumpOpts, uint64_t SectionIndex);<br>
<br>
-  /// Extracts a value in \p Data at offset \p *OffsetPtr. The information<br>
-  /// in \p FormParams is needed to interpret some forms. The optional<br>
-  /// \p Context and \p Unit allows extracting information if the form refers<br>
-  /// to other sections (e.g., .debug_str).<br>
-  bool extractValue(const DWARFDataExtractor &Data, uint32_t *OffsetPtr,<br>
-                    dwarf::FormParams FormParams,<br>
-                    const DWARFContext *Context = nullptr,<br>
-                    const DWARFUnit *Unit = nullptr);<br>
-<br>
+  /// Legacy interface for initializing a DWARFFormValue from data.<br>
   bool extractValue(const DWARFDataExtractor &Data, uint32_t *OffsetPtr,<br>
-                    dwarf::FormParams FormParams, const DWARFUnit *U) {<br>
-    return extractValue(Data, OffsetPtr, FormParams, nullptr, U);<br>
+                    dwarf::FormParams FormParams) {<br>
+    return extractValue(Data, OffsetPtr, FormParams, nullptr, nullptr);<br>
   }<br>
<br>
   bool isInlinedCStr() const {<br>
@@ -116,20 +117,6 @@ public:<br>
<br>
   /// Skip a form's value in \p DebugInfoData at the offset specified by<br>
   /// \p OffsetPtr.<br>
-  ///<br>
-  /// Skips the bytes for the current form and updates the offset.<br>
-  ///<br>
-  /// \param DebugInfoData The data where we want to skip the value.<br>
-  /// \param OffsetPtr A reference to the offset that will be updated.<br>
-  /// \param Params DWARF parameters to help interpret forms.<br>
-  /// \returns true on success, false if the form was not skipped.<br>
-  bool skipValue(DataExtractor DebugInfoData, uint32_t *OffsetPtr,<br>
-                 const dwarf::FormParams Params) const {<br>
-    return DWARFFormValue::skipValue(Form, DebugInfoData, OffsetPtr, Params);<br>
-  }<br>
-<br>
-  /// Skip a form's value in \p DebugInfoData at the offset specified by<br>
-  /// \p OffsetPtr.<br>
   ///<br>
   /// Skips the bytes for the specified form and updates the offset.<br>
   ///<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp Fri Mar  1 14:14:24 2019<br>
@@ -166,10 +166,8 @@ Optional<DWARFFormValue> DWARFAbbreviati<br>
       if (Spec.isImplicitConst())<br>
         return DWARFFormValue::createFromSValue(Spec.Form,<br>
                                                 Spec.getImplicitConstValue());<br>
-<br>
-      DWARFFormValue FormValue(Spec.Form);<br>
-      if (FormValue.extractValue(DebugInfoData, &Offset, U.getFormParams(), &U))<br>
-        return FormValue;<br>
+      return DWARFFormValue::createFromData(Spec.Form, U.getFormParams(), U,<br>
+                                            U.getDebugInfoExtractor(), &Offset);<br>
     }<br>
     // March Offset along until we get to the attribute we want.<br>
     if (auto FixedSize = Spec.getByteSize(U))<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp Fri Mar  1 14:14:24 2019<br>
@@ -212,15 +212,14 @@ parseV5DirFileTables(const DWARFDataExtr<br>
     if (*OffsetPtr >= EndPrologueOffset)<br>
       return false;<br>
     for (auto Descriptor : DirDescriptors) {<br>
-      DWARFFormValue Value(Descriptor.Form);<br>
       switch (Descriptor.Type) {<br>
       case DW_LNCT_path:<br>
-        if (!Value.extractValue(DebugLineData, OffsetPtr, FormParams, &Ctx, U))<br>
-          return false;<br>
-        IncludeDirectories.push_back(Value);<br>
+        IncludeDirectories.push_back(DWARFFormValue::createFromData(<br>
+            Descriptor.Form, FormParams, *U, DebugLineData, OffsetPtr, &Ctx));<br>
         break;<br>
       default:<br>
-        if (!Value.skipValue(DebugLineData, OffsetPtr, FormParams))<br>
+        if (!DWARFFormValue::skipValue(Descriptor.Form, DebugLineData,<br>
+                                       OffsetPtr, FormParams))<br>
           return false;<br>
       }<br>
     }<br>
@@ -240,9 +239,8 @@ parseV5DirFileTables(const DWARFDataExtr<br>
       return false;<br>
     DWARFDebugLine::FileNameEntry FileEntry;<br>
     for (auto Descriptor : FileDescriptors) {<br>
-      DWARFFormValue Value(Descriptor.Form);<br>
-      if (!Value.extractValue(DebugLineData, OffsetPtr, FormParams, &Ctx, U))<br>
-        return false;<br>
+      DWARFFormValue Value = DWARFFormValue::createFromData(<br>
+          Descriptor.Form, FormParams, *U, DebugLineData, OffsetPtr, &Ctx);<br>
       switch (Descriptor.Type) {<br>
       case DW_LNCT_path:<br>
         FileEntry.Name = Value;<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp Fri Mar  1 14:14:24 2019<br>
@@ -278,7 +278,8 @@ static void dumpAttribute(raw_ostream &O<br>
     OS << formatv(" [{0}]", Form);<br>
<br>
   DWARFUnit *U = Die.getDwarfUnit();<br>
-  DWARFFormValue FormValue = DWARFFormValue::createFromUnit(Form, U, OffsetPtr);<br>
+  DWARFFormValue FormValue = DWARFFormValue::createFromData(<br>
+      Form, U->getFormParams(), *U, U->getDebugInfoExtractor(), OffsetPtr);<br>
<br>
   OS << "\t(";<br>
<br>
@@ -686,8 +687,9 @@ void DWARFDie::attribute_iterator::updat<br>
     uint32_t ParseOffset = AttrValue.Offset;<br>
     auto U = Die.getDwarfUnit();<br>
     assert(U && "Die must have valid DWARF unit");<br>
-    AttrValue.Value = DWARFFormValue::createFromUnit(<br>
-        AbbrDecl.getFormByIndex(Index), U, &ParseOffset);<br>
+    AttrValue.Value = DWARFFormValue::createFromData(<br>
+        AbbrDecl.getFormByIndex(Index), U->getFormParams(), *U,<br>
+        U->getDebugInfoExtractor(), &ParseOffset);<br>
     AttrValue.ByteSize = ParseOffset - AttrValue.Offset;<br>
   } else {<br>
     assert(Index == NumAttrs && "Indexes should be [0, NumAttrs) only");<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARF/DWARFFormValue.cpp Fri Mar  1 14:14:24 2019<br>
@@ -97,11 +97,14 @@ DWARFFormValue DWARFFormValue::createFro<br>
   return DWARFFormValue(F, V);<br>
 }<br>
<br>
-DWARFFormValue DWARFFormValue::createFromUnit(dwarf::Form F, const DWARFUnit *U,<br>
-                                              uint32_t *OffsetPtr) {<br>
+DWARFFormValue DWARFFormValue::createFromData(dwarf::Form F,<br>
+                                              dwarf::FormParams FormParams,<br>
+                                              const DWARFUnit &U,<br>
+                                              const DWARFDataExtractor &Data,<br>
+                                              uint32_t *OffsetPtr,<br>
+                                              const DWARFContext *Ctx) {<br>
   DWARFFormValue FormValue(F);<br>
-  FormValue.extractValue(U->getDebugInfoExtractor(), OffsetPtr,<br>
-                         U->getFormParams(), U);<br>
+  FormValue.extractValue(Data, OffsetPtr, FormParams, &U, Ctx);<br>
   return FormValue;<br>
 }<br>
<br>
@@ -231,8 +234,8 @@ bool DWARFFormValue::isFormClass(DWARFFo<br>
<br>
 bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,<br>
                                   uint32_t *OffsetPtr, dwarf::FormParams FP,<br>
-                                  const DWARFContext *Ctx,<br>
-                                  const DWARFUnit *CU) {<br>
+                                  const DWARFUnit *CU,<br>
+                                  const DWARFContext *Ctx) {<br>
   if (!Ctx && CU)<br>
     Ctx = &CU->getContext();<br>
   C = Ctx;<br>
<br>
Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=355233&r1=355232&r2=355233&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=355233&r1=355232&r2=355233&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)<br>
+++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Fri Mar  1 14:14:24 2019<br>
@@ -584,7 +584,7 @@ unsigned DwarfLinker::shouldKeepVariable<br>
     MyInfo.InDebugMap = true;<br>
     return Flags | TF_Keep;<br>
   }<br>
-  <br>
+<br>
   Optional<uint32_t> LocationIdx =<br>
       Abbrev->findAttributeIndex(dwarf::DW_AT_location);<br>
   if (!LocationIdx)<br>
@@ -754,7 +754,9 @@ void DwarfLinker::keepDIEAndDependencies<br>
       continue;<br>
     }<br>
<br>
-    Val.extractValue(Data, &Offset, Unit.getFormParams(), &Unit);<br>
+    Val = DWARFFormValue::createFromData(AttrSpec.Form, Unit.getFormParams(),<br>
+                                         Unit, Unit.getDebugInfoExtractor(),<br>
+                                         &Offset);<br>
     CompileUnit *ReferencedCU;<br>
     if (auto RefDie = resolveDIEReference(*this, DMO, Units, Val, Unit, Die,<br>
                                           ReferencedCU)) {<br>
@@ -1552,9 +1554,9 @@ DIE *DwarfLinker::DIECloner::cloneDIE(<br>
       continue;<br>
     }<br>
<br>
-    DWARFFormValue Val(AttrSpec.Form);<br>
     uint32_t AttrSize = Offset;<br>
-    Val.extractValue(Data, &Offset, U.getFormParams(), &U);<br>
+    DWARFFormValue Val = DWARFFormValue::createFromData(<br>
+        AttrSpec.Form, U.getFormParams(), U, Data, &Offset);<br>
     AttrSize = Offset - AttrSize;<br>
<br>
     OutOffset += cloneAttribute(*Die, InputDIE, DMO, Unit, StringPool, Val,<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>