[PATCH] D34673: [ELF] - Do not set st_size field of SHT_UNDEF symbols

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1388-1392
+    // Do not set size for undefined symbols. This size is unreliable for case
+    // when linking DSO which links to another DSO. In that case it is possible
+    // that low level DSO may be changed in the way that alters symbol size. In
+    // such case if high-level DSO is rebuilt, its CRC will change, what may be
+    // undesirable. It is possible to avoid if we set st_size to zero.
----------------
ruiu wrote:
> It seems this comment describes something too specific. The most important part is that the size field for the undefined symbol is not significant, so it can be any value. You didn't mention that. I'd write:
> 
>   // Copy symbol size if it is a defined symbol. st_size is not significant for undefined symbols,
>   // so whether copying it or not is up to us if that's the case. We'll leave it as zero
>   // because by not setting a value, we can get the exact same outputs for two sets of input files that
>   // differ only in undefined symbol size in DSOs.
>   if (ESym->st_shndx != SHN_UNDEF)
>     ESym->st_size = Body->getSize<ELFT>();
Ok, thanks !


https://reviews.llvm.org/D34673