Re: two failures on libidn-1.22 testsuite

[Simon Josefsson]

Guido Trentalancia <address@hidden> writes:

> Hi Simon !
>
> Just a quick note to let you know that now with the latest version 1.22,
> I get two failures with any optimisation flag greater than -O1 on gcc
> (instead of just tst_idna2):
>
> PASS: tst_stringprep
> PASS: tst_punycode
> PASS: tst_idna
> ==15596== Invalid read of size 4
> ==15596==    at 0x402E47: idna_to_ascii_4z
> (in /usr/src/libidn-1.22/tests/tst_idna2)

Can you rebuild the library with CFLAGS="-O2 -g" and re-run 'make check'
to get line numbers where this happens?  The output you posted looks
like the same strlen issue as you reported before.

> With -O1 there are no failures.

Possibly because your compiler choses a faster strlen with -O2.

> However, if the problem lies in the library code rather than in the code
> of the test (which we could call a false-alarm),

It is not in the self tests, it is in libidn or below libidn (i.e.,
glibc/compiler).

> then what would be the outcome of a library produced with -O2 or -O3 ?
> I suppose that would be a buggy library by all means, at least on that
> specific piece of code...
>
> Please note that at the moment I have no time to follow up any better
> than this. So I don't know the details, but at a first sight, my opinion
> is still that such piece of code should be forcedly compiled with -O1 if
> gcc is being used (which can be checked very easily and -O1 will always
> override any other faster optimisation).

If this is the strlen issue again, the problem is not in libidn but in
your compiler or glibc that has an optimized strlen that reads in chunks
of 4 (because that is faster than reading in chunks of 1).  Not all
valgrind warnings indicates software bugs: you need to analyze the
warning to understand where it is coming from and why.  It is common for
strlen to be optimized to read in int-sizes rather than char-sizes.

/Simon

> Regards,
>
> Guido
>
> On Mon, 2011-04-25 at 23:35 +0200, Simon Josefsson wrote:
>> Guido Trentalancia <address@hidden> writes:
>> 
>> >> > Since we are not testing the compiler here, could we not avoid the
>> >> > optimisation even when the user has explictly requested it ?
>> >> 
>> >> Valgrind will always test the compiler, since valgrind is checking the
>> >> binary code.  I'm not sure what you mean by the second part -- having
>> >> the code try to out-smart the person building the code rarely works
>> >> well.  Since this was strlen, I suspect that you will get the same
>> >> Valgrind error from almost any code.
>> >> 
>> >> > Using strnlen for example would lead to the same optimisation ?
>> >> 
>> >> Maybe, but it would be wrong to use strnlen here since the string is
>> >> guaranteed to be zero terminated.
>> >
>> > Overriding inherited CFLAGS with -O1 in tests/Makefile (eventually just
>> > for Valgrind tests) ?
>> 
>> First, the code that triggers Valgrind is in the library, not in the
>> self test.  Also, -O1 is not a generic compiler flag, it is specific to
>> gcc-like compilers and libidn works with other C compilers.  Finally,
>> there is simply no way of knowing what flags "inverts" flags passed by
>> the user in general.  If another user passes CFLAGS=--foo when building
>> Libidn, we may need to provide --bar when building libidn to "invert"
>> the effects of what the user requested.  Building with "-O1 --bar" just
>> to deal with situation like yours does not lead to a robust project.
>> 
>> You are better off using the default build parameters unless you know
>> what you are doing and are prepared to deal any issues.
>> 
>> /Simon
>>