GNU bug report logs -
#55895
[PATCH] maint: Fix ptr_align signature to silence -Wmaybe-uninitialized
Previous Next
Reported by: Anders Kaseorg <andersk <at> mit.edu>
Date: Fri, 10 Jun 2022 22:31:02 UTC
Severity: normal
Tags: patch
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your bug report
#55895: [PATCH] maint: Fix ptr_align signature to silence -Wmaybe-uninitialized
which was filed against the coreutils package, has been closed.
The explanation is attached below, along with your original report.
If you require more details, please reply to 55895 <at> debbugs.gnu.org.
--
55895: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55895
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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.
[Message part 3 (message/rfc822, inline)]
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 <at> 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
This bug report was last modified 2 years and 341 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.