Discussion:
pcc-libs' crtbegin/end crashes in shared libs?
Rich Felker
2014-09-24 14:30:34 UTC
Permalink
Hi,

Based on a bug report by Rune, shared libraries built with pcc crash
when loaded by musl libc. I've tracked the issue down to the _init
function in the produced .so's and don't see how this could be working
at all:

00000320 <_init>:
320: 83 ec 0c sub $0xc,%esp
323: 89 c3 mov %eax,%ebx
325: e8 16 00 00 00 call 340 <***@plt>
32a: 83 c4 0c add $0xc,%esp
32d: c3 ret

This code, produced from pcc-libs' crtbegin.c (and from
crti.o/crtn.o), is loading ebx with junk then making a call via the
PLT, which depends on having ebx pointing to the GOT. I'm guessing the
correct address for the GOT was loaded into eax _before_ the asm
volatile (".section " #section); in crtbegin.c, and thus lost.

I first thought this was an incompatibility specific to musl (musl's
crti.o does not save ebx before the function fragments in _init and
_finit; glibc's does) but even if I added code to save/restore it on
musl's side, the value of ebx on entry to the PLT thunk would still be
nonsense.

I think the core problem is that the function call is being made via
the PLT to begin with. Rune reported that -Bsymbolic fixed the issue,
and indeed it would bypass the PLT, but it still clobbers ebx.

The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register. This hidden wrapper function will then
contain the correct prologue to save any call-saved registers that
might be clobbered, load the GOT register, and call the real
__do_global_ctors_aux via the PLT.

Does my description of the problem sound correct, and my proposed
solution ok? If so I can prepare a patch.

Rich
Joerg Sonnenberger
2014-09-24 14:52:23 UTC
Permalink
Post by Rich Felker
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register.
__do_global_ctors_aux must be hidden or local scope, everything else is
broken.

Joerg
Rich Felker
2014-09-25 16:56:55 UTC
Permalink
Post by Joerg Sonnenberger
Post by Rich Felker
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register.
__do_global_ctors_aux must be hidden or local scope, everything else is
broken.
In that case, I think just making it hidden should fix the problem.
I'll check.

Rich
Rich Felker
2014-09-27 02:05:02 UTC
Permalink
Post by Rich Felker
Post by Joerg Sonnenberger
Post by Rich Felker
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register.
__do_global_ctors_aux must be hidden or local scope, everything else is
broken.
In that case, I think just making it hidden should fix the problem.
I'll check.
Does the attached patch look acceptable? It seems to correct the code
generation.

Rich
Joerg Sonnenberger
2014-09-27 08:10:03 UTC
Permalink
Post by Rich Felker
Post by Rich Felker
Post by Joerg Sonnenberger
Post by Rich Felker
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register.
__do_global_ctors_aux must be hidden or local scope, everything else is
broken.
In that case, I think just making it hidden should fix the problem.
I'll check.
Does the attached patch look acceptable? It seems to correct the code
generation.
Looks sane enough.

Joerg
Anders Magnusson
2014-09-28 08:26:14 UTC
Permalink
Post by Rich Felker
Post by Rich Felker
Post by Joerg Sonnenberger
Post by Rich Felker
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register.
__do_global_ctors_aux must be hidden or local scope, everything else is
broken.
In that case, I think just making it hidden should fix the problem.
I'll check.
Does the attached patch look acceptable? It seems to correct the code
generation.
People seems happy with this so I have applied it. Thanks!

-- Ragge

u***@aetey.se
2014-09-25 16:47:08 UTC
Permalink
Post by Rich Felker
320: 83 ec 0c sub $0xc,%esp
323: 89 c3 mov %eax,%ebx
32a: 83 c4 0c add $0xc,%esp
32d: c3 ret
I think the core problem is that the function call is being made via
the PLT to begin with. Rune reported that -Bsymbolic fixed the issue,
and indeed it would bypass the PLT, but it still clobbers ebx.
The correct solution, of course, is not to call __do_global_ctors_aux
directly from _init, but to instead call a hidden function, which the
compiler knows it can call directly without the PLT and without
loading the GOT register. This hidden wrapper function will then
contain the correct prologue to save any call-saved registers that
might be clobbered, load the GOT register, and call the real
__do_global_ctors_aux via the PLT.
Does my description of the problem sound correct, and my proposed
solution ok? If so I can prepare a patch.
Judging by the [lack of] comments you are the most informed party
about the subject, so I would say "thanks, please go ahead".
I will be happy to test.

Rune
u***@aetey.se
2014-09-27 09:38:33 UTC
Permalink
Post by Rich Felker
Does the attached patch look acceptable? It seems to correct the code
generation.
Rich
I confirm that pcc on i386 with this patch produces (without -Bsymbolic)
shared libraries which do not cause crashes in my tests, among others
with libpcap.

Thanks Rich!

Rune
Post by Rich Felker
--- csu/linux/crtbegin.c.orig
+++ csu/linux/crtbegin.c
@@ -84,6 +84,7 @@
(**p++)();
}
+__attribute__((__visibility__("hidden"), __noinline__))
void
__do_global_ctors_aux(void)
{
@@ -95,6 +96,7 @@
}
}
+__attribute__((__visibility__("hidden"), __noinline__))
void
__do_global_dtors_aux(void)
{
_______________________________________________
Pcc mailing list
http://lists.ludd.ltu.se/cgi-bin/mailman/listinfo/pcc
Loading...