[PATCH] [PECOFF] Support yet another new type of weak symbol.

Rui Ueyama ruiu at google.com
Wed Mar 12 18:39:20 PDT 2014


Ping?


On Fri, Mar 7, 2014 at 5:39 PM, Rui Ueyama <ruiu at google.com> wrote:

> Hi Bigcheese, shankarke, kledzik,
>
> COMDAT_SELECT_LARGEST is a COMDAT type that make linker to choose the
> largest
> definition from among all of the definition of a symbol. If the size is the
> same, the choice is arbitrary.
>
> http://llvm-reviews.chandlerc.com/D3011
>
> Files:
>   include/lld/Core/DefinedAtom.h
>   lib/Core/SymbolTable.cpp
>   lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
>   lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
>   test/pecoff/Inputs/merge-largest1.obj.yaml
>   test/pecoff/Inputs/merge-largest2.obj.yaml
>   test/pecoff/merge-largest.test
>
> Index: include/lld/Core/DefinedAtom.h
> ===================================================================
> --- include/lld/Core/DefinedAtom.h
> +++ include/lld/Core/DefinedAtom.h
> @@ -99,6 +99,8 @@
>      mergeAsWeakAndAddressUsed, // Is C++ definition inline definition
> whose
>                                 // address was taken.
>      mergeSameNameAndSize,   // Another atom with different size is error
> +    mergeLargest,           // Choose the largest one from atoms with the
> same
> +                            // name
>      mergeByContent,         // Merge with other constants with same
> content.
>    };
>
> Index: lib/Core/SymbolTable.cpp
> ===================================================================
> --- lib/Core/SymbolTable.cpp
> +++ lib/Core/SymbolTable.cpp
> @@ -102,13 +102,14 @@
>    MCR_Error
>  };
>
> -static MergeResolution mergeCases[][5] = {
> -  // no          tentative      weak          weakAddress
> sameNameAndSize
> -  {MCR_Error,    MCR_First,     MCR_First,    MCR_First,
>  MCR_SameSize}, // no
> -  {MCR_Second,   MCR_Largest,   MCR_Second,   MCR_Second,
> MCR_SameSize}, // tentative
> -  {MCR_Second,   MCR_First,     MCR_First,    MCR_Second,
> MCR_SameSize}, // weak
> -  {MCR_Second,   MCR_First,     MCR_First,    MCR_First,
>  MCR_SameSize}, // weakAddress
> -  {MCR_SameSize, MCR_SameSize,  MCR_SameSize, MCR_SameSize,
> MCR_SameSize}, // sameSize
> +static MergeResolution mergeCases[][6] = {
> +  // no          tentative      weak          weakAddress
> sameNameAndSize largest
> +  {MCR_Error,    MCR_First,     MCR_First,    MCR_First,    MCR_SameSize,
>   MCR_Largest},  // no
> +  {MCR_Second,   MCR_Largest,   MCR_Second,   MCR_Second,   MCR_SameSize,
>   MCR_Largest},  // tentative
> +  {MCR_Second,   MCR_First,     MCR_First,    MCR_Second,   MCR_SameSize,
>   MCR_Largest},  // weak
> +  {MCR_Second,   MCR_First,     MCR_First,    MCR_First,    MCR_SameSize,
>   MCR_Largest},  // weakAddress
> +  {MCR_SameSize, MCR_SameSize,  MCR_SameSize, MCR_SameSize, MCR_SameSize,
>   MCR_SameSize}, // sameSize
> +  {MCR_Largest,  MCR_Largest,   MCR_Largest,  MCR_Largest,  MCR_SameSize,
>   MCR_Largest},  // largest
>  };
>
>  static MergeResolution mergeSelect(DefinedAtom::Merge first,
> @@ -148,19 +149,22 @@
>      case MCR_Second:
>        useNew = true;
>        break;
> -    case MCR_Largest:
> -      useNew = true;
> +    case MCR_Largest: {
> +      uint64_t existingSize = ((DefinedAtom*)existing)->size();
> +      uint64_t newSize = ((DefinedAtom*)&newAtom)->size();
> +      useNew = (newSize >= existingSize);
>        break;
> +    }
>      case MCR_SameSize: {
> -      uint64_t sa = ((DefinedAtom*)existing)->size();
> -      uint64_t sb = ((DefinedAtom*)&newAtom)->size();
> -      if (sa == sb) {
> +      uint64_t existingSize = ((DefinedAtom*)existing)->size();
> +      uint64_t newSize = ((DefinedAtom*)&newAtom)->size();
> +      if (existingSize == newSize) {
>          useNew = true;
>          break;
>        }
>        llvm::errs() << "Size mismatch: "
> -                   << existing->name() << " (" << sa << ") "
> -                   << newAtom.name() << " (" << sb << ")\n";
> +                   << existing->name() << " (" << existingSize << ") "
> +                   << newAtom.name() << " (" << newSize << ")\n";
>        // fallthrough
>      }
>      case MCR_Error:
> Index: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
> ===================================================================
> --- lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
> +++ lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
> @@ -253,8 +253,9 @@
>      return DefinedAtom::mergeByContent;
>    case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE:
>      return DefinedAtom::mergeSameNameAndSize;
> -  case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:
>    case llvm::COFF::IMAGE_COMDAT_SELECT_LARGEST:
> +    return DefinedAtom::mergeLargest;
> +  case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:
>    case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST:
>      // FIXME: These attributes has more complicated semantics than the
> regular
>      // weak symbol. These are mapped to mergeAsWeakAndAddressUsed for now
> Index: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
> ===================================================================
> --- lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
> +++ lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
> @@ -340,6 +340,7 @@
>      io.enumCase(value, "by-content",   lld::DefinedAtom::mergeByContent);
>      io.enumCase(value, "same-name-and-size",
>                  lld::DefinedAtom::mergeSameNameAndSize);
> +    io.enumCase(value, "largest", lld::DefinedAtom::mergeLargest);
>    }
>  };
>
> Index: test/pecoff/Inputs/merge-largest1.obj.yaml
> ===================================================================
> --- /dev/null
> +++ test/pecoff/Inputs/merge-largest1.obj.yaml
> @@ -0,0 +1,25 @@
> +---
> +header:
> +  Machine:         IMAGE_FILE_MACHINE_I386
> +  Characteristics: [  ]
> +sections:
> +  - Name:            .text
> +    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT,
> IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
> +    Alignment:       16
> +    SectionData:     00112233
> +symbols:
> +  - Name:            .text
> +    Value:           0
> +    SectionNumber:   1
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_NULL
> +    StorageClass:    IMAGE_SYM_CLASS_STATIC
> +    NumberOfAuxSymbols: 1
> +    AuxiliaryData:   0700000000000000C979F796000006000000
> +  - Name:            "_foo"
> +    Value:           0
> +    SectionNumber:   1
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
> +    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
> +...
> Index: test/pecoff/Inputs/merge-largest2.obj.yaml
> ===================================================================
> --- /dev/null
> +++ test/pecoff/Inputs/merge-largest2.obj.yaml
> @@ -0,0 +1,25 @@
> +---
> +header:
> +  Machine:         IMAGE_FILE_MACHINE_I386
> +  Characteristics: [  ]
> +sections:
> +  - Name:            .text
> +    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT,
> IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
> +    Alignment:       16
> +    SectionData:     0011223344556677
> +symbols:
> +  - Name:            .text
> +    Value:           0
> +    SectionNumber:   1
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_NULL
> +    StorageClass:    IMAGE_SYM_CLASS_STATIC
> +    NumberOfAuxSymbols: 1
> +    AuxiliaryData:   0700000000000000C979F796000006000000
> +  - Name:            "_foo"
> +    Value:           0
> +    SectionNumber:   1
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
> +    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
> +...
> Index: test/pecoff/merge-largest.test
> ===================================================================
> --- /dev/null
> +++ test/pecoff/merge-largest.test
> @@ -0,0 +1,22 @@
> +# RUN: yaml2obj %p/Inputs/merge-largest1.obj.yaml > %t1.obj
> +# RUN: yaml2obj %p/Inputs/merge-largest2.obj.yaml > %t2.obj
> +#
> +# RUN: lld -flavor link /out:%t.exe /subsystem:console /opt:noref /force \
> +# RUN:   -- %t1.obj %t2.obj > %t.log 2>&1
> +# RUN: echo foo >> %t.log
> +# RUN: FileCheck -check-prefix=OK %s < %t.log
> +#
> +# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=READOBJ %s
> +
> +OK-NOT: duplicate symbol error
> +
> +READOBJ:      Format: COFF-i386
> +READOBJ-NEXT: Arch: i386
> +READOBJ-NEXT: AddressSize: 32bit
> +READOBJ-NEXT: Sections [
> +READOBJ-NEXT:   Section {
> +READOBJ-NEXT:     Number: 1
> +READOBJ-NEXT:     Name: .text (2E 74 65 78 74 00 00 00)
> +READOBJ-NEXT:     VirtualSize: 0x8
> +READOBJ-NEXT:     VirtualAddress: 0x1000
> +READOBJ-NEXT:     RawDataSize: 8
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140312/f0cb9f5b/attachment.html>


More information about the llvm-commits mailing list