[PATCH] don't define SHARED macro in the source

[PATCH] don't define SHARED macro in the source

[Petr Hosek]

[-- Attachment #1.1: Type: text/plain, Size: 391 bytes --]

When building musl with pnacl-clang compiler I found a bug: crt/rcrt1.c
defines the SHARED macro which enables the conditional block in
src/ldso/dlstart.c, this breaks for us because pnacl-clang doesn't support
shared libraries nor inline assembly. If my understanding is correct, the
SHARED macro should be (and already is) defined only in CFLAGS_ALL_SHARED.
The attached patch fixes that.

[-- Attachment #1.2: Type: text/html, Size: 419 bytes --]

[-- Attachment #2: don-t-define-SHARED-macro-in-the-source.patch --]
[-- Type: text/x-patch, Size: 611 bytes --]

From 7b1348fa1a8dd2d02a9087639c6b02ce9585aef2 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek@chromium.org>
Date: Thu, 5 Nov 2015 21:55:20 -0800
Subject: [PATCH] don't define SHARED macro in the source

the SHARED macro should only be defined in CFLAGS_ALL_SHARED when
building musl as shared library.
---
 crt/rcrt1.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crt/rcrt1.c b/crt/rcrt1.c
index 9be117f..0ba78f8 100644
--- a/crt/rcrt1.c
+++ b/crt/rcrt1.c
@@ -1,4 +1,3 @@
-#define SHARED
 #define START "_start"
 #define _dlstart_c _start_c
 #include "../src/ldso/dlstart.c"
-- 
2.6.0.rc2.230.g3dd15c0

Re: [PATCH] don't define SHARED macro in the source

[Szabolcs Nagy]

* Petr Hosek <phosek@google.com> [2015-11-06 23:46:40 +0000]:
> When building musl with pnacl-clang compiler I found a bug: crt/rcrt1.c
> defines the SHARED macro which enables the conditional block in
> src/ldso/dlstart.c, this breaks for us because pnacl-clang doesn't support
> shared libraries nor inline assembly. If my understanding is correct, the
> SHARED macro should be (and already is) defined only in CFLAGS_ALL_SHARED.
> The attached patch fixes that.

but rcrt1.o is not built with CFLAGS_ALL_SHARED and SHARED is needed

rcrt1.o should be only used for static pie

> From 7b1348fa1a8dd2d02a9087639c6b02ce9585aef2 Mon Sep 17 00:00:00 2001
> From: Petr Hosek <phosek@chromium.org>
> Date: Thu, 5 Nov 2015 21:55:20 -0800
> Subject: [PATCH] don't define SHARED macro in the source
> 
> the SHARED macro should only be defined in CFLAGS_ALL_SHARED when
> building musl as shared library.
> ---
>  crt/rcrt1.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/crt/rcrt1.c b/crt/rcrt1.c
> index 9be117f..0ba78f8 100644
> --- a/crt/rcrt1.c
> +++ b/crt/rcrt1.c
> @@ -1,4 +1,3 @@
> -#define SHARED
>  #define START "_start"
>  #define _dlstart_c _start_c
>  #include "../src/ldso/dlstart.c"
> -- 
> 2.6.0.rc2.230.g3dd15c0
> 

Re: [PATCH] don't define SHARED macro in the source

[Petr Hosek]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

We don't support static pie in our toolchain so I haven't really tested
that scenario but now I see why the SHARED macro is needed here.

One option around this issue would be to avoid building rcrt1.c altogether,
but I'd like to avoid carrying an extra patch for that. The other option
would be to allow inline assembly in this case since it seems to be only
used as a guard.  I'll see if the second option is feasible.

On Sat, Nov 7, 2015 at 5:24 AM Szabolcs Nagy <nsz@port70.net> wrote:

> * Petr Hosek <phosek@google.com> [2015-11-06 23:46:40 +0000]:
> > When building musl with pnacl-clang compiler I found a bug: crt/rcrt1.c
> > defines the SHARED macro which enables the conditional block in
> > src/ldso/dlstart.c, this breaks for us because pnacl-clang doesn't
> support
> > shared libraries nor inline assembly. If my understanding is correct, the
> > SHARED macro should be (and already is) defined only in
> CFLAGS_ALL_SHARED.
> > The attached patch fixes that.
>
> but rcrt1.o is not built with CFLAGS_ALL_SHARED and SHARED is needed
>
> rcrt1.o should be only used for static pie
>
> > From 7b1348fa1a8dd2d02a9087639c6b02ce9585aef2 Mon Sep 17 00:00:00 2001
> > From: Petr Hosek <phosek@chromium.org>
> > Date: Thu, 5 Nov 2015 21:55:20 -0800
> > Subject: [PATCH] don't define SHARED macro in the source
> >
> > the SHARED macro should only be defined in CFLAGS_ALL_SHARED when
> > building musl as shared library.
> > ---
> >  crt/rcrt1.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/crt/rcrt1.c b/crt/rcrt1.c
> > index 9be117f..0ba78f8 100644
> > --- a/crt/rcrt1.c
> > +++ b/crt/rcrt1.c
> > @@ -1,4 +1,3 @@
> > -#define SHARED
> >  #define START "_start"
> >  #define _dlstart_c _start_c
> >  #include "../src/ldso/dlstart.c"
> > --
> > 2.6.0.rc2.230.g3dd15c0
> >
>
>

[-- Attachment #2: Type: text/html, Size: 2432 bytes --]

Re: [PATCH] don't define SHARED macro in the source

[Rich Felker]

On Sat, Nov 07, 2015 at 11:24:56PM +0000, Petr Hosek wrote:
> We don't support static pie in our toolchain so I haven't really tested
> that scenario but now I see why the SHARED macro is needed here.
> 
> One option around this issue would be to avoid building rcrt1.c altogether,
> but I'd like to avoid carrying an extra patch for that. The other option
> would be to allow inline assembly in this case since it seems to be only
> used as a guard.  I'll see if the second option is feasible.

As part of this release cycle, I plan to, by default, use the same .o
files (built as PIC) for both libc.so and libc.a, and remove the
SHARED macro. --disable-pic would then both imply --disable-shared and
inhibit building of rcrt1.o. I could add a separate
--disable-static-pie if you want to be able to use PIC but not static
pie (not generate rcrt1.o) and your arch could force all of these, but
my guess is that there's no such thing as PIC vs non-PIC for pnacl and
it's some kind of higher-level byte code. Anyway, does that sound
reasonable?

BTW, how are you doing cancellation without asm?

Rich

Re: [PATCH] don't define SHARED macro in the source

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2015-11-07 19:58:33 -0500]:
> 
> BTW, how are you doing cancellation without asm?
> 

are there even signals? :)

Re: [PATCH] don't define SHARED macro in the source

[Petr Hosek]

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

We do distinguish between PIC and non-PIC but only during translation from
bitcode down to native code when we link in the startup bits which have be
built as native binary. However, this part is handled separately from the C
library.

I got rcrt1.o building even with the SHARED macro by cherry-picking some of
the recent Clang changes related to inline asm handling into our compiler,
so this patch can be discarded. We don't really need rcrt1.o but as long as
we can build it, I don't mind it being there so the extra option is
probably not necessary.

Regarding cancellation, we handle these cases through intrinsics. We do
have signals but they're somewhat different from POSIX signals.

On Sat, Nov 7, 2015 at 4:58 PM Rich Felker <dalias@libc.org> wrote:

> On Sat, Nov 07, 2015 at 11:24:56PM +0000, Petr Hosek wrote:
> > We don't support static pie in our toolchain so I haven't really tested
> > that scenario but now I see why the SHARED macro is needed here.
> >
> > One option around this issue would be to avoid building rcrt1.c
> altogether,
> > but I'd like to avoid carrying an extra patch for that. The other option
> > would be to allow inline assembly in this case since it seems to be only
> > used as a guard.  I'll see if the second option is feasible.
>
> As part of this release cycle, I plan to, by default, use the same .o
> files (built as PIC) for both libc.so and libc.a, and remove the
> SHARED macro. --disable-pic would then both imply --disable-shared and
> inhibit building of rcrt1.o. I could add a separate
> --disable-static-pie if you want to be able to use PIC but not static
> pie (not generate rcrt1.o) and your arch could force all of these, but
> my guess is that there's no such thing as PIC vs non-PIC for pnacl and
> it's some kind of higher-level byte code. Anyway, does that sound
> reasonable?
>
> BTW, how are you doing cancellation without asm?
>
> Rich
>

[-- Attachment #2: Type: text/html, Size: 2318 bytes --]