[PATCH] D47602: Correct aligment computation for shared object symbols

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: ELF/InputFiles.cpp:899
+  // If no available alignment information from either section or
+  // symbol, return 1.
+  if (!(Sym.st_value || (0 < Sym.st_shndx && Sym.st_shndx < Sections.size())))
----------------
shenhan wrote:
> pcc wrote:
> > Is that correct? If there is a symbol at virtual address 0 in an unknown section, doesn't that mean that we should error out via the `Ret > UINT32_MAX` code path below since we don't know what alignment to use?
> We have quite a few tests that link with libm.so, which has quite a few symbols with st_value as 0 and shndx as "ABS".  Shall we handle symbols with "ABS" shndex separately (if symbols has absolute address, linker does not need to change its address or its address is not relevant, right?) 
> 
>    221: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.4
>    238: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.2.5
>    394: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.15
>    399: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.18
>   1736: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.15
>   1922: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.4
>   1953: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.18
>   2136: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  ABS GLIBC_2.2.5
We should only end up in this function if we need to create a copy relocation for a symbol. Those symbols appear to be associated with symbol versions so we wouldn't expect to be linking against them directly. Furthermore I don't think there is anything sensible that we can do if we're asked to create a copy relocation for an absolute symbol, so I think the correct behaviour would be to error out.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47602