| Summary: | gcc warns about suspicious constructs for unevaluted ?: operand | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Michael Karcher <gcc-bugzilla> |
| Component: | c | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | NEW --- | ||
| Severity: | enhancement | CC: | bugdal, glaubitz, jakub, segher |
| Priority: | P3 | Keywords: | diagnostic |
| Version: | 10.2.1 | ||
| Target Milestone: | --- | ||
| See Also: |
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69825 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88895 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107116 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=4210 |
||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2023-01-20 00:00:00 | |
| Attachments: | minimal example | ||
I doubt this will be changed anytime soon, see PR 4210 for the history on why. The testcase needs a NULL defined as (void *)0 . Thanks for the pointer to #4210. Note that 4210 is slightly different, though. In that report, the condition and the warnable expression are in different statements, and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=4210#c13 explicitly mentioned using a ternary expression to enable gcc to see the deadness of the code, using a flag called "skip_evaluation". This PR concerns a case that uses ?:, so I wonder whether skip_evaluation still exists, and could be used to gate the sizeof-pointer-div warning. (In reply to Andrew Pinski from comment #1) > I doubt this will be changed anytime soon, see PR 4210 for the history on > why. That PR is about an UB case though. In this case the code is perfectly well defined (just IB). The warning code here sees a sizeof divided by a sizeof and wants to warn for it as being maybe wrong, although it would be pretty easy to see it isn't. If you don't want to see the warning at all when using the macro, workaround is easy, just use (0+sizeof(x)/sizeof(*(x)) and be done with it.
If you want a warning if you use it say on int * type and no warning but 0 result if you use it on cv void * (note, the original
one only works with void * and not with const void * etc.), you could use e.g. following (verified both with gcc and clang),
of course if you don't care about const void * etc., you can simplify it a little bit.
//#define ARRAY_SIZE_MAYBENULL(x) ( __builtin_types_compatible_p(typeof(x), void*) ? 0 : (sizeof(x)/sizeof(*(x))) )
//#define ARRAY_SIZE_MAYBENULL(x) _Generic((x), void*: 0, const void*: 0, volatile void*: 0, const volatile void*: 0, default: (0+sizeof(x))/sizeof(*(x)))
//#define ARRAY_SIZE_MAYBENULL(x) _Generic((x), void*: 0, const void*: 0, volatile void*: 0, const volatile void*: 0, default: sizeof(x)/sizeof(*(x)))
#define ARRAY_SIZE_MAYBENULL(x) _Generic((x), void*: 0, const void*: 0, volatile void*: 0, const volatile void*: 0, default: \
sizeof(x)/sizeof(_Generic((x), void*: (x), const void*: (x), volatile void*: (x), const volatile void*: (x), default:*(x))))
void
foo (int *p)
{
int a[10];
p[0] = ARRAY_SIZE_MAYBENULL (a);
p[1] = ARRAY_SIZE_MAYBENULL ((void *) 0);
p[2] = ARRAY_SIZE_MAYBENULL ((const void *) 0);
p[3] = ARRAY_SIZE_MAYBENULL ((volatile void *) 0);
p[4] = ARRAY_SIZE_MAYBENULL ((void *restrict) 0);
p[5] = ARRAY_SIZE_MAYBENULL ((const volatile void *) 0);
p[6] = ARRAY_SIZE_MAYBENULL (p);
}
Seems kernel has #define NULL ((void *)0), so if it is solely about allowing NULL, you could also #define ARRAY_SIZE_MAYBENULL(x) _Generic((x), void*: (BUG_ON(x), 0), default: \ sizeof(x)/sizeof(_Generic((x), void*: (x), default:*(x)))) or similar, so that uses of the macro on anything but arrays (not pointers) or NULL pointer will be diagnosed with warning or runtime error (the latter case for non-NULL void * typed pointers). If it needs to be used in constant expression and you want warnings for the non-void * cases with pointers, not arrays and errors for use of void * pointers except for NULL, then maybe:
typedef __SIZE_TYPE__ size_t;
typedef __UINTPTR_TYPE__ uintptr_t;
#define NULL ((void *) 0)
#define ARRAY_SIZE_MAYBENULL(x) _Generic((x), void*: (size_t) (uintptr_t) ((char (*) [__builtin_constant_p (x) && (x) == 0 ? 1 : _Generic((x), void*: -1, default: 1)]) 0), default: \
sizeof(x)/sizeof(_Generic((x), void*: (x), default:*(x))))
int a[10];
int *q;
void *r;
int p[] = {
ARRAY_SIZE_MAYBENULL (a),
ARRAY_SIZE_MAYBENULL (NULL),
ARRAY_SIZE_MAYBENULL ((const void *) 0),
ARRAY_SIZE_MAYBENULL ((volatile void *) 0),
ARRAY_SIZE_MAYBENULL ((void *restrict) 0),
ARRAY_SIZE_MAYBENULL ((const volatile void *) 0),
ARRAY_SIZE_MAYBENULL (q),
ARRAY_SIZE_MAYBENULL (r),
ARRAY_SIZE_MAYBENULL ((void *) (uintptr_t) 0x1234abc)
};
This warns -Wsizeof-pointer-div on the const void *, volatile void *, const volatile void * and q cases and errors on the last 2.
clang seems to also emit those 4 -Wsizeof-pointer-div warnings, but doesn't emit any errors nor warnings on the last 2.
|
Created attachment 54318 [details] minimal example A well-known construct to determine array sizes at compile time is #define ARRAY_SIZE(x) (sizeof(x)/sizeof(*(x))) gcc helpfully warns for dangerous mis-use of this macro, as it only works with real arrays, not with pointer, for example. Assuming NULL is defined as ((void*)0), ARRAY_SIZE(NULL) yields a valid C expression, as long as we use the gcc extension that sizeof(void) equals one: ARRAY_SIZE(NULL) is expanded to essentially sizeof(void*)/sizeof(void) which yields 8 on usual 64-bit systems and 4 on usual 32-bit system. While this expression is valid, the result of this expression is likely not what the programmer intended, so the gcc warning "division ‘sizeof (void *) / sizeof (void)’ does not compute the number of array elements" is warranted. The Linux kernel contains a macro essentially being #define ARRAY_SIZE_MAYBENULL(x) ( __builtin_types_compatible_p(typeof(x), void*) ? 0 : (sizeof(x)/sizeof(*x)) ) which is intended to be invocable using actual array operands (returning the array size) or the compile-time constant NULL (returning zero). gcc correctly evaluates ARRAY_SIZE_MAYBENULL(NULL) to zero, but emits about the suspicious pattern in the third operand of the ternary operator. This is not helpful for the programmer, and breaks builds using -Wall -Werror. This is a feature request to omit warnings about dubious constructs like this if it can be statically determined that they are not evaluated. The example in the attachment compiles correctly and initializes x to 1, but emits the spurious warning about the unevaluated sizeof pattern.