Re: [musl] Re: musl bugs found through gnulib

[Rich Felker]

Some more updates..

On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote:
> Replacements of *printf, because of
> [...]
>   checking whether printf survives out-of-memory conditions... no

This was caused by the pointer-arithmetic overflow bug I just fixed in
git. It should no longer fail, and never failed before except in i386
binaries running on x86_64 kernels.

> Replacement of duplocale, because of
>   checking whether duplocale(LC_GLOBAL_LOCALE) works... no

POSIX does not specify any use of LC_GLOBAL_LOCALE except as an
argument to uselocale. Is there a reason it's needed? Perhaps more
importantly, is the replacement when libc doesn't provide this
functionality bloated/painful?

> Replacement of fdopen, because of
>   checking whether fdopen sets errno... no

Seems to have been fixed in gnulib.

> Replacement of getcwd, because of
>   checking whether getcwd handles long file names properly... no, but it is 
> partly working
>   checking whether getcwd aborts when 4k < cwd_length < 16k... no

Still unclear what the cause of these failures is. Anyone else looked
into them, or do I still need to?

> Replacement of iconv and iconv_open, because of
>   checking whether iconv supports conversion between UTF-8 and 
> UTF-{16,32}{BE,LE}... no

I fixed all the UTF-16-related bugs that were breaking this test. It
should pass now.

> Replacement of mktime, because of
>   checking for working mktime... no
> 
> Replacement of perror, because of
>   checking whether perror matches strerror... no
> 
> Replacement of popen, because of
>   checking whether popen works with closed stdin... no
> 
> Replacement of regex, because of
>   checking for working re_compile_pattern... no
> 
> Replacement of strtod, because of
>   checking whether strtod obeys C99... no
> 
> For each of the replacements, first look at the test program's results
> (in config.log), then look at the test program's source code (in m4/*.m4).
> 
> Furthermore we have test failures:
> 
> test-duplocale.c:70: assertion failed
> FAIL: test-duplocale
> 
> test-fcntl.c:382: assertion failed
> FAIL: test-fcntl

This is caused by the fact that the F_GETOWN fcntl on Linux is broken;
there's no way to distinguish error returns from non-error negative
return values. So we never set errno when calling F_GETOWN and assume
the return value is not an error. There's a new-ish Linux-specific
F_GETOWN_EX we could use when it's available, but the fallback code
would still fail just like it does now, because it's a fundamental
limitation in the API.

> test-fdatasync.c:50: assertion failed
> FAIL: test-fdatasync

This function was dummied-out for some reason. Fixed.

> test-fsync.c:50: assertion failed
> FAIL: test-fsync

Same.

> test-fwrite.c:53: assertion failed
> FAIL: test-fwrite

This seems like it might be a real bug. On musl, unbuffered files
actually have a one-byte buffer, but on writing, the buffer is
supposed to be flushed as soon as it fills, rather than waiting for
another write when it's full. I'll have to run some tests...

> test-getlogin_r.c:88: assertion failed
> FAIL: test-getlogin_r

This was broken; it should be fixed now.

> test-grantpt.c:34: assertion failed
> FAIL: test-grantpt

This is an invalid test. POSIX specifies this function "may fail", not
"shall fail", and since the function is inherently a no-op, it would
be idiotic to make it perform a syscall to check the validity of the
file descriptor...

> test-localeconv.c:41: assertion failed
> FAIL: test-localeconv

Fixed lots of issues; not sure if it works now.

> Segmentation fault
> FAIL: test-localename

This might be due to our incomplete locale implementation, or because
the test uses locale names that don't exist. I doubt it should
segfault though. I'll look into this one later.

> test-ptsname_r.c:118: assertion failed
> FAIL: test-ptsname_r

It's testing that ptsname_r both sets errno and returns the error
code, and that they're the same. Since this function is nonstandard,
there's no spec for it, so perhaps this is desirable; I was assuming
it should return -1 on failure.

> test-strerror_r.c:118: assertion failed
> FAIL: test-strerror_r

This test is looking for a null terminator at the n-1 position of the
buffer if strerror_r fails with ERANGE (buffer too small). I don't see
anywhere the function is specified to write to the buffer AT ALL on
failure, so this test seems invalid.

> test-wcwidth.c:71: assertion failed
> FAIL: test-wcwidth

It's checking for wcwidth(0x3000)==2. This definitely used to work,
but it might have been broken when I overhauled wcwidth. I'll look
into it..

> When I compile all of gnulib, I also get a compilation error
> (may be a musl or a gnulib problem, haven't investigated):
> fsusage.c: In function 'get_fs_usage':
> fsusage.c:222:17: error: storage size of 'fsd' isn't known
> fsusage.c:224:3: warning: implicit declaration of function 'statfs' 
> [-Wimplicit-function-declaration]
> fsusage.c:222:17: warning: unused variable 'fsd' [-Wunused-variable]
> make[4]: *** [fsusage.o] Error 1

This looks like a gnulib problem. On musl, statvfs should get used,
and this code should not even be compiled... Judging from the
comments, it looks like a hard-coded workaround for broken glibc
and/or Linux versions, but the header include seems to be missing in
the workaround case...

Rich