First step towards PR21189 -- Teach llvm-readobj to dump bits of COFF symbol subsections required to debug using VS2012+

[timurrrr]

Summary

Thanks to Andrey Guskov for his help investigating this!

[timurrrr]

ping?

[nrieck]

tools/llvm-readobj/COFFDumper.cpp

622

Minor nit. This should be DictScope because the content is not a list.

649

Ditto here. Does this subsection have any content not added yet? If not, I'd just print "ProcEnd".

[timurrrr]

tools/llvm-readobj/COFFDumper.cpp

622

Done!

Interesting, I kinda think I should actually use DictScope in a few other places of printCodeViewSymbolsSubsection()...
Can you please suggest me a good criteria of choosing ListScope vs DictScope?

649

Oh, good point!

I've actually went one step further and added an extra Size sanity check.

[timurrrr]

Addressed Nico's comments

[nrieck]

As a rule of thumb: Use DictScope for key-value pairs. Usually there's only a fixed amount of these per scope, like a single symbol info. Use ListScope if you want to delimit a list of things. Usually there's a loop printing the contents, and the amount is not fixed, like a list of symbols or sections. HTH!

[timurrrr]

Added offset checks.

[majnemer]

tools/llvm-readobj/COFFDumper.cpp

618

You don't have a check to ensure that we can succeed in this getU16 call.

629

36 makes me think we didn't take into account uint32_t CodeSize = DE.getU32(&Offset);

648

Is it not valid for the DataExtractor to be at the end of the file at this point?

[timurrrr]

tools/llvm-readobj/COFFDumper.cpp

618

Per offline discussion: the check at line 620 is enough.

629

Per offline discussion: 12+4+12+4+3+1 seems to be 36

648

It isn't, see line 673.

[majnemer]

LGTM

[timurrrr]

r220526

include/llvm/Support/COFF.h

struct ImportHeader {
 
     ImportNameType getNameType() const {
       return static_cast<ImportNameType>((TypeInfo & 0x1C) >> 3);
     }
   };
 
   enum CodeViewLineTableIdentifiers {
     DEBUG_SECTION_MAGIC           = 0x4,
+    DEBUG_SYMBOL_SUBSECTION       = 0xF1,
     DEBUG_LINE_TABLE_SUBSECTION   = 0xF2,
     DEBUG_STRING_TABLE_SUBSECTION = 0xF3,
-    DEBUG_INDEX_SUBSECTION        = 0xF4
+    DEBUG_INDEX_SUBSECTION        = 0xF4,
+
+    // Symbol subsections are split into records of different types.
+    DEBUG_SYMBOL_TYPE_PROC_START = 0x1147,
+    DEBUG_SYMBOL_TYPE_PROC_END   = 0x114F
   };
 
   inline bool isReservedSectionNumber(int32_t SectionNumber) {
     return SectionNumber <= 0;
   }
 
 } // End namespace COFF.
 } // End namespace llvm.
 
 #endif

test/tools/llvm-readobj/codeview-linetables.test

 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF5
 MFUN32-NEXT:     PayloadSize: 0x24
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF1
 MFUN32-NEXT:     PayloadSize: 0x4B
+MFUN32:          ProcStart {
+MFUN32-NEXT:       FunctionName: x
+MFUN32-NEXT:       Section: _x
+MFUN32-NEXT:       CodeSize: 0xA
+MFUN32-NEXT:     }
+MFUN32-NEXT:     ProcEnd
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF2
 MFUN32-NEXT:     PayloadSize: 0x30
 MFUN32:          FunctionName: _x
 MFUN32-NEXT:   ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF5
 MFUN32-NEXT:     PayloadSize: 0x24
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF1
 MFUN32-NEXT:     PayloadSize: 0x4B
+MFUN32:          ProcStart {
+MFUN32-NEXT:       FunctionName: y
+MFUN32-NEXT:       Section: _y
+MFUN32-NEXT:       CodeSize: 0xA
+MFUN32-NEXT:     }
+MFUN32-NEXT:     ProcEnd
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF2
 MFUN32-NEXT:     PayloadSize: 0x30
 MFUN32:          FunctionName: _y
 MFUN32-NEXT:   ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF5
 MFUN32-NEXT:     PayloadSize: 0x24
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF1
 MFUN32-NEXT:     PayloadSize: 0x4B
+MFUN32:          ProcStart {
+MFUN32-NEXT:       FunctionName: f
+MFUN32-NEXT:       Section: _f
+MFUN32-NEXT:       CodeSize: 0x14
+MFUN32-NEXT:     }
+MFUN32-NEXT:     ProcEnd
 MFUN32:        ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF2
 MFUN32-NEXT:     PayloadSize: 0x40
 MFUN32:          FunctionName: _f
 MFUN32-NEXT:   ]
 MFUN32-NEXT:   Subsection [
 MFUN32-NEXT:     Type: 0xF4

 MFUN64-NEXT:   Magic: 0x4
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF1
 MFUN64-NEXT:     PayloadSize: 0x52
 MFUN64:        ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF1
 MFUN64-NEXT:     PayloadSize: 0x4B
+MFUN64:          ProcStart {
+MFUN64-NEXT:       FunctionName: x
+MFUN64-NEXT:       Section: x
+MFUN64-NEXT:       CodeSize: 0xE
+MFUN64-NEXT:     }
+MFUN64-NEXT:     ProcEnd
 MFUN64:        ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF2
 MFUN64-NEXT:     PayloadSize: 0x30
 MFUN64:          FunctionName: x
 MFUN64-NEXT:   ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF1
 MFUN64-NEXT:     PayloadSize: 0x4B
+MFUN64:          ProcStart {
+MFUN64-NEXT:       FunctionName: y
+MFUN64-NEXT:       Section: y
+MFUN64-NEXT:       CodeSize: 0xE
+MFUN64-NEXT:     }
+MFUN64-NEXT:     ProcEnd
 MFUN64:        ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF2
 MFUN64-NEXT:     PayloadSize: 0x30
 MFUN64:          FunctionName: y
 MFUN64-NEXT:   ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF1
 MFUN64-NEXT:     PayloadSize: 0x4B
+MFUN64:          ProcStart {
+MFUN64-NEXT:       FunctionName: f
+MFUN64-NEXT:       Section: f
+MFUN64-NEXT:       CodeSize: 0x18
+MFUN64-NEXT:     }
+MFUN64-NEXT:     ProcEnd
 MFUN64:        ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF2
 MFUN64-NEXT:     PayloadSize: 0x40
 MFUN64:          FunctionName: f
 MFUN64-NEXT:   ]
 MFUN64-NEXT:   Subsection [
 MFUN64-NEXT:     Type: 0xF4

 MFILE32:        ]
 MFILE32-NEXT:   Subsection [
 MFILE32-NEXT:     Type: 0xF5
 MFILE32-NEXT:     PayloadSize: 0x24
 MFILE32:        ]
 MFILE32-NEXT:   Subsection [
 MFILE32-NEXT:     Type: 0xF1
 MFILE32-NEXT:     PayloadSize: 0x4B
+MFILE32:          ProcStart {
+MFILE32-NEXT:       FunctionName: f
+MFILE32-NEXT:       Section: _f
+MFILE32-NEXT:       CodeSize: 0x14
+MFILE32-NEXT:     }
+MFILE32-NEXT:     ProcEnd
 MFILE32:        ]
 MFILE32-NEXT:   Subsection [
 MFILE32-NEXT:     Type: 0xF2
 MFILE32-NEXT:     PayloadSize: 0x64
 MFILE32:          FunctionName: _f
 MFILE32-NEXT:   ]
 MFILE32-NEXT:   Subsection [
 MFILE32-NEXT:     Type: 0xF4

 MFILE64-NEXT:   Magic: 0x4
 MFILE64-NEXT:   Subsection [
 MFILE64-NEXT:     Type: 0xF1
 MFILE64-NEXT:     PayloadSize: 0x51
 MFILE64:        ]
 MFILE64-NEXT:   Subsection [
 MFILE64-NEXT:     Type: 0xF1
 MFILE64-NEXT:     PayloadSize: 0x4B
+MFILE64:          ProcStart {
+MFILE64-NEXT:       FunctionName: f
+MFILE64-NEXT:       Section: f
+MFILE64-NEXT:       CodeSize: 0x18
+MFILE64-NEXT:     }
+MFILE64-NEXT:     ProcEnd
 MFILE64:        ]
 MFILE64-NEXT:   Subsection [
 MFILE64-NEXT:     Type: 0xF2
 MFILE64-NEXT:     PayloadSize: 0x64
 MFILE64:          FunctionName: f
 MFILE64-NEXT:   ]
 MFILE64-NEXT:   Subsection [
 MFILE64-NEXT:     Type: 0xF4

 ;     return 0;
 ;   }
 ; using 32-version of CL v17.00.61030 and v18.00.21005.1 respectively.
 RUN: llvm-readobj -s -codeview-linetables %p/Inputs/comdat-function-linetables.obj.coff-2012-i386 \
 RUN:   | FileCheck %s -check-prefix MCOMDAT
 RUN: llvm-readobj -s -codeview-linetables %p/Inputs/comdat-function-linetables.obj.coff-2013-i386 \
 RUN:   | FileCheck %s -check-prefix MCOMDAT
 
+MCOMDAT:      ProcStart {
+MCOMDAT-NEXT:   FunctionName: f
+MCOMDAT-NEXT:   Section: ?f@@YAHXZ
+MCOMDAT-NEXT:   CodeSize: 0x7
+MCOMDAT-NEXT: }
 MCOMDAT:      FunctionLineTable [
-MCOMDAT-NEXT:        FunctionName: ?f@@YAHXZ
-MCOMDAT-NEXT:        CodeSize: 0x7
-MCOMDAT-NEXT:        FilenameSegment [
-MCOMDAT-NEXT:          Filename: c:\src\test.cc
-MCOMDAT-NEXT:          +0x0: 2
-MCOMDAT-NEXT:          +0x3: 3
-MCOMDAT-NEXT:          +0x5: 4
-MCOMDAT-NEXT:        ]
-MCOMDAT-NEXT:      ]
+MCOMDAT-NEXT:   FunctionName: ?f@@YAHXZ
+MCOMDAT-NEXT:   CodeSize: 0x7
+MCOMDAT-NEXT:   FilenameSegment [
+MCOMDAT-NEXT:     Filename: c:\src\test.cc
+MCOMDAT-NEXT:     +0x0: 2
+MCOMDAT-NEXT:     +0x3: 3
+MCOMDAT-NEXT:     +0x5: 4
+MCOMDAT-NEXT:   ]
+MCOMDAT-NEXT: ]
+MCOMDAT:      ProcStart {
+MCOMDAT-NEXT:   FunctionName: g
+MCOMDAT-NEXT:   Section: ?g@@YAHXZ
+MCOMDAT-NEXT:   CodeSize: 0x7
+MCOMDAT-NEXT: }
 MCOMDAT:      FunctionLineTable [
-MCOMDAT-NEXT:        FunctionName: ?g@@YAHXZ
-MCOMDAT-NEXT:        CodeSize: 0x7
-MCOMDAT-NEXT:        FilenameSegment [
-MCOMDAT-NEXT:          Filename: c:\src\test.cc
-MCOMDAT-NEXT:          +0x0: 7
-MCOMDAT-NEXT:          +0x3: 8
-MCOMDAT-NEXT:          +0x5: 9
-MCOMDAT-NEXT:        ]
-MCOMDAT-NEXT:      ]
+MCOMDAT-NEXT:   FunctionName: ?g@@YAHXZ
+MCOMDAT-NEXT:   CodeSize: 0x7
+MCOMDAT-NEXT:   FilenameSegment [
+MCOMDAT-NEXT:     Filename: c:\src\test.cc
+MCOMDAT-NEXT:     +0x0: 7
+MCOMDAT-NEXT:     +0x3: 8
+MCOMDAT-NEXT:     +0x5: 9
+MCOMDAT-NEXT:   ]
+MCOMDAT-NEXT: ]

tools/llvm-readobj/COFFDumper.cpp

private:
   void printDataDirectory(uint32_t Index, const std::string &FieldName);
 
   template <class PEHeader> void printPEHeader(const PEHeader *Hdr);
   void printBaseOfDataField(const pe32_header *Hdr);
   void printBaseOfDataField(const pe32plus_header *Hdr);
 
   void printCodeViewLineTables(const SectionRef &Section);
 
+  void printCodeViewSymbolsSubsection(StringRef Subsection,
+                                      const SectionRef &Section,
+                                      uint32_t Offset);
+
   void cacheRelocations();
 
   std::error_code resolveSymbol(const coff_section *Section, uint64_t Offset,
                                 SymbolRef &Sym);
   std::error_code resolveSymbolName(const coff_section *Section,
                                     uint64_t Offset, StringRef &Name);
   void printImportedSymbols(iterator_range<imported_symbol_iterator> Range);
 
void COFFDumper::printCodeViewLineTables(const SectionRef &Section) {
   if (error(Section.getContents(Data)))
     return;
 
   SmallVector<StringRef, 10> FunctionNames;
   StringMap<StringRef> FunctionLineTables;
 
   ListScope D(W, "CodeViewLineTables");
   {
+    // FIXME: Add more offset correctness checks.
     DataExtractor DE(Data, true, 4);
     uint32_t Offset = 0,
              Magic = DE.getU32(&Offset);
     W.printHex("Magic", Magic);
     if (Magic != COFF::DEBUG_SECTION_MAGIC) {
       error(object_error::parse_failed);
       return;
     }
while (DE.isValidOffset(Offset) && !Finished) {
       }
 
       // Print the raw contents to simplify debugging if anything goes wrong
       // afterwards.
       StringRef Contents = Data.substr(Offset, PayloadSize);
       W.printBinaryBlock("Contents", Contents);
 
       switch (SubSectionType) {
+      case COFF::DEBUG_SYMBOL_SUBSECTION:
+        printCodeViewSymbolsSubsection(Contents, Section, Offset);
+        break;
       case COFF::DEBUG_LINE_TABLE_SUBSECTION: {
         // Holds a PC to file:line table.  Some data to parse this subsection is
         // stored in the other subsections, so just check sanity and store the
         // pointers for deferred processing.
 
         if (PayloadSize < 12) {
           // There should be at least three words to store two function
           // relocations and size of the code.
while (DE.isValidOffset(Offset)) {
         char Buffer[32];
         format("+0x%X", PC).snprint(Buffer, 32);
         W.printNumber(Buffer, LineNumber);
       }
     }
   }
 }
 
+void COFFDumper::printCodeViewSymbolsSubsection(StringRef Subsection,
+                                                const SectionRef &Section,
+                                                uint32_t OffsetInSection) {
+  if (Subsection.size() == 0) {
+    error(object_error::parse_failed);
+    return;
+  }
+  DataExtractor DE(Subsection, true, 4);
+  uint32_t Offset = 0;
+
+  // Function-level subsections have "procedure start" and "procedure end"
+  // commands that should come in pairs and surround relevant info.
+  bool InFunctionScope = false;
+  while (DE.isValidOffset(Offset)) {
+    // Read subsection segments one by one.
+    uint16_t Size = DE.getU16(&Offset);
+    // The section size includes the size of the type identifier.
+    if (Size < 2 || !DE.isValidOffsetForDataOfSize(Offset, Size)) {
+      error(object_error::parse_failed);
+      return;
+    }
+    Size -= 2;
+    uint16_t Type = DE.getU16(&Offset);
+    switch (Type) {
+    case COFF::DEBUG_SYMBOL_TYPE_PROC_START: {
+      DictScope S(W, "ProcStart");
+      if (InFunctionScope || Size < 36) {
+        error(object_error::parse_failed);
+        return;
+      }
+      InFunctionScope = true;
+
+      // We're currently interested in a limited subset of fields in this
+      // segment, just ignore the rest of the fields for now.
+      uint8_t Unused[12];
+      DE.getU8(&Offset, Unused, 12);
+      uint32_t CodeSize = DE.getU32(&Offset);
+      DE.getU8(&Offset, Unused, 12);
+      StringRef SectionName;
+      if (error(resolveSymbolName(Obj->getCOFFSection(Section),
+                                  OffsetInSection + Offset, SectionName)))
+        return;
+      Offset += 4;
+      DE.getU8(&Offset, Unused, 3);
+      StringRef FunctionName = DE.getCStr(&Offset);
+      if (!DE.isValidOffset(Offset)) {
+        error(object_error::parse_failed);
+        return;
+      }
+      W.printString("FunctionName", FunctionName);
+      W.printString("Section", SectionName);
+      W.printHex("CodeSize", CodeSize);
+
+      break;
+    }
+    case COFF::DEBUG_SYMBOL_TYPE_PROC_END: {
+      W.startLine() << "ProcEnd\n";
+      if (!InFunctionScope || Size > 0) {
+        error(object_error::parse_failed);
+        return;
+      }
+      InFunctionScope = false;
+      break;
+    }
+    default:
+      Offset += Size;
+      break;
+    }
+  }
+
+  if (InFunctionScope)
+    error(object_error::parse_failed);
+}
+
 void COFFDumper::printSections() {
   ListScope SectionsD(W, "Sections");
   int SectionNumber = 0;
   for (const SectionRef &Sec : Obj->sections()) {
     ++SectionNumber;
     const coff_section *Section = Obj->getCOFFSection(Sec);
 
     StringRef Name;