[Bug c/116016] enhancement: add __builtin_set_counted_by(P->FAM, COUNT) or equivalent
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016 --- Comment #1 from Kees Cook --- Matching Clang feature request: https://github.com/llvm/llvm-project/issues/99774
[Bug c/116016] New: enhancement: add __builtin_set_counted_by(P->FAM, COUNT) or equivalent
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016
Bug ID: 116016
Summary: enhancement: add __builtin_set_counted_by(P->FAM,
COUNT) or equivalent
Product: gcc
Version: 15.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
With the wonderful addition of the 'counted_by' attribute and its wide roll-out
within the Linux kernel, we have found a use case that would be very nice to
have for object allocators: being able to set the counted_by counter variable
without knowing its name.
For example, given:
struct foo {
...
int counter;
...
struct bar array[] __attribute__((counted_by(counter)));
} *p;
one thought was to have __builtin_set_counted_by(P->FAM, COUNT), which would
have the behavior of:
if has_counted_by_attribute(P->FAM):
P->COUNT_MEMBER = COUNT
else
do nothing
The existing Linux object allocators are roughly:
#define alloc(P, FAM, COUNT) ({ \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
kmalloc(__size, GFP); \
})
Right now, any addition of a counted_by annotation must also include an
open-coded assignment of the counter variable after the allocation:
p = alloc(p, array, how_many);
p->counter = how_many;
Instead, the central allocator could be updated to:
#define alloc(P, FAM, COUNT) ({ \
typeof(P) __p; \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = kmalloc(__size, GFP); \
__builtin_set_counted_by(__p->FAM, COUNT); \
__p; \
})
And now structs can gain the counted_by attribute without needing additional
open-coded counter assignments for each struct, and unannotated structs could
still use the same allocator. (i.e. we are able to more cleanly continue to
migrate FAM structs.)
[Bug tree-optimization/109071] -Warray-bounds false positive warnings due to code duplication from jump threading
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109071 --- Comment #8 from Kees Cook --- The warning is about: val = >vals[index]; poc.c:20:20: warning: array subscript 4 is above array bounds of 'int[4]' [-Warray-bounds=] 20 | val = >vals[index]; |^~~ which happens before the warn(). And if the check is moved out of the "assign()" function, the warning goes away: val = >vals[index]; if (index >= MAX_ENTRIES) warn(); assign(0,ptr, index); assign(*val, ptr, index); Normally -Warray-bounds doesn't warn when a value is totally unknown (i.e. "index" here can be [-INT_MAX,INT_MAX]). Why does the warning change when the MAX_ENTRIES test is moved inside assign()?
[Bug tree-optimization/109071] -Warray-bounds false positive warnings due to code duplication from jump threading
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109071 --- Comment #6 from Kees Cook --- (In reply to qinzhao from comment #5) > adding __attribute__ ((noreturn)) to the routine "warn" can eliminate the > false positive warning. But it does return... it's not an assert.
[Bug c/53548] allow flexible array members in unions like zero-length arrays
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548 --- Comment #8 from Kees Cook --- Clang bug: https://github.com/llvm/llvm-project/issues/84565
[Bug c/53548] allow flexible array members in unions like zero-length arrays
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548 --- Comment #7 from Kees Cook --- There is still no way to use C99 flexible arrays in unions (or alone in structs) without syntactic obfuscation. The extension that already allows 0-sized arrays in unions should be extended to cover C99 arrays. This is especially important for projects migrating away from the various deprecated "fake" flexible array members to C99 flex array members, as they continue to depend on both union membership and single-member structs (i.e. the Linux kernel has lots of these, some even in UAPI). Please reopen this bug. :) Clang is also preparing to fix this issue.
[Bug c/53548] allow flexible array members in unions like zero-length arrays
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548 Kees Cook changed: What|Removed |Added CC||carlos at gcc dot gnu.org, ||kees at outflux dot net, ||ndesaulniers at google dot com, ||qing.zhao at oracle dot com --- Comment #6 from Kees Cook --- There is still no way to use C99 flexible arrays in unions (or alone in structs) without syntactic obfuscation. The extension that already allows 0-sized arrays in unions should be extended to cover C99 arrays. This is especially important for projects migrating away from the various deprecated "fake" flexible array members to C99 flex array members, as they continue to depend on both union membership and single-member structs (i.e. the Linux kernel has lots of these, some even in UAPI).
[Bug c/108896] provide "element_count" attribute to give more context to __builtin_dynamic_object_size() and -fsanitize=bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 --- Comment #42 from Kees Cook --- Exciting! Are you able to attach the latest patch? I'd love to try it out. I've been testing Clang's version as well: https://reviews.llvm.org/D148381
[Bug tree-optimization/109071] -Warray-bounds warning when array index checked via inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109071 --- Comment #3 from Kees Cook --- Is there a viable path to a solution here? This seems to cause enough false positives with -Warray-bounds that at least Linux can't enable the flag. I'd really like to have it enabled, though, since it finds plenty of real (and usually serious) bugs.
[Bug c/109071] New: -Warray-bounds warning when array index checked via inline
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109071
Bug ID: 109071
Summary: -Warray-bounds warning when array index checked via
inline
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
Created attachment 54611
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54611=edit
PoC for -Warray-bounds false positive
The Linux kernel is seeing -Warray-bounds warnings when array indexes are being
checked via inlines. This appears to be in the overly noisy/false positive
territory, but I don't actually know what's going on.
The upstream report is here:
https://lore.kernel.org/lkml/20230306220947.1982272-1-t...@redhat.com/
Originally I thought this was another -fsanitizer=shift issue, but after
reducing the test-case, it seems to be related to inlining or some other aspect
of optimization passes.
If the "assign" function is open-coded in the caller, the warning goes away.
If the index checks are moved before the "assign" calls, the warning goes away.
If there is only 1 call to "assign", the warning goes away.
Fundamentally there should be no warning at all since the value of "index" is
entirely unknown _except_ when it makes the call to "warn".
$ cat test.c
extern void warn(void);
#define MAX_ENTRIES 4
static inline void assign(int val, int *regs, int index)
{
if (index >= MAX_ENTRIES)
warn();
*regs = val;
}
struct nums {
int vals[MAX_ENTRIES];
};
void sparx5_psfp_sg_set(int *ptr, struct nums *sg, int index)
{
int *val;
val = >vals[index];
assign(0,ptr, index);
assign(*val, ptr, index);
}
$ gcc -Wall -O2 -c -o test.o test.c
test.c: In function 'sparx5_psfp_sg_set':
test.c:20:24: warning: array subscript 4 is above array bounds of 'int[4]'
[-Warray-bounds=]
20 | val = >vals[index];
|^~~
test.c:13:13: note: while referencing 'vals'
13 | int vals[MAX_ENTRIES];
| ^~~~
[Bug c/108896] provide "element_count" attribute to give more context to __builtin_dynamic_object_size() and -fsanitize=bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 --- Comment #6 from Kees Cook --- I really want to avoid the changes to sizeof() -- this will confuse a lot of other things. Sizeof is expected to be a constant expression, for example. I think the attribute is best since it avoids colliding with anything else.
[Bug c/108896] provide "element_count" attribute to give more context to __builtin_dynamic_object_size() and -fsanitize=bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 --- Comment #1 from Kees Cook --- The corresponding Clang feature request is here: https://github.com/llvm/llvm-project/issues/60928
[Bug c/108896] New: provide "element_count" attribute to give more context to __builtin_dynamic_object_size() and -fsanitize=bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
Bug ID: 108896
Summary: provide "element_count" attribute to give more context
to __builtin_dynamic_object_size() and
-fsanitize=bounds
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
Frequently a structure containing a flexible array member will also contain a
member where the count of array elements is stored. For example:
struct foo {
...
unsigned int count;
...
int data[];
};
struct foo *allocate_foo(unsigned int how_many)
{
struct foo *p;
p = malloc(sizeof(*p) + how_many * sizeof(*byte_array));
p->count = how_many;
return p;
}
While __builtin_dynamic_object_size(p->data, 1) will know the size within
"allocate_foo" due to malloc's __alloc_size hinting, this information is
immediately lost on return. However, the information _is_ still available in
p->count, but the compiler has no way to know about it.
Please provide a struct member attribute "element_count" that can be used to
associate the size of a flexible array to another struct member. For example:
struct foo {
...
unsigned int count;
...
int data[] __attribute__((__element_count__(count)));
};
Now any later examination of the size of "data" can be calculated. For example,
this equality will hold true:
__builtin_dynamic_object_size(p->data) == p->count * sizeof(*p->data)
and -fsanitize-bounds can examine this as well, to trap:
p->data[index] = ...; /* traps when index < 0, or index >= p->count */
[Bug sanitizer/108894] -fsanitize=bounds missing bounds provided by __builtin_dynamic_object_size()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108894 Kees Cook changed: What|Removed |Added Attachment #54508|0 |1 is obsolete|| --- Comment #2 from Kees Cook --- Created attachment 54509 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54509=edit PoC showing lack of __bdos support in -fsanitize=bounds (Updated PoC so the "unknown" case correctly said "ignored" instead of "ok"/"failure" -- the unknown size case must continue to be ignored.)
[Bug sanitizer/108894] -fsanitize=bounds missing bounds provided by __builtin_dynamic_object_size()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108894 --- Comment #1 from Kees Cook --- The matching Clang bug is: https://github.com/llvm/llvm-project/issues/60926
[Bug sanitizer/108894] New: -fsanitize=bounds missing bounds provided by __builtin_dynamic_object_size()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108894 Bug ID: 108894 Summary: -fsanitize=bounds missing bounds provided by __builtin_dynamic_object_size() Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: sanitizer Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org, jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org Target Milestone: --- Created attachment 54508 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54508=edit PoC showing lack of __bdos support in -fsanitize=bounds While -fsanitize-bounds is able to perform run-time bounds checking on fixed-size arrays (i.e. when __builtin_object_size(x, 1) does not return SIZE_MAX), it does not perform bounds checking when __builtin_dynamic_object_size(x, 1) is available. For example, the attached program produces _no_ bounds-checker warnings: $ gcc -Wall -O2 -fstrict-flex-arrays=3 -fsanitize=bounds -fstrict-flex-arrays=3 -o bounds bounds.c $ ./bounds p->array has a fixed size: 64 (16 elements of size 4) p->array[0] assignment: 255 (should be ok) p->array[16] assignment: 255 (should be failure) p->array has a dynamic size: 64 (16 elements of size 4) p->array[0] assignment: 255 (should be ok) p->array[16] assignment: 255 (should be failure) p->array has unknowable size p->array[0] assignment: 255 (should be ok) p->array[16] assignment: 255 (should be failure) Note that the first failure for a fixed size array implies that -fsanitize=bounds has also not been wired up to -fstrict-flex-arrays=3, so it is ignoring all trailing arrays.
[Bug tree-optimization/108306] false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306 --- Comment #7 from Kees Cook --- (In reply to Kees Cook from comment #6) > Sorry, I forgot to include those details fully! Here's how I'm seeing it: > > $ gcc --version > gcc (GCC) 13.0.0 20230105 (experimental) > ... > $ gcc -O2 -fno-strict-overflow -fsanitize=shift -Warray-bounds -c -o > /dev/null poc.c In function 'psi_group_change', inlined from 'psi_task_switch' at poc.c:25:2: poc.c:15:30: warning: array subscript 32 is above array bounds of 'unsigned int[4]' [-Warray-bounds=] 15 | tasks[t]++; | ~^~~ poc.c: In function 'psi_task_switch': poc.c:6:14: note: while referencing 'tasks' 6 | unsigned int tasks[NR_PSI_TASK_COUNTS]; | ^ (mispaste)
[Bug tree-optimization/108306] false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306
--- Comment #6 from Kees Cook ---
Sorry, I forgot to include those details fully! Here's how I'm seeing it:
$ gcc --version
gcc (GCC) 13.0.0 20230105 (experimental)
...
$ gcc -O2 -fno-strict-overflow -fsanitize=shift -Warray-bounds -c -o /dev/null
poc.c
enum psi_task_count {
NR_IOWAIT,
NR_PSI_TASK_COUNTS = 4,
};
unsigned int tasks[NR_PSI_TASK_COUNTS];
static void psi_group_change(unsigned int set)
{
unsigned int t;
unsigned int state_mask = 0;
for (t = 0; set; set &= ~(1 << t), t++)
if (set & (1 << t))
tasks[t]++;
}
void psi_task_switch(int sleep)
{
int set = 0;
if (sleep)
set |= (1 << NR_IOWAIT);
psi_group_change(set);
}
[Bug c/108306] false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306 Kees Cook changed: What|Removed |Added Attachment #54198|0 |1 is obsolete|| --- Comment #3 from Kees Cook --- Created attachment 54199 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54199=edit better PoC This is a better PoC showing the issue.
[Bug c/108306] false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306 --- Comment #2 from Kees Cook --- Ugh, sorry. The PoC is bad -- the bounds check isn't present. Let me try to get a another PoC.
[Bug c/108306] false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306 Kees Cook changed: What|Removed |Added CC||arnd at linaro dot org, ||garsilva at embeddedor dot com, ||pbrobinson at gmail dot com, ||pinskia at gcc dot gnu.org, ||qing.zhao at oracle dot com, ||rguenth at gcc dot gnu.org --- Comment #1 from Kees Cook --- FWIW, the Linux kernel is seeing these under GCC 13 (and 12, but we had to disable -Warray-bounds entirely in GCC 12 due to similar bugs).
[Bug c/108306] New: false-positive -Warray-bounds warning emitted with -fsanitize=shift
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306 Bug ID: 108306 Summary: false-positive -Warray-bounds warning emitted with -fsanitize=shift Product: gcc Version: 13.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- Created attachment 54198 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54198=edit reduced PoC This seems similar to bug #105679, but is still present in GCC 13 (and 12). Something about -fsanitize=shift really confuses -Warray-bounds: poc2.c: In function 'e': poc2.c:12:8: warning: array subscript 32 is above array bounds of 'int[4]' [-Warray-bounds=] 12 | c.d[f]++; | ~~~^~~ poc2.c:4:7: note: while referencing 'd' 4 | int d[MAX]; | ^ poc2.c:12:8: warning: array subscript 32 is above array bounds of 'int[4]' [-Warray-bounds=] 12 | c.d[f]++; | ~~~^~~ poc2.c:4:7: note: while referencing 'd' 4 | int d[MAX]; | ^
[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 --- Comment #11 from Kees Cook --- (In reply to Richard Biener from comment #10) > I sofar refrained from doing this because of the large amount of fallout and > followup changes and I think those are not warranted on the GCC 12 branch. Totally understandable! Thanks for considering it; I didn't know if it was "easy" or not, so I thought I'd ask. :)
[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Kees Cook changed: What|Removed |Added CC||qing.zhao at oracle dot com --- Comment #9 from Kees Cook --- Does anyone have some time to do this backport for GCC 12?
[Bug c/107162] New: -Wmisleading-indentation is blinded by comments
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107162
Bug ID: 107162
Summary: -Wmisleading-indentation is blinded by comments
Product: gcc
Version: 12.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
Hi,
Similar to this bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106528
-Wmissing-indentation is blinded by comments:
int square(int num) {
if (num == 1)
goto fail;
//if (num == 0)
goto fail;
if (num > 20)
return num * num;
else
return num + num;
fail:
return -1;
}
The above case does not warn in GCC, but does in Clang:
https://godbolt.org/z/bMG5jM9Ga
[Bug ipa/96503] attribute alloc_size effect lost after inlining
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #1 from Kees Cook --- Created attachment 53643 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53643=edit PoC showing unexpected __bdos results across inlines Fixing this is needed for the Linux kernel to do much useful with alloc_size. Most of the allocators are inline wrappers, for example. This can be additionally shown to break __builtin_dynamic_object_size(), which means all FORTIFY_SOURCE of alloc_size-marked inlines is broken. :( https://godbolt.org/z/jTKjY3s1j
[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 --- Comment #6 from Kees Cook --- (In reply to Richard Biener from comment #5) > Should be fixed on trunk. Can you check on the original unreduced testcase? Thanks! I've done test builds and can confirm these two false positives have been eliminated: arch/x86/kvm/emulate.c:251:27: warning: array subscript 32 is above array bounds of 'long unsigned int[17]' [-Warray-bounds] arch/x86/kvm/ioapic.c:213:33: warning: array subscript 32 is above array bounds of 'union kvm_ioapic_redirect_entry[24]' [-Warray-bounds] Is this backportable to gcc 12?
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836
--- Comment #41 from Kees Cook ---
(In reply to Bill Wendling from comment #40)
> The question then is if `-fstrict-flex-arrays=3' is used, what does a `[0]'
> at the end of a struct represent (assuming GCC no longer treats it as an
> FAM)?
It's a zero-sized object. The same as `struct { } foo;`
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #34 from Kees Cook --- -fstrict-flex-arrays=3 is still needed. (E.g. for proper FORTIFY coverage, etc.) I don't have an opinion about the -W options, though.(In reply to James Y Knight from comment #33) > (In reply to qinzhao from comment #32) > > there is a Bugzilla that has been filed for GCC to request the same warning > > for GCC: > > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=94428 > > > > -Wzero-length-array > > Great. Adding that flag, and eliminating the -fstrict-flex-arrays=3 option > from this proposal would be good. Hmm? No, -fstrict-flex-arrays=3 is still needed (because it changes compiler _behavior_, e.g. for proper FORTIFY coverage or trailing arrays, etc). I don't have a strong opinion about the -W options; but they can't warn if they just see a struct declaration with a 0 or 1 element array: userspace will have those for years to come. Maybe it would warn if such a struct member is ever actually used in the code? That kind of behavior would be useful for the Linux kernel at least.
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #21 from Kees Cook --- (In reply to Martin Sebor from comment #20) > Well, I just "asked" for such an option the same way you asked for > -fstrict-flex-arrays in comment #3, because I believe it would be useful to > make the BOS improvements you're looking for available even to code that > can't do a whole-hog replacement of all trailing arrays with flexible array Right, sorry, I meant, "I have a project waiting to use this feature right now", where as other projects might, upon discovering this feature, decide they also only need "-fstrict-flex-arrays". e.g. what option would GCC itself use? > members. The spelling of the option names doesn't seem important to me > (they could be separate options, or the same one with an argument). How about "-fnot-flex-arrays=N" to mean "trailing arrays with N or more elements will NOT be treated like a flex array"? Then code with sockaddr can use "-fnot-flex-arrays=15", code with "[1]" arrays can use "-fnot-flex-arrays=2", code with only "[0]" arrays can use "-fnot-flex-arrays=1", and "-fstrict-flex-arrays" can be an alias for "-fnot-flex-arrays=0", which Linux would use.
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #19 from Kees Cook --- (In reply to Martin Sebor from comment #18) > The zero size case exists (and is documented) solely as a substitute for > flexible array members. Treating is as an ordinary array would disable that > extension. It might be appropriate to provide a separate option to control > it but conflating it with the other cases (one or more elements) doesn't > seem like the robust design. > > As I mentioned in the review of the Clang change, > https://reviews.llvm.org/D126864, so that code bases that use some larger > number of elements than zero, such as one, and that can't easily change, can > still benefit from the BOS enhancement for the remaining cases, it would be > helpful for the new option to accept the minimum number of elements at which > a trailing array ceases to be considered a poor-man's flexible array member. I see your point about gaining the "trailing array" fix without breaking the older code bases, but that doesn't seem to fit the name (nor purpose) of -fstrict-flex-arrays, which should be considered a "complete" fix. To me it looks like -fstrict-flex-arrays should kill the [0] extension, the ancient [1] misuse, and the "anything trailing is flex" logic. If fixing _only_ the latter is desired, perhaps add an option for that, but no one is actually asking for it yet. :) The Linux kernel wants the "fully correct" mode.
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #17 from Kees Cook --- (In reply to qinzhao from comment #16) > additional work are needed in order to make this task complete: > > 1. add one more new gcc option: > > -fstrict-flex-arrays > > when it's on, only treat the following cases as flexing array: > > trailing array with size 0; > trailing array with size 1; > trailing flexible array; > > all other trailing arrays with size > 1 will be treated as normal arrays. Under -fstrict-flex-arrays, arrays of size 0 and 1 should *not* be treated as flex arrays. Only "[]" should be a flexible array. Everything else should be treated as having the literal size given.
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #13 from Kees Cook --- Maybe the enum needs to also be expanded so that [0] can be distinguished from []?
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #11 from Kees Cook --- and with a flex array to compare: https://godbolt.org/z/s9nb4Y7q4
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #10 from Kees Cook --- Here's a slightly reworked example: https://godbolt.org/z/EvehMax84
[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #9 from Kees Cook --- Just to clarify, __builtin_dynamic_object_size() shouldn't have anything to do with this. What's needed is something like -fstrict-flex-arrays so that all the "trailing array is a flex array" assumptions can be killed everywhere in GCC. Only an _actual_ flex array should be treated as such.
[Bug c/105679] erroneous -Warray-bounds warning with sanitizer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 --- Comment #1 from Kees Cook --- The Linux kernel has encountered at least two of these (seen as specifically "array subscript 32", though the root cause may be causing many others: ../drivers/net/wireless/ath/ath9k/mac.c:373:22: warning: array subscript 32 is above array bounds of 'struct ath9k_tx_queue_info[10]' [-Warray-bounds] ../arch/x86/kvm/ioapic.c:213:33: warning: array subscript 32 is above array bounds of 'union kvm_ioapic_redirect_entry[24]' [-Warray-bounds]
[Bug c/105679] New: erroneous -Warray-bounds warning with sanitizer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Bug ID: 105679 Summary: erroneous -Warray-bounds warning with sanitizer Product: gcc Version: 12.1.1 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- Created attachment 53010 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53010=edit test case minimized as much as possible Some combination of things (likely triggered by -fsanitize=shift and an array index being used with "<<" operator) is causing erroneous warnings about array bounds accesses: $ gcc -O2 -Warray-bounds -fno-strict-aliasing -fno-strict-overflow -fsanitize=shift -fsanitize-coverage=trace-pc -c -o /dev/null test.c test.c: In function 'work.isra': test.c:31:32: warning: array subscript 32 is above array bounds of 'struct object[2]' [-Warray-bounds] 31 | entry = instance->array[irq]; | ~~~^ test.c:19:23: note: while referencing 'array' 19 | struct object array[ELEMENTS]; | ^
[Bug middle-end/105539] -ftrivial-auto-var-init=zero happening too late?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105539 --- Comment #7 from Kees Cook --- Right, perhaps I should rename this bug? The much more surprising thing is the lack of warning about the uninit use. With or without -ftrivial-auto-var-init, I'd want to have the diagnostic that a UB may have happened.
[Bug c/105539] -ftrivial-auto-var-init=zero happening too late?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105539 --- Comment #2 from Kees Cook --- https://godbolt.org/z/99Pdro9Te
[Bug c/105539] -ftrivial-auto-var-init=zero happening too late?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105539 --- Comment #1 from Kees Cook --- (Also this doesn't warn about y being used uninitialized.)
[Bug c/105539] New: -ftrivial-auto-var-init=zero happening too late?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105539
Bug ID: 105539
Summary: -ftrivial-auto-var-init=zero happening too late?
Product: gcc
Version: 12.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
It looks like some pass is being run before the initializers are added:
int x (int z) {
int y;
if (z)
y = 10;
return y;
}
under "gcc -ftrivial-auto-var-init=zero -Wuninitialized -O2" does not test "z":
$ gcc -ftrivial-auto-var-init=zero -Wuninitialized -O2 -S x.c -o -
...
movl$10, %eax
ret
...
This should be testing "z", e.g.:
testl %edi, %edi
movl$10, %eax
cmovel %edi, %eax
retq
[Bug middle-end/99578] [11/12 Regression] gcc-11 -Warray-bounds or -Wstringop-overread warning when accessing a pointer from integer literal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #30 from Kees Cook --- The Linux kernel is seeing more and more of these warnings and it's becoming a bit of a burden. For example: https://lore.kernel.org/linux-hardening/20220227195918.705219-1-keesc...@chromium.org Given that this is a regression in behavior, and the kernel is not alone in tripping over this (9 duplicate bugs reported here already), can this be given greater priority? Would it be possible to add an option to just disable the "constant is a NULL pointer" logic directly?
[Bug middle-end/102276] -ftrivial-auto-var-init fails to initialize a variable, causes a spurious warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102276 --- Comment #4 from Kees Cook --- The kernel keeps gaining more of these cases, so it'll be important to get this fixed: https://lore.kernel.org/lkml/200fe5cb203ad5cc00c5c60b7ded2cd85c9b85ea.ca...@perches.com/
[Bug middle-end/104504] spurious -Wswitch-unreachable warning with -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104504 --- Comment #4 from Kees Cook --- (Ah, I knew this had been reported before. I found it now...) Duplicate of: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102276
[Bug middle-end/104504] spurious -Wswitch-unreachable warning with -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104504
--- Comment #2 from Kees Cook ---
As mentioned in a Linux kernel thread, isn't it possible to transform this:
switch (x) {
int y;
default:
y = x * 2;
return y;
}
into this:
{
int y;
switch (x) {
default:
y = x * 2;
return y;
}
}
[Bug middle-end/104504] spurious -Wswitch-unreachable warning with -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104504 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #1 from Kees Cook --- Similar issue exists in Clang, though Clang doesn't warn: https://github.com/llvm/llvm-project/issues/44261
[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77608
Kees Cook changed:
What|Removed |Added
CC||kees at outflux dot net
--- Comment #5 from Kees Cook ---
I think this behavior may, unfortunately, be "as expected", due to how the
memcpy overflow checks are working (they're checking surrounding object, yes,
like bos(0) would)? The constant-expression bos() calculations do appear to
understand the base pointer object, but when faced with "i", it can't know for
sure -- it might have room (if "i" is < 3), or not. So it must return -1 as it
lacks any other context (like memcpy's "size" argument).
There may, however, be a missing opportunity for tightening the memcpy checker?
For example:
...
volatile unsigned i;
struct weird {
char a[4];
char b[8];
};
int main (void)
{
{
struct weird instance;
char d [3];
P (d + i);
memcpy (d + i, "abcdef", 5); // always overflows d (the entire object)
i = 7;
P (instance.a + i); // can't see into "i"
P (instance.a + 7); // room left in instance (5), but not "a" (0)
memcpy (instance.a + i, "abcdef", 5); // misses a, doesn't overflow
instance. should this warn?
__builtin_printf ("%.0s", d);
}
}
-1 -1 0 0
-1 -1 0 0
5 0 5 5
[Bug c/94428] Reintroduce -Wzero-length-array
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94428 --- Comment #2 from Kees Cook --- Note that this needs a struct attribute that will allow structs to be excluded from the diagnostic (since the kernel needs to deal with legacy UAPI headers forever).
[Bug sanitizer/102317] signed integer overflow sanitizer cannot work well with -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317 --- Comment #11 from Kees Cook --- The trouble with "optimize" is that it just doesn't work. The kernel has banned its use because it results in all other optimization options being forgotten for the function in question.
[Bug target/102352] New: Add -mstack-protector-guard=... for arm32
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102352 Bug ID: 102352 Summary: Add -mstack-protector-guard=... for arm32 Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: target Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- As done for powerpc (1b3254e4bbe82245421a55324bc8fe34a99c6e3c), aarch64 (cd0b2d361df82c848dc7e1c3078651bb0624c3c6), riscv (c931e8d5a96463427040b0d11f9c4352ac22b2b0), and x86 (e1769bdd4cef522ada32aec863feba41116b183a), the arm32 target needs mstack-protector-guard= support as well, so that the kernel can gain per-thread stack canaries on arm32: https://github.com/KSPP/linux/issues/29
[Bug sanitizer/102317] signed integer overflow sanitizer cannot work well with -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317
--- Comment #9 from Kees Cook ---
(In reply to Jakub Jelinek from comment #8)
> So, instead (when building the kernel with sanitization) build with
> -fsanitize=signed-integer-overflow and no -fno-strict-overflow, and
> the routines where you want wrapv behavior and not runtime traps build with
> optimize ("wrapv", "wrapv-pointer") attribute?
__attribute__((optimize)) is documented as not for production use ("for
debugging purposes only"):
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-optimize-function-attribute
and the kernel has hit multiple problems with it. As such, all use has been
removed, for example:
https://lore.kernel.org/lkml/20201027205723.12514-1-a...@kernel.org/
https://lore.kernel.org/lkml/20201028080433.26799-1-a...@kernel.org/
https://lore.kernel.org/lkml/20210118105557.186614-3-adrian.ra...@collabora.com/
If there were an __attribute__((wrapv)) and __attribute__((wrapv-pointer)), we
could create the wrapping helpers with those and
__attribute__((no_sanitize("signed-integer-overflow")))
FWIW, I've been trying to track this issue in the kernel here:
https://github.com/KSPP/linux/issues/26
[Bug middle-end/102276] -ftrivial-auto-var-init fails to initialize a variable, causes a spurious warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102276 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #3 from Kees Cook --- Clang shares this problem, though it also lacks a warning, making it a silent missing initialization: https://bugs.llvm.org/show_bug.cgi?id=44916 FWIW, the Linux kernel has already purged all these cases, in preparation of using -ftrivial-auto-var-init: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep=Distribute+switch+variables+for+initialization
[Bug sanitizer/102317] signed integer overflow sanitizer cannot work well with -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317
--- Comment #7 from Kees Cook ---
The problem the kernel needs to solve is basically having our cake and eating
it too. :)
In _most_ situations, we want signed overflows to trap (i.e. get caught by
"-fsanitize=signed-integer-overflow").
In some limited situations, we want to be able to depend on 2s-complement
wrap-around on overflow -- and we would create a helper function to do this
work, marked with __attribute__((no_sanitize("signed-integer-overflow"))). (We
need this most for our atomic_t/refcount_t code, as well as all the open-coded
bounds checks -- see details below.)
For the latter case (wrap-around), the kernel must depend on the effects of
"-fwrapv-pointer" and "-fwrapv" because otherwise some cases end up being
elided due to being classified as undefined behavior.
(In reply to Jakub Jelinek from comment #6)
> That doesn't make sense. -fsanitize=signed-integer-overflow also removes
> that undefined behavior by defining what happens on signed integer overflow,
> one can choose whether to get a non-fatal runtime diagnostic + wrapv
> behavior, or fatal runtime diagnostic, or just abort.
There doesn't appear to be a way to remove the UB _and_ the diagnostic on a
case-by-case basis.
The matrix of behaviors:
code generation: UB, no UB (2s-complement wrap-around)
diagnostics: no diagnostic, warn or trap
-fno-strict-overflow provides "no UB", and "no diagnostic". ("warn or trap" are
not available.)
-fsanitize=signed-integer-overflow provides "no UB" and "warn or trap". ("no
diagnostic" is not available; using
__attribute__((no_sanitize("signed-integer-overflow"))) also removes "no UB")
So, on a per-function basis, we need to _either_ catch overflow _or_
deterministically perform 2s-complement wrap-around.
If there's a way to achieve this already, I would be very grateful, as I've not
been able to find the correct combination.
Background on the kernel's use of -fno-strict-overflow:
https://git.kernel.org/linus/a137802ee839ace40079bebde24cfb416f73208a
and (earlier) -fwrapv:
https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594
An example of wanting no-UB without diagnostic:
https://git.kernel.org/linus/adb03115f4590baa280ddc440a8eff08a6be0cb7
which ultimately had to be reverted:
https://git.kernel.org/linus/a6211caa634da39d861a47437ffcda8b38ef421b
and all of UBSAN integer overflow support was removed:
https://git.kernel.org/linus/6aaa31aeb9cf260e1b7155cc11ec864f052db5ec
[Bug middle-end/101891] New: Adjust -fzero-call-used-regs to always use XOR
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101891 Bug ID: 101891 Summary: Adjust -fzero-call-used-regs to always use XOR Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: middle-end Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- Currently -fzero-call-used-regs will use a pattern of: XOR regA,regA MOV regA,regB MOV regA,regC ... RET However, this introduces both a register ordering dependency (e.g. the CPU cannot clear regB without clearing regA first), and while greatly reduces available ROP gadgets, it does technically leave a set of "MOV" ROP gadgets at the end of functions (e.g. "MOV regA,regC; RET"). Please switch to always using XOR: XOR regA,regA XOR regB,regB XOR regC,regC ... RET
[Bug c/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 --- Comment #3 from Kees Cook --- Eww. That means _FORTIFY_SOURCE doesn't work correctly. Can there please be a -fstrict-flex-arrays or something to turn off all the heuristics so a code base can declare it only uses flex arrays for dynamic trailing objects?
[Bug c/101836] New: __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836
Bug ID: 101836
Summary: __builtin_object_size(P->M, 1) where M is an array and
the last member of a struct fails
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
Created attachment 51282
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51282=edit
struct layout changes bos1 behavior
When bos1 is used on a member who is both an array and at the end of a
structure, it fails to correctly resolve. This kind of behavior should only
happen for flexible array members:
struct trailing_array {
int a;
int b;
unsigned char c[16];
};
struct middle_array {
int a;
unsigned char c[16];
int b;
};
ok: sizeof(*working) == 24
ok: sizeof(working->c) == 16
ok: __builtin_object_size(working, 1) == -1
ok: __builtin_object_size(working->c, 1) == 16
ok: sizeof(*broken) == 24
ok: sizeof(broken->c) == 16
ok: __builtin_object_size(broken, 1) == -1
WAT: __builtin_object_size(broken->c, 1) == -1 (expected 16)
[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 --- Comment #5 from Kees Cook --- Perhaps the best question to ask is "given an arbitrary argument, how can code detect the remaining bytes of a member, including if the member contains a flexible array?" Because right now, this does not work: #define __bytes_until_end_of_member(p) __builtin_object_size(p, 1) since this gives different answers, depending on the level of dereference: __bytes_until_end_of_member(wrap) == -1 __bytes_until_end_of_member(>msg) == 16 __bytes_until_end_of_member(>msg.nlmsg_content) == -1 How can "wrap->msg" be 16 if "wrap" and "warp->msg.nlmsg_content" are -1?
[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 --- Comment #4 from Kees Cook --- It seems like this isn't about crossing field boundaries -- it's asking "how large is this particular member?" and bos can't know the answer because there is a flex-array. Why would __builtin_object_size(wrap->msg.nlmsg_content, 1); and __builtin_object_size(>msg, 1); differ? Or, if bos lacked "introspecition depth" to find the flex-array, why would __builtin_object_size(msg->nlmsg_content, 1); and __builtin_object_size(msg, 1); be the same? It seems like the latter pair (same results) is correct, and the former pair (differing result) is wrong.
[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 --- Comment #2 from Kees Cook --- Created attachment 51280 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51280=edit Same PoC, but with malloc to provide non-unlimited bounds
[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 --- Comment #1 from Kees Cook --- This is even more visible when the size IS known (via malloc hinting, for example): https://godbolt.org/z/4v5rKbhaf
[Bug c/101832] New: __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
Bug ID: 101832
Summary: __builtin_object_size(P->M, 1) where M ends with a
flex-array behaves like sizeof()
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: kees at outflux dot net
Target Milestone: ---
Created attachment 51279
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51279=edit
bos1 fails to recognize flex array under specific conditions
It is unclear to me if this is a duplicate of bug 64715.
bos1 in at least one situation fails to notice when a member contains a
flex-array, and returns sizeof() instead of -1. For example:
struct nlmsg {
__u32 nlmsg_len;
__u16 nlmsg_type;
__u16 nlmsg_flags;
__u32 nlmsg_seq;
__u32 nlmsg_pid;
__u8nlmsg_content[];
};
struct wrapper {
__u8 a;
__u8 b;
struct nlmsg msg;
};
ok: sizeof(wrap->msg) == 16
ok: __builtin_object_size(wrap->msg.nlmsg_content, 1) == -1
ok: __builtin_object_size(>msg, 0) == -1
WAT: __builtin_object_size(>msg, 1) == 16 (expected -1)
https://godbolt.org/z/95n4ofT53
[Bug c/101419] New: collapsing memset() calls can break __builtin_object_size()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101419 Bug ID: 101419 Summary: collapsing memset() calls can break __builtin_object_size() Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- Created attachment 51131 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51131=edit memset collapsing breaks __builtin_object_size() I've found a strange misoptimization around memset() and __builtin_object_size(). If there are two memset() calls that can be collapsed (due to being fully overlapping, I assume), use of __builtin_object_size() may return the wrong result. The example code shows that __builtin_object_size(_value, 1) returns 1 instead of 4: > $ gcc -Wall -Wextra -fno-strict-aliasing -fwrapv -O2 -c -o wat.o wat.c > In function ‘do_wipe’, > inlined from ‘loops’ at wat.c:24:3: > wat.c:15:3: warning: call to ‘__detected_overflow’ declared with attribute > warning: detected overflow [-Wattribute-warning] >15 | __detected_overflow(__builtin_object_size(>lg, 1), > sizeof(info->lg)); > | > ^~ Here, info->lg is int, but the call to __builtin_object_size() resolves to the size of info->sm (char). This can be seen directly in the resulting output: > $ objdump -rd wat.o > ... > : >0: 53 push %rbx >1: be 04 00 00 00 mov$0x4,%esi >6: 48 89 fbmov%rdi,%rbx >9: c6 07 00movb $0x0,(%rdi) >c: bf 01 00 00 00 mov$0x1,%edi > 11: e8 00 00 00 00 call 16 > 12: R_X86_64_PLT32 __detected_overflow-0x4 > 16: c7 03 00 00 00 00 movl $0x0,(%rbx) > 1c: 5b pop%rbx > 1d: c3 ret The first argument to __detected_overflow() is "1", instead of 4. Any changes to this example code makes the bug disappear (removal of loops, removal of empty asm, or reordering of memset() calls). Using Compiler Explorer, this bug appears to have been introduced between GCC 8.5 and 9.1: https://godbolt.org/z/oGq5K9fE4
[Bug sanitizer/96829] New: implement -fsanitize=unsigned-integer-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96829 Bug ID: 96829 Summary: implement -fsanitize=unsigned-integer-overflow Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: sanitizer Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org, jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org Target Milestone: --- While not technically undefined behavior, having -fsanitize=unsigned-integer-overflow would be nice to find similarly exploitable/unexpected cases of arithmetic overflow of unsigned types (as can be done with -fsanitize=signed-integer-overflow on signed types). (This would also match the same option available in Clang.)
[Bug c/94428] New: Reintroduce -Wzero-length-array
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94428 Bug ID: 94428 Summary: Reintroduce -Wzero-length-array Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- It would be nice to gain "-Wzero-length-array" so we can enforce this standard in the Linux kernel once all conversions have moved struct to flexible array members. Clang supports this as a distinct warning, but gcc currently only warns about this under "-pedantic -std=c99", which is not a workable combination for Linux. ;) $ clang -Wzero-length-array -o bounds-clang bounds.c bounds.c:20:15: warning: zero size arrays are an extension [-Wzero-length-array] char data[0]; ^ $ gcc -Wzero-length-array -o bounds-gcc bounds.c gcc: error: unrecognized command line option ‘-Wzero-length-array’ $ gcc -pedantic -std=c99 -o bounds-gcc bounds.c bounds.c:20:10: warning: ISO C forbids zero-size array ‘data’ [-Wpedantic] 20 | char data[0]; | ^~~~
[Bug sanitizer/92589] heuristic to avoid flexible array members too liberal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92589 --- Comment #8 from Kees Cook --- Created attachment 48153 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48153=edit updated PoC
[Bug sanitizer/92589] heuristic to avoid flexible array members too liberal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92589 --- Comment #7 from Kees Cook --- (In reply to Kees Cook from comment #6) > (In reply to Jakub Jelinek from comment #4) > > (In reply to Kees Cook from comment #2) > > > Is there anything to enforce a strict "only consider empty array size as > > > flexible array member" mode? This is an unfortunate weakening of the array > > > bounds checker as there are plenty of structures that have a fixed-size > > > array as the final member. > > > > There is -fsanitize=bounds-strict. > > This is too strict: it doesn't allow flexible arrays ([]) either. I'd like > something that ignores _only_ flexible arrays and fails on all other > trailing arrays beyond their size. Oops, sorry, my PoC was testing the corner cases, not the correct cases. -fsanitize=bounds-strict _does_ work (though I'd rather it disallowed [0], but I'll live): $ gcc -Wall -g3 -fsanitize=bounds-strict -fsanitize-undefined-trap-on-error -o bounds-gcc bounds.c $ ./bounds-gcc abc flex (should always be okay): ok (no trap!) zero (should be okay, treated as flex): ok (no trap!) one (should fail): Illegal instruction (core dumped)
[Bug sanitizer/92589] heuristic to avoid flexible array members too liberal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92589 --- Comment #6 from Kees Cook --- (In reply to Jakub Jelinek from comment #4) > (In reply to Kees Cook from comment #2) > > Is there anything to enforce a strict "only consider empty array size as > > flexible array member" mode? This is an unfortunate weakening of the array > > bounds checker as there are plenty of structures that have a fixed-size > > array as the final member. > > There is -fsanitize=bounds-strict. This is too strict: it doesn't allow flexible arrays ([]) either. I'd like something that ignores _only_ flexible arrays and fails on all other trailing arrays beyond their size.
[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307 --- Comment #5 from Kees Cook --- Hi! I recently learned that Clang has -fsanitizer-minimal-runtime that is very close to what I was expecting to use: https://bugs.llvm.org/show_bug.cgi?id=45295 That is close to what you're already suggesting. Would it be possible to do the same thing? That way the kernel can have just one "not the full debug details" handler. Thanks for looking at this!
[Bug sanitizer/94307] New: Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307 Bug ID: 94307 Summary: Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: sanitizer Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org, jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org Target Milestone: --- Instead of unconditionally calling __builtin_trap() for -fsanitize-undefined-trap-on-error it would help the Linux kernel's use of UBSAN to have a way to specify the trap function. With that, Linux can use its own internal exception handling routines and avoid various confused states: https://lore.kernel.org/linux-next/20200324164433.qusyu5h7ykx3f2bu@treble/ For example something like -fsanitize-undefined-trap-function=__ubsan_trap and "__ubsan_trap" can then be defined by the kernel itself. Using the standard handler routines (__ubsan_handle_*) are too heavy duty for some builds, so a regular trap is needed for the kernel, but this allows us to provide a "continue anyway" option as well.
[Bug sanitizer/92589] heuristic to avoid flexible array members too liberal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92589 --- Comment #2 from Kees Cook --- Is there anything to enforce a strict "only consider empty array size as flexible array member" mode? This is an unfortunate weakening of the array bounds checker as there are plenty of structures that have a fixed-size array as the final member.
[Bug sanitizer/92589] New: heuristic to avoid flexible array members too liberal
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92589 Bug ID: 92589 Summary: heuristic to avoid flexible array members too liberal Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: sanitizer Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org, jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org Target Milestone: --- Created attachment 47305 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47305=edit PoC for -fsanitize=bounds When choosing which arrays to instrument, -fsanitize=bounds tries to avoid flexible array members. Traditionally, this has been either [], [0], or [1]-sized arrays (though the latter two are realistically considered deprecated in C). However, the sanitizer appears to be ignoring _all_ trailing arrays in a structure, no matter what their size. Comparing the behavior between GCC and Clang, this is more visible: $ gcc -Wall -g3 -fsanitize=bounds -fsanitize-undefined-trap-on-error -o bounds-gcc bounds.c $ ./bounds-gcc abc flex non_flex non_trailing Illegal instruction (core dumped) $ clang -Wall -g3 -fsanitize=bounds -fsanitize-undefined-trap-on-error -o bounds-clang bounds.c $ ./bounds-clang abc flex non_flex Illegal instruction (core dumped) I would expect the trap during the non_flex structure over-index, as seen in the Clang-built binary.
[Bug middle-end/90673] A problem with 'copy destination size is too small' error in copy_from_user
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90673 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #5 from Kees Cook --- From the linked code: missing = copy_from_user(dbg_buff, buf, sizeof(buf)); dbg_buff is a global variable -- is writing to it thread safe? sizeof(buf) is 8. (it's a pointer not an array), so that seems the wrong size? I bet the error message for __bad_copy_to is busted and it really means __bad_copy_from.
[Bug c/85310] optimization ignoring strlen() results
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85310 --- Comment #4 from Kees Cook --- But it's optimizing away the check. If strlen() were suddenly acting like strnlen(), that'd be one thing, but the return value from strlen() is being used by the memcpy() without the actual test in between. That's not sensible.
[Bug c/85310] optimization ignoring strlen() results
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85310 Kees Cook changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED Resolution|INVALID |--- --- Comment #2 from Kees Cook --- I realize the compiler would like like it to be undefined, but it's just not: this worked before, and there's nothing about a static-sized character array that says it will be NULL terminated. No optimization of strlen() can make that assumption, since it's the _contents_ and not the _size_ that determine the results of strlen(). Specifically, this breaks the Linux kernel's FORTIFY_SOURCE implementation, which is exactly trying to protect against these kinds of unexpected memory contents, etc. With the ? : optimized away FORITFY_SOURCE ends up becoming actively dangerous.
[Bug c/85310] New: optimization ignoring strlen() results
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85310 Bug ID: 85310 Summary: optimization ignoring strlen() results Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- Created attachment 43889 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43889=edit test case Something is gcc 8 is broken when dealing with strlen() results. The correct behavior is seen in earlier versions of the compiler: $ gcc --version gcc (Ubuntu 7.3.0-14ubuntu1) 7.3.0 ... $ gcc -Wall -O2 -o testcase testcase.c $ ./testcase MY_SIZE: 16 dst: 15 done... The bad version happens in the current gcc trunk: $ /home/kees/bin/gcc-snapshot/gcc --version gcc (Ubuntu 20180322-1ubuntu1) 8.0.1 20180322 (experimental) [trunk revision 258755] ... $ /home/kees/bin/gcc-snapshot/gcc -Wall -O2 -o testcase testcase.c $ ./testcase MY_SIZE: 16 dst: 32 done... Segmentation fault (core dumped) Checking with godbolt confirms there is no test against "16" in the output: https://godbolt.org/g/63Sbir
[Bug target/82303] Better PIE/PIC code generation for kernel code (x86_64 & arm64)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82303 Kees Cook changed: What|Removed |Added CC||kees at outflux dot net --- Comment #3 from Kees Cook --- Any progress on getting this into a GCC release?
[Bug target/82411] const is not always read-only
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82411 --- Comment #3 from Kees Cook --- To clarify, using -mno-sdata means all things are removed from sdata, not just const, yes? I'd like to be able to leave writable stuff there, to avoid any additional performance penalty.
[Bug c/82411] New: const is not always read-only
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82411 Bug ID: 82411 Summary: const is not always read-only Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: kees at outflux dot net Target Milestone: --- On powerpc, a const variable may end up in the .sdata section, which is writable. This means authors cannot depend on the "const" marking to mean "read-only", as is required for sane Linux kernel memory protection security. Thread here: https://lkml.org/lkml/2017/10/2/488 At the very least, there should be a way to request never putting a const variable into a writable section.
[Bug testsuite/39536] add more VERIFY() calls to certain functions in libstdc++-v3 testsuite
--- Comment #2 from kees at outflux dot net 2010-06-27 17:55 --- http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02690.html -- kees at outflux dot net changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39536
[Bug testsuite/39536] New: add more VERIFY() calls to certain functions in libstdc++-v3 testsuite
This patch adds additional calls to VERIFY() on several functions to catch additional error conditions. -- Summary: add more VERIFY() calls to certain functions in libstdc++-v3 testsuite Product: gcc Version: 4.4.0 Status: UNCONFIRMED Severity: enhancement Priority: P3 Component: testsuite AssignedTo: unassigned at gcc dot gnu dot org ReportedBy: kees at outflux dot net http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39536
[Bug testsuite/39536] add more VERIFY() calls to certain functions in libstdc++-v3 testsuite
--- Comment #1 from kees at outflux dot net 2009-03-24 06:27 --- Created an attachment (id=17529) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17529action=view) more VERIFY() calls in libstdc++ testsuite -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39536
[Bug testsuite/39537] New: overhaul printf formats and type casts in testsuite
Ubuntu's builds of GCC enable non-standard options (specifically -Wformat-security and -Wformat). In an attempt to maintain parity with the upstream testsuite output, I've gone through the testsuite and located many architecture-dependent format-string/type discontinuities (e.g. %d should be %ld for a pointer on x86_64). This patch seeks to address all the mismatches through a standardized use of %p with (void*), and in several cases, a forced (int) cast. Are these changes interesting? I am hoping they are not needlessly invasive, but I am obviously open to feedback. -- Summary: overhaul printf formats and type casts in testsuite Product: gcc Version: 4.4.0 Status: UNCONFIRMED Severity: enhancement Priority: P3 Component: testsuite AssignedTo: unassigned at gcc dot gnu dot org ReportedBy: kees at outflux dot net http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39537
[Bug testsuite/39537] overhaul printf formats and type casts in testsuite
--- Comment #1 from kees at outflux dot net 2009-03-24 06:34 --- Created an attachment (id=17530) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17530action=view) testsuite updates for format strings and casts -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39537
[Bug testsuite/39537] overhaul printf formats and type casts in testsuite
--- Comment #3 from kees at outflux dot net 2009-03-24 07:10 --- g++.old-deja/g++.pt/t39.C: It looks like ptr[0],[1],[2] are either int or const char (i.e. ptr is int* or const char*). %p doesn't make much sense in those cases, so I opted for a cast. I'm not sure I followed your last sentence; I realize that many people find the Ubuntu default compiler flags inappropriate, but I'm hoping that can be set aside in the interests of helping to improve our ability to compare testsuite output. As you say, many of the changes are correct, and the rest have no negative impact. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39537
[Bug testsuite/39537] overhaul printf formats and type casts in testsuite
--- Comment #6 from kees at outflux dot net 2009-03-24 17:39 --- I'm trying to minimize the Ubuntu patch by getting changes accepted for the FSF GCC testsuite. I'm hoping to demonstrate that many of the changes are valid and represent stricter C coding, though none change the behavior of the test itself. Are there any parts of the above patchset that would be acceptable? Anything I can trim or change? (Also, please see bug 39536, which should also be trivial and useful to add.) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39537
[Bug target/38902] __builtin_strcpy doesn't work with -fstack-protector
--- Comment #5 from kees at outflux dot net 2009-01-18 18:12 --- Created an attachment (id=17135) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17135action=view) multiple tests for the regression This contains a series of tests, none of which should fail. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38902
[Bug c++/38562] [4.3/4.4 regression] mysql miscompiles and causes testsuite failures
--- Comment #9 from kees at outflux dot net 2008-12-22 01:01 --- Yes! Adding -fno-strict-aliasing to a normal (-O2) build seems to have fixed the problems so far. The full test suite takes a while, but the early failures are not present any more. I will report more once it finishes. Thank you for the hint! Is this a regression in gcc, or is this something upstreams need to add to their builds when doing the char/pointer manipulations you described? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] New: mysql miscompiles and causes testsuite failures
Building mysql 5.0.67 with gcc 4.2 and 4.3.2 produce okay results. Building on 4.3.3 20081210 (prerelease) and 4.4.0 20081212 (experimental) [trunk revision 142725] causes problems. Compiling with -O1 (instead of the default -O2) fixes the issue. At least one of the failures was reduced to sql/log_event.cc. Attaching log_event.cc and -O2 log_event.o from 4.3.2 and 4.4.0 20081212, and -O1 from 4.4.0 20081212 for now, until I can further isolate the problem. -- Summary: mysql miscompiles and causes testsuite failures Product: gcc Version: 4.3.3 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c++ AssignedTo: unassigned at gcc dot gnu dot org ReportedBy: kees at outflux dot net http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #1 from kees at outflux dot net 2008-12-18 00:35 --- Created an attachment (id=16919) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16919action=view) mysql source file -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #2 from kees at outflux dot net 2008-12-18 00:36 --- Created an attachment (id=16920) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16920action=view) obj file from -O2 gcc 4.3.2 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #3 from kees at outflux dot net 2008-12-18 00:38 --- Created an attachment (id=16921) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16921action=view) obj file from -O2 gcc 4.4.0 20081212 (experimental) [trunk revision 142725] -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #6 from kees at outflux dot net 2008-12-18 00:41 --- Created an attachment (id=16924) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16924action=view) obj file from -O1 gcc 4.4.0 20081212 (experimental) [trunk revision 142725] -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #4 from kees at outflux dot net 2008-12-18 00:39 --- Created an attachment (id=16922) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16922action=view) obj file from -O2 gcc 4.4.0 20081212 (experimental) [trunk revision 142725] -- kees at outflux dot net changed: What|Removed |Added Attachment #16921|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] mysql miscompiles and causes testsuite failures
--- Comment #5 from kees at outflux dot net 2008-12-18 00:40 --- Created an attachment (id=16923) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16923action=view) obj file from -O2 gcc 4.3.2 -- kees at outflux dot net changed: What|Removed |Added Attachment #16920|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c++/38562] [4.3/4.4 regression] mysql miscompiles and causes testsuite failures
--- Comment #7 from kees at outflux dot net 2008-12-18 01:07 --- Created an attachment (id=16925) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16925action=view) 4.4.0 -E output of log_event.cc (bzip2) $ /usr/lib/gcc-snapshot/bin/g++ -DMYSQL_SERVER -DDEFAULT_MYSQL_HOME=\/usr\ -DDATADIR=\/var/lib/mysql\ -DSHAREDIR=\/usr/share/mysql\ -DHAVE_CONFIG_H -I. -I. -I../include -I../innobase/include -I../innobase/include -I../ndb/include -I../ndb/include -I../ndb/include/ndbapi -I../ndb/include/mgmapi -I../include -I../include -I../regex -I. -DDBUG_OFF -DBIG_JOINS=1 -felide-constructors -fno-rtti -O1 -fno-implicit-templates -fno-exceptions -fno-rtti -MT log_event.o -MD -MP -MF .deps/log_event.Tpo -E log_event.cc /tmp/log_event.cc-E -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38562
[Bug c/35948] New: -D_FORTIFY_SOURCE discards qualifier overrides on {str,mem}cpy
The following source, without the (void*) overrides, will throw an warning
(as expected), when compiled with -Wall:
$ gcc -o memcpy-fortify -Wall memcpy-fortify.c
memcpy-fortify.c: In function 'main':
memcpy-fortify.c:21: warning: passing argument 1 of 'memcpy' discards
qualifiers from pointer target type
memcpy-fortify.c:22: warning: passing argument 1 of 'strcpy' discards
qualifiers from pointer target type
With (void*) it is (as expected) silent. With -O2, it is silent, but with
-D_FORTIFY_SOURCE != 0, the qualifier override is ignored:
$ gcc -o memcpy-fortify -Wall -O2 -D_FORTIFY_SOURCE=2 memcpy-fortify.c
memcpy-fortify.c: In function 'main':
memcpy-fortify.c:21: warning: passing argument 1 of 'memcpy' discards
qualifiers from pointer target type
memcpy-fortify.c:22: warning: passing argument 1 of 'strcpy' discards
qualifiers from pointer target type
This will cause problems for builds that run with -Werror.
/*
* gcc -o memcpy-fortify -Wall -Werror -O2 -D_FORTIFY_SOURCE=2 memcpy-fortify.c
*
*/
#include stdio.h
#include stdlib.h
#include unistd.h
#include stdint.h
#include string.h
#include stdint.h
#include inttypes.h
int main(int argc, char * argv[])
{
char *foo = strdup(string one);
char *bar = strdup(string two);
const char *baz = (const char *)foo;
printf(%s\n, foo);
memcpy((void*)baz, bar, strlen(bar)+1);
strcpy((void*)baz, bar);
printf(%s\n, foo);
return 0;
}
--
Summary: -D_FORTIFY_SOURCE discards qualifier overrides on
{str,mem}cpy
Product: gcc
Version: 4.2.3
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
AssignedTo: unassigned at gcc dot gnu dot org
ReportedBy: kees at outflux dot net
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35948
[Bug c/35948] -D_FORTIFY_SOURCE discards qualifier overrides on {str,mem}cpy
--- Comment #2 from kees at outflux dot net 2008-04-15 16:44 --- Erk, right. Sorry for the noise. -- kees at outflux dot net changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||INVALID http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35948
