[ELF][MC] Fix IFunc alias resolving issue

[zsrkmyn]

Summary

If we use '.set' directive to set a symbol as an alias of an IFunc, then the
symbol should also be IFunc. But it's evaluated to be the IFunc resolver at
this time. This patch tries to fix it.

[zsrkmyn]

Suppose we have foo.s as follows:

.text
.p2align 3

.globl foo_impl
.type foo_impl,@function
foo_impl:
  ret

.globl foo_resolver
.type foo_resolver,@function
foo_resolver:
  mov $foo_impl, %rax
  ret

.globl foo
.type foo,@gnu_indirect_function
.set foo,foo_resolver

.globl foo2
.set foo2,foo

and assemble foo.s with GNU as, and check foo.o with nm:

$ as foo.s -o foo.o
$ nm foo.o
0000000000000006 i foo
0000000000000006 i foo2   <- foo2 is an ifunc identical to foo
0000000000000000 T foo_impl
0000000000000006 T foo_resolver

and the 'foo2' is also an ifunc whose resolver is foo_resolver.

However, when assemble with llvm-mc, the 'foo2' becomes normal a function identical to 'foo_resolver':

$ llvm-mc foo.s -filetype obj -o foo.o
$ nm foo.o
0000000000000006 i foo
0000000000000006 T foo2   <- foo2 is a strong symbol identical to foo_resolver
0000000000000000 T foo_impl
0000000000000006 T foo_resolver

The reason is as follows:

When we 'getBaseSymbol' from 'ELFwriter::writeSymbol' in lib/MC/ELFObjectWriter.cpp, it recursively evaluates the 'Value' of the 'Symbol', and the two direcives -- '.set foo,foo_resolver' and '.set foo2,foo' -- make foo2 is evaluated as 'foo_resolver', and the 'Type' we get below becomes function, while it should be ifunc.

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

540

GNU as doesn't check the type of the alias

.type foo2, @function
.set foo2,foo   # foo2 is a STT_GNU_IFUNC

llvm/test/MC/ELF/ifunc-alias.s

5

.globl directives and instructions are not significant. You can simplify the test:

.type foo_resolver,@function
foo_resolver:

.type foo,@gnu_indirect_function
.set foo,foo_resolver

.set foo2,foo
.set foo3,foo2   # add another alias to show it is necessary to resolve it recursively

[zsrkmyn]

llvm/lib/MC/ELFObjectWriter.cpp

540

I'm sorry, but do you mean that I need to remove the '.type' directive in the test or that the IFunc check I do here is wrong? :D

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

515

Can you change the recursion to a loop?

llvm/test/MC/ELF/ifunc-alias.s

46

If Value, Size, Binding, etc are not significant. Delete those CHECK lines. Alternatively, use llvm-readelf -S which has a much conciser output.

[MaskRay]

llvm/lib/MC/ELFObjectWriter.cpp

514

If you make the parameter a pointer, SymbolPtr can be deleted.

[zsrkmyn]

Thank @MaskRay for reviewing. Would you mind helping me commit it? :-)

[MaskRay]

In D67206#1662128, @zsrkmyn wrote:

Thank @MaskRay for reviewing. Would you mind helping me commit it? :-)

With pleasure:) Sigh, I don't like this GNU extension but I have to learn more and more about it. Last time I committed a -z ifunc-noplt linker change for FreeBSD devs...

llvm/lib/MC/ELFObjectWriter.cpp

if (Type == ELF::STT_OBJECT || Type == ELF::STT_NOTYPE ||
         Type == ELF::STT_GNU_IFUNC || Type == ELF::STT_FUNC)
       Type = ELF::STT_TLS;
     break;
   }
 
   return Type;
 }
 
+static bool isIFunc(const MCSymbolELF &Symbol) {
+  if (Symbol.getType() == ELF::STT_GNU_IFUNC)
+    return true;
+
+  const MCSymbolRefExpr *Value;
+  if (!Symbol.isVariable() ||
+      !(Value = dyn_cast<MCSymbolRefExpr>(Symbol.getVariableValue())) ||
+      Value->getKind() != MCSymbolRefExpr::VK_None ||
+      mergeTypeForSet(Symbol.getType(), ELF::STT_GNU_IFUNC) != ELF::STT_GNU_IFUNC)
+    return false;
+  return isIFunc(cast<MCSymbolELF>(Value->getSymbol()));
+}
+
 void ELFWriter::writeSymbol(SymbolTableWriter &Writer, uint32_t StringIndex,
                             ELFSymbolData &MSD, const MCAsmLayout &Layout) {
   const auto &Symbol = cast<MCSymbolELF>(*MSD.Symbol);
   const MCSymbolELF *Base =
       cast_or_null<MCSymbolELF>(Layout.getBaseSymbol(Symbol));
 
   // This has to be in sync with when computeSymbolTable uses SHN_ABS or
   // SHN_COMMON.
   bool IsReserved = !Base || Symbol.isCommon();
 
   // Binding and Type share the same byte as upper and lower nibbles
   uint8_t Binding = Symbol.getBinding();
   uint8_t Type = Symbol.getType();
+  if (isIFunc(Symbol))
+    Type = ELF::STT_GNU_IFUNC;
   if (Base) {
     Type = mergeTypeForSet(Type, Base->getType());
   }
   uint8_t Info = (Binding << 4) | Type;
 
   // Other and Visibility share the same byte with Visibility using the lower
   // 2 bits
   uint8_t Visibility = Symbol.getVisibility();

llvm/test/MC/ELF/ifunc-alias.s

+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj --symbols | FileCheck %s
+.text
+
+.type  foo_impl,@function
+foo_impl:
+  ret
+
+.type  foo_resolver,@function
+foo_resolver:
+  mov $foo_impl, %rax
+  ret
+
+.type  foo,@gnu_indirect_function
+.set   foo,foo_resolver
+
+// All things below should be IFunc identical to 'foo'
+.set   foo2,foo
+.set   foo3,foo2
+.type  foo4,@function
+.set   foo4,foo3
+
+// But tls_object should not be IFunc
+.set   tls,foo
+.type  tls,@tls_object
+
+// CHECK:       Symbol {
+// CHECK:         Name: foo
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: GNU_IFunc
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: foo2
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: GNU_IFunc
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: foo3
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: GNU_IFunc
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: foo4
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: GNU_IFunc
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: foo_impl
+// CHECK-NEXT:    Value: 0x0
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: Function
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: foo_resolver
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: Function
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: tls
+// CHECK-NEXT:    Value: 0x1
+// CHECK-NEXT:    Size: 0
+// CHECK-NEXT:    Binding: Local
+// CHECK-NEXT:    Type: TLS
+// CHECK-NEXT:    Other: 0
+// CHECK-NEXT:    Section: .text
+// CHECK-NEXT:  }