[PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[H.J. Lu]

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

On Thu, Aug 31, 2017 at 10:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/31/2017 05:55 AM, H.J. Lu wrote:
>
>> One of malloc change caused:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=22052
>
> (uninit warning with -O3)
>
> I have not yet found a good way to suppress this. I don't know yet why
> GCC loses the information that the maybe_p variable is initialized if
> mem2chunk_check returns a non-null pointer.
>

This patch works for me.   OK for master?


-- 
H.J.

[-- Attachment #2: 0001-Silence-O3-Wall-warning-in-malloc-hooks.c-with-GCC-7.patch --]
[-- Type: text/x-patch, Size: 1468 bytes --]

From 735efe5ebc3eb6aa877b00b10ac0046524cee6e3 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 3 Sep 2017 08:39:55 -0700
Subject: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ
 #22052]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

realloc_check has

  unsigned char *magic_p;
...
  __libc_lock_lock (main_arena.mutex);
  const mchunkptr oldp = mem2chunk_check (oldmem, &magic_p);
  __libc_lock_unlock (main_arena.mutex);
  if (!oldp)
    malloc_printerr ("realloc(): invalid pointer");
...
  if (newmem == NULL)
    *magic_p ^= 0xFF;

with

static void malloc_printerr(const char *str) __attribute__ ((noreturn));

GCC 7 -O3 warns

hooks.c: In function ‘realloc_check’:
hooks.c:352:14: error: ‘magic_p’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     *magic_p ^= 0xFF;

This patch silences GCC 7 by initializing magic_p to NULL.

	[BZ #22052]
	* malloc/hooks.c (realloc_check): Initialize magic_p to NULL.
---
 malloc/hooks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/malloc/hooks.c b/malloc/hooks.c
index 01be076f5e..03bc086901 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -287,7 +287,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
 {
   INTERNAL_SIZE_T nb;
   void *newmem = 0;
-  unsigned char *magic_p;
+  unsigned char *magic_p = NULL;
 
   if (bytes + 1 == 0)
     {
-- 
2.13.5

Re: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[H.J. Lu]

On Sun, Sep 3, 2017 at 8:48 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 10:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/31/2017 05:55 AM, H.J. Lu wrote:
>>
>>> One of malloc change caused:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=22052
>>
>> (uninit warning with -O3)
>>
>> I have not yet found a good way to suppress this. I don't know yet why
>> GCC loses the information that the maybe_p variable is initialized if
>> mem2chunk_check returns a non-null pointer.
>>
>
> This patch works for me.   OK for master?
>

FYI, I opened a GCC bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82090

-- 
H.J.

Re: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[Florian Weimer]

On 09/03/2017 05:48 PM, H.J. Lu wrote:
> +  unsigned char *magic_p = NULL;

I think the current practice is to use <libc-diag.h> to suppress the
warning.

Is the issue that the memory clobber on the inline asm makes GCC not to
see the conditional initialization?

Thanks,
Florian

Re: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[H.J. Lu]

On Mon, Sep 4, 2017 at 1:33 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/03/2017 05:48 PM, H.J. Lu wrote:
>> +  unsigned char *magic_p = NULL;
>
> I think the current practice is to use <libc-diag.h> to suppress the
> warning.

Is there a usage example for this warning?

> Is the issue that the memory clobber on the inline asm makes GCC not to
> see the conditional initialization?

Possible.


-- 
H.J.

Re: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[Florian Weimer]

On 09/04/2017 10:29 PM, H.J. Lu wrote:
> On Mon, Sep 4, 2017 at 1:33 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 09/03/2017 05:48 PM, H.J. Lu wrote:
>>> +  unsigned char *magic_p = NULL;
>>
>> I think the current practice is to use <libc-diag.h> to suppress the
>> warning.
> 
> Is there a usage example for this warning?

Search for DIAG_PUSH_NEEDS_COMMENT in the sources.

Thanks,
Florian

Re: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ #22052]

[H.J. Lu]

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

On Mon, Sep 18, 2017 at 7:02 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/04/2017 10:29 PM, H.J. Lu wrote:
>>
>> On Mon, Sep 4, 2017 at 1:33 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> On 09/03/2017 05:48 PM, H.J. Lu wrote:
>>>>
>>>> +  unsigned char *magic_p = NULL;
>>>
>>>
>>> I think the current practice is to use <libc-diag.h> to suppress the
>>> warning.
>>
>>
>> Is there a usage example for this warning?
>
>
> Search for DIAG_PUSH_NEEDS_COMMENT in the sources.
>

Here is a patch.  Tested on x86-64.  I am going to check it in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Silence-O3-Wall-warning-in-malloc-hooks.c-with-GCC-7.patch --]
[-- Type: text/x-patch, Size: 1981 bytes --]

From aca3e9b42f9c172af368f2fc54d2c7299b231cc0 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 3 Sep 2017 08:39:55 -0700
Subject: [PATCH] Silence -O3 -Wall warning in malloc/hooks.c with GCC 7 [BZ
 #22052]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

realloc_check has

  unsigned char *magic_p;
...
  __libc_lock_lock (main_arena.mutex);
  const mchunkptr oldp = mem2chunk_check (oldmem, &magic_p);
  __libc_lock_unlock (main_arena.mutex);
  if (!oldp)
    malloc_printerr ("realloc(): invalid pointer");
...
  if (newmem == NULL)
    *magic_p ^= 0xFF;

with

static void malloc_printerr(const char *str) __attribute__ ((noreturn));

GCC 7 -O3 warns

hooks.c: In function ‘realloc_check’:
hooks.c:352:14: error: ‘magic_p’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     *magic_p ^= 0xFF;

This patch silences GCC 7 by using DIAG_IGNORE_NEEDS_COMMENT.

	[BZ #22052]
	* malloc/hooks.c (realloc_check): Use DIAG_IGNORE_NEEDS_COMMENT
	to silence -O3 -Wall warning GCC 7.
---
 malloc/hooks.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/malloc/hooks.c b/malloc/hooks.c
index 01be076f5e..17d8907727 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -345,11 +345,18 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
       newmem = _int_realloc (&main_arena, oldp, oldsize, nb);
     }
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about magic_p may be used uninitialized.  But we never
+     reach here if magic_p is uninitialized.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
+#endif
   /* mem2chunk_check changed the magic byte in the old chunk.
      If newmem is NULL, then the old chunk will still be used though,
      so we need to invert that change here.  */
   if (newmem == NULL)
     *magic_p ^= 0xFF;
+  DIAG_POP_NEEDS_COMMENT;
 
   __libc_lock_unlock (main_arena.mutex);
 
-- 
2.13.5