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

Han Shen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 31 14:02:21 PDT 2018


shenhan added a comment.

@pcc



================
Comment at: ELF/InputFiles.cpp:904
   if (0 < Sym.st_shndx && Sym.st_shndx < Sections.size())
-    Ret = std::min<uint64_t>(Ret, Sections[Sym.st_shndx].sh_addralign);
+    Ret = std::max<uint64_t>(Ret, Sections[Sym.st_shndx].sh_addralign);
 
----------------
shenhan wrote:
> ruiu wrote:
> > pcc wrote:
> > > Actually, now that I think about it, I think this was correct before. The `sh_addralign` field says what the maximum alignment of anything in the section is going to be, and the code on lines 899-900 (in the old code) uses the symbol's `st_value` to infer a smaller alignment wherever possible.
> > Hmm, you are probably right. Han, what are you trying to fix by this change?
> I cc'ed you and @pcc in the internal bug.
@pcc I debugged this function, the first 2 symbols this function processes are:

symbol1: shndx=18 st_value=0
symbol2: shndx=18 st_value=112

It appears to me st_value is more like a offset than a alignment information. (But it could be interpreted as alignment as well if we count the tailing zeros)

However, st_value=0 for symbol1 results in the final RET being 1 in the original code, that is apparently not correct.

So definitely the alignment calculation here is not correct. That's why if I change the variable name (so it does not appear as the first symbol) the program runs correctly.





Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47602





More information about the llvm-commits mailing list