--- Begin Message ---
|
Subject: |
[PATCH] maint: Fix ptr_align signature to silence -Wmaybe-uninitialized |
|
Date: |
Fri, 10 Jun 2022 18:24:15 -0400 (EDT) |
|
User-agent: |
Alpine 2.21.999 (DEB 260 2018-02-26) |
ptr_align is always called with a pointer to uninitialized memory, so
it does not make sense for that pointer to be const. This change
avoids -Wmaybe-uninitialized warnings from GCC 11.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
Some of the warnings from GCC 11.3.0 without this patch:
CC src/cksum-digest.o
src/digest.c: In function 'digest_check':
src/digest.c:1036:31: error: 'bin_buffer_unaligned' may be used uninitialized
[-Werror=maybe-uninitialized]
1036 | unsigned char *bin_buffer = ptr_align (bin_buffer_unaligned,
DIGEST_ALIGN);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/digest.c:24:
src/system.h:493:1: note: by argument 1 of type 'const void *' to 'ptr_align'
declared here
493 | ptr_align (void const *ptr, size_t alignment)
| ^~~~~~~~~
src/digest.c:1034:17: note: 'bin_buffer_unaligned' declared here
1034 | unsigned char bin_buffer_unaligned[DIGEST_BIN_BYTES + DIGEST_ALIGN];
| ^~~~~~~~~~~~~~~~~~~~
src/digest.c: In function 'main':
src/digest.c:1247:31: error: 'bin_buffer_unaligned' may be used uninitialized
[-Werror=maybe-uninitialized]
1247 | unsigned char *bin_buffer = ptr_align (bin_buffer_unaligned,
DIGEST_ALIGN);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/digest.c:24:
src/system.h:493:1: note: by argument 1 of type 'const void *' to 'ptr_align'
declared here
493 | ptr_align (void const *ptr, size_t alignment)
| ^~~~~~~~~
src/digest.c:1245:17: note: 'bin_buffer_unaligned' declared here
1245 | unsigned char bin_buffer_unaligned[DIGEST_BIN_BYTES + DIGEST_ALIGN];
| ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:20574: src/cksum-digest.o] Error 1
src/system.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/system.h b/src/system.h
index 0c5c9b900..120fd15e4 100644
--- a/src/system.h
+++ b/src/system.h
@@ -490,7 +490,7 @@ lcm (size_t u, size_t v)
locations. */
static inline void *
-ptr_align (void const *ptr, size_t alignment)
+ptr_align (void *ptr, size_t alignment)
{
char const *p0 = ptr;
char const *p1 = p0 + alignment - 1;
--
2.36.1
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#55895: [PATCH] maint: Fix ptr_align signature to silence -Wmaybe-uninitialized |
|
Date: |
Sat, 11 Jun 2022 09:12:08 -0700 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 |
On 6/10/22 21:11, Anders Kaseorg wrote:
It seems the important step I should
have included was CFLAGS=-O0.
Ah, OK. Since you're building from Git, I can refer you to
README-hacking which is intended for that. It says, "If you get warnings
with other configurations, you can run
'./configure --disable-gcc-warnings' or 'make WERROR_CFLAGS='
to build quietly or verbosely, respectively.
" Here, "other configurations" refers to what you're doing.
(With GCC 12.1.1 I get the same error and also additional errors that might merit further investigation.)
Like most static analysis tools, GCC generates a bunch of false
positives unless you baby it just right. We do the babying only for the
latest GCC with the default configuration; otherwise, it's typically not
worth the trouble. Feel free to investigate the other warnings, but
they're important only if they're true positives (and most likely
they're not, because gcc -O0 is dumber than gcc -O2).
there’s never a reason to call ptr_align with a const pointer, because if the
memory is initialized the pointer would have already been aligned
First, a const pointer can point to uninitialized storage. Second, even
if the referenced memory is initialized the pointer need not be aligned
already. For example, this is valid:
char *p = malloc (1024);
if (!p) return;
char const *q = p; // q points to uninitialized storage
char const *r = ptr_align (q, 512); // q is not aligned already
memset (p, 127, 1024);
...
Replacing 'malloc (1024)' with 'calloc (1024, 1)' (thus initializing the
storage before aligning the pointer) wouldn't affect the validity of the
code.
Also, the current signature converts a const pointer to a mutable pointer.
Yes, it's like strchr which is annoying but that's the best C can do.
You're right that changing it from void const * to void * won't hurt
coreutils' current callers but I'd rather not massage the code merely to
pacify nondefault configurations. There are too many nondefault
configurations to worry about and massaging the code to pacify them all
would waste our time and confuse the code. Instead, we pacify only
default configurations with current GCC.
--- End Message ---