[PATCH] Object/COFF: change data type of SymbolNumber from int16 to uint16.

[Rui Ueyama]

On Fri, Mar 14, 2014 at 3:06 PM, Rui Ueyama <ruiu at google.com> wrote:

> Hi Bigcheese,
>
> Microsoft PE/COFF Spec clearly states that the field is of unsigned
> interger
>

Confusing typo: ... clearly states that the field is of *signed* integer

type. However, in reality, it's unsigned. If cl.exe needs to create a large
> number of sections for COMDAT sections, it will just create more than 32768
> sections. Handling large section number as negative number is not correct.
> I think this is a spec bug.
>
> http://llvm-reviews.chandlerc.com/D3088
>
> Files:
>   include/llvm/Object/COFF.h
>   include/llvm/Support/COFF.h
>   test/MC/COFF/feat00.s
>   test/MC/COFF/weak.s
>   tools/llvm-nm/llvm-nm.cpp
>
> Index: include/llvm/Object/COFF.h
> ===================================================================
> --- include/llvm/Object/COFF.h
> +++ include/llvm/Object/COFF.h
> @@ -193,7 +193,7 @@
>    } Name;
>
>    support::ulittle32_t Value;
> -  support::little16_t SectionNumber;
> +  support::ulittle16_t SectionNumber;
>
>    support::ulittle16_t Type;
>
> Index: include/llvm/Support/COFF.h
> ===================================================================
> --- include/llvm/Support/COFF.h
> +++ include/llvm/Support/COFF.h
> @@ -138,8 +138,8 @@
>    };
>
>    enum SymbolSectionNumber {
> -    IMAGE_SYM_DEBUG     = -2,
> -    IMAGE_SYM_ABSOLUTE  = -1,
> +    IMAGE_SYM_DEBUG     = 0xFFFE,
> +    IMAGE_SYM_ABSOLUTE  = 0xFFFF,
>      IMAGE_SYM_UNDEFINED = 0
>    };
>
> Index: test/MC/COFF/feat00.s
> ===================================================================
> --- test/MC/COFF/feat00.s
> +++ test/MC/COFF/feat00.s
> @@ -6,7 +6,7 @@
>  // CHECK: Symbol {
>  // CHECK:   Name: @feat.00
>  // CHECK:   Value: 123
> -// CHECK:   Section: (-1)
> +// CHECK:   Section: (65535)
>  // CHECK:   BaseType: Null (0x0)
>  // CHECK:   ComplexType: Null (0x0)
>  // CHECK:   StorageClass: External (0x2)
> Index: test/MC/COFF/weak.s
> ===================================================================
> --- test/MC/COFF/weak.s
> +++ test/MC/COFF/weak.s
> @@ -52,7 +52,7 @@
>  // CHECK:      Symbol {
>  // CHECK:        Name:                .weak._test_weak.default
>  // CHECK-NEXT:   Value:               0
> -// CHECK-NEXT:   Section:             (-1)
> +// CHECK-NEXT:   Section:             (65535)
>  // CHECK-NEXT:   BaseType:            Null
>  // CHECK-NEXT:   ComplexType:         Null
>  // CHECK-NEXT:   StorageClass:        External
> Index: tools/llvm-nm/llvm-nm.cpp
> ===================================================================
> --- tools/llvm-nm/llvm-nm.cpp
> +++ tools/llvm-nm/llvm-nm.cpp
> @@ -317,7 +317,9 @@
>      return Ret;
>
>    uint32_t Characteristics = 0;
> -  if (Symb->SectionNumber > 0) {
> +  if (Symb->SectionNumber > 0 &&
> +      Symb->SectionNumber != llvm::COFF::IMAGE_SYM_DEBUG &&
> +      Symb->SectionNumber != llvm::COFF::IMAGE_SYM_ABSOLUTE) {
>      section_iterator SecI = Obj.section_end();
>      if (error(SymI->getSection(SecI)))
>        return '?';
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140314/9ef6bc1a/attachment.html>