[DWARF] Fix crash for DWARFDie::dump.

[ayermolo]

Summary

When DIE is extracted manually, the DieArray is empty. When dump is invoked on aforementioned DIE it tries to extract child, even if Dump options say otherwise. Resulting in crash.

[dblaikie]

Test case?

[ayermolo]

In D99698#2665550, @dblaikie wrote:

Test case?

I am not sure how to test it. I think only way to trigger it is to manually extract DIE with children like done here: https://github.com/facebookincubator/BOLT/blob/68abc968b706b55585b1b8be315aef5d3bf90b1c/bolt/src/DWARFRewriter.cpp#L129
Then invoke dump() on it.

I guess one way would be to add this method of an extraction to an API, @maksfb or I were thinking of doing it, and modify one of the tools to use it.
Another write a small program that does it and have it as part of the test suite.

My understanding it's done that way vs using current APIs is to keep memory footprint down.

[dblaikie]

In D99698#2669200, @ayermolo wrote:

In D99698#2665550, @dblaikie wrote:

Test case?

I am not sure how to test it.

How about a unit test? We have some for the DWARF parser already.

[ayermolo]

In D99698#2669581, @dblaikie wrote:

In D99698#2669200, @ayermolo wrote:

In D99698#2665550, @dblaikie wrote:

Test case?

I am not sure how to test it.

How about a unit test? We have some for the DWARF parser already.

Ah missed it.
Are those YAML tests manually constructed? I tried obj2yaml and copied just debug parts.

1. parsing failed
2. Syntax is different then what is in the tests.
   • Name: .debug_info Type: SHT_PROGBITS AddressAlign: 0x1 Content: A6000000040000000000080100000000210086000000000000008F000000700540000000000044000000027005400000000000120000000156D5000000E100000001028C0000000302917CF200000001028C00000000049005400000000000240000000156ED00000001068C00000003029178F600000001068C00000003029170FB0000000106930000000005E900000005040698000000069D00000007A20000000500010000060100

[dblaikie]

In D99698#2672788, @ayermolo wrote:

In D99698#2669581, @dblaikie wrote:

In D99698#2669200, @ayermolo wrote:

In D99698#2665550, @dblaikie wrote:

Test case?

I am not sure how to test it.

How about a unit test? We have some for the DWARF parser already.

Ah missed it.
Are those YAML tests manually constructed? I tried obj2yaml

Sorry, I don't know too much about the yaml2obj/obj2yaml stuff - not sure what state it's in @jhenderson and @labath might have more context on the current state.

but there are also pure unit tests that use LLVM APIs to build DWARF and then test that - perhaps one of those would be suitable here?

and copied just debug parts.

1. parsing failed
2. Syntax is different then what is in the tests.
   • Name: .debug_info Type: SHT_PROGBITS AddressAlign: 0x1 Content: A6000000040000000000080100000000210086000000000000008F000000700540000000000044000000027005400000000000120000000156D5000000E100000001028C0000000302917CF200000001028C00000000049005400000000000240000000156ED00000001068C00000003029178F600000001068C00000003029170FB0000000106930000000005E900000005040698000000069D00000007A20000000500010000060100

[ayermolo]

Added a test using APIs, moved getHeaderSize from protected to public. Aligns with other get APIs that are already public.

[ayermolo]

nit

[ayermolo]

nit

[ayermolo]

nit changes

[dblaikie]

llvm/unittests/DebugInfo/DWARF/DWARFDieManualExtractTest.cpp

68

Pass in a str_ostream or the like, and validate that the dump does what the comment above says (dumping only the current DIE and not any children)

[ayermolo]

Updated test with check of dump().

[dblaikie]

Looks good, thanks!

[ayermolo]

nit: fix up clang tidy

[ayermolo]

In D99698#2687086, @dblaikie wrote:

Looks good, thanks!

Thanks for reviewing. Will ask teammates to commit once it passes all the tests. Looks like it failed clang tidy.

[ayermolo]

fixing ling/tidy.

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

uint32_t getDIEIndex(const DWARFDebugInfoEntry *Die) {
     auto First = DieArray.data();
     assert(Die >= First && Die < First + DieArray.size());
     return Die - First;
   }
 
 protected:
   const DWARFUnitHeader &getHeader() const { return Header; }
 
-  /// Size in bytes of the parsed unit header.
-  uint32_t getHeaderSize() const { return Header.getSize(); }
-
   /// Find the unit's contribution to the string offsets table and determine its
   /// length and form. The given offset is expected to be derived from the unit
   /// DIE's DW_AT_str_offsets_base attribute.
   Expected<Optional<StrOffsetsContributionDescriptor>>
   determineStringOffsetsTableContribution(DWARFDataExtractor &DA);
 
   /// Find the unit's contribution to the string offsets table and determine its
   /// length and form. The given offset is expected to be 0 in a dwo file or,
const dwarf::FormParams &getFormParams() const {
     return Header.getFormParams();
   }
   uint16_t getVersion() const { return Header.getVersion(); }
   uint8_t getAddressByteSize() const { return Header.getAddressByteSize(); }
   uint8_t getRefAddrByteSize() const { return Header.getRefAddrByteSize(); }
   uint8_t getDwarfOffsetByteSize() const {
     return Header.getDwarfOffsetByteSize();
   }
+  /// Size in bytes of the parsed unit header.
+  uint32_t getHeaderSize() const { return Header.getSize(); }
   uint64_t getLength() const { return Header.getLength(); }
   dwarf::DwarfFormat getFormat() const { return Header.getFormat(); }
   uint8_t getUnitType() const { return Header.getUnitType(); }
   bool isTypeUnit() const { return Header.isTypeUnit(); }
   uint64_t getAbbrOffset() const { return Header.getAbbrOffset(); }
   uint64_t getNextUnitOffset() const { return Header.getNextUnitOffset(); }
   const DWARFSection &getLineSection() const { return LineSection; }
   StringRef getStringSection() const { return StringSection; }

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

if (abbrCode) {
           OS << format(" [%u] %c", abbrCode,
                        AbbrevDecl->hasChildren() ? '*' : ' ');
         OS << '\n';
 
         // Dump all data in the DIE for the attributes.
         for (const DWARFAttribute &AttrValue : attributes())
           dumpAttribute(OS, *this, AttrValue, Indent, DumpOpts);
 
-        DWARFDie child = getFirstChild();
-        if (DumpOpts.ShowChildren && DumpOpts.ChildRecurseDepth > 0 && child) {
+        if (DumpOpts.ShowChildren && DumpOpts.ChildRecurseDepth > 0) {
+          DWARFDie Child = getFirstChild();
           DumpOpts.ChildRecurseDepth--;
           DIDumpOptions ChildDumpOpts = DumpOpts;
           ChildDumpOpts.ShowParents = false;
-          while (child) {
-            child.dump(OS, Indent + 2, ChildDumpOpts);
-            child = child.getSibling();
+          while (Child) {
+            Child.dump(OS, Indent + 2, ChildDumpOpts);
+            Child = Child.getSibling();
           }
         }
       } else {
         OS << "Abbreviation code not found in 'debug_abbrev' class for code: "
            << abbrCode << '\n';
       }
     } else {
       OS.indent(Indent) << "NULL\n";

llvm/unittests/DebugInfo/DWARF/CMakeLists.txt

add_llvm_unittest(DebugInfoDWARFTests
   DwarfUtils.cpp
   DWARFAcceleratorTableTest.cpp
   DWARFDataExtractorTest.cpp
   DWARFDebugArangeSetTest.cpp
   DWARFDebugFrameTest.cpp
   DWARFDebugInfoTest.cpp
   DWARFDebugLineTest.cpp
   DWARFDieTest.cpp
+  DWARFDieManualExtractTest.cpp
   DWARFExpressionCompactPrinterTest.cpp
   DWARFFormValueTest.cpp
   DWARFListTableTest.cpp
   DWARFLocationExpressionTest.cpp
   )
 
 target_link_libraries(DebugInfoDWARFTests PRIVATE LLVMTestingSupport)

llvm/unittests/DebugInfo/DWARF/DWARFDieManualExtractTest.cpp

+//===- llvm/unittest/DebugInfo/DWARFDieManualExtractTest.cpp
+//---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DwarfGenerator.h"
+#include "DwarfUtils.h"
+#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::dwarf;
+using namespace utils;
+using object::SectionedAddress;
+
+namespace {
+
+TEST(DWARFDie, manualExtractDump) {
+  typedef uint32_t AddrType;
+  uint16_t Version = 4;
+  Triple Triple = getDefaultTargetTripleForAddrSize(sizeof(AddrType));
+  if (!isObjectEmissionSupported(Triple))
+    return;
+
+  auto ExpectedDG = dwarfgen::Generator::create(Triple, Version);
+  ASSERT_THAT_EXPECTED(ExpectedDG, Succeeded());
+  dwarfgen::Generator *DG = ExpectedDG.get().get();
+  dwarfgen::CompileUnit &DGCU = DG->addCompileUnit();
+  dwarfgen::DIE CUDie = DGCU.getUnitDIE();
+
+  CUDie.addAttribute(DW_AT_name, DW_FORM_strp, "/tmp/main.c");
+  CUDie.addAttribute(DW_AT_language, DW_FORM_data2, DW_LANG_C);
+
+  dwarfgen::DIE SubprogramDie = CUDie.addChild(DW_TAG_subprogram);
+  SubprogramDie.addAttribute(DW_AT_name, DW_FORM_strp, "main");
+  SubprogramDie.addAttribute(DW_AT_low_pc, DW_FORM_addr, 0x1000U);
+  SubprogramDie.addAttribute(DW_AT_high_pc, DW_FORM_addr, 0x2000U);
+
+  StringRef FileBytes = DG->generate();
+  MemoryBufferRef FileBuffer(FileBytes, "dwarf");
+  auto Obj = object::ObjectFile::createObjectFile(FileBuffer);
+  EXPECT_TRUE((bool)Obj);
+  std::unique_ptr<DWARFContext> Ctx = DWARFContext::create(**Obj);
+
+  DWARFCompileUnit *CU = Ctx->getCompileUnitForOffset(0);
+  ASSERT_NE(nullptr, CU);
+  // Manually extracting DWARF DIE.
+  uint64_t DIEOffset = CU->getOffset() + CU->getHeaderSize();
+  uint64_t NextCUOffset = CU->getNextUnitOffset();
+  DWARFDebugInfoEntry DieInfo;
+  DWARFDataExtractor DebugInfoData = CU->getDebugInfoExtractor();
+  uint32_t Depth = 0;
+  ASSERT_TRUE(
+      DieInfo.extractFast(*CU, &DIEOffset, DebugInfoData, NextCUOffset, Depth));
+  DWARFDie Die(CU, &DieInfo);
+  ASSERT_TRUE(Die.isValid());
+  ASSERT_TRUE(Die.hasChildren());
+  // Since we have extracted manually DieArray is empty.
+  // Dump function should respect the default flags and print just current DIE,
+  // and not explore children.
+  Die.dump();
+}
+
+} // end anonymous namespace