[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Initialize the entire obstack struct [BZ #17919]
From: |
Carlos O'Donell |
Subject: |
Re: [PATCH] Initialize the entire obstack struct [BZ #17919] |
Date: |
Tue, 03 Feb 2015 11:49:26 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
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.
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Siddhesh Poyarekar, 2015/02/03
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Andreas Schwab, 2015/02/03
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919],
Carlos O'Donell <=
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Carlos O'Donell, 2015/02/03
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Carlos O'Donell, 2015/02/03
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Mark Wielaard, 2015/02/03
- Re: [PATCH] Initialize the entire obstack struct [BZ #17919], Siddhesh Poyarekar, 2015/02/05