[PATCH] D36466: [ELF] - Do not omit common symbols when -Map is given.

[Rafael Avila de Espindola via llvm-commits]

LGTM with Rui's suggestion.

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 110534.
> grimar added a comment.
>
> - Added testcase.
>
>
> https://reviews.llvm.org/D36466
>
> Files:
>   ELF/MapFile.cpp
>   test/ELF/map-commons.s
>   test/ELF/map-file.s
>
> Index: test/ELF/map-file.s
> ===================================================================
> --- test/ELF/map-file.s
> +++ test/ELF/map-file.s
> @@ -45,6 +45,7 @@
>  // CHECK-NEXT: 0000000000201014 0000000000000000     0                 baz
>  // CHECK-NEXT: 0000000000202000 0000000000000004    16 .bss
>  // CHECK-NEXT: 0000000000202000 0000000000000004    16         <internal>:(COMMON)
> +// CHECK-NEXT: 0000000000202000 0000000000000004     0                 common
>  // CHECK-NEXT: 0000000000000000 0000000000000008     1 .comment
>  // CHECK-NEXT: 0000000000000000 0000000000000008     1         <internal>:(.comment)
>  // CHECK-NEXT: 0000000000000000 00000000000000f0     8 .symtab
> Index: test/ELF/map-commons.s
> ===================================================================
> --- test/ELF/map-commons.s
> +++ test/ELF/map-commons.s
> @@ -0,0 +1,15 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +
> +# RUN: ld.lld %t.o -o %t -Map=- | FileCheck %s
> +# CHECK:      .bss
> +# CHECK-NEXT:         <internal>:(COMMON)
> +# CHECK-NEXT:                 foo
> +
> +# RUN: ld.lld %t.o -r -o %t -Map=- | FileCheck %s --check-prefix=NOCOMMON
> +# NOCOMMON-NOT: COMMON
> +
> +# RUN: ld.lld %t.o --no-define-common -o %t -Map=- | \
> +# RUN:   FileCheck %s --check-prefix=NOCOMMON
> +
> +.comm foo,4,4
> Index: ELF/MapFile.cpp
> ===================================================================
> --- ELF/MapFile.cpp
> +++ ELF/MapFile.cpp
> @@ -25,6 +25,7 @@
>  #include "OutputSections.h"
>  #include "Strings.h"
>  #include "SymbolTable.h"
> +#include "SyntheticSections.h"
>  #include "Threads.h"
>  
>  #include "llvm/Support/raw_ostream.h"
> @@ -35,8 +36,8 @@
>  using namespace lld;
>  using namespace lld::elf;
>  
> -typedef DenseMap<const SectionBase *, SmallVector<DefinedRegular *, 4>>
> -    SymbolMapTy;
> +typedef DenseMap<const SectionBase *, SmallVector<Defined *, 4>> SymbolMapTy;
> +typedef DenseMap<Defined *, std::string> SymbolStringsMapTy;
>  
>  // Print out the first three columns of a line.
>  static void writeHeader(raw_ostream &OS, uint64_t Addr, uint64_t Size,
> @@ -48,49 +49,57 @@
>  static std::string indent(int Depth) { return std::string(Depth * 8, ' '); }
>  
>  // Returns a list of all symbols that we want to print out.
> -template <class ELFT> static std::vector<DefinedRegular *> getSymbols() {
> -  std::vector<DefinedRegular *> V;
> -  for (ObjFile<ELFT> *File : ObjFile<ELFT>::Instances)
> -    for (SymbolBody *B : File->getSymbols())
> -      if (auto *DR = dyn_cast<DefinedRegular>(B))
> +template <class ELFT> static std::vector<Defined *> getSymbols() {
> +  std::vector<Defined *> V;
> +  for (ObjFile<ELFT> *File : ObjFile<ELFT>::Instances) {
> +    for (SymbolBody *B : File->getSymbols()) {
> +      if (auto *DR = dyn_cast<DefinedRegular>(B)) {
>          if (DR->getFile() == File && !DR->isSection() && DR->Section &&
>              DR->Section->Live)
>            V.push_back(DR);
> +      } else if (auto *DC = dyn_cast<DefinedCommon>(B)) {
> +        if (InX::Common)
> +          V.push_back(cast<DefinedCommon>(B));
> +      }
> +    }
> +  }
>    return V;
>  }
>  
>  // Returns a map from sections to their symbols.
> -static SymbolMapTy getSectionSyms(ArrayRef<DefinedRegular *> Syms) {
> +static SymbolMapTy getSectionSyms(ArrayRef<Defined *> Syms) {
>    SymbolMapTy Ret;
> -  for (DefinedRegular *S : Syms)
> -    Ret[S->Section].push_back(S);
> +  for (Defined *S : Syms) {
> +    if (auto *DR = dyn_cast<DefinedRegular>(S))
> +      Ret[DR->Section].push_back(S);
> +    else
> +      Ret[InX::Common].push_back(S);
> +  }
>  
>    // Sort symbols by address. We want to print out symbols in the
>    // order in the output file rather than the order they appeared
>    // in the input files.
>    for (auto &It : Ret) {
> -    SmallVectorImpl<DefinedRegular *> &V = It.second;
> -    std::sort(V.begin(), V.end(), [](DefinedRegular *A, DefinedRegular *B) {
> -      return A->getVA() < B->getVA();
> -    });
> +    SmallVectorImpl<Defined *> &V = It.second;
> +    std::sort(V.begin(), V.end(),
> +              [](Defined *A, Defined *B) { return A->getVA() < B->getVA(); });
>    }
>    return Ret;
>  }
>  
>  // Construct a map from symbols to their stringified representations.
>  // Demangling symbols (which is what toString() does) is slow, so
>  // we do that in batch using parallel-for.
>  template <class ELFT>
> -static DenseMap<DefinedRegular *, std::string>
> -getSymbolStrings(ArrayRef<DefinedRegular *> Syms) {
> +static SymbolStringsMapTy getSymbolStrings(ArrayRef<Defined *> Syms) {
>    std::vector<std::string> Str(Syms.size());
>    parallelForEachN(0, Syms.size(), [&](size_t I) {
>      raw_string_ostream OS(Str[I]);
>      writeHeader(OS, Syms[I]->getVA(), Syms[I]->template getSize<ELFT>(), 0);
>      OS << indent(2) << toString(*Syms[I]);
>    });
>  
> -  DenseMap<DefinedRegular *, std::string> Ret;
> +  SymbolStringsMapTy Ret;
>    for (size_t I = 0, E = Syms.size(); I < E; ++I)
>      Ret[Syms[I]] = std::move(Str[I]);
>    return Ret;
> @@ -109,9 +118,9 @@
>    }
>  
>    // Collect symbol info that we want to print out.
> -  std::vector<DefinedRegular *> Syms = getSymbols<ELFT>();
> +  std::vector<Defined *> Syms = getSymbols<ELFT>();
>    SymbolMapTy SectionSyms = getSectionSyms(Syms);
> -  DenseMap<DefinedRegular *, std::string> SymStr = getSymbolStrings<ELFT>(Syms);
> +  SymbolStringsMapTy SymStr = getSymbolStrings<ELFT>(Syms);
>  
>    // Print out the header line.
>    int W = ELFT::Is64Bits ? 16 : 8;
> @@ -132,7 +141,7 @@
>          writeHeader(OS, OSec->Addr + IS->OutSecOff, IS->getSize(),
>                      IS->Alignment);
>          OS << indent(1) << toString(IS) << '\n';
> -        for (DefinedRegular *Sym : SectionSyms[IS])
> +        for (Defined *Sym : SectionSyms[IS])
>            OS << SymStr[Sym] << '\n';
>        }
>      }