https://github.com/kees approved this pull request.
Thanks for the updates! Let's get this in and continue with the rest of the
support. :)
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -0,0 +1,187 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+
+struct bar;
+
+struct not_found {
+ int count;
+ struct bar *fam[] __counted_by(bork); // expected-error {{use of undeclared
identifier 'bork'}}
+};
+
kees wrote:
> Consider this example. It tries to illustrate why putting `__counted_by()` on
> a pointer to a structs containing flexible array members doesn't make sense.
>
> ```c
> struct HasFAM {
> int count;
> char buffer[] __counted_by(count); // This is OK
> };
>
> struct
kees wrote:
> As @apple-fcloutier @rapidsna noted this is how `-fbounds-safety` is
> currently implemented (because its much simpler) but it is a restriction that
> could be lifted in future by only requiring `struct bar` to be defined at the
> point that `foo::bar` is used rather than when
https://github.com/kees closed https://github.com/llvm/llvm-project/pull/89707
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kees wrote:
My thinking about this attribute tends to follow from my desire not to change
the C type system, but rather to adjust the behavior of the sanitizers. This
means that it is possible to still build the Linux kernel without the
sanitizers (the build just ignores the attribute), or
@@ -4781,6 +4782,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType
*FTy, StringRef Name,
}
}
setDSOLocal(F);
+ markRegisterParameterAttributes(F);
kees wrote:
Ah-ha, thanks! Okay, I've updated the comments with just a
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/89707
>From c061c8f49f2b916bb5e60ec35d3e448ac13f2b72 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Mon, 22 Apr 2024 17:53:32 -0700
Subject: [PATCH 1/4] [CodeGen][i386] Move -mregparm storage earlier and fix
Runtime
@@ -4781,6 +4782,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType
*FTy, StringRef Name,
}
}
setDSOLocal(F);
+ markRegisterParameterAttributes(F);
kees wrote:
Comment added. Is this what you had in mind?
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/89707
>From c061c8f49f2b916bb5e60ec35d3e448ac13f2b72 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Mon, 22 Apr 2024 17:53:32 -0700
Subject: [PATCH 1/3] [CodeGen][i386] Move -mregparm storage earlier and fix
Runtime
@@ -4781,6 +4782,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType
*FTy, StringRef Name,
}
}
setDSOLocal(F);
+ markRegisterParameterAttributes(F);
kees wrote:
This seems like a large proposed change; is it worth it for
@@ -4781,6 +4782,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType
*FTy, StringRef Name,
}
}
setDSOLocal(F);
+ markRegisterParameterAttributes(F);
kees wrote:
Oh, I think I see what you mean -- this is the common place
@@ -4781,6 +4782,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType
*FTy, StringRef Name,
}
}
setDSOLocal(F);
+ markRegisterParameterAttributes(F);
kees wrote:
I was trying to basically duplicate what was already done for
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/89707
>From c061c8f49f2b916bb5e60ec35d3e448ac13f2b72 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Mon, 22 Apr 2024 17:53:32 -0700
Subject: [PATCH 1/2] [CodeGen][i386] Move -mregparm storage earlier and fix
Runtime
kees wrote:
This needs test cases, which I'll add tomorrow. I just wanted to get the core
logic up for review before I hit EOD...
https://github.com/llvm/llvm-project/pull/89707
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/kees created https://github.com/llvm/llvm-project/pull/89707
When building the Linux kernel for i386, the -mregparm=3 option is enabled.
Crashes were observed in the sanitizer handler functions, and the problem was
found to be mismatched calling convention.
As was fixed in
kees wrote:
Does this still work for cases where there are multiple flexible arrays? e.g.
```
struct weird_protocol {
unsigned int cmd_type;
unsigned int data_len;
union {
struct cmd_one one[];
struct cmd_two two[];
struct cmd_three three[];
unsigned
https://github.com/kees approved this pull request.
Tests and logic adjustment looks good to me.
https://github.com/llvm/llvm-project/pull/89126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
kees wrote:
This now passes my behavioral testing suite for wrapping; yay! (The earlier
version didn't cover truncate, so this is very nice now.)
https://github.com/llvm/llvm-project/pull/86618
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
kees wrote:
I guess I don't have a strong opinion here, since these helpers are specific to
C++, and I've been generally trying to remove fixed-size 0-sized arrays in C
projects (i.e. the Linux kernel). I do care about C flexible arrays (and their
associated extensions), though. I suspect
https://github.com/kees commented:
I can't speak to the implementation details, but this passes my PoC tests that
examine subobjects.
https://github.com/llvm/llvm-project/pull/86858
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
kees wrote:
> My natural inclination is that it is array-like, but... that just makes me
> want `__is_array` to return `true` for it all the more.
Yes. An array is an array, regardless of its size. The size is just a storage
characteristic. It'd almost be like arguing that `NaN` isn't a
https://github.com/kees closed https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From 59c81a85cd9652d02b15a79553259351a59e8534 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH] [Clang][Sema] Allow flexible arrays in unions and alone in
structs
GNU
@@ -271,6 +271,9 @@ Improvements to Clang's diagnostics
- Clang now correctly diagnoses no arguments to a variadic macro parameter as
a C23/C++20 extension.
Fixes #GH84495.
+- ``-Wmicrosoft`` or ``-Wgnu`` is now required to diagnose C99 flexible
+ array members in a union
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/8] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
@@ -21,10 +27,76 @@ struct __attribute((packed, aligned(4))) { char a; int x;
char z[]; } e = { 1, 2
struct { int x; char y[]; } f = { 1, { 13, 15 } };
// CHECK: @f ={{.*}} global <{ i32, [2 x i8] }> <{ i32 1, [2 x i8] c"\0D\0F" }>
-union {
- struct {
-int a;
-char
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/7] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/7] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
kees wrote:
Ah, well, regardless, I think I found where the
`StructuredList->setInitializedFieldInUnion` was actually missing, and then I
could undo my zero-init-only and everything still appears fixed. Doing a full
debug build test run now...
https://github.com/llvm/llvm-project/pull/84428
kees wrote:
> > because we don't yet support non-zero initialization (as described in
> > commit
> > [5955a0f](https://github.com/llvm/llvm-project/commit/5955a0f9375a8c0b134eeb4a8de5155dcce7c94f))
>
> I'm confused. We support non-zero init, and there are tests for non-zero init
> in that
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/6] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
kees wrote:
> `InitListChecker::CheckStructUnionTypes` never calls
> `StructuredList->setInitializedFieldInUnion`
Ah-ha, thank you for the pointer. I think I've figured this out: initialization
was avoiding flexible arrays because we don't yet support non-zero
initialization (as described in
kees wrote:
> Is this an existing bug? i.e. it's the CodeGen test for `union { char x[]; }
> x = {0};` ... :P
Confirmed. Adding a CodeGen test for `union { char x[]; } x = {0};` without any
of the changes from this PR still hits the assert. I assume this was from
making flex array
kees wrote:
Hmpf. Build failure encountered under an Assert:
```
# | Assertion failed: VarSize == CstSize && "Emitted constant has unexpected
size", file C:\ws\src\clang\lib\CodeGen\CodeGenModule.cpp, line 5294
# | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/
and
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/5] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/3] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,158 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
-// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility -x c++
+// RUN: %clang_cc1 %s
@@ -1,13 +1,58 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only
// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
// RUN: %clang_cc1 %s -verify -fsyntax-only
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH 1/2] [Clang][Sema]: Allow flexible arrays in unions and alone
in structs
@@ -1,13 +1,58 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only
// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
// RUN: %clang_cc1 %s -verify -fsyntax-only
@@ -1,13 +1,58 @@
-// RUN: %clang_cc1 %s -verify=c -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only
// RUN: %clang_cc1 %s -verify -fsyntax-only -x c++
-// RUN: %clang_cc1 %s -verify -fsyntax-only -fms-compatibility
// RUN: %clang_cc1 %s -verify -fsyntax-only
kees wrote:
> That one ends up not being a problem, but presumably you are wanting to
> change that top-level 'struct' to be a 'union'?
No, I want to collapse the entire macro into just `TYPE NAME[]`. Right now the
Linux kernel uses the `DECLARE_FLEX_ARRAY` macro _in_ over 200 unions and
kees wrote:
> There are currently over 200 separate unions using the work-around.
Specifically, this is what Linux uses for getting C99 flexible arrays in unions
and alone in structs:
```
#define DECLARE_FLEX_ARRAY(TYPE, NAME)\
struct { \
struct { } __empty_ ##
kees wrote:
> C99 added flexible array members, and the C99 rationale says the feature was
> added specifically as a replacement for the common idiom known as the "struct
> hack" for creating a structure containing a variable-size array.
This is my reasoning as well -- we (Linux dev hat on)
kees wrote:
> Left my comment on the main list, but I don't see this as a well motivated
> change, and even if GCC supported it, it would still be a very difficult to
> motivate extension without massive historical workloads already using it.
This is needed by the Linux kernel, and is in
kees wrote:
For historical reference, the first version of this PR is visible here now:
https://github.com/kees/llvm-project/commit/ce31f1d75f060b32e5dbc5756fe41cc8eaac83a6
https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees updated https://github.com/llvm/llvm-project/pull/84428
>From eb5138b45fa450737600050ad8dabdcb27513d42 Mon Sep 17 00:00:00 2001
From: Kees Cook
Date: Thu, 7 Mar 2024 17:03:09 -0800
Subject: [PATCH] [Clang][Sema]: Allow flexible arrays in unions and alone in
structs
GNU
kees wrote:
GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548
Clang: https://github.com/llvm/llvm-project/issues/84565
https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
kees wrote:
> > I didn't do this because it seemed like this would change a lot of existing
> > test cases
>
> Can you give some examples of tests that would fail? If we have tests
> checking that these fail, then perhaps those tests should add
> `-Werror=pedantic` so that they can continue
kees wrote:
> Rather than have a `-f` flag to opt into this extension, I think instead you
> should just make it always available, then have tests that it can be used,
> but will trigger diagnostics under `-Wpedantic` since it's technically a
> language extension (IIUC).
I didn't do this
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 %s -verify=c -fsyntax-only -fflex-array-extensions
+
+// The test checks that flexible array members do not emit warnings when
+// -fflex-array-extensions when used in a union or alone in a structure.
+
+struct already_hidden {
+ int a;
kees wrote:
With PR #82432 landed, this PR is redundant. Thanks for changing the option
name! Closing...
https://github.com/llvm/llvm-project/pull/80089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/kees closed https://github.com/llvm/llvm-project/pull/80089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/84428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees created https://github.com/llvm/llvm-project/pull/84428
GNU and MSVC have extensions where flexible array members (or their equivalent)
can be in unions or alone in structs. This is already fully supported in Clang
through the 0-sized array ("fake flexible array")
https://github.com/kees approved this pull request.
Working as expected for me!
https://github.com/llvm/llvm-project/pull/82432
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kees wrote:
This doesn't seem to do anything for me with the Linux kernel's -next branch
(which supports -sio as `CONFIG_UBSAN_SIGNED_WRAP=y`). e.g. I see no behavioral
difference with test_ubsan.ko nor the expected atomic overflows.
https://github.com/llvm/llvm-project/pull/82432
kees wrote:
GCC folks have not answered. Adding -wrap keeps the behavior for -overflow the
same between GCC and Clang. Can we please move this forward and land it as is?
We can trivially change this in the future if we need to.
https://github.com/llvm/llvm-project/pull/80089
kees wrote:
> Sure -fwrapv makes wraparound defined, but it doesn't prevent us from making
> -fsanitize=signed-integer-overflow useful. "-fwrapv => no
> signed-integer-overflow" is not a solid argument.
>
> I think we can try making -fsanitize=signed-integer-overflow effective even
> when
kees wrote:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317
https://github.com/llvm/llvm-project/pull/80089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kees wrote:
> > > > > Why not just enforce -fsanitize=signed-integer-overflow with -fwrapv?
> > > > > I suspect it's just overlook, and not intentional behavior.
> > > >
> > > >
> > > > +1
> > > > We should consider this direction
> > >
> > >
> > > The UB-vs-non-UB seemed to be a really
kees wrote:
> > Why not just enforce -fsanitize=signed-integer-overflow with -fwrapv? I
> > suspect it's just overlook, and not intentional behavior.
>
> +1
>
> We should consider this direction
The UB-vs-non-UB seemed to be a really specific goal in the existing code. i.e.
that the
kees wrote:
> > and likely in production kernels. :)
>
> I doubt you meant running in production.
I very much do -- I expect to use this like we use the array-bounds sanitizer,
which is on in production in at least Ubuntu and Android. It may be a long road
to getting sane coverage without
kees wrote:
I've tested this with the Linux kernel, and it is working as expected. It is
ready and waiting for this option to gain back a bunch of sanitizer coverage
for CIs and likely in production kernels. :)
https://github.com/llvm/llvm-project/pull/80089
kees wrote:
> Hey, does anyone know why the CI bot (buildkite) fails my builds for a
> trailing whitespace in a file I did not touch?
>
> It says there's some trailing whitespace in some documentation files that my
> PR doesn't touch (unless I'm misinterpreting its output):
>
https://github.com/kees approved this pull request.
I can't speak to the LLVM code changes, but behaviorally, this passes all the
torture tests I'd expect it to (kernel-tools/fortify/array-bounds.c and full
kernel builds with its hundreds of `counted_by` annotations).
kees wrote:
That's it! Everything works. :) Ship it!
https://github.com/llvm/llvm-project/pull/76348
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kees wrote:
Thanks! It's fixed for me now. I continue to be highly unlucky, though, and
keep finding weird stuff. It seems to segfault just parsing libc headers under
`-D_FORTIFY_SOURCE=3` (but not `=2` or lower):
```
0. Program arguments:
kees wrote:
Accessing `mi->ints` is unambiguous (it would use the declared `count_ints`)
but I'm fine to wait and fix that later. The flex array union is gloriously
rare in the kernel. As for whole object, I say pick smallest from all available
under bdos(x, 1) and largest for bdos(x, 0), or
kees wrote:
> @kees, the issue should be fixed with the newest push.
Nice! This is so extremely close. It fixed all but 1 instance in the torture
testing. The remaining seems to be union order sensitive. O_o This arrangement
still reports non-zero for the `int count_bytes` one, unless I move
kees wrote:
> > but the value is nonsense, so we must return 0 so that anything checking
> > lengths will not write anything to the array.
>
> @kees Oh, I see. I did not know such the convention but it makes sense. Is it
> documented somewhere?
This is new territory (having a multiplier for
kees wrote:
> > This prints a wrapped calculation instead of the expected "0".
>
> Shouldn't `getDefaultBuiltinObjectSizeResult` return `-1`? Have you tried
> `-2` to see if it's returning the negative count value or it happens to be
> equal to the default value?
Well, maybe it's not
kees wrote:
Thanks! The update fixes the anon struct issue I hit. I've found one more
issue, though this appears to be a miscalculation with a pathological `count`
value (i.e. `count` is signed type and contains a negative value):
```
struct annotated {
unsigned long flags;
int count;
kees wrote:
Possibly due to bug #72032 , I can get this tree to crash using the latest
`array-bounds.c` test from
https://github.com/kees/kernel-tools/tree/trunk/fortify
Specifically:
```
struct anon_struct {
unsigned long flags;
long count;
int array[]
kees wrote:
> ```
> int foo(struct s *p, int index) {
> return __builtin_dynamic_object_size((++p)->array[index], 1);
> }
> ```
>
> This _shouldn't_ increment `p`, but we need to get the array size of the
> element _after_ `p`. I suspect that this is probably a horrible security
> violation
@@ -455,18 +455,11 @@ void DiagnosticInfoDontCall::print(DiagnosticPrinter )
const {
SmallVector DiagnosticInfoDontCall::getInliningDecisions() const {
SmallVector InliningDecisions;
- if (MDN) {
-const MDOperand = MDN->getOperand(0);
-if (auto *MDT =
kees wrote:
Yeah, this is the "compiler doesn't know if pointer points into an array of
structs or not" that has driven me crazy for years. But we can now reliably
disambiguate this for structs that end with a flexible array member (or future
pointers marked with `__single`). It's been a long
https://github.com/kees commented:
For the test cases, I wonder if it might be good to add __bdos() calls with
type 0 as well. The results should always be the same, but we do want to check
for that. i.e.:
` p->array[index] = __builtin_dynamic_object_size(>count, 0);`
@@ -827,6 +827,165 @@ CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const
Expr *E, unsigned Type,
return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true);
}
+llvm::Value *
+CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
+
https://github.com/kees edited https://github.com/llvm/llvm-project/pull/71877
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/kees approved this pull request.
Yes, this looks correct. We don't want to let this feature leak into the "fake"
flex array logic.
https://github.com/llvm/llvm-project/pull/72347
___
cfe-commits mailing list
kees wrote:
This is great! Thanks for fixing this!
https://github.com/llvm/llvm-project/pull/67772
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: Kees Cook
Date: 2023-09-03T22:24:37-07:00
New Revision: 00e54d04ae2802d498741097d4b83e898bc99c5b
URL:
https://github.com/llvm/llvm-project/commit/00e54d04ae2802d498741097d4b83e898bc99c5b
DIFF:
https://github.com/llvm/llvm-project/commit/00e54d04ae2802d498741097d4b83e898bc99c5b.diff
Author: Kees Cook
Date: 2022-10-01T18:45:45-07:00
New Revision: aef03c9b3bed5cef5a1940774b80128aefcb4095
URL:
https://github.com/llvm/llvm-project/commit/aef03c9b3bed5cef5a1940774b80128aefcb4095
DIFF:
https://github.com/llvm/llvm-project/commit/aef03c9b3bed5cef5a1940774b80128aefcb4095.diff
97 matches
Mail list logo
