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

[grimar]

Summary

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.

[grimar]

• Added forgotten input file.

[ruiu]

LGTM with comment change.

ELF/SyntheticSections.cpp

1388–1392

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>();

[grimar]

ELF/SyntheticSections.cpp

1388–1392

Ok, thanks !

ELF/SyntheticSections.cpp

for (SymbolTableEntry &Ent : Symbols) {
     if (Body->isLocal()) {
       ESym->setBindingAndType(STB_LOCAL, Body->Type);
     } else {
       ESym->setBindingAndType(Body->symbol()->computeBinding(), Body->Type);
       ESym->setVisibility(Body->symbol()->Visibility);
     }
 
     ESym->st_name = Ent.StrTabOffset;
-    ESym->st_size = Body->getSize<ELFT>();
 
     // Set a section index.
     if (const OutputSection *OutSec = Body->getOutputSection())
       ESym->st_shndx = OutSec->SectionIndex;
     else if (isa<DefinedRegular>(Body))
       ESym->st_shndx = SHN_ABS;
     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).
     if (!Config->DefineCommon && isa<DefinedCommon>(Body))
       ESym->st_value = cast<DefinedCommon>(Body)->Alignment;
     else
       ESym->st_value = Body->getVA();
 

test/ELF/dso-undef-size.s

+# 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