diff mbox

Initialize pad outside the conditional to prevent uninitialized data warnings.

Message ID CAOraFgDEbbq06LuN-RpB4K1WrCXiarWRHqTfVRYp-83WZFJfmQ@mail.gmail.com
State New, archived
Headers

Commit Message

Patsy Franklin June 25, 2018, 2:18 p.m. UTC 
  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

Florian Weimer June 25, 2018, 2:39 p.m. UTC | #1 
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
Patsy Franklin June 25, 2018, 5:15 p.m. UTC | #2 
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
Florian Weimer June 25, 2018, 5:18 p.m. UTC | #3 
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
diff mbox

Patch

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.

diff --git a/nptl/sem_open.c b/nptl/sem_open.c
index 1d7f142134..c5389f6873 100644
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -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;