Initialize pad outside the conditional to prevent uninitialized data warnings.
Commit Message
Notes:
In sem_open.c, pad was only initialized when __HAVE_64B_ATOMICS was
not true causing valgrind to warn about uninitialized data on some
arches. This patch moves the initialization of pad outside of the
conditional.
Prior to this change, valgrind warned about unitialized bytes on
ppc64, ppc64le, s390x, and aarch64.
Tested on ppc64le with no regressions. Used valgrind to confirm that
the uninitialized bytes warning was fixed.
Comments
On 06/25/2018 04:18 PM, Patsy Franklin wrote:
> Prior to this change, valgrind warned about unitialized bytes on
> ppc64, ppc64le, s390x, and aarch64.
How come that valgrind warns when it's supposed to be uninitialized
padding? Do you have a valgrind trace (preferably with debugging symbols)?
Thanks.
Florian
Hi Florian,
On Mon, Jun 25, 2018 at 10:39 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/25/2018 04:18 PM, Patsy Franklin wrote:
>>
>> Prior to this change, valgrind warned about unitialized bytes on
>> ppc64, ppc64le, s390x, and aarch64.
>
>
> How come that valgrind warns when it's supposed to be uninitialized padding?
The problem occurs when the partially initialized struct is written to
the file system.
I should have stated this in the original description.
Thanks,
Patsy
On 06/25/2018 07:15 PM, Patsy Franklin wrote:
> Hi Florian,
>
> On Mon, Jun 25, 2018 at 10:39 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 06/25/2018 04:18 PM, Patsy Franklin wrote:
>>>
>>> Prior to this change, valgrind warned about unitialized bytes on
>>> ppc64, ppc64le, s390x, and aarch64.
>>
>>
>> How come that valgrind warns when it's supposed to be uninitialized padding?
>
> The problem occurs when the partially initialized struct is written to
> the file system.
>
> I should have stated this in the original description.
Looks okay to me if you put this into the commit message.
Thanks,
Florian
2018-06-22 Patsy Franklin <pfrankli@redhat.com>
* nptl/sem_open.c [!__HAVE_64B_ATOMICS] (sem_open): Don't update pad.
(sem_open): Set sem.newsem.pad to zero for valgrind.
@@ -215,10 +215,11 @@ sem_open (const char *name, int oflag, ...)
sem.newsem.data = value;
#else
sem.newsem.value = value << SEM_VALUE_SHIFT;
- /* pad is used as a mutex on pre-v9 sparc and ignored otherwise. */
- sem.newsem.pad = 0;
sem.newsem.nwaiters = 0;
#endif
+ /* pad is used as a mutex on pre-v9 sparc and ignored otherwise. */
+ sem.newsem.pad = 0;
+
/* This always is a shared semaphore. */
sem.newsem.private = FUTEX_SHARED;