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

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
Herald added a subscriber: emaste.

This fixes PR33598.

It looks may be usefult for next case:
When we have 2 DSO's, like in this PR, if lower level DSO may change slightly (in part of some symbol's st_size) 
and higher-level DSO is rebuilt, then tools that monitoring checksum of high level DSO file can notice it and trigger 
cascade of some other unnecessary actions. If we set st_size to zero, that can be avoided.

gABI says: "st_size
Many symbols have associated sizes. For example, a data object's size is the number of bytes contained in the object. 
This member holds 0 if the symbol has no size or an unknown size."
(http://refspecs.linuxbase.org/elf/gabi4+/ch4.symtab.html)

Looks setting zero is correct from gABI size because in the given example above symbol size can be different in runtime 
and therefore can be probably called 'unknown' when linking.


https://reviews.llvm.org/D34673

Files:
  ELF/SyntheticSections.cpp
  test/ELF/dso-undef-size.s


Index: test/ELF/dso-undef-size.s
===================================================================
--- test/ELF/dso-undef-size.s
+++ test/ELF/dso-undef-size.s
@@ -0,0 +1,32 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/dso-undef-size.s -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1.so
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2.o
+# RUN: ld.lld -shared %t2.o %t1.so -o %t2.so
+# RUN: llvm-readobj -symbols -dyn-symbols %t2.so
+
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding:
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+# CHECK:      DynamicSymbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding:
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.text
+.global foo
Index: ELF/SyntheticSections.cpp
===================================================================
--- ELF/SyntheticSections.cpp
+++ ELF/SyntheticSections.cpp
@@ -1376,7 +1376,6 @@
     }
 
     ESym->st_name = Ent.StrTabOffset;
-    ESym->st_size = Body->getSize<ELFT>();
 
     // Set a section index.
     if (const OutputSection *OutSec = Body->getOutputSection())
@@ -1386,6 +1385,13 @@
     else if (isa<DefinedCommon>(Body))
       ESym->st_shndx = SHN_COMMON;
 
+    // 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.
+    ESym->st_size = (ESym->st_shndx == SHN_UNDEF) ? 0 : Body->getSize<ELFT>();
+
     // st_value is usually an address of a symbol, but that has a
     // special meaining for uninstantiated common symbols (this can
     // occur if -r is given).


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34673.104109.patch
Type: text/x-patch
Size: 2215 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170627/1276d5c2/attachment-0001.bin>