[lld] r246264 - Make sure we output symbols in the same order on 32 and 64 bit builds.

[Rafael Espíndola via llvm-commits]

This is why I was asking about DenseMap iteration order in case you
were curious.

Thanks for the help.

Cheers,
Rafael


On 27 August 2015 at 22:46, Rafael Espindola via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: rafael
> Date: Thu Aug 27 21:46:41 2015
> New Revision: 246264
>
> URL: http://llvm.org/viewvc/llvm-project?rev=246264&view=rev
> Log:
> Make sure we output symbols in the same order on 32 and 64 bit builds.
>
> Modified:
>     lld/trunk/ELF/Writer.cpp
>     lld/trunk/test/elf2/symbols.s
>
> Modified: lld/trunk/ELF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=246264&r1=246263&r2=246264&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Writer.cpp (original)
> +++ lld/trunk/ELF/Writer.cpp Thu Aug 27 21:46:41 2015
> @@ -16,6 +16,7 @@
>  #include "Symbols.h"
>
>  #include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/STLExtras.h"
>  #include "llvm/Support/FileOutputBuffer.h"
>  #include "llvm/Support/raw_ostream.h"
>
> @@ -273,7 +274,18 @@ void StringTableSection<Is64Bits>::write
>    memcpy(Buf, Data.data(), Data.size());
>  }
>
> +template <class ELFT>
> +static int compareSym(const typename ELFFile<ELFT>::Elf_Sym *A,
> +                      const typename ELFFile<ELFT>::Elf_Sym *B) {
> +  uint32_t AN = A->st_name;
> +  uint32_t BN = B->st_name;
> +  assert(AN != BN);
> +  return AN - BN;
> +}
> +
>  template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
> +  uint8_t *BufStart = Buf;
> +
>    Buf += sizeof(Elf_Sym);
>    llvm::StringTableBuilder &Builder = Table.getStringBuilder();
>    for (auto &P : Table.getSymbols()) {
> @@ -326,6 +338,15 @@ template <class ELFT> void SymbolTableSe
>
>      Buf += sizeof(Elf_Sym);
>    }
> +
> +  // The order the global symbols are in is not defined. We can use an arbitrary
> +  // order, but it has to be reproducible. That is true even when cross linking.
> +  // The default hashing of StringRef produces different results on 32 and 64
> +  // bit systems so we sort by st_name. That is arbitrary but deterministic.
> +  // FIXME: Experiment with passing in a custom hashing instead.
> +  auto *Syms = reinterpret_cast<Elf_Sym *>(BufStart);
> +  ++Syms;
> +  array_pod_sort(Syms, Syms + Table.getSymbols().size(), compareSym<ELFT>);
>  }
>
>  template <bool Is64Bits>
>
> Modified: lld/trunk/test/elf2/symbols.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/symbols.s?rev=246264&r1=246263&r2=246264&view=diff
> ==============================================================================
> --- lld/trunk/test/elf2/symbols.s (original)
> +++ lld/trunk/test/elf2/symbols.s Thu Aug 27 21:46:41 2015
> @@ -57,31 +57,22 @@ abs = 0x123
>  // CHECK-NEXT:     Section: Undefined (0x0)
>  // CHECK-NEXT:   }
>  // CHECK-NEXT:   Symbol {
> -// CHECK-NEXT:     Name: zed
> -// CHECK-NEXT:     Value: 0x1004
> +// CHECK-NEXT:     Name: _start
> +// CHECK-NEXT:     Value: 0x1000
>  // CHECK-NEXT:     Size: 0
>  // CHECK-NEXT:     Binding: Global (0x1)
> -// CHECK-NEXT:     Type: None
> +// CHECK-NEXT:     Type: Function
>  // CHECK-NEXT:     Other: 0
> -// CHECK-NEXT:     Section: foobar
> +// CHECK-NEXT:     Section: .text
>  // CHECK-NEXT:   }
>  // CHECK-NEXT:   Symbol {
> -// CHECK-NEXT:     Name: zed3
> -// CHECK-NEXT:     Value: 0x100C
> -// CHECK-NEXT:     Size: 4
> +// CHECK-NEXT:     Name: abs
> +// CHECK-NEXT:     Value: 0x123
> +// CHECK-NEXT:     Size: 0
>  // CHECK-NEXT:     Binding: Global
>  // CHECK-NEXT:     Type: None
>  // CHECK-NEXT:     Other: 0
> -// CHECK-NEXT:     Section: foobar
> -// CHECK-NEXT:   }
> -// CHECK-NEXT:   Symbol {
> -// CHECK-NEXT:     Name: _start
> -// CHECK-NEXT:     Value: 0x1000
> -// CHECK-NEXT:     Size: 0
> -// CHECK-NEXT:     Binding: Global (0x1)
> -// CHECK-NEXT:     Type: Function
> -// CHECK-NEXT:     Other: 0
> -// CHECK-NEXT:     Section: .text
> +// CHECK-NEXT:     Section: Absolute
>  // CHECK-NEXT:   }
>  // CHECK-NEXT:   Symbol {
>  // CHECK-NEXT:     Name: bar
> @@ -93,15 +84,6 @@ abs = 0x123
>  // CHECK-NEXT:     Section: Undefined (0x0)
>  // CHECK-NEXT:   }
>  // CHECK-NEXT:   Symbol {
> -// CHECK-NEXT:     Name: abs
> -// CHECK-NEXT:     Value: 0x123
> -// CHECK-NEXT:     Size: 0
> -// CHECK-NEXT:     Binding: Global
> -// CHECK-NEXT:     Type: None
> -// CHECK-NEXT:     Other: 0
> -// CHECK-NEXT:     Section: Absolute
> -// CHECK-NEXT:   }
> -// CHECK-NEXT:   Symbol {
>  // CHECK-NEXT:     Name: foo
>  // CHECK-NEXT:     Value: 0x1000
>  // CHECK-NEXT:     Size: 0
> @@ -111,6 +93,24 @@ abs = 0x123
>  // CHECK-NEXT:     Section: .text
>  // CHECK-NEXT:   }
>  // CHECK-NEXT:   Symbol {
> +// CHECK-NEXT:     Name: zed
> +// CHECK-NEXT:     Value: 0x1004
> +// CHECK-NEXT:     Size: 0
> +// CHECK-NEXT:     Binding: Global (0x1)
> +// CHECK-NEXT:     Type: None
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: foobar
> +// CHECK-NEXT:   }
> +// CHECK-NEXT:   Symbol {
> +// CHECK-NEXT:     Name: zed3
> +// CHECK-NEXT:     Value: 0x100C
> +// CHECK-NEXT:     Size: 4
> +// CHECK-NEXT:     Binding: Global
> +// CHECK-NEXT:     Type: None
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: foobar
> +// CHECK-NEXT:   }
> +// CHECK-NEXT:   Symbol {
>  // CHECK-NEXT:     Name: zed2
>  // CHECK-NEXT:     Value: 0x1008
>  // CHECK-NEXT:     Size: 0
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits