Re: [PATCH 9/10] GLIBC ABI patches

Re: [PATCH 9/10] GLIBC ABI patches

[idunham]

>
>>>>>>> Just nonsense aliases GNU uses...
>>>>>>> Needed for ABI compatability.
>>>>>> could we mark them as such? at least with a comment.
>>>>>> I really like that musl is so readable. This patch adds some
>>>>>> obfuscation that can simply be countered by marking it as "ok this
>>>>>> is only here for reason X."
>>>>> I would like to see those options behind a compile time option : It
>>>>> bloats musl with in many cases unneeded code. I test my compiles with
>>>>> musl, and I like it lean and mean.
>>>> These are just aliases, not code. There's no bloat there.
>>>>
>>>> One of the advantages of musl is its LACK of configurability: If you
>>>> have *musl*, you know what precisely you're getting.
>>>>
>>>> With valediction,
>>>> - Gregor Richards
>>>>
>>> While I agree with the above, I still have a few objections :
>>>
>>> - We don't want glibc compatibility. We want a good libc.
>>> - That we even need those aliases is usually a case of bad automake /
>>> autoconf / bad feature detection.
>>>
>>> Why bloat code with stuff to provide glibc compatibility ?
>>>
>>>
>>> 	Igmar
>>
>> These are for ABI compatibility, not API compatibility. Nobody using
>> glibc uses these symbols intentionally, they are renamed and aliased by
>> the library. Last I checked, musl is shockingly close to ABI
>> compatibility with glibc, and like it or not, that's a valuable feature.
>> If you don't like the *bloat* of it, you'll have to dig out a lot of the
>> existing aliases too.
>
> I've seen lots of code who use internal glibc functions / data structures.
> We want to prevent them from being used, that's why I personally have a
> problem with adding code like this. Unless it actually serves a real use.
>
> 	Igmar

The real use here, from what orc was saying, is running the proprietary
nvidia driver on a musl-based system.

Rich:
I have three questions:

1. Should it be possible to weak_alias an externally defined function?
ie, src/stdlib/strtod.c defines strtod, then src/glibc-abi/stdlib.c goes:
#include "libc.h"
#include <stdlib.h>
weak_alias(strtod, __strtod_internal)
2. Would this result in obfustication of the source?
3. Would a subdirectory of src/ solely for glibc ABI/API compatability be
appropriate?
The point would be to allow removing gnuish extensions without too much
trouble.
Even if 1 or 2 mean that weak_alias stuff goes in the same file, this
could contain functions like the ones added for gnulib (src/stdio/ext*.c).

Isaac Dunham

Re: [PATCH 9/10] GLIBC ABI patches

[Rich Felker]

On Tue, Jul 24, 2012 at 11:00:54PM -0400, idunham@lavabit.com wrote:
> Rich:
> I have three questions:
> 
> 1. Should it be possible to weak_alias an externally defined function?
> ie, src/stdlib/strtod.c defines strtod, then src/glibc-abi/stdlib.c goes:
> #include "libc.h"
> #include <stdlib.h>
> weak_alias(strtod, __strtod_internal)

This is not possible the way traditional assemblers/linkers work. You
can make a wrapper function but that adds bloat and has a runtime
performance cost if the wrapper is used.

> 2. Would this result in obfustication of the source?

I'm not sure.

> 3. Would a subdirectory of src/ solely for glibc ABI/API compatability be
> appropriate?
> The point would be to allow removing gnuish extensions without too much
> trouble.
> Even if 1 or 2 mean that weak_alias stuff goes in the same file, this
> could contain functions like the ones added for gnulib (src/stdio/ext*.c).

I think that's confusing matters. The ones added for gnulib are not
for glibc compatibility; glibc does not even have them. They're added
so that gnulib will not attempt to poke at musl internals and generate
musl-version-specific binaries which would crash on different musl
versions. Removing them is a very bad idea. Moreover on i386 they're
55 bytes of code. Removing something like that for space savings is on
the same order of ridiculousness as some of busybox's optimizations...

Rich

Re: [PATCH 9/10] GLIBC ABI patches

[Luca Barbato]

On 07/25/2012 05:06 PM, Rich Felker wrote:
> On Tue, Jul 24, 2012 at 11:00:54PM -0400, idunham@lavabit.com wrote:
>> Rich:
>> I have three questions:
>>
>> 1. Should it be possible to weak_alias an externally defined function?
>> ie, src/stdlib/strtod.c defines strtod, then src/glibc-abi/stdlib.c goes:
>> #include "libc.h"
>> #include <stdlib.h>
>> weak_alias(strtod, __strtod_internal)
> 
> This is not possible the way traditional assemblers/linkers work. You
> can make a wrapper function but that adds bloat and has a runtime
> performance cost if the wrapper is used.

The only way I can see to keep them apart of the code is moving them in
an header and conditionally include them when compatibility is enabled
at builtime. Not sure if it would worth it.

>> 3. Would a subdirectory of src/ solely for glibc ABI/API compatability be
>> appropriate?
>> The point would be to allow removing gnuish extensions without too much
>> trouble.
>> Even if 1 or 2 mean that weak_alias stuff goes in the same file, this
>> could contain functions like the ones added for gnulib (src/stdio/ext*.c).
> 
> I think that's confusing matters. The ones added for gnulib are not
> for glibc compatibility; glibc does not even have them. They're added
> so that gnulib will not attempt to poke at musl internals and generate
> musl-version-specific binaries which would crash on different musl
> versions. Removing them is a very bad idea. Moreover on i386 they're
> 55 bytes of code. Removing something like that for space savings is on
> the same order of ridiculousness as some of busybox's optimizations...

And probably leaving a note for the poor soul really needing to shave
those last 55/100 bytes would be enough.

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero

Re: [PATCH 9/10] GLIBC ABI patches

[idunham]

> On Wed, Jul 25, 2012 at 04:12:59PM +0200, Luca Barbato wrote:
>> On 07/23/2012 03:38 AM, Isaac Dunham wrote:
>> > +weak_alias(poll, __poll);
>>
>> > +weak_alias(fscanf, __isoc99_fscanf);
>>
>> > +weak_alias(sscanf, __isoc99_sscanf);
>>
>> > -char *strndup(const char *s, size_t n)
>> > +char *__strndup(const char *s, size_t n)
>>
>> > +weak_alias(__strndup, strndup);
>>
>> Why strndup is different?
>
> I think the idea is that we might want to use __strndup internally in
> functions which can't expose the strndup name.

Precisely.

> However, as we haven't
> yet had a need for that, I suspect it's unlikely. Also, __strndup
> isn't really an ugly name (it makes sense as the "internal" name for
> strndup if such usage were needed), but __isoc99_scanf is a huge WTF
> unless you know the reason it exists in glibc (and then it just makes
> you hate glibc even more...).
>
> With that said, for now I'd probably prefer to keep plain strndup as
> the "real" name.

OK.

Re: [PATCH 9/10] GLIBC ABI patches

[Rich Felker]

On Wed, Jul 25, 2012 at 05:52:46PM +0200, Luca Barbato wrote:
> > I think the idea is that we might want to use __strndup internally in
> > functions which can't expose the strndup name. However, as we haven't
> > yet had a need for that, I suspect it's unlikely. Also, __strndup
> > isn't really an ugly name (it makes sense as the "internal" name for
> > strndup if such usage were needed), but __isoc99_scanf is a huge WTF
> > unless you know the reason it exists in glibc (and then it just makes
> > you hate glibc even more...).
> 
> Would be nice make all those alias consistent, might be interesting see
> if linker scripts could be use for similar purposes, keeping the core
> code cleaner.

Considering that the linker is never run on the static library that
would be rather difficult. Even if it were possible, I think it would
just subtract a trivial amount of cruft that's easy to ignore from the
.c files at the expense of making the build system much uglier and
more GNU-binutils-dependent.

If lots of people think it's confusing having a mix of aliases that
are for internal/legitimate purposes and for ugly ABI purposes, I
either add comments to all the ABI-only ones that they can be ignored
by somebody reading the source, or I could do something like the LFS64
macros and have a separate macro for the ABI-only aliases that would
be self-documenting of their purpose and that could be nulled out by
extremists who want them gone...

Rich

Re: [PATCH 9/10] GLIBC ABI patches

[Luca Barbato]

On 07/25/2012 05:19 PM, Rich Felker wrote:
> On Wed, Jul 25, 2012 at 04:12:59PM +0200, Luca Barbato wrote:
>> On 07/23/2012 03:38 AM, Isaac Dunham wrote:
>>> +weak_alias(poll, __poll);
>>
>>> +weak_alias(fscanf, __isoc99_fscanf);
>>
>>> +weak_alias(sscanf, __isoc99_sscanf);
>>
>>> -char *strndup(const char *s, size_t n)
>>> +char *__strndup(const char *s, size_t n)
>>
>>> +weak_alias(__strndup, strndup);
>>
>> Why strndup is different?
> 
> I think the idea is that we might want to use __strndup internally in
> functions which can't expose the strndup name. However, as we haven't
> yet had a need for that, I suspect it's unlikely. Also, __strndup
> isn't really an ugly name (it makes sense as the "internal" name for
> strndup if such usage were needed), but __isoc99_scanf is a huge WTF
> unless you know the reason it exists in glibc (and then it just makes
> you hate glibc even more...).

Would be nice make all those alias consistent, might be interesting see
if linker scripts could be use for similar purposes, keeping the core
code cleaner.

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero

Re: [PATCH 9/10] GLIBC ABI patches

[Rich Felker]

On Wed, Jul 25, 2012 at 04:12:59PM +0200, Luca Barbato wrote:
> On 07/23/2012 03:38 AM, Isaac Dunham wrote:
> > +weak_alias(poll, __poll);
> 
> > +weak_alias(fscanf, __isoc99_fscanf);
> 
> > +weak_alias(sscanf, __isoc99_sscanf);
> 
> > -char *strndup(const char *s, size_t n)
> > +char *__strndup(const char *s, size_t n)
> 
> > +weak_alias(__strndup, strndup);
> 
> Why strndup is different?

I think the idea is that we might want to use __strndup internally in
functions which can't expose the strndup name. However, as we haven't
yet had a need for that, I suspect it's unlikely. Also, __strndup
isn't really an ugly name (it makes sense as the "internal" name for
strndup if such usage were needed), but __isoc99_scanf is a huge WTF
unless you know the reason it exists in glibc (and then it just makes
you hate glibc even more...).

With that said, for now I'd probably prefer to keep plain strndup as
the "real" name.

Rich

Re: [PATCH 9/10] GLIBC ABI patches

[Luca Barbato]

On 07/23/2012 03:38 AM, Isaac Dunham wrote:
> +weak_alias(poll, __poll);

> +weak_alias(fscanf, __isoc99_fscanf);

> +weak_alias(sscanf, __isoc99_sscanf);

> -char *strndup(const char *s, size_t n)
> +char *__strndup(const char *s, size_t n)

> +weak_alias(__strndup, strndup);

Why strndup is different?

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero

Re: [PATCH 9/10] GLIBC ABI patches

[Arvid E. Picciani]

On Tue, 24 Jul 2012 20:33:31 +0200, Igmar Palsenberg wrote:

> I've seen lots of code who use internal glibc functions / data
> structures. We want to prevent them from being used, that's why I
> personally have a problem with adding code like this. Unless it
> actually serves a real use.

That was sort of the point of making it an alias, i think.
Code that uses it still doesn't compile, but if it was compiled with 
gnulibc headers it at least links against musl.
This satisfies both requirements of "musl should encourage standards 
compliance" and "musl should be able to run nvidia drivers"

I just want it to also satisfy "musl code should serve as reference 
implementation of a standards compliant libc", which it doesnt
if gnu compatibility code is mixed in indifferent.

-- 
Arvid E. Picciani

Re: [PATCH 9/10] GLIBC ABI patches

[Rich Felker]

On Tue, Jul 24, 2012 at 08:33:31PM +0200, Igmar Palsenberg wrote:
> >> Why bloat code with stuff to provide glibc compatibility ?
> >> 
> >> 
> >> 	Igmar
> > 
> > “That we even need those aliases is usually a case of bad automake
> > / autoconf / bad feature detection”
> > 
> > These are for ABI compatibility, not API compatibility. Nobody
> > using glibc uses these symbols intentionally, they are renamed and
> > aliased by the library. Last I checked, musl is shockingly close
> > to ABI compatibility with glibc, and like it or not, that's a
> > valuable feature. If you don't like the “bloat” of it, you'll have
> > to dig out a lot of the existing aliases too.
> 
> I've seen lots of code who use internal glibc functions / data
> structures. We want to prevent them from being used, that's why I
> personally have a problem with adding code like this. Unless it
> actually serves a real use.

OK, let me clarify a bit of musl's policy about glibc compatibility.
The intent is that programs or .o files built against glibc headers
and linked against glibc libc.so should work on musl as long as they
are conforming C/POSIX programs or programs which just use nonstandard
but widely available functions provided by musl and which are
otherwise conforming C/POSIX programs. There's no explicit goal of
supporting glibc programs which are poking at glibc internals (like
the stdio hacks in gnulib) or otherwise dependent on the way glibc
implements a particular interface, since one of the major goals of
musl is to be able to drop and replace the internals of any interface
with a new/better implementation without breaking programs linked
against musl libc.so.

To achieve the above level of glibc ABI compatibility, it's necessary
to provide a minimal set of symbols not specified by C or POSIX but
which glibc uses for implementing things specified by C or POSIX. One
good example is the __isoc99_*scanf functions. Since glibc's plain
*scanf functions are nonconformant, building against glibc with
-std=c99 causes the program to use the __isoc99_*scanf functions
instead. This is not a glibc-specific hack in the program but instead
a hack induced by the glibc header files. There's also minimal cost in
supporting it; it's just an extra symbol in the global symbol table,
no extra code. Other examples are the *64 functions for 64-bit off_t
support. An example where there is some actual code/data bloat is the
tables used for implementing ctype.h is*() functions. I added these
with some hesitation, but unfortunately they're necessary to be able
to run any conforming C program that was built against glibc which
happens to use ctype.h, and thankfully they're not too big.

With that said, there's also some benefit to being able to use nasty
binaryware video drivers that were built against glibc, specifically
the nvidia junk. I'm not a fan (personally, I would never buy an
nvidia card until their policies change, and even then I'd be hesitant
to buy the watt-guzzling SUV of video cards...) but many people have
them and want to use them, and being able to get the driver working
with musl is quite a nice hack. So if we can provide the symbols or a
minimal amount of compatibility glue to make these drivers work, I
think it's worthwhile to do so.

Rich

Re: [PATCH 9/10] GLIBC ABI patches

[Igmar Palsenberg]

>>>>>> Just nonsense aliases GNU uses...
>>>>>> Needed for ABI compatability.
>>>>> could we mark them as such? at least with a comment.
>>>>> I really like that musl is so readable. This patch adds some obfuscation that can simply be countered by marking it as "ok this is only here for reason X."
>>>> I would like to see those options behind a compile time option : It bloats musl with in many cases unneeded code. I test my compiles with musl, and I like it lean and mean.
>>> These are just aliases, not code. There's no bloat there.
>>> 
>>> One of the advantages of musl is its LACK of configurability: If you have “musl”, you know what precisely you're getting.
>>> 
>>> With valediction,
>>> - Gregor Richards
>>> 
>> While I agree with the above, I still have a few objections :
>> 
>> - We don't want glibc compatibility. We want a good libc.
>> - That we even need those aliases is usually a case of bad automake / autoconf / bad feature detection.
>> 
>> Why bloat code with stuff to provide glibc compatibility ?
>> 
>> 
>> 	Igmar
> 
> “That we even need those aliases is usually a case of bad automake / autoconf / bad feature detection”
> 
> These are for ABI compatibility, not API compatibility. Nobody using glibc uses these symbols intentionally, they are renamed and aliased by the library. Last I checked, musl is shockingly close to ABI compatibility with glibc, and like it or not, that's a valuable feature. If you don't like the “bloat” of it, you'll have to dig out a lot of the existing aliases too.

I've seen lots of code who use internal glibc functions / data structures. We want to prevent them from being used, that's why I personally have a problem with adding code like this. Unless it actually serves a real use.



	Igmar

Re: [PATCH 9/10] GLIBC ABI patches

[Gregor Richards]

On 07/24/12 14:23, Igmar Palsenberg wrote:
>
>>>>> Just nonsense aliases GNU uses...
>>>>> Needed for ABI compatability.
>>>> could we mark them as such? at least with a comment.
>>>> I really like that musl is so readable. This patch adds some obfuscation that can simply be countered by marking it as "ok this is only here for reason X."
>>> I would like to see those options behind a compile time option : It bloats musl with in many cases unneeded code. I test my compiles with musl, and I like it lean and mean.
>> These are just aliases, not code. There's no bloat there.
>>
>> One of the advantages of musl is its LACK of configurability: If you have “musl”, you know what precisely you're getting.
>>
>> With valediction,
>> - Gregor Richards
>>
> While I agree with the above, I still have a few objections :
>
> - We don't want glibc compatibility. We want a good libc.
> - That we even need those aliases is usually a case of bad automake / autoconf / bad feature detection.
>
> Why bloat code with stuff to provide glibc compatibility ?
>
>
> 	Igmar

“That we even need those aliases is usually a case of bad automake / 
autoconf / bad feature detection”

These are for ABI compatibility, not API compatibility. Nobody using 
glibc uses these symbols intentionally, they are renamed and aliased by 
the library. Last I checked, musl is shockingly close to ABI 
compatibility with glibc, and like it or not, that's a valuable feature. 
If you don't like the “bloat” of it, you'll have to dig out a lot of the 
existing aliases too.

With valediction,
  - Gregor Richards

Re: [PATCH 9/10] GLIBC ABI patches

[Igmar Palsenberg]

>>>> Just nonsense aliases GNU uses...
>>>> Needed for ABI compatability.
>>> could we mark them as such? at least with a comment.
>>> I really like that musl is so readable. This patch adds some obfuscation that can simply be countered by marking it as "ok this is only here for reason X."
>> I would like to see those options behind a compile time option : It bloats musl with in many cases unneeded code. I test my compiles with musl, and I like it lean and mean.

> These are just aliases, not code. There's no bloat there.
> 
> One of the advantages of musl is its LACK of configurability: If you have “musl”, you know what precisely you're getting.
> 
> With valediction,
> - Gregor Richards
> 

While I agree with the above, I still have a few objections : 

- We don't want glibc compatibility. We want a good libc.
- That we even need those aliases is usually a case of bad automake / autoconf / bad feature detection.

Why bloat code with stuff to provide glibc compatibility ?


	Igmar

Re: [PATCH 9/10] GLIBC ABI patches

[Gregor Richards]

On 07/24/12 14:15, Igmar Palsenberg wrote:
>
>>> Just nonsense aliases GNU uses...
>>> Needed for ABI compatability.
>> could we mark them as such? at least with a comment.
>> I really like that musl is so readable. This patch adds some obfuscation that can simply be countered by marking it as "ok this is only here for reason X."
> I would like to see those options behind a compile time option : It bloats musl with in many cases unneeded code. I test my compiles with musl, and I like it lean and mean.
>
>
> Regards,
>
>
>
> 	Igmar

These are just aliases, not code. There's no bloat there.

One of the advantages of musl is its LACK of configurability: If you 
have “musl”, you know what precisely you're getting.

With valediction,
  - Gregor Richards

Re: [PATCH 9/10] GLIBC ABI patches

[Igmar Palsenberg]

>> Just nonsense aliases GNU uses...
>> Needed for ABI compatability.
> 
> could we mark them as such? at least with a comment.
> I really like that musl is so readable. This patch adds some obfuscation that can simply be countered by marking it as "ok this is only here for reason X."

I would like to see those options behind a compile time option : It bloats musl with in many cases unneeded code. I test my compiles with musl, and I like it lean and mean.


Regards,



	Igmar

Re: [PATCH 9/10] GLIBC ABI patches

[Arvid E. Picciani]

On Sun, 22 Jul 2012 18:38:28 -0700, Isaac Dunham wrote:

> Just nonsense aliases GNU uses...
> Needed for ABI compatability.

could we mark them as such? at least with a comment.
I really like that musl is so readable. This patch adds some 
obfuscation that can simply be countered by marking it as "ok this is 
only here for reason X."

-- 
Arvid E. Picciani

Re: [PATCH 9/10] GLIBC ABI patches

[Isaac Dunham]

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

On Sun, 22 Jul 2012 18:13:32 -0700
Isaac Dunham <idunham@lavabit.com> wrote:

> This patch series is basically a reworked version of orc's previous
> patch.
> From what orc said, the first patch should provide enough to build
> Xorg; I haven't tested this yet.
> A few more patches are syscall wrappers (splice) or trivial
> functions (finite).
> Finally, there are several aliases.

Just nonsense aliases GNU uses...
Needed for ABI compatability.

Isaac Dunham

[-- Attachment #2: 9-__glibc.diff --]
[-- Type: text/x-patch, Size: 1545 bytes --]

diff --git a/src/select/poll.c b/src/select/poll.c
index f1e73e8..b2f8b00 100644
--- a/src/select/poll.c
+++ b/src/select/poll.c
@@ -6,3 +6,5 @@ int poll(struct pollfd *fds, nfds_t n, int timeout)
 {
 	return syscall_cp(SYS_poll, fds, n, timeout);
 }
+
+weak_alias(poll, __poll);
diff --git a/src/stdio/fscanf.c b/src/stdio/fscanf.c
index 51fc9b3..155f8ac 100644
--- a/src/stdio/fscanf.c
+++ b/src/stdio/fscanf.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include <stdarg.h>
+#include "libc.h"
 
 int fscanf(FILE *f, const char *fmt, ...)
 {
@@ -10,3 +11,5 @@ int fscanf(FILE *f, const char *fmt, ...)
 	va_end(ap);
 	return ret;
 }
+
+weak_alias(fscanf, __isoc99_fscanf);
diff --git a/src/stdio/sscanf.c b/src/stdio/sscanf.c
index a1cea69..8c16343 100644
--- a/src/stdio/sscanf.c
+++ b/src/stdio/sscanf.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include <stdarg.h>
+#include "libc.h"
 
 int sscanf(const char *s, const char *fmt, ...)
 {
@@ -10,3 +11,5 @@ int sscanf(const char *s, const char *fmt, ...)
 	va_end(ap);
 	return ret;
 }
+
+weak_alias(sscanf, __isoc99_sscanf);
diff --git a/src/string/strndup.c b/src/string/strndup.c
index 617d27b..b162d8b 100644
--- a/src/string/strndup.c
+++ b/src/string/strndup.c
@@ -1,7 +1,8 @@
 #include <stdlib.h>
 #include <string.h>
+#include "libc.h"
 
-char *strndup(const char *s, size_t n)
+char *__strndup(const char *s, size_t n)
 {
 	size_t l = strnlen(s, n);
 	char *d = malloc(l+1);
@@ -10,3 +11,5 @@ char *strndup(const char *s, size_t n)
 	d[l] = 0;
 	return d;
 }
+
+weak_alias(__strndup, strndup);