[PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

When switching optimization to higher levels (-O3) and enable link time
optimization (-flto) gcc finds two instances of the __pthread_tsd_main
variable that are declared with different sizes.

The real size that is needed is known in both source files. Just use
equivalent definitions.

1	1	src/thread/pthread_self.c

diff --git a/src/thread/pthread_self.c b/src/thread/pthread_self.c
index 23dbaa5..32a6e5d 100644
--- a/src/thread/pthread_self.c
+++ b/src/thread/pthread_self.c
@@ -3,7 +3,7 @@
 static struct pthread *main_thread = &(struct pthread){0};
 
 /* pthread_key_create.c overrides this */
-static const void *dummy[1] = { 0 };
+static const void *dummy[PTHREAD_KEYS_MAX] = { 0 };
 weak_alias(dummy, __pthread_tsd_main);
 
 static int init_main_thread()
-- 
1.7.9.5

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Rich Felker]

On Sun, Feb 10, 2013 at 11:32:47PM +0100, Jens Gustedt wrote:
> When switching optimization to higher levels (-O3) and enable link time
> optimization (-flto) gcc finds two instances of the __pthread_tsd_main
> variable that are declared with different sizes.
> 
> The real size that is needed is known in both source files. Just use
> equivalent definitions.
> 
> 1	1	src/thread/pthread_self.c
> 
> diff --git a/src/thread/pthread_self.c b/src/thread/pthread_self.c
> index 23dbaa5..32a6e5d 100644
> --- a/src/thread/pthread_self.c
> +++ b/src/thread/pthread_self.c
> @@ -3,7 +3,7 @@
>  static struct pthread *main_thread = &(struct pthread){0};
>  
>  /* pthread_key_create.c overrides this */
> -static const void *dummy[1] = { 0 };
> +static const void *dummy[PTHREAD_KEYS_MAX] = { 0 };
>  weak_alias(dummy, __pthread_tsd_main);

Nope, that defeats the whole purpose, which is to avoid wasting space
when it's not needed. This warning is bogus. There's no reason a weak
symbol can't have different size than a strong one that optionally
replaces it.

Rich

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Sonntag, den 10.02.2013, 19:31 -0500 schrieb Rich Felker:
> Nope, that defeats the whole purpose, which is to avoid wasting space
> when it's not needed. This warning is bogus. There's no reason a weak
> symbol can't have different size than a strong one that optionally
> replaces it.

Hm, about wasting space I am not very convinced, but maybe I didn't
understand well enough why you need all these aliases after all, and
why you can't refer to the real symbol directly.

In any case, this is perhaps better done with the tool chain. I have
good experience by having

  -fdata-sections -ffunction-sections

for the compiler options and then

   -Wl, --gc-sections

for the link. In the case of musl, this removes exactly all the dummy
sections :) plus two others (pad and sccp), see below.

Also I observed that the .so when compiled with -O3 and -flto is
smaller than with the default build options.

Jens

/usr/bin/ld: Removing unused section '.rodata.dummy_file.17696.2847' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.rodata.dummy.18033.2845' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.rodata.dummy.18516.2843' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.rodata.dummy_file.24416.2840' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.rodata.dummy.24420.2836' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.bss.dummy.25545.2834' in file '/tmp/ccKGiYdU.ltrans0.ltrans.o'
/usr/bin/ld: Removing unused section '.text.pad.21403' in file '/tmp/ccKGiYdU.ltrans2.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.16099.2724' in file '/tmp/ccKGiYdU.ltrans8.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy_0.14753.2904' in file '/tmp/ccKGiYdU.ltrans8.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy_0.14278.2989' in file '/tmp/ccKGiYdU.ltrans8.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.14134.2991' in file '/tmp/ccKGiYdU.ltrans8.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy0.11727.2430' in file '/tmp/ccKGiYdU.ltrans13.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy1.11721.2433' in file '/tmp/ccKGiYdU.ltrans13.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy0.11661.2464' in file '/tmp/ccKGiYdU.ltrans13.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy1.11656.2467' in file '/tmp/ccKGiYdU.ltrans13.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy_0.20439.2558' in file '/tmp/ccKGiYdU.ltrans17.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy_0.24290.2666' in file '/tmp/ccKGiYdU.ltrans20.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.23638.2864' in file '/tmp/ccKGiYdU.ltrans20.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.23456.2867' in file '/tmp/ccKGiYdU.ltrans20.ltrans.o'
/usr/bin/ld: Removing unused section '.text.sccp.23444.2870' in file '/tmp/ccKGiYdU.ltrans20.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy_1.26947.2522' in file '/tmp/ccKGiYdU.ltrans22.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.5449.2666' in file '/tmp/ccKGiYdU.ltrans28.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.5419.2669' in file '/tmp/ccKGiYdU.ltrans28.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.5020.2796' in file '/tmp/ccKGiYdU.ltrans28.ltrans.o'
/usr/bin/ld: Removing unused section '.text.dummy.2522.3034' in file '/tmp/ccKGiYdU.ltrans28.ltrans.o'


-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 08:40:20 +0100]:
> Am Sonntag, den 10.02.2013, 19:31 -0500 schrieb Rich Felker:
> > Nope, that defeats the whole purpose, which is to avoid wasting space
> > when it's not needed. This warning is bogus. There's no reason a weak
> > symbol can't have different size than a strong one that optionally
> > replaces it.
> 
> Hm, about wasting space I am not very convinced, but maybe I didn't
> understand well enough why you need all these aliases after all, and
> why you can't refer to the real symbol directly.
> 

aliases are there for a reason

> In any case, this is perhaps better done with the tool chain. I have
> good experience by having
> 
>   -fdata-sections -ffunction-sections
> 
> for the compiler options and then
> 
>    -Wl, --gc-sections
> 
> for the link. In the case of musl, this removes exactly all the dummy
> sections :) plus two others (pad and sccp), see below.
> 

are you sure about the correctness of these?

they seem to be broken to me: weak aliases are not respected
and functions are dropped even if there are weak references to
them which is bad..

eg if sccp is dropped then in any code that does not
use pthread, the cancellable syscalls will be broken
(if i understand these right)

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2013-02-11 12:22:37 +0100]:
> * Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 08:40:20 +0100]:
> > In any case, this is perhaps better done with the tool chain. I have
> > good experience by having
> > 
> >   -fdata-sections -ffunction-sections
> > 
> > for the compiler options and then
> > 
> >    -Wl, --gc-sections
> > 
> > for the link. In the case of musl, this removes exactly all the dummy
> > sections :) plus two others (pad and sccp), see below.
> > 
> 
> are you sure about the correctness of these?
> 
> they seem to be broken to me: weak aliases are not respected
> and functions are dropped even if there are weak references to
> them which is bad..
> 
> eg if sccp is dropped then in any code that does not
> use pthread, the cancellable syscalls will be broken
> (if i understand these right)

ah sorry, --gc-sections is for dynamic linking,
there the weak aliases dont matter

and in case of static linking -fdata-sections
and -ffunction-sections just makes the elfheader
bigger and the linking slower (sum size of sections
may be a bit smaller or bigger because of alignment
things)

so these flags may be useful for building .so

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Montag, den 11.02.2013, 13:08 +0100 schrieb Szabolcs Nagy:
> * Szabolcs Nagy <nsz@port70.net> [2013-02-11 12:22:37 +0100]:
> > are you sure about the correctness of these?
> > 
> > they seem to be broken to me: weak aliases are not respected
> > and functions are dropped even if there are weak references to
> > them which is bad..
> > 
> > eg if sccp is dropped then in any code that does not
> > use pthread, the cancellable syscalls will be broken
> > (if i understand these right)

the weak alias for sccp is dropped in my link for .so since there is
the real sccp symbol that is linked in. (for pad I didn't check, there
are several places with "pad" as names :)

> ah sorry, --gc-sections is for dynamic linking,
> there the weak aliases dont matter

Hm, I don't think that this is for dynamic linking, only.

> and in case of static linking -fdata-sections
> and -ffunction-sections just makes the elfheader
> bigger and the linking slower (sum size of sections
> may be a bit smaller or bigger because of alignment
> things)

Still I have good experience by using it on another library that
defines a lot of weak symbols that aren't used.

The only real problem I have for libmusl.so is "environ", as of my
other patch. There is probably a good reason that this is made
weak. But why must it be realized as an alias? Wouldn't it be
sufficient to just have it as a weak symbol? something like

// implementation header file
extern char **__environ __attribute__((__weak ));

//__environ.c
char **environ __attribute__((__weak )) = 0;
weak_alias(environ, ___environ);
weak_alias(environ, __environ);
weak_alias(environ, _environ);

> so these flags may be useful for building .so

As I said in my previous mail:

> Also I observed that the .so when compiled with -O3 and -flto is
> smaller than with the default build options.

this is true even without the gc-section stuff. So this is perhaps a
thing to consider.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 13:51:24 +0100]:
> > and in case of static linking -fdata-sections
> > and -ffunction-sections just makes the elfheader
> > bigger and the linking slower (sum size of sections
> > may be a bit smaller or bigger because of alignment
> > things)
> 
> Still I have good experience by using it on another library that
> defines a lot of weak symbols that aren't used.
> 

i've just tried it and the .a just got bigger
and size reports minimal difference probably
due to alignment differences, which does not matter

$ size libc.a.sections |awk '{s+=$4}END{print s}'
410638
$ size libc.a |awk '{s+=$4}END{print s}'
410698

> The only real problem I have for libmusl.so is "environ", as of my
> other patch. There is probably a good reason that this is made
> weak. But why must it be realized as an alias? Wouldn't it be
> sufficient to just have it as a weak symbol? something like
> 
> // implementation header file
> extern char **__environ __attribute__((__weak ));
> 
> //__environ.c
> char **environ __attribute__((__weak )) = 0;
> weak_alias(environ, ___environ);
> weak_alias(environ, __environ);
> weak_alias(environ, _environ);
> 

there are posix requirements for environ so it must be weak
i'm not sure about the alias though

> > so these flags may be useful for building .so
> 
> As I said in my previous mail:
> 
> > Also I observed that the .so when compiled with -O3 and -flto is
> > smaller than with the default build options.
> 
> this is true even without the gc-section stuff. So this is perhaps a
> thing to consider.
> 

yes, it is known
but before adding -flto the code should be audited for correctness

(there might be missing barriers where the code works now because
without lto the optimizer cannot reorder code around extern functions,
this probably affects math functions that access fenv, note that
c99 FENV_ACCESS pragma is not respected by gcc and if float operations
are reordered around fe* functions the behaviour changes)

and of course all these flags are fairly recent so they need
configure script changes (and they might be broken so they
need testing)

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Montag, den 11.02.2013, 14:09 +0100 schrieb Szabolcs Nagy:
> i've just tried it and the .a just got bigger
> and size reports minimal difference probably
> due to alignment differences, which does not matter
> 
> $ size libc.a.sections |awk '{s+=$4}END{print s}'
> 410638
> $ size libc.a |awk '{s+=$4}END{print s}'
> 410698

yes, for musl it probably isn't a big deal, because musl is basically
already divided in one compilation unit per function. As I said, the
only unused sections that drop out are those for the weak aliases that
finally have their real symbol linked in.

For other libraries that define weak symbols that are not needed later
this is more relevant. With P99 I have the opposite position to musl
(sort of). Everything are inline functions if possible (so they don't
even define a symbol) only some functions have to be weak because they
access to some static data or use setjmp. (P99 doesn't even have a
libp99.a or libp99.so file).

With the gc-section trick I can suck out the unused symbols from the
executable at the end.

> > The only real problem I have for libmusl.so is "environ", as of my
> > other patch. There is probably a good reason that this is made
> > weak. But why must it be realized as an alias? Wouldn't it be
> > sufficient to just have it as a weak symbol? something like
> > 
> > // implementation header file
> > extern char **__environ __attribute__((__weak ));
> > 
> > //__environ.c
> > char **environ __attribute__((__weak )) = 0;
> > weak_alias(environ, ___environ);
> > weak_alias(environ, __environ);
> > weak_alias(environ, _environ);
> > 
> 
> there are posix requirements for environ so it must be weak
> i'm not sure about the alias though

Anyhow the use of environ versus __environ was just not consistent
through the rest of musl. I have send another patch that should heal
that.

But thinking of it, I am not sure that the way that is done makes any
sense. If the reason to have environ weak is that the user code could
supply its own environ object, then the use of __environ all over musl
will certainly break things. Then environ and __environ could be two
different objects and the application code and musl would not be
consistent in their access to the environment.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Rich Felker]

On Mon, Feb 11, 2013 at 02:38:08PM +0100, Jens Gustedt wrote:
> > there are posix requirements for environ so it must be weak
> > i'm not sure about the alias though
> 
> Anyhow the use of environ versus __environ was just not consistent
> through the rest of musl. I have send another patch that should heal
> that.

The current intended usage is that __environ is used in ISO C
functions and startup code that must not reference the name environ
(since it's in the namespace reserved for the application), and POSIX
and extension functions use environ. There's no reason the latter
_need_ to use environ though; it was just more convenient and less
implementation-specific. If you think there's a good reason to change
it after reading this, let's discuss it.

Rich

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Montag, den 11.02.2013, 08:44 -0500 schrieb Rich Felker:
> The current intended usage is that __environ is used in ISO C
> functions and startup code that must not reference the name environ
> (since it's in the namespace reserved for the application), and POSIX
> and extension functions use environ.

If this was the intended use, the effective use was not in line with
it. execv and execvp had it differently.

> There's no reason the latter
> _need_ to use environ though; it was just more convenient and less
> implementation-specific. If you think there's a good reason to change
> it after reading this, let's discuss it.

With what I have added in my other mail (and you have snipped :) in
the current model there is even the danger that environ and __environ
split into two different objects.

char **environ;

int main(void) {
  return (long)&environ;
}

When compile-linking this with musl I get both symbols environ and
__environ linked in.

So it would at least be good that the internal use of it in musl would
be consistent and the C library would not see two distinct objects.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 15:07:28 +0100]:
> the current model there is even the danger that environ and __environ
> split into two different objects.
> 
> char **environ;
> 
> int main(void) {
>   return (long)&environ;
> }
> 
> When compile-linking this with musl I get both symbols environ and
> __environ linked in.
> 

and that is precisely what posix requires


> So it would at least be good that the internal use of it in musl would
> be consistent and the C library would not see two distinct objects.
> 

no


"The ISO C standard and this volume of POSIX.1-2008 do not conflict
on the use of environ, but some historical implementations of environ
may cause a conflict. As long as environ is treated in the same way
as an entry point (for example, fork()), it conforms to both standards.
A library can contain fork(), but if there is a user-provided fork(),
that fork() is given precedence and no problem ensues.
The situation is similar for environ: the definition in this volume of
POSIX.1-2008 is to be used if there is no user-provided environ to take
precedence. At least three implementations are known to exist that
solve this problem."

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Montag, den 11.02.2013, 15:39 +0100 schrieb Szabolcs Nagy:
> * Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 15:07:28 +0100]:
> > So it would at least be good that the internal use of it in musl would
> > be consistent and the C library would not see two distinct objects.
> > 
> 
> no

I take it that this "no" is only for the second part of the
assertion, and that you would subscribe to the shortend phrase

   So it would at least be good that the internal use of it in musl
   would be consistent.

If it is stated in POSIX that a userspace environ is a different
object from the one in the library, I am perfectly fine with
that. Documented behavior is a good thing, and userspace should see
its environ.

As it is currently, without my patch, execv would see __environ and
execvp would see the user space environ. I am still convinced that
this isn't desirable, both should see __environ, and that is what I
meant with my subphrase "and the C library would not see two distinct
objects".

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 17:30:23 +0100]:
> Am Montag, den 11.02.2013, 15:39 +0100 schrieb Szabolcs Nagy:
> > * Jens Gustedt <jens.gustedt@inria.fr> [2013-02-11 15:07:28 +0100]:
> > > So it would at least be good that the internal use of it in musl would
> > > be consistent and the C library would not see two distinct objects.
> > > 
> > 
> > no
> 
> I take it that this "no" is only for the second part of the
> assertion, and that you would subscribe to the shortend phrase
> 
>    So it would at least be good that the internal use of it in musl
>    would be consistent.
> 
> If it is stated in POSIX that a userspace environ is a different
> object from the one in the library, I am perfectly fine with
> that. Documented behavior is a good thing, and userspace should see
> its environ.
> 
> As it is currently, without my patch, execv would see __environ and
> execvp would see the user space environ. I am still convinced that
> this isn't desirable, both should see __environ, and that is what I
> meant with my subphrase "and the C library would not see two distinct
> objects".
> 

you are right

execv and execvp should consistently use __environ

it seems execvp was changed to fix a bug in posix_spawn
but that's no longer relevant with the new implementation

" fix parent-memory-clobber in posix_spawn (environ)"

btw system uses environ but internal/libc.h defines environ
to be __environ, i think that's hideous

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Jens Gustedt]

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

Am Montag, den 11.02.2013, 18:08 +0100 schrieb Szabolcs Nagy:
> btw system uses environ but internal/libc.h defines environ
> to be __environ, i think that's hideous

Yes, I had seen that, my second version of the patch addresses that,
too. Perhaps one should also remove the define from internal/libc.h ?

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Rich Felker]

On Mon, Feb 11, 2013 at 06:21:54PM +0100, Jens Gustedt wrote:
> Am Montag, den 11.02.2013, 18:08 +0100 schrieb Szabolcs Nagy:
> > btw system uses environ but internal/libc.h defines environ
> > to be __environ, i think that's hideous
> 
> Yes, I had seen that, my second version of the patch addresses that,
> too. Perhaps one should also remove the define from internal/libc.h ?

I agree that's hideous and should be removed.

Rich

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Rich Felker]

On Mon, Feb 11, 2013 at 03:07:28PM +0100, Jens Gustedt wrote:
> Am Montag, den 11.02.2013, 08:44 -0500 schrieb Rich Felker:
> > The current intended usage is that __environ is used in ISO C
> > functions and startup code that must not reference the name environ
> > (since it's in the namespace reserved for the application), and POSIX
> > and extension functions use environ.
> 
> If this was the intended use, the effective use was not in line with
> it. execv and execvp had it differently.
> 
> > There's no reason the latter
> > _need_ to use environ though; it was just more convenient and less
> > implementation-specific. If you think there's a good reason to change
> > it after reading this, let's discuss it.
> 
> With what I have added in my other mail (and you have snipped :) in
> the current model there is even the danger that environ and __environ
> split into two different objects.
> 
> char **environ;
> 
> int main(void) {
>   return (long)&environ;
> }

This is a conforming C program, but not a conforming POSIX program.
The current code in musl should allow it to work as long as it does
not call functions in the standard library which are not defined by
ISO C. If any POSIX functions are used, the program is non-conforming
and invokes undefined behavior.

With that said, I don't see any reason this behavior is _desirable_ or
beneficial, so I'm fine with changing it and always using __environ
internally. We could even add a visibility attribute on it so that
GOT/PC-relative accesses get used in compilers which support
visibility, instead of the more expensive accesses through the GOT.

Rich

Re: [PATCH 2/3] Have different definitions of __pthread_tsd_main agree in size

[Rich Felker]

On Mon, Feb 11, 2013 at 04:47:50PM -0500, Rich Felker wrote:
> With that said, I don't see any reason this behavior is _desirable_ or
> beneficial, so I'm fine with changing it and always using __environ
> internally. We could even add a visibility attribute on it so that
> GOT/PC-relative accesses get used in compilers which support
> visibility, instead of the more expensive accesses through the GOT.

Actually this is wrong. __environ might be relocated to the main
executable via copy relocations, so it can't be accessed GOT-relative.
Giving it protected visibility would actually break this, because of
bugs in GCC...

Rich