Object/COFF: add function to check if section number is reserved one

[ruiu]

Summary

Looks like there's no pass to run the pieces of code in COFFObjectFile.cpp
that I modified. Are these dead code? I changed the code but can't write test
for it.

[Bigcheese]

It was probably just never tested.

[ruiu]

• added tests

[ruiu]

OK I figured out the test that uses getSection on a COFF file. Added a new test for it.

[Bigcheese]

lgtm

[ruiu]

Closed by commit rL204199 (authored by @ruiu).

llvm/trunk/include/llvm/Support/COFF.h

 
 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 #include <cstring>
 
 namespace llvm {
 namespace COFF {
 
+  // The maximum number of sections that a COFF object can have (inclusive).
+  const int MaxNumberOfSections = 65299;
+
   // The PE signature bytes that follows the DOS stub header.
   static const char PEMagic[] = { 'P', 'E', '\0', '\0' };
 
   // Sizes in bytes of various things in the COFF format.
   enum {
     HeaderSize     = 20,
     NameSize       = 8,
     SymbolSize     = 18,
namespace COFF {
 
   enum CodeViewLineTableIdentifiers {
     DEBUG_SECTION_MAGIC           = 0x4,
     DEBUG_LINE_TABLE_SUBSECTION   = 0xF2,
     DEBUG_STRING_TABLE_SUBSECTION = 0xF3,
     DEBUG_INDEX_SUBSECTION        = 0xF4
   };
 
+  inline bool isReservedSectionNumber(int N) {
+    return N == IMAGE_SYM_UNDEFINED || N > MaxNumberOfSections;
+  }
+
 } // End namespace COFF.
 } // End namespace llvm.
 
 #endif

llvm/trunk/lib/Object/COFFObjectFile.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/COFF.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/COFF.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cctype>
 #include <limits>
 
 using namespace llvm;
 using namespace object;
 
error_code COFFObjectFile::getSymbolType(DataRefImpl Ref,
   Result = SymbolRef::ST_Other;
   if (Symb->StorageClass == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
       Symb->SectionNumber == COFF::IMAGE_SYM_UNDEFINED) {
     Result = SymbolRef::ST_Unknown;
   } else if (Symb->getComplexType() == COFF::IMAGE_SYM_DTYPE_FUNCTION) {
     Result = SymbolRef::ST_Function;
   } else {
     uint32_t Characteristics = 0;
-    if (Symb->SectionNumber > 0) {
+    if (!COFF::isReservedSectionNumber(Symb->SectionNumber)) {
       const coff_section *Section = NULL;
       if (error_code EC = getSection(Symb->SectionNumber, Section))
         return EC;
       Characteristics = Section->Characteristics;
     }
     if (Characteristics & COFF::IMAGE_SCN_MEM_READ &&
         ~Characteristics & COFF::IMAGE_SCN_MEM_WRITE) // Read only.
       Result = SymbolRef::ST_Data;
error_code COFFObjectFile::getSymbolSize(DataRefImpl Ref,
   else
     Result = 0;
   return object_error::success;
 }
 
 error_code COFFObjectFile::getSymbolSection(DataRefImpl Ref,
                                             section_iterator &Result) const {
   const coff_symbol *Symb = toSymb(Ref);
-  if (Symb->SectionNumber <= COFF::IMAGE_SYM_UNDEFINED)
+  if (COFF::isReservedSectionNumber(Symb->SectionNumber)) {
     Result = section_end();
-  else {
+  } else {
     const coff_section *Sec = 0;
     if (error_code EC = getSection(Symb->SectionNumber, Sec)) return EC;
     DataRefImpl Ref;
     Ref.p = reinterpret_cast<uintptr_t>(Sec);
     Result = section_iterator(SectionRef(Ref, this));
   }
   return object_error::success;
 }
if (Index > NumEnt)
     return object_error::parse_failed;
   Res = &DataDirectory[Index];
   return object_error::success;
 }
 
 error_code COFFObjectFile::getSection(int32_t Index,
                                       const coff_section *&Result) const {
   // Check for special index values.
-  if (Index == COFF::IMAGE_SYM_UNDEFINED ||
-      Index == COFF::IMAGE_SYM_ABSOLUTE ||
-      Index == COFF::IMAGE_SYM_DEBUG)
+  if (COFF::isReservedSectionNumber(Index))
     Result = NULL;
   else if (Index > 0 && Index <= COFFHeader->NumberOfSections)
     // We already verified the section table data, so no need to check again.
     Result = SectionTable + (Index - 1);
   else
     return object_error::parse_failed;
   return object_error::success;
 }

llvm/trunk/test/tools/llvm-readobj/file-headers.test

 COFF32-NEXT: Format: COFF-i386
 COFF32-NEXT: Arch: i386
 COFF32-NEXT: AddressSize: 32bit
 COFF32-NEXT: ImageFileHeader {
 COFF32-NEXT:   Machine: IMAGE_FILE_MACHINE_I386 (0x14C)
 COFF32-NEXT:   SectionCount: 2
 COFF32-NEXT:   TimeDateStamp: 2013-03-20 17:56:46 (0x5149F85E)
 COFF32-NEXT:   PointerToSymbolTable: 0xA5
-COFF32-NEXT:   SymbolCount: 7
+COFF32-NEXT:   SymbolCount: 9
 COFF32-NEXT:   OptionalHeaderSize: 0
 COFF32-NEXT:   Characteristics [ (0x0)
 COFF32-NEXT:   ]
 COFF32-NEXT: }
 
 COFF64:      File: {{(.*[/\\])?}}trivial.obj.coff-x86-64
 COFF64-NEXT: Format: COFF-x86-64
 COFF64-NEXT: Arch: x86_64

llvm/trunk/test/tools/llvm-readobj/symbols.test

 RUN: llvm-readobj -t %p/Inputs/trivial.obj.coff-i386 \
 RUN:   | FileCheck %s -check-prefix COFF
 RUN: llvm-readobj -t %p/Inputs/trivial.obj.elf-i386 \
 RUN:   | FileCheck %s -check-prefix ELF
 
 COFF:      Symbols [
 COFF-NEXT:   Symbol {
+COFF-NEXT:     Name: @comp.id
+COFF-NEXT:    Value: 14766605
+COFF-NEXT:    Section:  (65535)
+COFF-NEXT:     BaseType: Null (0x0)
+COFF-NEXT:     ComplexType: Null (0x0)
+COFF-NEXT:    StorageClass: Static (0x3)
+COFF-NEXT:    AuxSymbolCount: 0
+COFF-NEXT:   }
+COFF-NEXT:   Symbol {
+COFF-NEXT:    Name: @feat.00
+COFF-NEXT:    Value: 2147484049
+COFF-NEXT:     Section:  (65535)
+COFF-NEXT:     BaseType: Null (0x0)
+COFF-NEXT:    ComplexType: Null (0x0)
+COFF-NEXT:    StorageClass: Static (0x3)
+COFF-NEXT:     AuxSymbolCount: 0
+COFF-NEXT:   }
+COFF-NEXT:   Symbol {
 COFF-NEXT:     Name: .text
 COFF-NEXT:     Value: 0
 COFF-NEXT:     Section: .text (1)
 COFF-NEXT:     BaseType: Null (0x0)
 COFF-NEXT:     ComplexType: Null (0x0)
 COFF-NEXT:     StorageClass: Static (0x3)
 COFF-NEXT:     AuxSymbolCount: 1
 COFF-NEXT:     AuxSectionDef {

llvm/trunk/tools/llvm-nm/llvm-nm.cpp

 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Support/COFF.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
char Ret = StringSwitch<char>(Name)
                  .StartsWith(".debug", 'N')
                  .StartsWith(".sxdata", 'N')
                  .Default('?');
 
   if (Ret != '?')
     return Ret;
 
   uint32_t Characteristics = 0;
-  if (Symb->SectionNumber > 0 &&
-      Symb->SectionNumber != llvm::COFF::IMAGE_SYM_DEBUG &&
-      Symb->SectionNumber != llvm::COFF::IMAGE_SYM_ABSOLUTE) {
+  if (!COFF::isReservedSectionNumber(Symb->SectionNumber)) {
     section_iterator SecI = Obj.section_end();
     if (error(SymI->getSection(SecI)))
       return '?';
     const coff_section *Section = Obj.getCOFFSection(*SecI);
     Characteristics = Section->Characteristics;
   }
 
   switch (Symb->SectionNumber) {

llvm/trunk/tools/llvm-readobj/COFFDumper.cpp

if (Symbol->StorageClass     == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
       W.printNumber("TotalSize", Aux->TotalSize);
       W.printHex("PointerToLineNumber", Aux->PointerToLineNumber);
       W.printHex("PointerToNextFunction", Aux->PointerToNextFunction);
       W.printBinary("Unused", makeArrayRef(Aux->Unused));
 
     } else if (
         Symbol->StorageClass   == COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL ||
         (Symbol->StorageClass  == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
-         Symbol->SectionNumber == 0 &&
+         Symbol->SectionNumber == COFF::IMAGE_SYM_UNDEFINED &&
          Symbol->Value         == 0)) {
       const coff_aux_weak_external_definition *Aux;
       if (error(getSymbolAuxData(Obj, Symbol + I, Aux)))
         break;
 
       const coff_symbol *Linked;
       StringRef LinkedName;
       error_code EC;