[PATCH] bugfix: initialize a state variable in lio_wait

[PATCH] bugfix: initialize a state variable in lio_wait

[Jens Gustedt]

got_err was only set to 1 in case of error, but never written
otherwise. This resulted in situations where lio_listio returned -1 for a
request with cnt == 1, but where that sole request had completed.
---
 src/aio/lio_listio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/aio/lio_listio.c b/src/aio/lio_listio.c
index 64a6ebc..07145dd 100644
--- a/src/aio/lio_listio.c
+++ b/src/aio/lio_listio.c
@@ -13,7 +13,7 @@ struct lio_state {
 
 static int lio_wait(struct lio_state *st)
 {
-	int i, err, got_err;
+	int i, err, got_err = 0;
 	int cnt = st->cnt;
 	struct aiocb **cbs = st->cbs;
 
-- 
1.7.9.5

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Szabolcs Nagy]

* Jens Gustedt <Jens.Gustedt@inria.fr> [2013-06-16 00:01:20 +0200]:
> got_err was only set to 1 in case of error, but never written
> otherwise.

i wonder why gcc -Wmaybe-uninitialized does not catch this

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Rich Felker]

On Sun, Jun 16, 2013 at 12:16:27AM +0200, Szabolcs Nagy wrote:
> * Jens Gustedt <Jens.Gustedt@inria.fr> [2013-06-16 00:01:20 +0200]:
> > got_err was only set to 1 in case of error, but never written
> > otherwise.
> 
> i wonder why gcc -Wmaybe-uninitialized does not catch this

Indeed, this is very strange. Anyway, I'm committing the fix.

Rich

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Jens Gustedt]

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

Hello,

Am Sonntag, den 16.06.2013, 01:40 -0400 schrieb Rich Felker:
> On Sun, Jun 16, 2013 at 12:16:27AM +0200, Szabolcs Nagy wrote:
> > * Jens Gustedt <Jens.Gustedt@inria.fr> [2013-06-16 00:01:20 +0200]:
> > > got_err was only set to 1 in case of error, but never written
> > > otherwise.
> > 
> > i wonder why gcc -Wmaybe-uninitialized does not catch this
> 
> Indeed, this is very strange. Anyway, I'm committing the fix.

Looking a bit deeper into it, I think this is not only strange but
suspicious.

First, this means that we have not enough coverage for the aio
routines to find simple bugs like that one. aio isn't much used,
obviously.

But then I also checked with clang, and it doesn't find this
either. First, I thought that the control flow surrounding this must
be fundamentally flawed if the compilers aren't capable to track such
simple cases of uninitialized variables. But even by simplifying, it
doesn't trigger, so I am not sure what is going on.

It makes not much sense to me to have an infinite loop at the
outside. I think the control flow should merely be something as
below. Still I am not sure that the strategy of continuing after an
error has been detected is paying something. I would be more in favor
in erroring out as early as possible and return err in errno to let
the application know that there was a problem. Waiting until we
checked everybody and then returning EIO is throwing away information.

Jens

static int lio_wait(struct lio_state *st)
{
  int got_err = 0;
  struct aiocb **const cbs = st->cbs;

  for (int i=0, cnt = st->cnt; i<cnt; i++) {
    if (cbs[i]) {
    RETRY:;
      switch(aio_error(cbs[i])) {
      case EINPROGRESS:
        if (aio_suspend((void *)cbs, cnt, 0))
          return -1;
        else
          goto RETRY;
      case 0:
        break;
      default:
        got_err=1;
        cbs[i] = 0;
        break;
      }
    }
  }
  if (got_err) {
    errno = EIO;
    return -1;
  } else
    return 0;
}



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



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

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 09:22 +0200 schrieb Jens Gustedt:
> But then I also checked with clang, and it doesn't find this
> either. First, I thought that the control flow surrounding this must
> be fundamentally flawed if the compilers aren't capable to track such
> simple cases of uninitialized variables. But even by simplifying, it
> doesn't trigger, so I am not sure what is going on.

I checked the assembler that is produced, and the reason that the bug
might not trigger for most people is that the variable ends up in ebx
which is nulled anyway at the beginning of the function. Then,
depending on how that (static) function might be inlined or not in the
context of the calling function and in which hardware register the
variable ends up, there, this could trigger the bug or not.

In all, this re-enforces my opinion to *always* initialize variables,
unless I know that its address is immediately passed to an
initialization function. If the compiler is good in computing the flow
control for a given case, it will optimize it out if it is
superfluous. If the flow control is too complicated for the compiler,
it is probably too complicated for me, too.

BTW, I used valgrind to help me finding such stuff on initialization,
but the tons of false positives that musl triggers because of the str
functions and calloc are really a pain.

Jens

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




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

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Justin Cormack]

On Sun, Jun 16, 2013 at 10:31 AM, Jens Gustedt <jens.gustedt@inria.fr> wrote:
> BTW, I used valgrind to help me finding such stuff on initialization,
> but the tons of false positives that musl triggers because of the str
> functions and calloc are really a pain.

It would be nice to ship a valgrind suppressions file shipped with
Musl to ignore these, I have always found it very helpful to have as a
part of the documentation in order to not worry about trying to work
out myself which are false positives when working with a project. You
can use valgrind --gen-suppressions to produce them, but I guess we
need to check that they are not in fact bugs!

(The valgrind maintainers are also very responsive to issues)

Justin

valgrind problems

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 11:18 +0100 schrieb Justin Cormack:
> On Sun, Jun 16, 2013 at 10:31 AM, Jens Gustedt <jens.gustedt@inria.fr> wrote:
> > BTW, I used valgrind to help me finding such stuff on initialization,
> > but the tons of false positives that musl triggers because of the str
> > functions and calloc are really a pain.
> 
> It would be nice to ship a valgrind suppressions file shipped with
> Musl to ignore these, I have always found it very helpful to have as a
> part of the documentation in order to not worry about trying to work
> out myself which are false positives when working with a project. You
> can use valgrind --gen-suppressions to produce them, but I guess we
> need to check that they are not in fact bugs!
> 
> (The valgrind maintainers are also very responsive to issues)

yes, I know about that possibility and already used this in the
past. What would bother me more is that by switching of the check for
strlen, e.g., valgrind wouldn't find real user code errors when I pass
an uninitialized char[] to it. Switching off all the checks for the
str*** functions would somehow be counter productive in the context of
valgrind.

Also for calloc I am not sure that when we switch off the false
positive (where musl assumes a certain gcc behavior for what in
general is UB) valgrind would still capture the fact that the malloced
object is 0 initialized. If it doesn't we would get even more false
positives.

Jens


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



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

Re: valgrind problems

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-06-16 12:43:08 +0200]:
> Am Sonntag, den 16.06.2013, 11:18 +0100 schrieb Justin Cormack:
> > It would be nice to ship a valgrind suppressions file shipped with
> > Musl to ignore these, I have always found it very helpful to have as a
> yes, I know about that possibility and already used this in the
> past. What would bother me more is that by switching of the check for
> strlen, e.g., valgrind wouldn't find real user code errors when I pass
> an uninitialized char[] to it. Switching off all the checks for the
> str*** functions would somehow be counter productive in the context of
> valgrind.
...

note that valgrind cannot emulate 80bit long double arithmetics
(uses 64bit arithmetics instead) which breaks floating-point
printf and strtod functions in musl (and most math code on x86
take slightly different code paths, some break badly)

so valgrind has issues with floating-point code and there were
limitations with threading as well

if the address sanitizer support works in gcc or clang that
might be a better option for catching memory errors

Re: valgrind problems

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 13:39 +0200 schrieb Szabolcs Nagy:
> note that valgrind cannot emulate 80bit long double arithmetics
> (uses 64bit arithmetics instead) which breaks floating-point
> printf and strtod functions in musl (and most math code on x86
> take slightly different code paths, some break badly)
> 
> so valgrind has issues with floating-point code and there were
> limitations with threading as well

Hm, valgrind seems to be *the* tool that is used everywhere. And on
glibc platforms it gives valuable insight and finds a lot of bugs. As
a naïve user of musl I would just expect it to work.

Jens

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



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

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 04:16:43PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 16.06.2013, 13:39 +0200 schrieb Szabolcs Nagy:
> > note that valgrind cannot emulate 80bit long double arithmetics
> > (uses 64bit arithmetics instead) which breaks floating-point
> > printf and strtod functions in musl (and most math code on x86
> > take slightly different code paths, some break badly)
> > 
> > so valgrind has issues with floating-point code and there were
> > limitations with threading as well
> 
> Hm, valgrind seems to be *the* tool that is used everywhere. And on
> glibc platforms it gives valuable insight and finds a lot of bugs. As
> a naïve user of musl I would just expect it to work.

I agree with you Jens -- user expectation is that valgrind should
work (reasonable) and that valgrind is the best available tool for the
job (which I believe is still true).

However the floating point issue is real; valgrind will badly
mis-execute code calling strtod or any of the scanf family of
functions for floating point parsing, and will give subtly different
results for a large portion of the math library.

If anyone has a good working relationship with the valgrind team, I
think we should raise this issue politely and aim to get correct
floating point emulation added to valgrind. Obviously it may have to
be done in software rather than using the host's floating point, but
there's already a good, presumably bit-exact, GPLv2 implementation of
i387 floating point emulation in the kernel if nobody wants to write a
new one.

Rich

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 12:43:08PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 16.06.2013, 11:18 +0100 schrieb Justin Cormack:
> > On Sun, Jun 16, 2013 at 10:31 AM, Jens Gustedt <jens.gustedt@inria.fr> wrote:
> > > BTW, I used valgrind to help me finding such stuff on initialization,
> > > but the tons of false positives that musl triggers because of the str
> > > functions and calloc are really a pain.
> > 
> > It would be nice to ship a valgrind suppressions file shipped with
> > Musl to ignore these, I have always found it very helpful to have as a
> > part of the documentation in order to not worry about trying to work
> > out myself which are false positives when working with a project. You
> > can use valgrind --gen-suppressions to produce them, but I guess we
> > need to check that they are not in fact bugs!
> > 
> > (The valgrind maintainers are also very responsive to issues)
> 
> yes, I know about that possibility and already used this in the
> past. What would bother me more is that by switching of the check for
> strlen, e.g., valgrind wouldn't find real user code errors when I pass
> an uninitialized char[] to it. Switching off all the checks for the
> str*** functions would somehow be counter productive in the context of
> valgrind.

Is there no way to make the suppression more fine-grained? For
example, "ignore up to wordsize-1 sequential invalid reads in this
function" would give the desired behavior. glibc has some sort of
suppression for these functions; what does valgrind do for glibc?

> Also for calloc I am not sure that when we switch off the false
> positive (where musl assumes a certain gcc behavior for what in
> general is UB)

musl is not assuming any gcc behavior. It's assuming its own
implementation of malloc, which is valid because calloc is part of the
malloc implementation. From an implementation-internal standpoint,
musl's malloc does not just return a pointer to an object of size n.
It returns a pointer to offset 2*sizeof(size_t) in an object of size
2*sizeof(size_t)+n which is possibly (depending on the bits in the
extra header) a subobject of a much larger object.

> valgrind would still capture the fact that the malloced
> object is 0 initialized. If it doesn't we would get even more false
> positives.

I wish we could find a good way around this. Unfortunately the obvious
solution (accessing malloc via an alias) would break since we have two
different malloc implementations that can be used in static linking,
and I don't see any way to make the alias bind to the one that's
actually going to be used.

Is there any fancier sort of suppression we could do?

Rich

Re: valgrind problems

[Jens Gustedt]

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

Hello,

Am Sonntag, den 16.06.2013, 10:31 -0400 schrieb Rich Felker:
> Is there no way to make the suppression more fine-grained? For
> example, "ignore up to wordsize-1 sequential invalid reads in this
> function" would give the desired behavior. glibc has some sort of
> suppression for these functions; what does valgrind do for glibc?

I think it basically wraps its own code around some functions. Or
maybe just replaces them?

Such an approach obviously doesn't work if we link the C library
statically. I just wanted to try to link statically against glibc to
see what happens, but got stuck with other stuff.

> > Also for calloc I am not sure that when we switch off the false
> > positive (where musl assumes a certain gcc behavior for what in
> > general is UB)
> 
> musl is not assuming any gcc behavior.

unfortunately it does. as an optimization shortcut it reads the newly
allocated bytes and if they are already 0, it doesn't write to
them. So this uses the fact that the newly allocated memory has an
unspecified, but determined value to do some optimization.

It is exactly that line of calloc.c, namely the `if` in line 20 that
triggers under valgrind.

In a recent discussion with the C committee I have learned that there
is no consensus of whether an unspecified value is determined and
wouldn't change once it is observed or whether it could be "wobbling"
(the term that someone used, there). There are even people that
propose to have a sort of state outside the object representation that
captures if memory has been initialized or not.

Jens

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




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

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 05:26:19PM +0200, Jens Gustedt wrote:
> Hello,
> 
> Am Sonntag, den 16.06.2013, 10:31 -0400 schrieb Rich Felker:
> > Is there no way to make the suppression more fine-grained? For
> > example, "ignore up to wordsize-1 sequential invalid reads in this
> > function" would give the desired behavior. glibc has some sort of
> > suppression for these functions; what does valgrind do for glibc?
> 
> I think it basically wraps its own code around some functions. Or
> maybe just replaces them?
> 
> Such an approach obviously doesn't work if we link the C library
> statically. I just wanted to try to link statically against glibc to
> see what happens, but got stuck with other stuff.

I see. Have you tried dynamic linking with musl?

> > > Also for calloc I am not sure that when we switch off the false
> > > positive (where musl assumes a certain gcc behavior for what in
> > > general is UB)
> > 
> > musl is not assuming any gcc behavior.
> 
> unfortunately it does. as an optimization shortcut it reads the newly
> allocated bytes and if they are already 0, it doesn't write to
> them.

This is not gcc behavior. It's behavior of it's own implementation of
malloc. From the implementation's standpoint, the object returned by
malloc does not have indeterminate value; otherwise it would be
impossible to link it with its bookkeeping information, contained in
the header of the object, and to later free it. In other words, if you
required the implementation to treat malloc as a black box, it would
be impossible to implement free.

> So this uses the fact that the newly allocated memory has an
> unspecified, but determined value to do some optimization.
> 
> It is exactly that line of calloc.c, namely the `if` in line 20 that
> triggers under valgrind.
> 
> In a recent discussion with the C committee I have learned that there
> is no consensus of whether an unspecified value is determined and
> wouldn't change once it is observed or whether it could be "wobbling"
> (the term that someone used, there). There are even people that
> propose to have a sort of state outside the object representation that
> captures if memory has been initialized or not.

This applies to applications but not to the implementation itself.
That's why musl is compiled with -ffreestanding: to tell the compiler
that functions with standard names are not black boxes it can assume
behave according to how they would on a hosted implementation, but
that we're implementing them.

Rich

Re: valgrind problems

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 11:31 -0400 schrieb Rich Felker:
> > > > Also for calloc I am not sure that when we switch off the false
> > > > positive (where musl assumes a certain gcc behavior for what in
> > > > general is UB)
> > > 
> > > musl is not assuming any gcc behavior.
> > 
> > unfortunately it does. as an optimization shortcut it reads the newly
> > allocated bytes and if they are already 0, it doesn't write to
> > them.
> 
> This is not gcc behavior.

yes, I think it is gcc+platform behavior. the C standard makes no
guarantee that this

   if (*z) *z=0;

guarantees that *z is observably 0 afterwards. And the problem is not
that *z could be UB (if size_t had traps) but that there is no
guarantee that an undetermined value is stable between two reads (the
first with that "if" and the second from the application that receives
this).

> It's behavior of it's own implementation of
> malloc. From the implementation's standpoint, the object returned by
> malloc does not have indeterminate value; otherwise it would be
> impossible to link it with its bookkeeping information, contained in
> the header of the object, and to later free it. In other words, if you
> required the implementation to treat malloc as a black box, it would
> be impossible to implement free.

I agree with all of this, if you can guarantee that the memory *is*
effectively zero'ed after calloc. With the current implementation in
musl you can only do that with additional knowledge about the
platform.

Jens

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



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

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 05:58:52PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 16.06.2013, 11:31 -0400 schrieb Rich Felker:
> > > > > Also for calloc I am not sure that when we switch off the false
> > > > > positive (where musl assumes a certain gcc behavior for what in
> > > > > general is UB)
> > > > 
> > > > musl is not assuming any gcc behavior.
> > > 
> > > unfortunately it does. as an optimization shortcut it reads the newly
> > > allocated bytes and if they are already 0, it doesn't write to
> > > them.
> > 
> > This is not gcc behavior.
> 
> yes, I think it is gcc+platform behavior. the C standard makes no
> guarantee that this
> 
>    if (*z) *z=0;
> 
> guarantees that *z is observably 0 afterwards. And the problem is not
> that *z could be UB (if size_t had traps) but that there is no
> guarantee that an undetermined value is stable between two reads (the
> first with that "if" and the second from the application that receives
> this).

It's not an indeterminate value. It's simply a value. Because malloc
is not special here. It's just like any other function defined in C
code on a freestanding implementation.

> > It's behavior of it's own implementation of
> > malloc. From the implementation's standpoint, the object returned by
> > malloc does not have indeterminate value; otherwise it would be
> > impossible to link it with its bookkeeping information, contained in
> > the header of the object, and to later free it. In other words, if you
> > required the implementation to treat malloc as a black box, it would
> > be impossible to implement free.
> 
> I agree with all of this, if you can guarantee that the memory *is*
> effectively zero'ed after calloc. With the current implementation in
> musl you can only do that with additional knowledge about the
> platform.

No, the only special knowledge musl is using is that it's part of a
hosted implementation written to be used on a freestanding
implementation (gcc -ffreestanding); in particular, the fact that
"malloc" is not the malloc function of a hosted implementation.

Rich

Re: valgrind problems

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 12:04 -0400 schrieb Rich Felker:
> It's not an indeterminate value. It's simply a value. Because malloc
> is not special here. It's just like any other function defined in C
> code on a freestanding implementation.

I agree that malloc is not special here, because it is freestanding,
no problem. But malloc also gets the memory from some system calls, if
you don't have a fixed static pool.

These system calls guarantee you that the value of the memory is
determined. So this is a platform specific guarantee, and valgrind
doesn't seem to share that information.

In particular, valgrind claimed that calloc was using memory
unitialized that was received with brk. It is platform specific to
assume that memory returned by brk is initialized to some value, be it
0 or not. If this is a linux specific guarantee, valgrind seems to be
missing it.

Jens

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



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

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 07:39:43PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 16.06.2013, 12:04 -0400 schrieb Rich Felker:
> > It's not an indeterminate value. It's simply a value. Because malloc
> > is not special here. It's just like any other function defined in C
> > code on a freestanding implementation.
> 
> I agree that malloc is not special here, because it is freestanding,
> no problem. But malloc also gets the memory from some system calls, if
> you don't have a fixed static pool.
> 
> These system calls guarantee you that the value of the memory is
> determined. So this is a platform specific guarantee, and valgrind
> doesn't seem to share that information.
> 
> In particular, valgrind claimed that calloc was using memory
> unitialized that was received with brk. It is platform specific to
> assume that memory returned by brk is initialized to some value, be it
> 0 or not. If this is a linux specific guarantee, valgrind seems to be
> missing it.

I'm pretty sure valgrind's failure here is not missing the fact that
brk (or any new anonymous pages) are zero pages; it's seeing the call
to a function named "malloc" and treating the memory pointed to by the
result as containing indeterminate values. If valgrind's logic were
merely considering anonymous memory from brk or mmap as indeterminate,
it could not catch errors due to use of indeterminate values in memory
obtained by malloc that was recycled from an earlier call to free.

In any case, the behavior of brk and mmap has nothing to do with gcc,
so it's not a matter of gcc-specific assumptions.

Rich

Re: valgrind problems

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2013-06-16 15:16:28 -0400]:
> On Sun, Jun 16, 2013 at 07:39:43PM +0200, Jens Gustedt wrote:
> > In particular, valgrind claimed that calloc was using memory
> > unitialized that was received with brk. It is platform specific to
> > assume that memory returned by brk is initialized to some value, be it
> > 0 or not. If this is a linux specific guarantee, valgrind seems to be
> > missing it.
> 
> I'm pretty sure valgrind's failure here is not missing the fact that
> brk (or any new anonymous pages) are zero pages; it's seeing the call
> to a function named "malloc" and treating the memory pointed to by the
> result as containing indeterminate values. If valgrind's logic were
> merely considering anonymous memory from brk or mmap as indeterminate,
> it could not catch errors due to use of indeterminate values in memory
> obtained by malloc that was recycled from an earlier call to free.

the problem only shows up with static linking
where valgrind does not see the malloc call,
only brk

valgrind thinks that brk is uninitialized

it is easy to demonstrate even with glibc
(using static linking and valgind --track-origins=yes)

so we should just let valgrind know that brk
is ok

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 09:38:25PM +0200, Szabolcs Nagy wrote:
> > I'm pretty sure valgrind's failure here is not missing the fact that
> > brk (or any new anonymous pages) are zero pages; it's seeing the call
> > to a function named "malloc" and treating the memory pointed to by the
> > result as containing indeterminate values. If valgrind's logic were
> > merely considering anonymous memory from brk or mmap as indeterminate,
> > it could not catch errors due to use of indeterminate values in memory
> > obtained by malloc that was recycled from an earlier call to free.
> 
> the problem only shows up with static linking
> where valgrind does not see the malloc call,
> only brk
> 
> valgrind thinks that brk is uninitialized
> 
> it is easy to demonstrate even with glibc
> (using static linking and valgind --track-origins=yes)
> 
> so we should just let valgrind know that brk
> is ok

Oh, okay -- I misunderstood the problem then. And indeed the solution
is just to tell valgrind that new memory from brk and anonymous mmap
is zero-filled. I'm actually fairly surprised it doesn't already know
that...

Rich

Re: valgrind problems

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-06-16 17:58:52 +0200]:
> yes, I think it is gcc+platform behavior. the C standard makes no
> guarantee that this
> 
>    if (*z) *z=0;
> 
> guarantees that *z is observably 0 afterwards. And the problem is not
> that *z could be UB (if size_t had traps) but that there is no
> guarantee that an undetermined value is stable between two reads (the
> first with that "if" and the second from the application that receives
> this).

there is no undefined/implementation defined behaviour there

as rich said in freestanding mode malloc is not special
(musl gets initialized memory from the system through syscalls
and are always well defined at that point)

as far as i can see in freestanding mode you can only get
indeterminate values in
- objects with automatic storage duration
- objects modified from a signal handler
- unnamed members of a structure
none of these apply to *z in calloc

but i think you are wrong about the if (*z) *z=0; more generally:

accessing objects with indeterminate value is not undefined
in general, at least the definition of 'indeterminate value'
does not allow that, unless it may be a trap representation:
http://port70.net/~nsz/c/c11/n1570.html#3.19.2p1

if it is intended to be undefined then the normative text should
say so

the only explicitly undefined case i see is
http://port70.net/~nsz/c/c11/n1570.html#6.3.2.1p2
but that does not apply to accessing objects through a pointer

Re: valgrind problems

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 19:40 +0200 schrieb Szabolcs Nagy:
> but i think you are wrong about the if (*z) *z=0; more generally:
> 
> accessing objects with indeterminate value is not undefined
> in general, at least the definition of 'indeterminate value'
> does not allow that, unless it may be a trap representation:
> http://port70.net/~nsz/c/c11/n1570.html#3.19.2p1
> 
> if it is intended to be undefined then the normative text should
> say so

as I said up-thread, this is not my own opinion (I personally would
argue as you do) but expressed in a recent discussion on the list of
the C standards committee. In addition there is an 10 year old reply
to a DR that goes in that direction.

And to clarify that, this is not about UB, but about the stability of
the value.

Jens


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




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

Re: valgrind problems

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-06-16 20:02:00 +0200]:
> as I said up-thread, this is not my own opinion (I personally would
> argue as you do) but expressed in a recent discussion on the list of
> the C standards committee. In addition there is an 10 year old reply
> to a DR that goes in that direction.

i see

> And to clarify that, this is not about UB, but about the stability of
> the value.

i don't see any way "unstable" value can be allowed in c
without invoking UB

objects have to retain their last stored value or their
initial value
(so object access cannot be unstable)

the abstract machine is defined in terms of sequencing
side effects and value computations
(so indeterminate values can only have unstable meaning
if all operators have well defined semantics for them:
eg. the semantics of '+' should say that the value of
the result is the sum of the operands or indeterminate
if any of the operands is indeterminate)

Re: valgrind problems

[Rich Felker]

On Sun, Jun 16, 2013 at 09:25:47PM +0200, Szabolcs Nagy wrote:
> * Jens Gustedt <jens.gustedt@inria.fr> [2013-06-16 20:02:00 +0200]:
> > as I said up-thread, this is not my own opinion (I personally would
> > argue as you do) but expressed in a recent discussion on the list of
> > the C standards committee. In addition there is an 10 year old reply
> > to a DR that goes in that direction.
> 
> i see
> 
> > And to clarify that, this is not about UB, but about the stability of
> > the value.
> 
> i don't see any way "unstable" value can be allowed in c
> without invoking UB
> 
> objects have to retain their last stored value or their
> initial value
> (so object access cannot be unstable)
> 
> the abstract machine is defined in terms of sequencing
> side effects and value computations
> (so indeterminate values can only have unstable meaning
> if all operators have well defined semantics for them:
> eg. the semantics of '+' should say that the value of
> the result is the sum of the operands or indeterminate
> if any of the operands is indeterminate)

I agree completely. From an optimizing compiler standpoint, I _wish_
the language admitted such propagation of indeterminate values without
invoking full UB, but I don't see any way that the current language of
the standard could be interpreted as allowing that. At one point,
indeterminate values were deemed to invoke UB (see the vestigial item
in J.2) but it seems that the standard has been amended so that
indeterminate values are "safe" in most cases as long as the type does
not have trap representations.

Rich

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Rich Felker]

On Sun, Jun 16, 2013 at 11:31:58AM +0200, Jens Gustedt wrote:
> In all, this re-enforces my opinion to *always* initialize variables,
> unless I know that its address is immediately passed to an

I disagree with this principle. If you initialize the variable to a
dummy value, the compiler (or other static analysis tools) cannot
catch erroneous use of the variable before a real value is stored in
it. Also valgrind cannot catch them. Your "always initialize"
principle only makes sense when (as in this case) there's an obvious
"default value" the variable should have if it's not set elsewhere.
But often that's not the case, especially for pointers.

Rich

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Jens Gustedt]

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

Am Sonntag, den 16.06.2013, 10:24 -0400 schrieb Rich Felker:
> On Sun, Jun 16, 2013 at 11:31:58AM +0200, Jens Gustedt wrote:
> > In all, this re-enforces my opinion to *always* initialize variables,
> > unless I know that its address is immediately passed to an
> 
> I disagree with this principle. If you initialize the variable to a
> dummy value, the compiler (or other static analysis tools) cannot
> catch erroneous use of the variable before a real value is stored in
> it. Also valgrind cannot catch them. Your "always initialize"
> principle only makes sense when (as in this case) there's an obvious
> "default value" the variable should have if it's not set elsewhere.
> But often that's not the case, especially for pointers.

For pointers and all other types of objects there is the C default
initialization with 0. Statically allocated objects are initialized
with that, and so in general code should be made "robust" with respect
to such an initialization, be it just by failing loud and clear when
we run over a pointer that is 0 initialized. (and analysis tools
should be capable to tell us potential dereferences of 0-pointers, the
same as they should be able to find unitialized use.)

The other aspect of that is the possibility of C99 to declare
variables as late as possible. 90% of the variables could be declared
at their first write, which eliminates most of these problems.

Whereas such a 0-initialization strategy mostly works for application
code, there might in fact be issues for a C library such as musl. I
agree that there are circumstances where it just can't fail. But these
code spots need special care, anyhow, I think.

The musl functions that I looked into are short and sweet, but
unfortunately somewhat "obfuscated" by this
top-of-function-declaration mania. I find a style as I posted it
earlier for lio_wait much more readable, for me and for the compiler:

static int lio_wait(struct lio_state *st)
{
  int got_err = 0;
  struct aiocb **const cbs = st->cbs;
  for (int i=0, cnt = st->cnt; i<cnt; i++) {
...

Jens

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



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

Re: [PATCH] bugfix: initialize a state variable in lio_wait

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2013-06-16 00:16:27 +0200]:
> i wonder why gcc -Wmaybe-uninitialized does not catch this

seems to be a long standing, hard to fix bug in gcc

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501