Re: [PATCH] Initialize the entire obstack struct [BZ #17919]

[Carlos O'Donell]

On 02/03/2015 11:38 AM, Siddhesh Poyarekar wrote:
> [Also looping in bug-gnulib]
> 
> On Tue, Feb 03, 2015 at 11:30:17AM -0500, Carlos O'Donell wrote:
>> Is this a valgrind bug, false positive in valgrind, or bug in glibc?
>>
>> It clearly says we have a move that depends on an uninitizlied value,
>> so something read the value and tried to do something with it.
> 
> It is a combination of multiple things actually.  The uninitialized
> value here is the padding in the structure and we have fixed such
> warnings in the past; see the nscd stats warning[1] for example.

That case was not as performance sensitive as creating and using
obstacks.
 
> This warning doesn't trigger on x86 though due to the code that gcc
> generates for it - it explicitly ANDs the bit field in the struct
> before testing it.  On s390, the optimizer does away with the AND
> operation and tests the entire thing for some reason - I haven't
> figured out why but the instruction combiner RTL pass does away with
> the AND operation.
> 
> This is also why I started out filing a gcc bug, but then changed my
> mind and fixed it in glibc instead, since I presumed that the compiler
> can assume that the padding is also appropriately initialized.  But
> then, I am not completely sure if the compiler is allowed to make that
> assumption.

Thanks for the clarification.

IMO zero-initialized padding, for this case, isn't something you can
expect. Therefore I think it's a compiler bug.

I think it's OK to work around this in glibc, but it needs a comment
with a reference to the filed gcc bug. I do not think it is valid
for gcc on s390x to use the entire bit field along with padding and
I believe it could result in incorrect operation.

Cheers,
Carlos.