[llvm-branch-commits] [llvm] f3f90ec - [MC][ELF] Improve st_size propagation rule

[Tom Stellard via llvm-branch-commits]

Author: Fangrui Song
Date: 2022-05-11T14:45:04-07:00
New Revision: f3f90ec42ae674afe1858b4f7d9f270731872b80

URL: https://github.com/llvm/llvm-project/commit/f3f90ec42ae674afe1858b4f7d9f270731872b80
DIFF: https://github.com/llvm/llvm-project/commit/f3f90ec42ae674afe1858b4f7d9f270731872b80.diff

LOG: [MC][ELF] Improve st_size propagation rule

`.symver foo, foo at ver` creates the MCSymbolELF `foo at ver` whose almost all
attributes (including st_size) should inherit from `foo` (GNU as behavior).

a041ef1bd8905f0d58e301c6830b183002ff1847 added st_size propagation which works
for many cases but fails for the following one:

```
.set __GLIBC_2_12_sys_errlist, _sys_errlist_internal
.type   __GLIBC_2_12_sys_errlist, at object
.size   __GLIBC_2_12_sys_errlist, 1080
.symver __GLIBC_2_12_sys_errlist, sys_errlist at GLIBC_2.12
...
_sys_errlist_internal:
.size   _sys_errlist_internal, 1072
```

`sys_errlist at GLIBC_2.12`'s st_size is 1072 (incorrect), which does not match
`__GLIBC_2_12_sys_errlist`'s st_size: 1080.

The problem is that `Base` is (the final) `_sys_errlist_internal` while we want
to respect (the intermediate) `__GLIBC_2_12_sys_errlist`'s st_size.
Fix this by following the MCSymbolRefExpr assignment chain and finding
the closest non-null `getSize()`, which covers most needs. Notably MCBinaryExpr
is not handled, but it is rare enough to matter.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D123283

(cherry picked from commit e07dfa5328b0ca1465ae7b749e1ac2d994741e27)

Added: 
    

Modified: 
    llvm/lib/MC/ELFObjectWriter.cpp
    llvm/test/MC/ELF/offset.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 6fd2f7e7a7185..e6d28141dd06b 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -549,9 +549,27 @@ void ELFWriter::writeSymbol(SymbolTableWriter &Writer, uint32_t StringIndex,
   uint64_t Size = 0;
 
   const MCExpr *ESize = MSD.Symbol->getSize();
-  if (!ESize && Base)
+  if (!ESize && Base) {
+    // For expressions like .set y, x+1, if y's size is unset, inherit from x.
     ESize = Base->getSize();
 
+    // For `.size x, 2; y = x; .size y, 1; z = y; z1 = z; .symver y, y at v1`, z,
+    // z1, and y at v1's st_size equals y's. However, `Base` is `x` which will give
+    // us 2. Follow the MCSymbolRefExpr assignment chain, which covers most
+    // needs. MCBinaryExpr is not handled.
+    const MCSymbolELF *Sym = &Symbol;
+    while (Sym->isVariable()) {
+      if (auto *Expr =
+              dyn_cast<MCSymbolRefExpr>(Sym->getVariableValue(false))) {
+        Sym = cast<MCSymbolELF>(&Expr->getSymbol());
+        if (!Sym->getSize())
+          continue;
+        ESize = Sym->getSize();
+      }
+      break;
+    }
+  }
+
   if (ESize) {
     int64_t Res;
     if (!ESize->evaluateKnownAbsolute(Res, Layout))

diff  --git a/llvm/test/MC/ELF/offset.s b/llvm/test/MC/ELF/offset.s
index 8d7fd3cd1459c..199675dd2b738 100644
--- a/llvm/test/MC/ELF/offset.s
+++ b/llvm/test/MC/ELF/offset.s
@@ -14,15 +14,15 @@
 # CHECK-NEXT:    000000000000000d    42 OBJECT  GLOBAL DEFAULT [[#A]]  d1
 # CHECK-NEXT:    000000000000000d    42 OBJECT  GLOBAL DEFAULT [[#A]]  d2
 # CHECK-NEXT:    0000000000000001    41 OBJECT  GLOBAL DEFAULT [[#A]]  e
-# CHECK-NEXT:    0000000000000001    42 OBJECT  GLOBAL DEFAULT [[#A]]  e1
-# CHECK-NEXT:    0000000000000001    42 OBJECT  GLOBAL DEFAULT [[#A]]  e2
+# CHECK-NEXT:    0000000000000001    41 OBJECT  GLOBAL DEFAULT [[#A]]  e1
+# CHECK-NEXT:    0000000000000001    41 OBJECT  GLOBAL DEFAULT [[#A]]  e2
 # CHECK-NEXT:    0000000000000002    42 OBJECT  GLOBAL DEFAULT [[#A]]  e3
 # CHECK-NEXT:    0000000000000005     0 NOTYPE  GLOBAL DEFAULT [[#A]]  test2_a
 # CHECK-NEXT:    0000000000000005     0 NOTYPE  GLOBAL DEFAULT [[#A]]  test2_b
 # CHECK-NEXT:    0000000000000009     0 NOTYPE  GLOBAL DEFAULT [[#A]]  test2_c
 # CHECK-NEXT:    0000000000000009     0 NOTYPE  GLOBAL DEFAULT [[#A]]  test2_d
 # CHECK-NEXT:    0000000000000004     0 NOTYPE  GLOBAL DEFAULT  ABS    test2_e
-# CHECK-NEXT:    0000000000000001    42 OBJECT  GLOBAL DEFAULT [[#A]]  e at v1
+# CHECK-NEXT:    0000000000000001    41 OBJECT  GLOBAL DEFAULT [[#A]]  e at v1
 
 
         .data
@@ -45,7 +45,8 @@ d2 = d1
 
 e = a + (1 - 1)
 .size e, 41
-## FIXME These st_size fields inherit from e instead of a.
+## These st_size fields inherit from e instead of a.
+## TODO e3's st_size should inherit from e.
 .set e1, e
 .set e2, e1
 e3 = e1 + 1