* Re: musl bugs found through gnulib [not found] ` <20120611182202.1ee4d019@newbook> @ 2012-06-17 22:49 ` Bruno Haible 2012-06-17 23:54 ` Rich Felker ` (4 more replies) 0 siblings, 5 replies; 35+ messages in thread From: Bruno Haible @ 2012-06-17 22:49 UTC (permalink / raw) To: bug-gnulib, musl; +Cc: Isaac Dunham, Paul Eggert, Reuben Thomas [CCing the musl list] Isaac Dunham wrote in <http://lists.gnu.org/archive/html/bug-gnulib/2012-06/msg00101.html>: > musl is designed for standards conformance, There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, that explains how to use gnulib to check a libc against bugs. When I apply this to musl-0.9.1, I get this list of problems: Replacements of *printf, because of checking whether printf supports infinite 'long double' arguments... no checking whether printf supports the 'ls' directive... no checking whether printf survives out-of-memory conditions... no Replacement of duplocale, because of checking whether duplocale(LC_GLOBAL_LOCALE) works... no Replacement of fdopen, because of checking whether fdopen sets errno... no Replacement of futimens, because of checking whether futimens works... no 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 Replacement of getopt, because of checking whether getopt is POSIX compatible... no Replacement of glob, because of checking for GNU glob interface version 1... no (not sure this is a bug or just an incompatibility compared to glibc) Replacement of iconv and iconv_open, because of checking whether iconv supports conversion between UTF-8 and UTF-{16,32}{BE,LE}... no 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 test-fdatasync.c:50: assertion failed FAIL: test-fdatasync test-fma2.h:116: assertion failed FAIL: test-fma2 test-fsync.c:50: assertion failed FAIL: test-fsync test-fwrite.c:53: assertion failed FAIL: test-fwrite test-getlogin_r.c:88: assertion failed FAIL: test-getlogin_r test-grantpt.c:34: assertion failed FAIL: test-grantpt test-localeconv.c:41: assertion failed FAIL: test-localeconv Segmentation fault FAIL: test-localename test-ptsname_r.c:118: assertion failed FAIL: test-ptsname_r test-strerror_r.c:118: assertion failed FAIL: test-strerror_r test-wcwidth.c:71: assertion failed FAIL: test-wcwidth 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 Bruno ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible @ 2012-06-17 23:54 ` Rich Felker 2012-06-18 8:21 ` Szabolcs Nagy 2012-06-18 0:16 ` [musl] " idunham ` (3 subsequent siblings) 4 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-17 23:54 UTC (permalink / raw) To: musl On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > Replacements of *printf, because of > checking whether printf supports infinite 'long double' arguments... no I'm just going to change isnanl to handle the invalid bit patterns because I'm sick of this issue... > checking whether printf supports the 'ls' directive... no Fixed in git on Jun 8. > checking whether printf survives out-of-memory conditions... no This test must be doing something wrong. musl's printf does not use any dynamic memory. > Replacement of duplocale, because of > checking whether duplocale(LC_GLOBAL_LOCALE) works... no I'll look into this. Locales are not presently supported, but the interfaces are intended to work and behave as they should for an implementation that has exactly one possible locale. > Replacement of fdopen, because of > checking whether fdopen sets errno... no I'll look into it. > Replacement of futimens, because of > checking whether futimens works... no Odd, this is just a syscall wrapper, no? > 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 I'm not sure if it's related, but musl defines PATH_MAX and does not attempt to support explicit pathnames longer than PATH_MAX. I'll have to look at what the test is doing. > Replacement of getopt, because of > checking whether getopt is POSIX compatible... no This was a bug I reported to gnulib. The test is checking for GNU semantics which are incompatible with POSIX, i.e. the test is misnamed. > Replacement of glob, because of > checking for GNU glob interface version 1... no > (not sure this is a bug or just an incompatibility compared to glibc) I think gnulib just wants GNU glob functionality. This is legitimate for it to replace since we (at present, at least) don't attempt to offer any GNU functionality in glob. > Replacement of iconv and iconv_open, because of > checking whether iconv supports conversion between UTF-8 and UTF-{16,32}{BE,LE}... no These conversions are definitely supported. The only related thing we don't support is BOMs and stateful/endian-detected UTF-16/32. > Replacement of mktime, because of > checking for working mktime... no I need to check this. I've suspected it might have a bug. > Replacement of perror, because of > checking whether perror matches strerror... no Odd, it uses strerror.. > Replacement of popen, because of > checking whether popen works with closed stdin... no Sounds like a bug; I'll check it out. > Replacement of regex, because of > checking for working re_compile_pattern... no This is a check for the GNU regex interfaces as opposed to POSIX regcomp/regexec. > Replacement of strtod, because of > checking whether strtod obeys C99... no This was caused by a missing negative zero case for "-0x" followed by non-hex chars. It's fixed in git. > 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). OK, looking... Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-17 23:54 ` Rich Felker @ 2012-06-18 8:21 ` Szabolcs Nagy 2012-06-18 13:02 ` John Spencer 0 siblings, 1 reply; 35+ messages in thread From: Szabolcs Nagy @ 2012-06-18 8:21 UTC (permalink / raw) To: musl * Rich Felker <dalias@aerifal.cx> [2012-06-17 19:54:26 -0400]: > On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > > Replacements of *printf, because of > > checking whether printf supports infinite 'long double' arguments... no > > I'm just going to change isnanl to handle the invalid bit patterns > because I'm sick of this issue... > i wonder if these gnulib bugs will get fixed.. gnulib should go over an audit (otherwise it's not credible considering all the false tests so far) some of the gnulib tests will find musl bugs but many tests are wrong i think we should not invest too much time into gnulib but work on a proper libc testsuit instead.. > > 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 > > I'm not sure if it's related, but musl defines PATH_MAX and does not > attempt to support explicit pathnames longer than PATH_MAX. I'll have > to look at what the test is doing. > heh it sounds as if gnulib wants getcwd to abort that's wrong > > Replacement of perror, because of > > checking whether perror matches strerror... no > > Odd, it uses strerror.. > this one is musl, in perror.c: - if (msg) { + if (msg && *msg) { but the gnulib replacement looks dangerous/wrong (why would you want perror to abort?) ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 8:21 ` Szabolcs Nagy @ 2012-06-18 13:02 ` John Spencer 2012-06-18 14:55 ` Rich Felker 0 siblings, 1 reply; 35+ messages in thread From: John Spencer @ 2012-06-18 13:02 UTC (permalink / raw) To: musl; +Cc: Szabolcs Nagy, Rich Felker, Reuben Thomas, Bruno Haible On 06/18/2012 10:21 AM, Szabolcs Nagy wrote: > * Rich Felker<dalias@aerifal.cx> [2012-06-17 19:54:26 -0400]: > >> On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: >>> Replacements of *printf, because of >>> checking whether printf supports infinite 'long double' arguments... no >> I'm just going to change isnanl to handle the invalid bit patterns >> because I'm sick of this issue... >> please don't do this. this would be a win of Evil against Good. do you see were this goes ? first they wanted you to implement __freadahead, now they want 4 such functions plus a broken __MUSL__ macro. this is usually refered to using the proverb "give them the small finger and they take the entire hand". the tactics of the gnulib folks is to wear you down, until they have no work to do at all. also note how Bruno changed the subjects in "gnulib portability issues" thread on the gnulib ML into something that suggests the opposite, i.e. that musl is broken. there's only one broken program out there, which is gnu coreutils' "od". if someone wants to use coreutils and cares about this uncommon issue, he/she can simply use a patch for this specific program. > i wonder if these gnulib bugs will get fixed.. > > gnulib should go over an audit > (otherwise it's not credible considering all > the false tests so far) > > some of the gnulib tests will find musl bugs > but many tests are wrong > > i think we should not invest too much time > into gnulib but work on a proper libc > testsuit instead.. full ACK. now that we know which tests are responsible for detecting many musl functions (wrongly) as broken, we can simply create some "gnulib-fix" program that makes these tests always succeed, thus keeping their bloat outside. PLUS add a warning at a prominent spot that programs using GNULIB should be built statically to avoid future incompatibilities due to their illegal poking at internals. sabotage currently uses the following approach to beat gnulib into functioning: empty_file() { rm -f "$1" touch "$1" } ./configure -C --prefix=/ || exit 1 #fixing gnulib cd lib for i in printf.c fprintf.c freadahead.c closein.c fseterr.c vfprintf.c ; do empty_file "$i" done echo "#define __printf__ printf" >> config.h echo "#define rpl_fprintf fprintf" >> config.h cd .. sed -i 's,atexit (close_stdin),,' gzip.c # probably better to just echo "void close_stdin(void) {}" > close_in.c --JS ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 13:02 ` John Spencer @ 2012-06-18 14:55 ` Rich Felker 2012-06-18 15:26 ` Szabolcs Nagy 2012-06-19 13:26 ` John Spencer 0 siblings, 2 replies; 35+ messages in thread From: Rich Felker @ 2012-06-18 14:55 UTC (permalink / raw) To: musl; +Cc: Bruno Haible On Mon, Jun 18, 2012 at 03:02:16PM +0200, John Spencer wrote: > On 06/18/2012 10:21 AM, Szabolcs Nagy wrote: > >* Rich Felker<dalias@aerifal.cx> [2012-06-17 19:54:26 -0400]: > > > >>On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > >>>Replacements of *printf, because of > >>> checking whether printf supports infinite 'long double' arguments... no > >>I'm just going to change isnanl to handle the invalid bit patterns > >>because I'm sick of this issue... > >> > please don't do this. this would be a win of Evil against Good. > > do you see were this goes ? > first they wanted you to implement __freadahead, now they want 4 > such functions Actually at first they were trying to do it in gnulib, and I told them rather than trying to put ugly musl-specific hacks in gnulib that will break in the future, I'd prefer to just provide the function, as long as it can be detected by configure and not... > plus a broken __MUSL__ macro. This is rejected. Bruno has criticized my rejection of it as purely theoretical, ignoring practical needs, etc. But you've seen it on the list and irc: how many times have new users/contributors shown up trying to get a package working with musl, asked if there's __MUSL__ or similar, and then LEARNED SOMETHING and FIXED THE PACKAGE RIGHT when told that there's no such thing and that the package's configure script should be fixed to detect things rather than hard-coding assumptions? I hate to think how many packages would have broken #ifdef __MUSL__ patches in them by now if the people submitting patches had that available to work with...and then the package maintainers would get frustrated with musl in a hurry when they had to fix things again later because the assumption they hard-coded was bad. > this is usually refered to using the proverb "give them the small > finger and they take the entire hand". I do feel a bit like my interaction with Eric versus later interaction with Bruce has been something of a bait-and-switch. But you know the old saying: never attribute to malice what can be adequately explained.... > the tactics of the gnulib folks is to wear you down, until they have > no work to do at all. I don't follow your reasoning here. If anything Bruno has been pushing to make more work for himself by adding more special cases that have to be maintained, instead of adopting a general solution.. > also note how Bruno changed the subjects in "gnulib portability > issues" thread on the gnulib ML into something that suggests the > opposite, i.e. that musl is broken. > > there's only one broken program out there, which is gnu coreutils' "od". > if someone wants to use coreutils and cares about this uncommon > issue, he/she can simply use a patch for this specific program. The problem is that gnulib is potentially replacing printf in many programs (I haven't checked this; would you care to check?) over this stupid issue that only affects one broken program, resulting in massive bloat for users, and potentially breaking things. > >i think we should not invest too much time > >into gnulib but work on a proper libc > >testsuit instead.. > > full ACK. now that we know which tests are responsible for detecting I think nsz was talking about testing, not about app compatibility. The gnulib tests have turned out to be 50-50 or worse for detecting actual bugs versus broken tests. Off-hand, that's slightly worse than Open POSIX Test Suite's rate... In any case, I think we've gotten almost all we can out of the gnulib tests in terms of fixing bugs. > many musl functions (wrongly) as broken, we can simply create some > "gnulib-fix" program that makes these tests always succeed, thus > keeping their bloat outside. If you'd like to post a script to do this that actually works with all gnulib programs, I'll consider that as an alternative to working with the gnulib developers... ;-) > PLUS add a warning at a prominent spot that programs using GNULIB > should be built statically to avoid future incompatibilities due to > their illegal poking at internals. I don't think there's any poking at internals yet, and the proposal is NOT to include any of that stuff. > sabotage currently uses the following approach to beat gnulib into > functioning: > > empty_file() { > rm -f "$1" > touch "$1" > } > > ../configure -C --prefix=/ || exit 1 > > #fixing gnulib > > cd lib > for i in printf.c fprintf.c freadahead.c closein.c fseterr.c vfprintf.c ; do > empty_file "$i" > done > > echo "#define __printf__ printf" >> config.h > echo "#define rpl_fprintf fprintf" >> config.h > > cd .. This looks like ugly hackery that's poking at gnulib internals rather than operating at the configure level like it should. I think it's a bit hypocritical to criticize them for poking at libc internals and then turn around and poke at their internals... > sed -i 's,atexit (close_stdin),,' gzip.c > # probably better to just echo "void close_stdin(void) {}" > close_in.c This will make some of the GNU coreutils non-conformant to POSIX. I've already discussed on this list why using an atexit function for closing stdin or stdout is the heart of the problem and the source of all the complexity. If the main program just closed the stream at the natural point in the normal program flow, it would be trivial to do correctly and portably. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 14:55 ` Rich Felker @ 2012-06-18 15:26 ` Szabolcs Nagy 2012-06-18 16:00 ` Rich Felker 2012-06-19 13:26 ` John Spencer 1 sibling, 1 reply; 35+ messages in thread From: Szabolcs Nagy @ 2012-06-18 15:26 UTC (permalink / raw) To: musl * Rich Felker <dalias@aerifal.cx> [2012-06-18 10:55:45 -0400]: > > there's only one broken program out there, which is gnu coreutils' "od". > > if someone wants to use coreutils and cares about this uncommon > > issue, he/she can simply use a patch for this specific program. > > The problem is that gnulib is potentially replacing printf in many > programs (I haven't checked this; would you care to check?) over this > stupid issue that only affects one broken program, resulting in > massive bloat for users, and potentially breaking things. > hm, what's the issue with od? is it about printing random binary data as long double? if od invokes undefined behaviour then it's a coreutils bug it sounds bad that based on a broken test gnulib may replace a correct printf implementation with its own broken one just to make a broken coreutils tool to work so now we need a libc workaround for this issue.. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 15:26 ` Szabolcs Nagy @ 2012-06-18 16:00 ` Rich Felker 0 siblings, 0 replies; 35+ messages in thread From: Rich Felker @ 2012-06-18 16:00 UTC (permalink / raw) To: musl On Mon, Jun 18, 2012 at 05:26:32PM +0200, Szabolcs Nagy wrote: > * Rich Felker <dalias@aerifal.cx> [2012-06-18 10:55:45 -0400]: > > > there's only one broken program out there, which is gnu coreutils' "od". > > > if someone wants to use coreutils and cares about this uncommon > > > issue, he/she can simply use a patch for this specific program. > > > > The problem is that gnulib is potentially replacing printf in many > > programs (I haven't checked this; would you care to check?) over this > > stupid issue that only affects one broken program, resulting in > > massive bloat for users, and potentially breaking things. > > > > hm, what's the issue with od? > is it about printing random binary data as long double? Yes. > if od invokes undefined behaviour then it's a coreutils bug Yes. > it sounds bad that based on a broken test gnulib may replace > a correct printf implementation with its own broken one > just to make a broken coreutils tool to work so now we > need a libc workaround for this issue.. Coreutils should at least be changed to call isnanl() in the data before calling printf. Then they can get by with just replacing isnanl (ugly but relatively cheap and non-intrusive) instead of replacing all of printf. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 14:55 ` Rich Felker 2012-06-18 15:26 ` Szabolcs Nagy @ 2012-06-19 13:26 ` John Spencer 1 sibling, 0 replies; 35+ messages in thread From: John Spencer @ 2012-06-19 13:26 UTC (permalink / raw) To: musl; +Cc: Rich Felker, Bruno Haible On 06/18/2012 04:55 PM, Rich Felker wrote: > > If you'd like to post a script to do this that actually works with all > gnulib programs, I'll consider that as an alternative to working with > the gnulib developers... ;-) > http://sprunge.us/dKaK it works by passing configure cache variables that will skip over the bogus conformance tests. unfortunately the compilation of fseterr/freadahead is unconditonal, thus the 2 C files must be "fixed" to not produce the well known compile errors. the only test that isn't turned off is the one for "POSIX getopt", because apparently the meaning is "GNU getopt", so using musl's getopt instead of the replacement one might cause problems. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible 2012-06-17 23:54 ` Rich Felker @ 2012-06-18 0:16 ` idunham 2012-06-19 0:11 ` Rich Felker ` (2 subsequent siblings) 4 siblings, 0 replies; 35+ messages in thread From: idunham @ 2012-06-18 0:16 UTC (permalink / raw) To: musl; +Cc: Isaac Dunham, Paul Eggert, bug-gnulib, Reuben Thomas > [CCing the musl list] > Isaac Dunham wrote in > <http://lists.gnu.org/archive/html/bug-gnulib/2012-06/msg00101.html>: >> musl is designed for standards conformance, > > There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, > that explains how to use gnulib to check a libc against bugs. Be warned: a bad test can cause failures as well. It's been one of the musl developers' complaints about gnulib that the tests are buggy and frequently check for glibc behavior instead of standard behavior. > When I apply this to musl-0.9.1, I get this list of problems: > > Replacements of *printf, because of > checking whether printf supports infinite 'long double' arguments... no > checking whether printf supports the 'ls' directive... no > checking whether printf survives out-of-memory conditions... no At least one of these (infinite long double, IIRC) is invalid or a test for a GNU-ism. This was previously discussed on the musl ML. OOM behavior is undefined AFAICT (feel free to point out a standard), and the scenario is a lot less likely with musl than glibc for several reasons. > Replacement of duplocale, because of > checking whether duplocale(LC_GLOBAL_LOCALE) works... no Need to check this one > Replacement of fdopen, because of > checking whether fdopen sets errno... no I presume this is nonconformance to POSIX ("otherwise, a null pointer shall be returned and errno set...")? > Replacement of futimens, because of > checking whether futimens works... no Could be a bug. > Replacement of getcwd, because of > checking whether getcwd handles long file names properly... no, but it > is partly working Is this a test for ERANGE handling (error on name >= size)? Other than that, I see no specification covering this. > checking whether getcwd aborts when 4k < cwd_length < 16k... no AFAICT, only required to error when size =< cwd_length. If size !< (cwd_length + 1), that is conformant behavior. (See man 3posix getcwd) > Replacement of getopt, because of > checking whether getopt is POSIX compatible... no We'd need to see this test...(will look later). > Replacement of glob, because of > checking for GNU glob interface version 1... no > (not sure this is a bug or just an incompatibility compared to glibc) Looks like an incompatability, since it specifies "GNU interface"... > Replacement of iconv and iconv_open, because of > checking whether iconv supports conversion between UTF-8 and > UTF-{16,32}{BE,LE}... no Not "nonconformant" from the standpoint of POSIX, AFAICT, but it is incomplete. musl is UTF8 native, but I don't think it supports UTF16/UTF32 yet. > 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 Look like bugs, if the description is correct. > Replacement of regex, because of > checking for working re_compile_pattern... no This is #ifdef __USE_GNU I'm not aware of any standard covering GNU APIs... > 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). > Thanks, Isaac Dunham ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible 2012-06-17 23:54 ` Rich Felker 2012-06-18 0:16 ` [musl] " idunham @ 2012-06-19 0:11 ` Rich Felker 2012-06-19 2:07 ` [musl] " Eric Blake 2012-06-19 10:45 ` musl, printf out-of-memory test Bruno Haible 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker 2012-06-20 19:28 ` Rich Felker 4 siblings, 2 replies; 35+ messages in thread From: Rich Felker @ 2012-06-19 0:11 UTC (permalink / raw) To: musl; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, Reuben Thomas Some updates... On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, > that explains how to use gnulib to check a libc against bugs. When I apply > this to musl-0.9.1, I get this list of problems: > > Replacements of *printf, because of > [...] > checking whether printf survives out-of-memory conditions... no No idea. Copying out the test and running it directly, it passes just fine for me. Maybe gnulib has already replaced printf with its own malloc-using version by the time it gets to this test?? > Replacement of fdopen, because of > checking whether fdopen sets errno... no There was one bug here (failure to set errno when mode string was invalid) but I don't think that's the case gnulib was testing for. It seems gnulib wants an error for the "may fail" when the fd is invalid. > Replacement of futimens, because of > checking whether futimens works... no gnulib always forces this test to fail if __linux__ is defined. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib 2012-06-19 0:11 ` Rich Felker @ 2012-06-19 2:07 ` Eric Blake 2012-06-19 2:52 ` Rich Felker 2012-06-19 10:45 ` musl, printf out-of-memory test Bruno Haible 1 sibling, 1 reply; 35+ messages in thread From: Eric Blake @ 2012-06-19 2:07 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, musl, Reuben Thomas [-- Attachment #1: Type: text/plain, Size: 2821 bytes --] On 06/18/2012 06:11 PM, Rich Felker wrote: > Some updates... > > On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: >> There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, >> that explains how to use gnulib to check a libc against bugs. When I apply >> this to musl-0.9.1, I get this list of problems: >> >> Replacements of *printf, because of >> [...] >> checking whether printf survives out-of-memory conditions... no > > No idea. Copying out the test and running it directly, it passes just > fine for me. Maybe gnulib has already replaced printf with its own > malloc-using version by the time it gets to this test?? No; configure-time tests are relatively independent, and all done prior to any replacements at compile-time. You should be able to inspect config.log to see the actual test that configure ran. > >> Replacement of fdopen, because of >> checking whether fdopen sets errno... no > > There was one bug here (failure to set errno when mode string was > invalid) but I don't think that's the case gnulib was testing for. It > seems gnulib wants an error for the "may fail" when the fd is invalid. The 'EBADF may fail' condition is rather weak. And since glibc guarantees a definite failure, it is nicer to program to the glibc interface that guarantees immediate failure on a bad fd at fdopen() time than it is to deal with the surprises that result when fdopen() succeeds but later attempts to use the stream fail. Perhaps it might be worth splitting this into two gnulib modules, one for the strict POSIX compliance and one for the glibc interface, but that depends on how likely people are to want to program to the weaker POSIX interface when it is just as easy to make fdopen() guarantee failure on bad fds and save efforts up front. > >> Replacement of futimens, because of >> checking whether futimens works... no > > gnulib always forces this test to fail if __linux__ is defined. That's because the Linux kernel got it wrong for quite some time, and worse, it was file-system dependent - even if it worked on one machine and file system, compiling in support for futimens and then running on another file system would experience random compliance failures due to the poor file system implementation. It's been a while, so maybe we can finally graduate this module into assuming that the Linux kernel is better behaved by default, and make the user specifically request the fallbacks if they are worried about using the binary on a file system that still has bugs. But don't take it personally - this is not a case of musl getting it wrong, but of the kernel getting it wrong. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 620 bytes --] ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-19 2:07 ` [musl] " Eric Blake @ 2012-06-19 2:52 ` Rich Felker 2012-06-19 11:03 ` musl, fdopen test Bruno Haible 0 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-19 2:52 UTC (permalink / raw) To: Eric Blake; +Cc: musl, Isaac Dunham, Paul Eggert, bug-gnulib, Reuben Thomas On Mon, Jun 18, 2012 at 08:07:40PM -0600, Eric Blake wrote: > On 06/18/2012 06:11 PM, Rich Felker wrote: > > Some updates... > > > > On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > >> There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, > >> that explains how to use gnulib to check a libc against bugs. When I apply > >> this to musl-0.9.1, I get this list of problems: > >> > >> Replacements of *printf, because of > >> [...] > >> checking whether printf survives out-of-memory conditions... no > > > > No idea. Copying out the test and running it directly, it passes just > > fine for me. Maybe gnulib has already replaced printf with its own > > malloc-using version by the time it gets to this test?? > > No; configure-time tests are relatively independent, and all done prior > to any replacements at compile-time. You should be able to inspect > config.log to see the actual test that configure ran. OK, then no idea what's causing this. I was going to try running the test but I didn't have autotools installed on the system I wanted to test on, so I put it off.. > >> Replacement of fdopen, because of > >> checking whether fdopen sets errno... no > > > > There was one bug here (failure to set errno when mode string was > > invalid) but I don't think that's the case gnulib was testing for. It > > seems gnulib wants an error for the "may fail" when the fd is invalid. > > The 'EBADF may fail' condition is rather weak. And since glibc > guarantees a definite failure, it is nicer to program to the glibc > interface that guarantees immediate failure on a bad fd at fdopen() time > than it is to deal with the surprises that result when fdopen() succeeds > but later attempts to use the stream fail. Perhaps it might be worth The only real-world situation I can think of where you'd care that fdopen detect EBADF is when you've just called a function that allocates the file descriptor and directly passed it to fdopen without first checking the return value. For instance: FILE *f = fdopen(open(pathname, O_RDWR|O_CLOEXEC), "rb+"); instead of: int fd = open(pathname, O_RDWR|O_CLOEXEC); if (fd<0) goto error; FILE *f = fdopen(fd, "rb+"); The former is rather lazy programming, but maybe gnulib intends to support this kind of programming. > splitting this into two gnulib modules, one for the strict POSIX > compliance and one for the glibc interface, but that depends on how > likely people are to want to program to the weaker POSIX interface when > it is just as easy to make fdopen() guarantee failure on bad fds and > save efforts up front. My thought in having musl skip the test is to maximize performance of fdopen, assuming you might be using it in a situation like on a newly accept()ed network connection where every syscall counts (think multi-threaded httpd). For read-only fdopen, no syscalls are needed, but if writing is possible, fdopen has to make a syscall to check whether the fd is a tty in order to decide whether to enable line buffering or full buffering, and in principle it could detect EBADF at the same time for no cost. > >> Replacement of futimens, because of > >> checking whether futimens works... no > > > > gnulib always forces this test to fail if __linux__ is defined. > > That's because the Linux kernel got it wrong for quite some time, and > worse, it was file-system dependent - even if it worked on one machine > and file system, compiling in support for futimens and then running on > another file system would experience random compliance failures due to > the poor file system implementation. > > It's been a while, so maybe we can finally graduate this module into > assuming that the Linux kernel is better behaved by default, and make > the user specifically request the fallbacks if they are worried about > using the binary on a file system that still has bugs. But don't take > it personally - this is not a case of musl getting it wrong, but of the > kernel getting it wrong. Yes, it might be nice if the test output made it clear that it was hard-coded to fail so nobody goes looking for nonexistant bugs.. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, fdopen test 2012-06-19 2:52 ` Rich Felker @ 2012-06-19 11:03 ` Bruno Haible 2012-06-19 11:09 ` Jim Meyering 0 siblings, 1 reply; 35+ messages in thread From: Bruno Haible @ 2012-06-19 11:03 UTC (permalink / raw) To: bug-gnulib, musl; +Cc: Rich Felker, Eric Blake, Isaac Dunham Rich Felker wrote: > > >> Replacement of fdopen, because of > > >> checking whether fdopen sets errno... no > > > > > > There was one bug here (failure to set errno when mode string was > > > invalid) but I don't think that's the case gnulib was testing for. It > > > seems gnulib wants an error for the "may fail" when the fd is invalid. Indeed, the possibility that fdopen(invalid fd, ...) can succeed is supported by POSIX. When looking at the gnulib documentation doc/posix-functions/fdopen.texi and the unit test tests/test-fdopen.c it appears that it was not the intent of gnulib to prohibit this behaviour. Rather, musl is the first platform to exhibit this behaviour, and gnulib's intent was to make sure that fdopen(invalid fd, ...) 1. does not crash, 2. sets errno when it fails. Eric Blake replied: > > The 'EBADF may fail' condition is rather weak. And since glibc > > guarantees a definite failure, it is nicer to program to the glibc > > interface that guarantees immediate failure on a bad fd at fdopen() time > > than it is to deal with the surprises that result when fdopen() succeeds > > but later attempts to use the stream fail. Perhaps it might be worth The glibc documentation contains this warning: In some other systems, `fdopen' may fail to detect that the modes for file descriptor do not permit the access specified by `opentype'. The GNU C library always checks for this. So, I think few programmers will explicitly want to exploit this glibc specific behaviour. Rich Felker replied: > The only real-world situation I can think of where you'd care that > fdopen detect EBADF is when you've just called a function that > allocates the file descriptor and directly passed it to fdopen without > first checking the return value. For instance: > > FILE *f = fdopen(open(pathname, O_RDWR|O_CLOEXEC), "rb+"); > > instead of: > > int fd = open(pathname, O_RDWR|O_CLOEXEC); > if (fd<0) goto error; > FILE *f = fdopen(fd, "rb+"); > > The former is rather lazy programming, but maybe gnulib intends to > support this kind of programming. No, gnulib does not intend to encourage this kind of lazy programming. > My thought in having musl skip the test is to maximize performance of > fdopen, assuming you might be using it in a situation like on a newly > accept()ed network connection where every syscall counts (think > multi-threaded httpd). For read-only fdopen, no syscalls are needed, Sounds reasonable. Here's a proposed patch to remove gnulib's unintentional requirement. 2012-06-19 Bruno Haible <bruno@clisp.org> fdopen: Allow implementations that don't reject invalid fd arguments. * m4/fdopen.m4 (gl_FUNC_FDOPEN): Let the test pass if fdopen(-1,...) succeeds. Reported by Rich Felker <dalias@aerifal.cx>. --- m4/fdopen.m4.orig Tue Jun 19 13:00:23 2012 +++ m4/fdopen.m4 Tue Jun 19 13:00:05 2012 @@ -1,4 +1,4 @@ -# fdopen.m4 serial 2 +# fdopen.m4 serial 3 dnl Copyright (C) 2011-2012 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -25,10 +25,8 @@ FILE *fp; errno = 0; fp = fdopen (-1, "r"); - if (fp != NULL) + if (fp == NULL && errno == 0) return 1; - if (errno == 0) - return 2; return 0; }]])], [gl_cv_func_fdopen_works=yes], ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, fdopen test 2012-06-19 11:03 ` musl, fdopen test Bruno Haible @ 2012-06-19 11:09 ` Jim Meyering 2012-06-20 20:52 ` Bruno Haible 0 siblings, 1 reply; 35+ messages in thread From: Jim Meyering @ 2012-06-19 11:09 UTC (permalink / raw) To: Bruno Haible; +Cc: musl, Rich Felker, bug-gnulib, Eric Blake, Isaac Dunham Bruno Haible wrote: ... >> My thought in having musl skip the test is to maximize performance of >> fdopen, assuming you might be using it in a situation like on a newly >> accept()ed network connection where every syscall counts (think >> multi-threaded httpd). For read-only fdopen, no syscalls are needed, > > Sounds reasonable. > > Here's a proposed patch to remove gnulib's unintentional requirement. > > > 2012-06-19 Bruno Haible <bruno@clisp.org> > > fdopen: Allow implementations that don't reject invalid fd arguments. > * m4/fdopen.m4 (gl_FUNC_FDOPEN): Let the test pass if fdopen(-1,...) > succeeds. > Reported by Rich Felker <dalias@aerifal.cx>. Thanks, Bruno. That patch looks perfect. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, fdopen test 2012-06-19 11:09 ` Jim Meyering @ 2012-06-20 20:52 ` Bruno Haible 0 siblings, 0 replies; 35+ messages in thread From: Bruno Haible @ 2012-06-20 20:52 UTC (permalink / raw) To: Jim Meyering; +Cc: bug-gnulib, musl, Eric Blake, Rich Felker, Isaac Dunham > > 2012-06-19 Bruno Haible <bruno@clisp.org> > > > > fdopen: Allow implementations that don't reject invalid fd arguments. > > * m4/fdopen.m4 (gl_FUNC_FDOPEN): Let the test pass if fdopen(-1,...) > > succeeds. > > Reported by Rich Felker <dalias@aerifal.cx>. > > Thanks, Bruno. > That patch looks perfect. I have applied the patch. Bruno ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-19 0:11 ` Rich Felker 2012-06-19 2:07 ` [musl] " Eric Blake @ 2012-06-19 10:45 ` Bruno Haible 2012-06-19 19:16 ` Rich Felker 1 sibling, 1 reply; 35+ messages in thread From: Bruno Haible @ 2012-06-19 10:45 UTC (permalink / raw) To: bug-gnulib; +Cc: Rich Felker, musl, Isaac Dunham Rich Felker wrote: > > Replacements of *printf, because of > > [...] > > checking whether printf survives out-of-memory conditions... no > > No idea. Copying out the test and running it directly, it passes just > fine for me. Maybe gnulib has already replaced printf with its own > malloc-using version by the time it gets to this test?? Strange indeed. With a testdir of all of gnulib, I got configure:17615: checking whether printf survives out-of-memory conditions configure:17786: /arch/x86-linux/inst-musl/bin/musl-gcc -std=gnu99 -o conftest -g -O2 -Wall conftest.c >&5 configure:17789: $? = 0 configure:17837: result: yes but with a testdir of only the POSIX related modules of gnulib, I got configure:13657: checking whether printf survives out-of-memory conditions configure:13828: /arch/x86-linux/inst-musl/bin/musl-gcc -std=gnu99 -o conftest -g -O2 -Wall conftest.c >&5 configure:13831: $? = 0 configure:13879: result: no The '$? = 0' line prints only the linker's exit code, not the runtime exit code. I'm adding a second output line for the runtime exit code. Then I get: configure:8919: checking whether printf survives out-of-memory conditions configure:9090: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 configure:9093: $? = 0 configure:9097: $? = 1 configure:9142: result: no After adding a printf to stderr: Once I get configure:8919: checking whether printf survives out-of-memory conditions configure:9093: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 configure:9096: $? = 0 printf's return value = 5000002, errno = 0 configure:9100: $? = 0 configure:9145: result: yes In another configure run I get: configure:8919: checking whether printf survives out-of-memory conditions configure:9093: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 configure:9096: $? = 0 configure:9100: $? = 1 configure:9145: result: no So, the exit code 1 must have come from the crash handler. Without this crash handler: 7x I get configure:8919: checking whether printf survives out-of-memory conditions configure:8979: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 configure:8982: $? = 0 printf's return value = 5000002, errno = 0 configure:8986: $? = 0 configure:9031: result: yes but once I get configure:8979: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 configure:8982: $? = 0 configure:8986: $? = 139 configure:9031: result: no So, apparently, under memory stress, musl's printf has a probability of between 10% and 50% of crashing with SIGSEGV (139 = 128 + 11). Bruno 2012-06-19 Bruno Haible <bruno@clisp.org> *printf-posix: Put more info into config.log. * m4/printf.m4 (gl_PRINTF_ENOMEM): Emit conftest's error output and exit code into config.log. --- m4/printf.m4.orig Tue Jun 19 12:41:56 2012 +++ m4/printf.m4 Tue Jun 19 12:41:53 2012 @@ -1,4 +1,4 @@ -# printf.m4 serial 48 +# printf.m4 serial 49 dnl Copyright (C) 2003, 2007-2012 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -1028,8 +1028,9 @@ changequote([,])dnl ])]) if AC_TRY_EVAL([ac_link]) && test -s conftest$ac_exeext; then - (./conftest + (./conftest 2>&AS_MESSAGE_LOG_FD result=$? + _AS_ECHO_LOG([\$? = $result]) if test $result != 0 && test $result != 77; then result=1; fi exit $result ) >/dev/null 2>/dev/null ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl, printf out-of-memory test 2012-06-19 10:45 ` musl, printf out-of-memory test Bruno Haible @ 2012-06-19 19:16 ` Rich Felker 2012-06-19 20:04 ` Bruno Haible 0 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-19 19:16 UTC (permalink / raw) To: musl; +Cc: bug-gnulib On Tue, Jun 19, 2012 at 12:45:50PM +0200, Bruno Haible wrote: > So, the exit code 1 must have come from the crash handler. Without this crash > handler: 7x I get > > configure:8919: checking whether printf survives out-of-memory conditions > configure:8979: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 > configure:8982: $? = 0 > printf's return value = 5000002, errno = 0 > configure:8986: $? = 0 > configure:9031: result: yes > > but once I get > > configure:8979: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 > configure:8982: $? = 0 > configure:8986: $? = 139 > configure:9031: result: no > > So, apparently, under memory stress, musl's printf has a probability of > between 10% and 50% of crashing with SIGSEGV (139 = 128 + 11). musl's printf does not do anything with memory except using a small constant amount of stack space (a few hundred bytes for non-float, somewhere around 5-7k for floating point). This is completely independent of the width/padding/precision; the implementation actually goes to a good bit of trouble to ensure that it can print any amount of padding efficiently without large or unbounded stack space usage. Is there any way the rlimits put in place could be preventing the stack from expanding beyond even one page the current number of pages, etc.? Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-19 19:16 ` Rich Felker @ 2012-06-19 20:04 ` Bruno Haible 2012-06-19 20:08 ` Rich Felker 0 siblings, 1 reply; 35+ messages in thread From: Bruno Haible @ 2012-06-19 20:04 UTC (permalink / raw) To: bug-gnulib; +Cc: Rich Felker, musl Rich Felker wrote: > > but once I get > > > > configure:8979: /arch/x86-linux/inst-musl/bin/musl-gcc -o conftest -g -O2 -Wall conftest.c >&5 > > configure:8982: $? = 0 > > configure:8986: $? = 139 > > configure:9031: result: no > > > > So, apparently, under memory stress, musl's printf has a probability of > > between 10% and 50% of crashing with SIGSEGV (139 = 128 + 11). > > musl's printf does not do anything with memory except using a small > constant amount of stack space (a few hundred bytes for non-float, > somewhere around 5-7k for floating point). This is completely > independent of the width/padding/precision; the implementation > actually goes to a good bit of trouble to ensure that it can print any > amount of padding efficiently without large or unbounded stack space > usage. > > Is there any way the rlimits put in place could be preventing the > stack from expanding beyond even one page the current number of pages, > etc.? I can reduce the program and the compilation options: =============================== conftest.c ============================= #include <stdio.h> #include <errno.h> int main() { int ret; int err; ret = printf ("%.5000000f", 1.0); err = errno; fprintf (stderr, "printf's return value = %d, errno = %d\n", ret, err); return !(ret == 5000002 || (ret < 0 && err == ENOMEM)); } ======================================================================== $ musl-gcc -g -Wall conftest.c -o conftest $ ./conftest > /dev/null ; echo $? printf's return value = 5000002, errno = 0 0 $ ./conftest > /dev/null ; echo $? printf's return value = 5000002, errno = 0 0 $ ./conftest > /dev/null ; echo $? printf's return value = 5000002, errno = 0 0 $ ./conftest > /dev/null ; echo $? Speicherzugriffsfehler (Speicherabzug geschrieben) 139 $ ./conftest > /dev/null ; echo $? Speicherzugriffsfehler (Speicherabzug geschrieben) 139 I couldn't get useful info from gdb. This is on Linux, 32-bit mode on a 64-bit system. Can you reproduce this? Bruno ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-19 20:04 ` Bruno Haible @ 2012-06-19 20:08 ` Rich Felker 2012-06-19 21:17 ` Bruno Haible 0 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-19 20:08 UTC (permalink / raw) To: Bruno Haible; +Cc: musl, bug-gnulib On Tue, Jun 19, 2012 at 10:04:57PM +0200, Bruno Haible wrote: > I can reduce the program and the compilation options: > > =============================== conftest.c ============================= > #include <stdio.h> > #include <errno.h> > int main() > { > int ret; > int err; > ret = printf ("%.5000000f", 1.0); > err = errno; > fprintf (stderr, "printf's return value = %d, errno = %d\n", ret, err); > return !(ret == 5000002 || (ret < 0 && err == ENOMEM)); > } > ======================================================================== > $ musl-gcc -g -Wall conftest.c -o conftest > $ ./conftest > /dev/null ; echo $? > printf's return value = 5000002, errno = 0 > 0 > $ ./conftest > /dev/null ; echo $? > printf's return value = 5000002, errno = 0 > 0 > $ ./conftest > /dev/null ; echo $? > printf's return value = 5000002, errno = 0 > 0 > $ ./conftest > /dev/null ; echo $? > Speicherzugriffsfehler (Speicherabzug geschrieben) > 139 > $ ./conftest > /dev/null ; echo $? > Speicherzugriffsfehler (Speicherabzug geschrieben) > 139 > > I couldn't get useful info from gdb. > > This is on Linux, 32-bit mode on a 64-bit system. Can you reproduce this? I can't reproduce it. Do you have a dynamic-linked musl or just static? I tried both and couldn't reproduce with either. Did you set resource limits before running it? Are you using any strange kernel mods? I once heard of a patched kernel setting up other mappings over top of the not-yet-expanded-into stack space, but I'd be surprised if more weren't breaking on such a system... What happened in gdb? Were you unable to get it to crash? What if you run it under strace? Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-19 20:08 ` Rich Felker @ 2012-06-19 21:17 ` Bruno Haible 2012-06-20 1:52 ` Rich Felker 0 siblings, 1 reply; 35+ messages in thread From: Bruno Haible @ 2012-06-19 21:17 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, musl Rich Felker wrote: > Do you have a dynamic-linked musl or just static? Dynamically linked: $ readelf -d conftest Dynamic section at offset 0xf3c contains 18 entries: Tag Type Name/Value 0x00000001 (NEEDED) Shared library: [libc.so] 0x0000000c (INIT) 0x804832c 0x0000000d (FINI) 0x80484ec 0x00000004 (HASH) 0x80481a0 0x6ffffef5 (GNU_HASH) 0x80481dc 0x00000005 (STRTAB) 0x80482b0 0x00000006 (SYMTAB) 0x8048210 0x0000000a (STRSZ) 83 (bytes) 0x0000000b (SYMENT) 16 (bytes) 0x00000015 (DEBUG) 0x0 0x00000003 (PLTGOT) 0x8049ff4 0x00000002 (PLTRELSZ) 32 (bytes) 0x00000014 (PLTREL) REL 0x00000017 (JMPREL) 0x804830c 0x00000011 (REL) 0x8048304 0x00000012 (RELSZ) 8 (bytes) 0x00000013 (RELENT) 8 (bytes) 0x00000000 (NULL) 0x0 $ readelf -l conftest Elf file type is EXEC (Executable file) Entry point 0x8048390 There are 9 program headers, starting at offset 52 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align PHDR 0x000034 0x08048034 0x08048034 0x00120 0x00120 R E 0x4 INTERP 0x000154 0x08048154 0x08048154 0x00026 0x00026 R 0x1 [Requesting program interpreter: /arch/x86-linux/inst-musl/lib/libc.so] LOAD 0x000000 0x08048000 0x08048000 0x00578 0x00578 R E 0x1000 LOAD 0x000f28 0x08049f28 0x08049f28 0x000ec 0x000f8 RW 0x1000 DYNAMIC 0x000f3c 0x08049f3c 0x08049f3c 0x000b8 0x000b8 RW 0x4 NOTE 0x00017c 0x0804817c 0x0804817c 0x00024 0x00024 R 0x4 GNU_EH_FRAME 0x000528 0x08048528 0x08048528 0x00014 0x00014 R 0x4 GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4 GNU_RELRO 0x000f28 0x08049f28 0x08049f28 0x000d8 0x000d8 R 0x1 Section to Segment mapping: Segment Sections... 00 01 .interp 02 .interp .note.gnu.build-id .hash .gnu.hash .dynsym .dynstr .rel.dyn .rel.plt .init .plt .text .fini .rodata .eh_frame_hdr .eh_frame 03 .ctors .dtors .jcr .dynamic .got.plt .data .bss 04 .dynamic 05 .note.gnu.build-id 06 .eh_frame_hdr 07 08 .ctors .dtors .jcr .dynamic $ readelf --dyn-syms conftest Symbol table '.dynsym' contains 10 entries: Num: Value Size Type Bind Vis Ndx Name 0: 00000000 0 NOTYPE LOCAL DEFAULT UND 1: 00000000 0 FUNC GLOBAL DEFAULT UND printf 2: 00000000 0 FUNC GLOBAL DEFAULT UND fprintf 3: 00000000 0 FUNC GLOBAL DEFAULT UND __errno_location 4: 00000000 0 FUNC GLOBAL DEFAULT UND __libc_start_main 5: 0804a014 0 NOTYPE GLOBAL DEFAULT ABS _edata 6: 0804a020 0 NOTYPE GLOBAL DEFAULT ABS _end 7: 08048390 0 NOTYPE GLOBAL DEFAULT 11 _start 8: 0804a014 0 NOTYPE GLOBAL DEFAULT ABS __bss_start 9: 0804a014 4 OBJECT GLOBAL DEFAULT 22 stderr > Did you set resource limits before running it? No. $ ulimit -a core file size (blocks, -c) unlimited data seg size (kbytes, -d) unlimited scheduling priority (-e) 0 file size (blocks, -f) unlimited pending signals (-i) 29019 max locked memory (kbytes, -l) 64 max memory size (kbytes, -m) unlimited open files (-n) 1024 pipe size (512 bytes, -p) 8 POSIX message queues (bytes, -q) 819200 real-time priority (-r) 0 stack size (kbytes, -s) 8192 cpu time (seconds, -t) unlimited max user processes (-u) 29019 virtual memory (kbytes, -v) unlimited file locks (-x) unlimited > Are you using any strange kernel mods? No. Stock openSUSE 12.1. $ uname -srv Linux 3.1.10-1.9-desktop #1 SMP PREEMPT Thu Apr 5 18:48:38 UTC 2012 (4a97ec8) > What happened in gdb? The stack trace in gdb is unusable. $ gdb conftest GNU gdb (GDB) SUSE (7.3-41.1.2) Copyright (C) 2011 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-suse-linux". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /data/bruno/tmp/testdir3/conftest...done. (gdb) set solib-search-path /arch/x86-linux/inst-musl/lib (gdb) run Starting program: /data/bruno/tmp/testdir3/conftest warning: Could not load shared library symbols for linux-gate.so.1. Do you need "set solib-search-path" or "set sysroot"? Program received signal SIGSEGV, Segmentation fault. 0xf7fc76c3 in fmt_fp () from /data/arch/x86-linux/inst-musl/lib/libc.so (gdb) where #0 0xf7fc76c3 in fmt_fp () from /data/arch/x86-linux/inst-musl/lib/libc.so #1 0x00000000 in ?? () This is a bit useless, since libc.so is compiled without debugging information. If I rebuild with "-O1 -g" instead of "-Os" and "-O3", I get this stack trace: $ gdb conftest GNU gdb (GDB) SUSE (7.3-41.1.2) Copyright (C) 2011 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-suse-linux". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /data/bruno/tmp/testdir3/conftest...done. (gdb) set solib-search-path /arch/x86-linux/inst-musl/lib (gdb) run Starting program: /data/bruno/tmp/testdir3/conftest warning: Could not load shared library symbols for linux-gate.so.1. Do you need "set solib-search-path" or "set sysroot"? Program received signal SIGSEGV, Segmentation fault. fmt_fp (f=0xf7ff9200, y=0, w=0, p=5000000, fl=0, t=102) at src/stdio/vfprintf.c:326 326 x = *d % i; (gdb) where #0 fmt_fp (f=0xf7ff9200, y=0, w=0, p=5000000, fl=0, t=102) at src/stdio/vfprintf.c:326 #1 0xf7fcacf3 in printf_core (f=0xf7ff9200, fmt=<optimized out>, ap=0xffffc13c, nl_arg=0xffffc09c, nl_type=0xffffc114) at src/stdio/vfprintf.c:614 #2 0xf7fcb0eb in vfprintf (f=0xf7ff9200, fmt=0x80484f4 "%.5000000f", ap=0xffffc1a4 "") at src/stdio/vfprintf.c:659 #3 0xf7fcd967 in vprintf (fmt=0x80484f4 "%.5000000f", ap=0xffffc1a4 "") at src/stdio/vprintf.c:5 #4 0xf7fc8463 in printf (fmt=0x80484f4 "%.5000000f") at src/stdio/printf.c:9 #5 0x0804845f in main () at conftest.c:7 (gdb) info locals x = <optimized out> big = {524288, 0 <repeats 1750 times>, 4160552156, 0, 0, 0, 0, 0, 0, 0, 4160720884, 8, 8, 134513329, 4160343432, 134513332, 4160609540, 1, 0 <repeats 46 times>, 134513908, 4160721408, 4160517969, 4160727464, 134513908, 0, 0, 0, 0, 0, 4160720884, 4160711907, 0, 0, 4160524786} a = 0xffffa2b0 d = 0x218b40 r = 0xffffa2b0 z = 0x218b44 e2 = 0 e = 0 i = <optimized out> j = 9 l = <optimized out> buf = '\000' <repeats 24 times> s = <optimized out> prefix = 0xf7ff6cb4 "0X+0X 0X-0x+0x 0x" pl = 0 ebuf0 = '\000' <repeats 11 times> ebuf = 0xffffa293 "" estr = <optimized out> (gdb) up #1 0xf7fcacf3 in printf_core (f=0xf7ff9200, fmt=<optimized out>, ap=0xffffc13c, nl_arg=0xffffc09c, nl_type=0xffffc114) at src/stdio/vfprintf.c:614 614 l = fmt_fp(f, arg.f, w, p, fl, t); (gdb) info locals a = <optimized out> z = 0xffffbff0 "" s = 0x80484fe "" l10n = 0 litpct = <optimized out> fl = 0 w = 0 p = 5000000 arg = {i = 9223372036854775808, f = 1, p = 0x0} argpos = -1 st = <optimized out> ps = 0 cnt = 0 l = 0 i = <optimized out> buf = "A\370\367\374\371\370\367\000\000\000\000\021", '\000' <repeats 27 times>, "\377", <incomplete sequence \367> prefix = 0xf7ff6cd2 "-+ 0X0x" t = 102 pl = 0 wc = L"\xf7f9c62d\xf7f899ac" ws = <optimized out> mb = "\271\202\004\b" (gdb) up #2 0xf7fcb0eb in vfprintf (f=0xf7ff9200, fmt=0x80484f4 "%.5000000f", ap=0xffffc1a4 "") at src/stdio/vfprintf.c:659 659 ret = printf_core(f, fmt, &ap2, nl_arg, nl_type); (gdb) info locals ap2 = 0xffffc1ac "" nl_type = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0} nl_arg = {{i = 150189233701, f = 0, p = 0xf7f9d625}, {i = 4307434622, f = <invalid float value>, p = 0xbe3c7e}, { i = 4024693728518132, f = 0, p = 0x8049ff4}, {i = 0, f = <invalid float value>, p = 0x0}, {i = 98599429607984, f = 0, p = 0xf7fa1230}, {i = 17868614760971370496, f = -0, p = 0x0}, {i = 17870160128724931592, f = 0, p = 0xf7ff9408}, {i = 13791, f = 0, p = 0x35df}, {i = 47244701668, f = <invalid float value>, p = 0xefe4}, { i = 824633720832, f = 0, p = 0x0}} internal_buf = "h\334\375\367", '\000' <repeats 12 times>"\364, \217\377\367\340\216\377\367\270\300\377\377\"\000\000\000:\310\371\367\270\300\377\377\000\000\000\000\210\000\000\000\260\202\004\b\000\224\377\367\000\000\000\000\000\000\000\000\364\217\377\367H\224\377\367@\301\377\377\000\340\377\377" saved_buf = 0x0 ret = <optimized out> __need_unlock = 0 (gdb) up #3 0xf7fcd967 in vprintf (fmt=0x80484f4 "%.5000000f", ap=0xffffc1a4 "") at src/stdio/vprintf.c:5 5 return vfprintf(stdout, fmt, ap); (gdb) info locals No locals. (gdb) up #4 0xf7fc8463 in printf (fmt=0x80484f4 "%.5000000f") at src/stdio/printf.c:9 9 ret = vprintf(fmt, ap); (gdb) info locals ret = 9 ap = 0xffffc1a4 "" (gdb) up #5 0x0804845f in main () at conftest.c:7 7 ret = printf ("%.5000000f", 1.0); (gdb) info locals ret = 0 err = 0 The SIGSEGV occurs because d = 0x218b40 but the address ranges are these: 08048000-08049000 r-xp 00000000 08:05 26174991 /data/bruno/tmp/testdir3/conftest 08049000-0804b000 rwxp 00000000 08:05 26174991 /data/bruno/tmp/testdir3/conftest f7f84000-f7ff8000 r-xp 00000000 08:05 26168372 /data/arch/x86-linux/inst-musl/lib/libc.so f7ff8000-f7ffa000 rwxp 00073000 08:05 26168372 /data/arch/x86-linux/inst-musl/lib/libc.so f7ffa000-f7ffe000 rwxp 00000000 00:00 0 fffdc000-ffffe000 rwxp 00000000 00:00 0 [stack] ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso] > What if you run it under strace? Yes. When it succeeds, the strace output looks normal. When it fails, it's this: $ strace ./conftest execve("./conftest", ["./conftest"], [/* 133 vars */]) = 0 [ Process PID=2858 runs in 32 bit mode. ] --- {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xe7664} (Segmentation fault) --- +++ killed by SIGSEGV (core dumped) +++ Speicherzugriffsfehler (Speicherabzug geschrieben) Hope this helps. Bruno ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-19 21:17 ` Bruno Haible @ 2012-06-20 1:52 ` Rich Felker 2012-06-20 7:30 ` Szabolcs Nagy 2012-06-20 9:35 ` Bruno Haible 0 siblings, 2 replies; 35+ messages in thread From: Rich Felker @ 2012-06-20 1:52 UTC (permalink / raw) To: Bruno Haible; +Cc: musl, bug-gnulib On Tue, Jun 19, 2012 at 11:17:33PM +0200, Bruno Haible wrote: > [...] > The SIGSEGV occurs because d = 0x218b40 but the address ranges are these: > 08048000-08049000 r-xp 00000000 08:05 26174991 /data/bruno/tmp/testdir3/conftest > 08049000-0804b000 rwxp 00000000 08:05 26174991 /data/bruno/tmp/testdir3/conftest > f7f84000-f7ff8000 r-xp 00000000 08:05 26168372 /data/arch/x86-linux/inst-musl/lib/libc.so > f7ff8000-f7ffa000 rwxp 00073000 08:05 26168372 /data/arch/x86-linux/inst-musl/lib/libc.so > f7ffa000-f7ffe000 rwxp 00000000 00:00 0 > fffdc000-ffffe000 rwxp 00000000 00:00 0 [stack] > ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso] > > > What if you run it under strace? > > Yes. When it succeeds, the strace output looks normal. When it fails, > it's this: > > $ strace ./conftest > execve("./conftest", ["./conftest"], [/* 133 vars */]) = 0 > [ Process PID=2858 runs in 32 bit mode. ] > --- {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xe7664} (Segmentation fault) --- > +++ killed by SIGSEGV (core dumped) +++ > Speicherzugriffsfehler (Speicherabzug geschrieben) > > Hope this helps. Yes, it helped a lot. Thanks! The problem was an obscure pointer-arithmetic overflow that could only happen in 32-bit binaries running on a 64-bit kernel where the stack pointer is near the 4GB boundary. This is why I couldn't reproduce it: I'm on a 32-bit kernel where the stack is at 3GB and there's no way an offset bounded by INT_MAX/9 could reach past 4GB. That's my excuse for why it was never noticed before, but it still doesn't justify the bug, which is a nasty instance of UB (pointer arithmetic outside array bounds). Anyway, it's fixed now. Rich P.S. I just realized - I meant to credit you for finding it in the commit message but somehow I forgot to. Sorry about that! ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl, printf out-of-memory test 2012-06-20 1:52 ` Rich Felker @ 2012-06-20 7:30 ` Szabolcs Nagy 2012-06-20 9:35 ` Bruno Haible 1 sibling, 0 replies; 35+ messages in thread From: Szabolcs Nagy @ 2012-06-20 7:30 UTC (permalink / raw) To: musl * Rich Felker <dalias@aerifal.cx> [2012-06-19 21:52:49 -0400]: > On Tue, Jun 19, 2012 at 11:17:33PM +0200, Bruno Haible wrote: > > Hope this helps. > > Yes, it helped a lot. Thanks! The problem was an obscure > pointer-arithmetic overflow that could only happen in 32-bit binaries > running on a 64-bit kernel where the stack pointer is near the 4GB > boundary. This is why I couldn't reproduce it: I'm on a 32-bit > kernel where the stack is at 3GB and there's no way an offset bounded > by INT_MAX/9 could reach past 4GB. That's my excuse for why it was > never noticed before, but it still doesn't justify the bug, which is a > nasty instance of UB (pointer arithmetic outside array bounds). > > Anyway, it's fixed now. > you mentioned another potential out of bound pointer arithmetics there but it's not yet fixed: diff --git a/src/stdio/vfprintf.c b/src/stdio/vfprintf.c index a3bf18d..116e1ce 100644 --- a/src/stdio/vfprintf.c +++ b/src/stdio/vfprintf.c @@ -319,7 +319,7 @@ static int fmt_fp(FILE *f, long double y, int w, int p, int fl, int t) if (j < 9*(z-r-1)) { uint32_t x; /* We avoid C's broken division of negative numbers */ - d = r + 1 + (j+9*LDBL_MAX_EXP)/9 - LDBL_MAX_EXP; + d = r + 1 + ((j+9*LDBL_MAX_EXP)/9 - LDBL_MAX_EXP); j += 9*LDBL_MAX_EXP; j %= 9; for (i=10, j++; j<9; i*=10, j++); ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-20 1:52 ` Rich Felker 2012-06-20 7:30 ` Szabolcs Nagy @ 2012-06-20 9:35 ` Bruno Haible 2012-06-20 11:00 ` Jim Meyering 1 sibling, 1 reply; 35+ messages in thread From: Bruno Haible @ 2012-06-20 9:35 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, musl Rich Felker wrote: > The problem was an obscure pointer-arithmetic overflow ... > where the stack pointer is near the 4GB boundary. This explains also why it occurred only with a certain probability outside gdb, but with 100% probability from within gdb: Apparently gdb runs the program without address space layout randomization. > Anyway, it's fixed now. I confirm that http://git.etalabs.net/cgi-bin/gitweb.cgi?p=musl;a=commitdiff;h=914949d321448bd2189bdcbce794dbae2c8ed16e fixes the bug. Bruno ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-20 9:35 ` Bruno Haible @ 2012-06-20 11:00 ` Jim Meyering 2012-06-21 19:58 ` Tom Tromey 0 siblings, 1 reply; 35+ messages in thread From: Jim Meyering @ 2012-06-20 11:00 UTC (permalink / raw) To: Bruno Haible; +Cc: Rich Felker, musl, bug-gnulib Bruno Haible wrote: > Rich Felker wrote: >> The problem was an obscure pointer-arithmetic overflow ... >> where the stack pointer is near the 4GB boundary. > > This explains also why it occurred only with a certain probability > outside gdb, but with 100% probability from within gdb: Apparently gdb > runs the program without address space layout randomization. That is correct. It is a feature of gdb-7.0 and newer. You can inspect (watch/break-at/etc.) the same address and expect it to refer to the same memory location in multiple invocations. This makes gdb's command-line history even more useful. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: musl, printf out-of-memory test 2012-06-20 11:00 ` Jim Meyering @ 2012-06-21 19:58 ` Tom Tromey 0 siblings, 0 replies; 35+ messages in thread From: Tom Tromey @ 2012-06-21 19:58 UTC (permalink / raw) To: musl; +Cc: Bruno Haible, Rich Felker, bug-gnulib >>>>> "Jim" == Jim Meyering <jim@meyering.net> writes: Jim> That is correct. It is a feature of gdb-7.0 and newer. Jim> You can inspect (watch/break-at/etc.) the same address and expect it Jim> to refer to the same memory location in multiple invocations. Jim> This makes gdb's command-line history even more useful. gdb defaults to this for development convenience. You can change it though, see "set disable-randomization". Tom ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible ` (2 preceding siblings ...) 2012-06-19 0:11 ` Rich Felker @ 2012-06-20 3:04 ` Rich Felker 2012-06-20 4:10 ` [musl] " Eric Blake ` (3 more replies) 2012-06-20 19:28 ` Rich Felker 4 siblings, 4 replies; 35+ messages in thread From: Rich Felker @ 2012-06-20 3:04 UTC (permalink / raw) To: musl; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, Reuben Thomas 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 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker @ 2012-06-20 4:10 ` Eric Blake 2012-06-20 13:27 ` Rich Felker 2012-06-20 7:32 ` Szabolcs Nagy ` (2 subsequent siblings) 3 siblings, 1 reply; 35+ messages in thread From: Eric Blake @ 2012-06-20 4:10 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, musl, Reuben Thomas [-- Attachment #1: Type: text/plain, Size: 3678 bytes --] On 06/19/2012 09:04 PM, Rich Felker wrote: >> 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? Unfortunately, you are out of date. POSIX _does_ require duplocale(LC_GLOBAL_LOCALE) to work: http://austingroupbugs.net/view.php?id=301 If the locobj argument is LC_GLOBAL_LOCALE, duplocale() shall create a new locale object containing a copy of the global locale determined by the setlocale() function. The behavior is undefined if the locobj argument is not a valid locale object handle. After line 24978 add a new paragraph to APPLICATION USAGE: The duplocale() function can also be used in conjunction with uselocale((locale_t)0). This returns the locale in effect for the calling thread, but can have the value LC_GLOBAL_LOCALE. Passing LC_GLOBAL_LOCALE to functions such as isalnum_l() results in undefined behavior, but applications can convert it into a usable locale object by using 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. Yes, Linux 2.6.32 introduced F_GETOWN_EX for precisely this reason, and you should be using it. >> 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... This is one of the cases where gnulib prefers to emulate the shall fail semantics of glibc, as they are more useful to program around. >> 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. There _is_ a proposed standard for it now: http://austingroupbugs.net/view.php?id=508 which requires only the return value to be 0 or an errno value, and not that errno be set. gnulib should only be checking for a valid return value. > >> 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. This is a case where POSIX is rather weak, but where quality of implementation demands that the most useful interface is one that provides the most information back to the user. glibc had a number of bugs that were fixed in this area to improve QoI, and gnulib now prefers to rely on those improvements. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 620 bytes --] ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-20 4:10 ` [musl] " Eric Blake @ 2012-06-20 13:27 ` Rich Felker 0 siblings, 0 replies; 35+ messages in thread From: Rich Felker @ 2012-06-20 13:27 UTC (permalink / raw) To: musl; +Cc: Isaac Dunham, Paul Eggert, bug-gnulib, Reuben Thomas On Tue, Jun 19, 2012 at 10:10:11PM -0600, Eric Blake wrote: > Unfortunately, you are out of date. POSIX _does_ require > duplocale(LC_GLOBAL_LOCALE) to work: > > http://austingroupbugs.net/view.php?id=301 OK. I'll add support. For now all it requires is avoiding dereferencing the pointer, anyway. > Yes, Linux 2.6.32 introduced F_GETOWN_EX for precisely this reason, and > you should be using it. When I wrote that code, 2.6.32 was new enough, and the issue seemed minor enough, that I didn't bother. Now I agree it would be nice though. > >> 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... > > This is one of the cases where gnulib prefers to emulate the shall fail > semantics of glibc, as they are more useful to program around. I don't see how it's nicer. All it does it make pty acquisition slightly slower (one extra useless syscall). The only time you would call grantpt without knowing that the fd is valid is right after calling posix_openpt without checking the return value, and in that case, it seems unlikely that you'd check the return value of grantpt. And last time I asked, I remember being told that gnulib does not intend to facilitate this sort of lazy programming anyway. In any case, if you are relying on lazy error checking like that, unlockpt will already report the error... > >> 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. > > There _is_ a proposed standard for it now: > > http://austingroupbugs.net/view.php?id=508 > > which requires only the return value to be 0 or an errno value, and not > that errno be set. gnulib should only be checking for a valid return value. Okay, I'll update it to match this. I wish they'd just standardized the superior BSD openpty function instead... > >> 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. > > This is a case where POSIX is rather weak, but where quality of > implementation demands that the most useful interface is one that > provides the most information back to the user. glibc had a number of > bugs that were fixed in this area to improve QoI, and gnulib now prefers > to rely on those improvements. I don't see anything which forbids it from writing in this case, so I suppose I could change it. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker 2012-06-20 4:10 ` [musl] " Eric Blake @ 2012-06-20 7:32 ` Szabolcs Nagy 2012-06-22 10:39 ` grantpt test Bruno Haible 2012-07-02 22:33 ` [musl] Re: musl bugs found through gnulib Pádraig Brady 3 siblings, 0 replies; 35+ messages in thread From: Szabolcs Nagy @ 2012-06-20 7:32 UTC (permalink / raw) To: musl * Rich Felker <dalias@aerifal.cx> [2012-06-19 23:04:45 -0400]: > Some more updates.. > > On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > > Replacement of perror, because of > > checking whether perror matches strerror... no > > this is a musl issue: diff --git a/src/stdio/perror.c b/src/stdio/perror.c index 4349ac5..fdcb4d7 100644 --- a/src/stdio/perror.c +++ b/src/stdio/perror.c @@ -10,7 +10,7 @@ void perror(const char *msg) FLOCK(f); - if (msg) { + if (msg && *msg) { fwrite(msg, strlen(msg), 1, f); fputc(':', f); fputc(' ', f); ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: grantpt test 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker 2012-06-20 4:10 ` [musl] " Eric Blake 2012-06-20 7:32 ` Szabolcs Nagy @ 2012-06-22 10:39 ` Bruno Haible 2012-07-02 22:33 ` [musl] Re: musl bugs found through gnulib Pádraig Brady 3 siblings, 0 replies; 35+ messages in thread From: Bruno Haible @ 2012-06-22 10:39 UTC (permalink / raw) To: bug-gnulib; +Cc: Rich Felker, musl, Isaac Dunham, Paul Eggert, Reuben Thomas Rich Felker wrote: > > 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... Looking at the (few) callers of grantpt() in gnulib, it indeed seems unlikely that people will want to rely on the failure for invalid file descriptors. So I'm relaxing the requirements of gnulib. 2012-06-22 Bruno Haible <bruno@clisp.org> grantpt: Relax requirement regarding invalid file descriptors. * lib/grantpt.c: Don't include <fcntl.h>. (grantpt): Don't verify the validity of the file descriptor. * modules/grantpt (Depends-on): Remove fcntl-h. * tests/test-grantpt.c (main): Allow grantpt to succeed for invalid file descriptors. * doc/posix-functions/grantpt.texi: Document more platforms on which grantpt succeeds for invalid file descriptors. Reported by Rich Felker <dalias@aerifal.cx>. --- doc/posix-functions/grantpt.texi.orig Fri Jun 22 12:33:55 2012 +++ doc/posix-functions/grantpt.texi Fri Jun 22 12:33:52 2012 @@ -20,5 +20,5 @@ IRIX 5.3. @item This function reports success for invalid file descriptors on some platforms: -Cygwin 1.7.9. +OpenBSD, Cygwin 1.7.9, musl libc. @end itemize --- lib/grantpt.c.orig Fri Jun 22 12:33:55 2012 +++ lib/grantpt.c Fri Jun 22 12:14:57 2012 @@ -21,7 +21,6 @@ #include <assert.h> #include <errno.h> -#include <fcntl.h> #include <string.h> #include <sys/wait.h> #include <unistd.h> @@ -50,8 +49,6 @@ #if defined __OpenBSD__ /* On OpenBSD, master and slave of a pseudo-terminal are allocated together, through an ioctl on /dev/ptm. There is no need for grantpt(). */ - if (fcntl (fd, F_GETFD) < 0) - return -1; return 0; #else /* This function is most often called from a process without 'root' --- modules/grantpt.orig Fri Jun 22 12:33:55 2012 +++ modules/grantpt Fri Jun 22 12:15:14 2012 @@ -9,7 +9,6 @@ Depends-on: stdlib extensions -fcntl-h [test $HAVE_GRANTPT = 0] pt_chown [test $HAVE_GRANTPT = 0] waitpid [test $HAVE_GRANTPT = 0] configmake [test $HAVE_GRANTPT = 0] --- tests/test-grantpt.c.orig Fri Jun 22 12:33:55 2012 +++ tests/test-grantpt.c Fri Jun 22 12:14:31 2012 @@ -28,22 +28,36 @@ int main (void) { - /* Test behaviour for invalid file descriptors. */ + /* Test behaviour for invalid file descriptors. + These calls don't fail on OpenBSD (with gnulib's replacement) and on + musl libc. */ { + int ret; + errno = 0; - ASSERT (grantpt (-1) == -1); - ASSERT (errno == EBADF - || errno == EINVAL /* seen on FreeBSD 6.4 */ - || errno == 0 /* seen on Solaris 8 */ - ); + ret = grantpt (-1); + if (ret != 0) + { + ASSERT (ret == -1); + ASSERT (errno == EBADF + || errno == EINVAL /* seen on FreeBSD 6.4 */ + || errno == 0 /* seen on Solaris 8 */ + ); + } } { + int ret; + errno = 0; - ASSERT (grantpt (99) == -1); - ASSERT (errno == EBADF - || errno == EINVAL /* seen on FreeBSD 6.4 */ - || errno == 0 /* seen on Solaris 8 */ - ); + ret = grantpt (99); + if (ret != 0) + { + ASSERT (ret == -1); + ASSERT (errno == EBADF + || errno == EINVAL /* seen on FreeBSD 6.4 */ + || errno == 0 /* seen on Solaris 8 */ + ); + } } return 0; ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker ` (2 preceding siblings ...) 2012-06-22 10:39 ` grantpt test Bruno Haible @ 2012-07-02 22:33 ` Pádraig Brady 3 siblings, 0 replies; 35+ messages in thread From: Pádraig Brady @ 2012-07-02 22:33 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, musl, Reuben Thomas On 06/20/2012 05:04 AM, Rich Felker wrote: > Some more updates.. > > On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: >> 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... This is addressed in different ways in these two commits: http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commit;h=defe5737 http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commit;h=2ab2617e cheers, Pádraig. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible ` (3 preceding siblings ...) 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker @ 2012-06-20 19:28 ` Rich Felker 2012-06-21 2:21 ` Rich Felker 4 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-20 19:28 UTC (permalink / raw) To: musl; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, Reuben Thomas On Mon, Jun 18, 2012 at 12:49:44AM +0200, Bruno Haible wrote: > [CCing the musl list] > Isaac Dunham wrote in > <http://lists.gnu.org/archive/html/bug-gnulib/2012-06/msg00101.html>: > > musl is designed for standards conformance, > > There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, > that explains how to use gnulib to check a libc against bugs. When I apply > this to musl-0.9.1, I get this list of problems: > > Replacements of *printf, because of > checking whether printf supports infinite 'long double' arguments... no Fixed. (Not really a bug, but fixed anyway.) > checking whether printf supports the 'ls' directive... no Previously fixed. > checking whether printf survives out-of-memory conditions... no Fixed. > Replacement of duplocale, because of > checking whether duplocale(LC_GLOBAL_LOCALE) works... no Fixed. > Replacement of fdopen, because of > checking whether fdopen sets errno... no Not a bug. I believe this was fixed in gnulib. > Replacement of futimens, because of > checking whether futimens works... no Not a bug; just confusing message. > 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 open; probably not a bug. > Replacement of getopt, because of > checking whether getopt is POSIX compatible... no Not a bug. > Replacement of glob, because of > checking for GNU glob interface version 1... no > (not sure this is a bug or just an incompatibility compared to glibc) Not supported. > Replacement of iconv and iconv_open, because of > checking whether iconv supports conversion between UTF-8 and UTF-{16,32}{BE,LE}... no Fixed. > > Replacement of mktime, because of > checking for working mktime... no Still open. > Replacement of perror, because of > checking whether perror matches strerror... no Fixed. > Replacement of popen, because of > checking whether popen works with closed stdin... no Fixed. > Replacement of regex, because of > checking for working re_compile_pattern... no Not supported. > Replacement of strtod, because of > checking whether strtod obeys C99... no Previously fixed. > test-duplocale.c:70: assertion failed > FAIL: test-duplocale Fixed. > test-fcntl.c:382: assertion failed > FAIL: test-fcntl Pending; intend to fix. > test-fdatasync.c:50: assertion failed > FAIL: test-fdatasync Fixed. > test-fma2.h:116: assertion failed > FAIL: test-fma2 Unknown. Asking nsz.. > test-fsync.c:50: assertion failed > FAIL: test-fsync Fixed. > test-fwrite.c:53: assertion failed > FAIL: test-fwrite Fixed. > test-getlogin_r.c:88: assertion failed > FAIL: test-getlogin_r Fixed. > test-grantpt.c:34: assertion failed > FAIL: test-grantpt Buggy/useless test. > test-localeconv.c:41: assertion failed > FAIL: test-localeconv Fixed. > Segmentation fault > FAIL: test-localename Still open. > test-ptsname_r.c:118: assertion failed > FAIL: test-ptsname_r Fixed. > test-strerror_r.c:118: assertion failed > FAIL: test-strerror_r Fixed. > test-wcwidth.c:71: assertion failed > FAIL: test-wcwidth Fixed. > 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 OK, this is valid fallback code for when statvfs fails, but the headers required for it have not been included. Basically the only still-open issues are getcwd, mktime, fma, localename, so I'll avoid future spam by just addressing them. I've kept all the Cc's so far, but if this is getting OT for gnulib folks, I'll be happy to drop the Cc. Just let me know. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-20 19:28 ` Rich Felker @ 2012-06-21 2:21 ` Rich Felker 2012-06-21 8:52 ` [musl] " Paul Eggert 0 siblings, 1 reply; 35+ messages in thread From: Rich Felker @ 2012-06-21 2:21 UTC (permalink / raw) To: musl; +Cc: bug-gnulib, Isaac Dunham, Paul Eggert, Reuben Thomas On Wed, Jun 20, 2012 at 03:28:02PM -0400, Rich Felker wrote: > > Replacement of getcwd, because of > > checking whether getcwd handles long file names properly... no, but it is partly working This test is failing because musl uses the kernel to resolve the current directory name, and the kernel does not support pathnames longer than PATH_MAX. For some reason, the test only considers this an error if AT_FDCWD is defined. Does gnulib aim to provide a getcwd that always works regardless of path depth? If so, replacing getcwd is the right action for gnulib on musl. > > checking whether getcwd aborts when 4k < cwd_length < 16k... no No is the correct result here. This test is looking for a bug that only exists on some archs with large page sizes (>4k), and no means it did not find the bug. > > Replacement of mktime, because of > > checking for working mktime... no This test is buggy; it goes into an infinite loop due to integer overflow UB, because the condition to break out of the loop is only checked when the test does not fail: for (j = 1; ; j <<= 1) if (! bigtime_test (j)) result |= 4; else if (INT_MAX / 2 < j) break; However this does indicate a bug in musl. The relevant code is very old and I suspect it's not checking for integer overflows at all, just generating huge time_t values that get truncated rather than mapped to (time_t)-1. Both need to be fixed. > > test-fcntl.c:382: assertion failed > > FAIL: test-fcntl > > Pending; intend to fix. Fixed. > > test-fma2.h:116: assertion failed > > FAIL: test-fma2 > > Unknown. Asking nsz.. Fixed by nsz. :-) Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib 2012-06-21 2:21 ` Rich Felker @ 2012-06-21 8:52 ` Paul Eggert 0 siblings, 0 replies; 35+ messages in thread From: Paul Eggert @ 2012-06-21 8:52 UTC (permalink / raw) To: Rich Felker; +Cc: bug-gnulib, Reuben Thomas, musl, Isaac Dunham On 06/20/2012 07:21 PM, Rich Felker wrote: >>> Replacement of mktime, because of >>> checking for working mktime... no > > This test is buggy; it goes into an infinite loop due to integer > overflow UB, because the condition to break out of the loop is only > checked when the test does not fail: Thanks, I pushed the following patch into gnulib: --- ChangeLog | 9 +++++++++ m4/mktime.m4 | 25 ++++++++++++++----------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 199b06c..1661a62 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2012-06-21 Paul Eggert <eggert@cs.ucla.edu> + + mktime: fix integer overflow in 'configure'-time test + * m4/mktime.m4 (gl_FUNC_MKTIME): Do not rely on undefined behavior + after integer overflow. Problem reported by Rich Felker in + <http://lists.gnu.org/archive/html/bug-gnulib/2012-06/msg00257.html>. + Also, don't look for further instances of a bug if we've already + found one instance; this helps 'configure' run faster. + 2012-06-20 John Darrington <john@darrington.wattle.id.au> (tiny change) tmpfile, clean-temp: Fix invocation of GetVersionEx. diff --git a/m4/mktime.m4 b/m4/mktime.m4 index 5e05dfa..14fcf7f 100644 --- a/m4/mktime.m4 +++ b/m4/mktime.m4 @@ -1,4 +1,4 @@ -# serial 21 +# serial 22 dnl Copyright (C) 2002-2003, 2005-2007, 2009-2012 Free Software Foundation, dnl Inc. dnl This file is free software; the Free Software Foundation @@ -192,20 +192,23 @@ main () if (tz_strings[i]) putenv (tz_strings[i]); - for (t = 0; t <= time_t_max - delta; t += delta) + for (t = 0; t <= time_t_max - delta && (result & 1) == 0; t += delta) if (! mktime_test (t)) result |= 1; - if (! (mktime_test ((time_t) 1) - && mktime_test ((time_t) (60 * 60)) - && mktime_test ((time_t) (60 * 60 * 24)))) + if ((result & 2) == 0 + && ! (mktime_test ((time_t) 1) + && mktime_test ((time_t) (60 * 60)) + && mktime_test ((time_t) (60 * 60 * 24)))) result |= 2; - for (j = 1; ; j <<= 1) - if (! bigtime_test (j)) - result |= 4; - else if (INT_MAX / 2 < j) - break; - if (! bigtime_test (INT_MAX)) + for (j = 1; (result & 4) == 0; j <<= 1) + { + if (! bigtime_test (j)) + result |= 4; + if (INT_MAX / 2 < j) + break; + } + if ((result & 8) == 0 && ! bigtime_test (INT_MAX)) result |= 8; } if (! irix_6_4_bug ()) -- 1.7.6.5 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [musl] Re: musl bugs found through gnulib @ 2012-06-18 0:05 idunham 2012-06-18 0:29 ` Rich Felker 0 siblings, 1 reply; 35+ messages in thread From: idunham @ 2012-06-18 0:05 UTC (permalink / raw) To: musl; +Cc: Isaac Dunham, Paul Eggert, bug-gnulib, Reuben Thomas > [CCing the musl list] > Isaac Dunham wrote in > <http://lists.gnu.org/archive/html/bug-gnulib/2012-06/msg00101.html>: >> musl is designed for standards conformance, > > There is a recipe, in <http://sourceware.org/glibc/wiki/Testing/Gnulib>, > that explains how to use gnulib to check a libc against bugs. Be warned: a bad test can cause failures as well. It's been one of the musl developers' complaints about gnulib that the tests are buggy and frequently check for glibc behavior instead of standard behavior. > When I apply this to musl-0.9.1, I get this list of problems: > > Replacements of *printf, because of > checking whether printf supports infinite 'long double' arguments... no > checking whether printf supports the 'ls' directive... no > checking whether printf survives out-of-memory conditions... no At least one of these (infinite long double, IIRC) is invalid or a test for a GNU-ism. This was previously discussed on the musl ML. OOM behavior is undefined AFAICT (feel free to point out a standard), and the scenario is a lot less likely with musl than glibc for several reasons. > Replacement of duplocale, because of > checking whether duplocale(LC_GLOBAL_LOCALE) works... no Need to check this one > Replacement of fdopen, because of > checking whether fdopen sets errno... no I presume this is nonconformance to POSIX ("otherwise, a null pointer shall be returned and errno set...")? > Replacement of futimens, because of > checking whether futimens works... no Could be a bug. > Replacement of getcwd, because of > checking whether getcwd handles long file names properly... no, but it > is partly working Is this a test for ERANGE handling (error on name >= size)? Other than that, I see no specification covering this. > checking whether getcwd aborts when 4k < cwd_length < 16k... no AFAICT, only required to error when size =< cwd_length. If size !< (cwd_length + 1), that is conformant behavior. (See man 3posix getcwd) > Replacement of getopt, because of > checking whether getopt is POSIX compatible... no We'd need to see this test...(will look later). > Replacement of glob, because of > checking for GNU glob interface version 1... no > (not sure this is a bug or just an incompatibility compared to glibc) Looks like an incompatability, since it specifies "GNU interface"... > Replacement of iconv and iconv_open, because of > checking whether iconv supports conversion between UTF-8 and > UTF-{16,32}{BE,LE}... no Not "nonconformant" from the standpoint of POSIX, AFAICT, but it is incomplete. musl is UTF8 native, but I don't think it supports UTF16/UTF32 yet. > 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 Look like bugs, if the description is correct. > Replacement of regex, because of > checking for working re_compile_pattern... no This is #ifdef __USE_GNU I'm not aware of any standard covering GNU APIs... > 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). > Thanks, Isaac Dunham ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: musl bugs found through gnulib 2012-06-18 0:05 idunham @ 2012-06-18 0:29 ` Rich Felker 0 siblings, 0 replies; 35+ messages in thread From: Rich Felker @ 2012-06-18 0:29 UTC (permalink / raw) To: musl On Sun, Jun 17, 2012 at 05:05:23PM -0700, idunham@lavabit.com wrote: > > Replacement of fdopen, because of > > checking whether fdopen sets errno... no > I presume this is nonconformance to POSIX ("otherwise, a null pointer > shall be returned and errno set...")? The EBADF error is a "may fail", not a "shall fail", per POSIX. Since it requires an extra syscall in the general case (although in some special cases, we're already making a syscall that could detect EBADF), I find it's probably just an undesirable performance hit. Applications should interpret the "may fail" as meaning it's not valid/portable to call fdopen with an invalid file descriptor and assume you can get a FILE* that will generate EBADF when it's used later, not as meaning that the implementation will do their error-checking for them. > > Replacement of futimens, because of > > checking whether futimens works... no > Could be a bug. See the #ifdef __linux__ in the test file. Due to old buggy kernels, they ALWAYS consider futimens broken on Linux. > > Replacement of getcwd, because of > > checking whether getcwd handles long file names properly... no, but it > > is partly working > Is this a test for ERANGE handling (error on name >= size)? Other than > that, I see no specification covering this. > > checking whether getcwd aborts when 4k < cwd_length < 16k... no > AFAICT, only required to error when size =< cwd_length. If size !< > (cwd_length + 1), that is conformant behavior. (See man 3posix getcwd) I think musl is correct on everything for getcwd, but I'd like to double-check. > > Replacement of getopt, because of > > checking whether getopt is POSIX compatible... no > We'd need to see this test...(will look later). It's testing for GNU behavior not POSIX behavior. > > Replacement of glob, because of > > checking for GNU glob interface version 1... no > > (not sure this is a bug or just an incompatibility compared to glibc) > Looks like an incompatability, since it specifies "GNU interface"... Same. > > Replacement of iconv and iconv_open, because of > > checking whether iconv supports conversion between UTF-8 and > > UTF-{16,32}{BE,LE}... no > Not "nonconformant" from the standpoint of POSIX, AFAICT, but it is > incomplete. musl is UTF8 native, but I don't think it supports UTF16/UTF32 > yet. It does. musl's iconv supports all UTFs (but no endian auto-detection; explicit endianness is required in the argument to iconv_open), all ISO-8859s, lots of other legacy 8bit charsets, and some legacy Chinese and Japanese charsets as source charsets only (not destination). > > 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 > Look like bugs, if the description is correct. Yes, I'm looking into these as possible bugs, but I don't think anything's wrong with perror... > > Replacement of regex, because of > > checking for working re_compile_pattern... no > This is #ifdef __USE_GNU > I'm not aware of any standard covering GNU APIs... Well gnulib wants to offer this API, so if it doesn't exist, gnulib must provide it.. Rich ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2012-07-02 22:33 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20120609230541.47eac2de@newbook> [not found] ` <4FD55156.7050302@cs.ucla.edu> [not found] ` <20120611182202.1ee4d019@newbook> 2012-06-17 22:49 ` musl bugs found through gnulib Bruno Haible 2012-06-17 23:54 ` Rich Felker 2012-06-18 8:21 ` Szabolcs Nagy 2012-06-18 13:02 ` John Spencer 2012-06-18 14:55 ` Rich Felker 2012-06-18 15:26 ` Szabolcs Nagy 2012-06-18 16:00 ` Rich Felker 2012-06-19 13:26 ` John Spencer 2012-06-18 0:16 ` [musl] " idunham 2012-06-19 0:11 ` Rich Felker 2012-06-19 2:07 ` [musl] " Eric Blake 2012-06-19 2:52 ` Rich Felker 2012-06-19 11:03 ` musl, fdopen test Bruno Haible 2012-06-19 11:09 ` Jim Meyering 2012-06-20 20:52 ` Bruno Haible 2012-06-19 10:45 ` musl, printf out-of-memory test Bruno Haible 2012-06-19 19:16 ` Rich Felker 2012-06-19 20:04 ` Bruno Haible 2012-06-19 20:08 ` Rich Felker 2012-06-19 21:17 ` Bruno Haible 2012-06-20 1:52 ` Rich Felker 2012-06-20 7:30 ` Szabolcs Nagy 2012-06-20 9:35 ` Bruno Haible 2012-06-20 11:00 ` Jim Meyering 2012-06-21 19:58 ` Tom Tromey 2012-06-20 3:04 ` Re: musl bugs found through gnulib Rich Felker 2012-06-20 4:10 ` [musl] " Eric Blake 2012-06-20 13:27 ` Rich Felker 2012-06-20 7:32 ` Szabolcs Nagy 2012-06-22 10:39 ` grantpt test Bruno Haible 2012-07-02 22:33 ` [musl] Re: musl bugs found through gnulib Pádraig Brady 2012-06-20 19:28 ` Rich Felker 2012-06-21 2:21 ` Rich Felker 2012-06-21 8:52 ` [musl] " Paul Eggert 2012-06-18 0:05 idunham 2012-06-18 0:29 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).