From e07dfa5328b0ca1465ae7b749e1ac2d994741e27 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Fri, 8 Apr 2022 10:06:43 -0700 Subject: [PATCH] [MC][ELF] Improve st_size propagation rule `.symver foo, foo@ver` creates the MCSymbolELF `foo@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,@object .size __GLIBC_2_12_sys_errlist, 1080 .symver __GLIBC_2_12_sys_errlist, sys_errlist@GLIBC_2.12 ... _sys_errlist_internal: .size _sys_errlist_internal, 1072 ``` `sys_errlist@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 --- llvm/lib/MC/ELFObjectWriter.cpp | 20 +++++++++++++++++++- llvm/test/MC/ELF/offset.s | 9 +++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp index fe11768..08d1b12 100644 --- a/llvm/lib/MC/ELFObjectWriter.cpp +++ b/llvm/lib/MC/ELFObjectWriter.cpp @@ -547,9 +547,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@v1`, z, + // z1, and y@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(Sym->getVariableValue(false))) { + Sym = cast(&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 8d7fd3c..199675d 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@v1 +# CHECK-NEXT: 0000000000000001 41 OBJECT GLOBAL DEFAULT [[#A]] e@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 -- 2.7.4