Page MenuHomePhabricator

[ELF][MC] Fix IFunc alias resolving issue
ClosedPublic

Authored by zsrkmyn on Sep 4 2019, 6:25 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

zsrkmyn created this revision.Sep 4 2019, 6:25 PM
zsrkmyn added a comment.EditedSep 4 2019, 6:30 PM

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 added inline comments.Sep 4 2019, 7:32 PM
llvm/lib/MC/ELFObjectWriter.cpp
539 ↗(On Diff #218822)

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
4 ↗(On Diff #218822)

.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 added inline comments.Sep 4 2019, 8:02 PM
llvm/lib/MC/ELFObjectWriter.cpp
539 ↗(On Diff #218822)

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

zsrkmyn updated this revision to Diff 219039.Sep 6 2019, 12:51 AM
zsrkmyn marked 3 inline comments as done.
MaskRay added inline comments.Sep 6 2019, 12:55 AM
llvm/lib/MC/ELFObjectWriter.cpp
515 ↗(On Diff #219039)

Can you change the recursion to a loop?

llvm/test/MC/ELF/ifunc-alias.s
46 ↗(On Diff #219039)

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

zsrkmyn updated this revision to Diff 219046.Sep 6 2019, 1:53 AM
zsrkmyn marked 2 inline comments as done.
zsrkmyn updated this revision to Diff 219047.Sep 6 2019, 1:56 AM
MaskRay added inline comments.Sep 6 2019, 9:32 AM
llvm/lib/MC/ELFObjectWriter.cpp
514 ↗(On Diff #219047)

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

zsrkmyn updated this revision to Diff 219225.Sep 7 2019, 12:27 AM
zsrkmyn marked an inline comment as done.
MaskRay accepted this revision.Sep 7 2019, 12:54 AM
This revision is now accepted and ready to land.Sep 7 2019, 12:54 AM

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

This revision was automatically updated to reflect the committed changes.

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...