[PATCH] D36812: Add support for writing 64-bit symbol tables for archives when offsets become too large for 32-bit

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 14 15:16:53 PDT 2017


I think my main concern is that we now have the archive Kind and a Is64
Boolean. Can we instead rename K_MIPS64 to K_GNU64 and have the code
switch from K_GNU to K_GNU64 if needed?

That way printNBits would not need an extra argument for example (it
already takes Kind).

Cheers,
Rafael

Jake Ehrlich via Phabricator <reviews at reviews.llvm.org> writes:
> Index: test/Object/archive-SYM64-write.test
> ===================================================================
> --- /dev/null
> +++ test/Object/archive-SYM64-write.test
> @@ -0,0 +1,27 @@
> +# RUN: yaml2obj %s > %t
> +# RUN: rm -f %t.lib
> +# RUN: llvm-ar cr %t.lib %t
> +# RUN: mv %t %t2
> +# RUN: llvm-ar cr %t.lib %t2 %p/Inputs/trivial-object-test.elf-x86-64

Is it important that there are two runs of llvm-ar? Can't you just
include %t twice? It would look like:

llvm-ar rc %t.lib %t %t %p/Inputs/trivial-object-test.elf-x86-64

> +bool isArchiveSymbol(const object::BasicSymbolRef &S) {
> +  uint32_t Symflags = S.getFlags();
> +  if (Symflags & object::SymbolRef::SF_FormatSpecific)
> +    return false;
> +  if (!(Symflags & object::SymbolRef::SF_Global))
> +    return false;
> +  if (Symflags & object::SymbolRef::SF_Undefined &&
> +      !(Symflags & object::SymbolRef::SF_Indirect))
> +    return false;
> +  return true;
> +}
> +

This can be static.

This is a nice refactoring. Could you commit that first and rebase?
Thanks!


> -      unsigned NameOffset = NameOS.tell();
> +      uint64_t NameOffset = NameOS.tell();

NameOffset is still printed as 32 bits.

Cheers,
Rafael


More information about the llvm-commits mailing list