[PATCH] D144136: Add a "remark" to report on array accesses

2023-09-26 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D144136#4651030 , @void wrote: > or can it be simplified into something like this: > > array-bounds.c:341:2: remark: accessing flexible array, counted by 'count', > with 'index - 1' [-Rarray-bounds] > 341 |

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-09-15 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D148381#4645600 , @void wrote: > Added more error messages. Changed some code around to align with coding > practices. Added some more test cases. Doing a full "allmodconfig" Linux kernel build with my 200ish __counted_by

[PATCH] D148381: [Clang] Add counted_by attribute

2023-09-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I can generate warnings for anonymous structs were the `__counted_by` member is reported as "not found". For example: little.c:7:28: warning: counted_by field 'count' not found 7 | int array[] __counted_by(count); |

[PATCH] D144136: Add a "remark" to report on array accesses

2023-09-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Can you refresh this patch to work with https://reviews.llvm.org/D148381 ? My testing seems to imply that it doesn't know the size of the array. I assume the `if (!IsUnboundedArray)` check is incomplete now. i.e. for a `__counted_by` array, I see the "unknown" remark:

[PATCH] D148381: [Clang] Add counted_by attribute

2023-09-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D148381#4639436 , @xbolva00 wrote: > Will gcc use counted_by or element_count ? GCC is using `__counted_by`: https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628459.html Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D159373: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2023-09-03 Thread Kees Cook via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG00e54d04ae28: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be… (authored by kees). Repository: rG LLVM Github

[PATCH] D159373: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2023-09-01 Thread Kees Cook via Phabricator via cfe-commits
kees created this revision. kees added reviewers: nickdesaulniers, aaron.ballman. Herald added a project: All. kees requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was deprecated in Clang 16 and scheduled for removal in Clang 18. Time

[PATCH] D148381: [WIP][Clang] Add element_count attribute

2023-05-03 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This is great! It passes my own testing for the sanitizer too. I'm looking forward to __bdos support. :) FWIW, similar progress is being made on the GCC side: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D144136: Add a "remark" to report on array accesses

2023-02-23 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This gets me all 6 reports. The details about the array and the index don't really matter for the basic metrics: diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/Diagnostic SemaKinds.td index ba831c026342..29d2167b504b 100644

[PATCH] D144136: Add a "remark" to report on array accesses

2023-02-18 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Here's a test-case. I'd expect 6 remarks from building this: /* Build with -Wall -O2 -fstrict-flex-arrays=3 -fsanitize=bounds -Rarray-bounds */ #include #include #include #include #define report_size(p, index) do {\ const size_t bdos =

[PATCH] D144136: Add a "remark" to report on array accesses

2023-02-18 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This appears to be working for me. For before/after changes, the other half is still needed, i.e. a "accessing array of unknown size" and eventually splitting the dynamic sizing check off of that one (once -fsanitize=bounds checks __builtin_dynamic_object_size). For

[PATCH] D144136: Add a "remark" to report on array accesses

2023-02-15 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This information will be useful for evaluating the coverage of the bounds checker for a given program, which in turn can help guide both improvements to the bounds checker (e.g. adding knowledge from `__builtin_dynamic_object_size()`) and improvements to the built code

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-10-26 Thread Kees Cook via Phabricator via cfe-commits
kees accepted this revision. kees added a comment. The self-tests all look correct to me, so I expect it is working how I'd expect. I haven't had a chance to do kernel builds with this yet, but I'm hoping to make time soon. I'd say if others are happy, let's land it, and if anything unexpected

[PATCH] D135727: [clang] Correct sanitizer behavior in union FAMs

2022-10-20 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This looks great to me. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews.llvm.org/D135727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; //

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-13 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/Sema/array-bounds-ptr-arith.c:15 { -return (void *)es->s_uuid + 80; // expected-warning {{refers past the end of the array}} +return (void *)es->s_uuid + 80; // expected-warning {{refers past the end of the

[PATCH] D135727: [clang] Correct sanitizer behavior in union FAMs

2022-10-13 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D135727#3856978 , @void wrote: > I think we opened the can or worms. :-) At this point I think the can we opened has worms with cans of worms with cans of worms. I'd say it's turtles all the way down, but it seems to just be

[PATCH] D135727: [clang] Correct sanitizer behavior in union FAMs

2022-10-12 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D135727#3853896 , @void wrote: > @kees @serge-sans-paille: It appears to me that a terminating array of size > > 2 *isn't* treated as a FAM in Clang, at least Clang warns about it. The first > failure above

[PATCH] D135727: [clang] Correct sanitizer behavior in union FAMs

2022-10-11 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Change log typo? "but for all" should be "but not for all" ? Comment at: clang/lib/AST/Expr.cpp:223 // arrays to be treated as flexible-array-members, we still emit diagnostics // as if they are not. Pending further discussion... +if

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-10-11 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D134902#3848595 , @serge-sans-paille wrote: > I second the opinion here. C99 says nothing about flexible array member for > unions, that's already a "language extension". (and so not be considered as > FAM by

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-10-10 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D134902#3848246 , @void wrote: > @rsmith, @serge-sans-paille, and @kees, I need some advice. There's a test in > `clang/test/CodeGen/bounds-checking.c` that's checking bounds stuff on > unions. The behavior is...weird to me. It

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-10-07 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D135411#3841841 , @samitolvanen wrote: > That being said, I did compile a 64-bit MIPS kernel using this pass, but I > didn't try booting it. I would expect to run into quite a few issues > initially. I've done a test build

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-10-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. What's the best way to test this in the kernel? I assume add `ARCH_SUPPORTS_CFI_CLANG` to an arch, and see what blows up? :) Have you tried this on any other arch yet? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-10-06 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/CodeGen/object-size-flex-array.c:45 + // CHECK-STRICT-2: ret i32 -1 + // CHECK-STRICT-3: ret i32 0 return OBJECT_SIZE_BUILTIN(f->c, 1); serge-sans-paille wrote: > This one worries me a bit, as an array of

[PATCH] D135107: [clang][NFC] Use enum for -fstrict-flex-arrays

2022-10-05 Thread Kees Cook via Phabricator via cfe-commits
kees accepted this revision. kees added a comment. This revision is now accepted and ready to land. L Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135107/new/ https://reviews.llvm.org/D135107 ___

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-10-01 Thread Kees Cook via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kees marked an inline comment as done. Closed by commit rGaef03c9b3bed: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will… (authored by kees). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-10-01 Thread Kees Cook via Phabricator via cfe-commits
kees marked an inline comment as done. kees added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3464-3465 + D.Diag(diag::warn_ignored_clang_option) + << "-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-" +

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-10-01 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 464476. kees added a comment. Update optional removal target release to Clang 18 2 releases was the suggested time to wait between deprecation and removal for this option. As this change was originally written during the Clang 15 development window, and we're

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-10-01 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 464474. kees added a comment. use Group Instead of a custom warning, use the clang_ignored_legacy_options_Group, as MaskRay suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-09-30 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 464381. kees added a comment. Keep git-clang-format happy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142 Files: clang/docs/ReleaseNotes.rst

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-09-30 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 464337. kees retitled this revision from "[clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang" to "[clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang". kees added

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-09-30 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/CodeGen/bounds-checking-fam.c:23 }; // CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}( I would expect to see here: ``` struct Zero { int a[0]; }; // CHECK-LABEL: define {{.*}} @{{.*}}test_zero{{.*}}(

[PATCH] D125142: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2022-09-30 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 464146. kees retitled this revision from "[clang][auto-init] Remove -enable flag for "zero" mode" to "[clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang". kees added a comment. rebase and tweak Rebase to main,

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-09-29 Thread Kees Cook via Phabricator via cfe-commits
kees marked an inline comment as done. kees added a comment. In D125142#3825972 , @MaskRay wrote: >> [clang][auto-init] Remove -enable flag for "zero" mode > > The subject should be updated to mention the exact option name, not a vague > `-enable` and

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

2022-09-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Excellent! Thank you for getting this prepared. Having this properly managed in the kernel means we don't have to do horrible ugly hacks to work around old 0-length arrays in UAPI (which have all been unioned with a proper flexible array now). Repository: rG LLVM

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-09-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Thanks for all the careful consideration; I appreciate it! I'll land this tomorrow unless there are new specific objections. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142

[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

2022-09-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a subscriber: nathanchance. kees added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8686 +def warn_cast_function_type_strict : Warning, + InGroup, DefaultIgnore; def err_cast_pointer_to_non_pointer_int : Error<

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-13 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Example of the bug I want to block: struct foo { int stuff; u32 data[0]; }; struct foo *deserialize(u8 *str, int len) { struct foo *instance; size_t bytes; bytes = sizeof(*instance) + sizeof(instance->data) * (len / sizeof(u32));

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-13 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3646854 , @jyknight wrote: > In D126864#3645994 , @kees wrote: > >> I should clarify: I still need the =3 mode. Since sizeof([0]) == 0 and >> sizeof([]) == error, they are being

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-12 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I should clarify: I still need the =3 mode. Since sizeof([0]) == 0 and sizeof([]) == error, they are being treated differently already by the compiler causing bugs in Linux. The kernel must still have a way to reject the _use_ of a [0] array. We cannot reject

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-12 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3641939 , @serge-sans-paille wrote: > @kees are you ok with current state? Build tests are still looking correct on my end. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-11 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:906 // member, only a T[0] or T[] member gets that treatment. + // Under StrictFlexArraysLevel, obey c99+ that disallows FAM in union, see + // C11 6.7.2.1 §18 jyknight wrote:

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-06-23 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3598257 , @serge-sans-paille wrote: > @kees does the new version looks good to you? Hi, yes, this is working as expected in my kernel builds. Yay! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-15 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3584536 , @serge-sans-paille wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 does toward > `-fstrict-flex-arrays=` with > > - `n=0` ⇒ `-fno-strict-flex-arrays`, current state (the default) > - `n=1` ⇒ only

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-14 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3582360 , @nickdesaulniers wrote: > If the GCC developers split this into two distinct flags, should we land > something we're just going to have to turn around and modify to match the new > flags/semantics they've

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-14 Thread Kees Cook via Phabricator via cfe-commits
kees accepted this revision. kees added a comment. This revision is now accepted and ready to land. Are the presubmit build failures unrelated? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/ https://reviews.llvm.org/D126864 ___

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-08 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3567647 , @nickdesaulniers wrote: > @kees maybe we should think about what would be needed for toolchains that > don't yet support `-fstrict-flex-arrays` in kernel sources? Does this become > a toolchain portability

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-08 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3564519 , @efriedma wrote: > Do we want some builtin define so headers can check if we're in > -fstrict-flex-arrays mode? It might be hard to mess with the definitions > otherwise. Hm, maybe? It wonder if that's more

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-07 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D126864#3556262 , @efriedma wrote: > I'm a little concerned about the premise of this, though. See > https://github.com/llvm/llvm-project/issues/29694 for why we relaxed this > check in the first place. I mean, the Linux

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-07 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Thanks for working on this! Doing test builds with the Linux kernel correctly detects a number of trailing arrays that were being treated as flexible arrays (and need to be fixed in the kernel). This is exactly what was expected and wanted. :) Comment

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-11 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D125142#3505767 , @jfb wrote: > I think the most relevant post from @rsmith is: > https://discourse.llvm.org/t/making-ftrivial-auto-var-init-zero-a-first-class-option/55143/40 > > He has a prototype:

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-10 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This is marked "needs revision", but I think it just needs wider review? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142 ___ cfe-commits

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-10 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D125142#3502732 , @MaskRay wrote: > This cannot be committed as is. In particular, @rsmith's "We do not want to > create or encourage the creation of language dialects and non-portable code," > concern on >

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-09 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 428224. kees edited the summary of this revision. kees added a comment. add release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142 Files:

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-09 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 428173. kees added a comment. This revision is now accepted and ready to land. update with suggestions - Add FIXME comment with deprecation schedule - Report deprecation warning when -enable flag is used Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-08 Thread Kees Cook via Phabricator via cfe-commits
kees updated this revision to Diff 427916. kees added a comment. Report flag as "unused" Adjust flags and tests to have the option warn about being unused now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-08 Thread Kees Cook via Phabricator via cfe-commits
kees planned changes to this revision. kees added a comment. In D125142#3499010 , @brooks wrote: > It would be somewhat helpful as a transition aid if > `-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang` > remained as a no-op

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

2022-04-19 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I tested this (with D123958 ), and it appears to be working as intended. These two fptrs are the first and second listed, and are happily randomized: [0.00] LSM: offsetof(struct security_hook_heads, ptrace_access_check): 816

[PATCH] D123958: [randstruct] Randomize all elements of a record

2022-04-19 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D123958#3459205 , @aaron.ballman wrote: > I had assumed that any structure not marked for randomization would not be > randomized. Based on that, I don't think inner structure objects (anonymous > or otherwise) should

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. FWIW, related problems with `pskb_expand_head` were seen again here: https://github.com/ClangBuiltLinux/linux/issues/1599 I have trouble reproducing it, but I think the kernel patch there solves the problem created by `__alloc_size` vs `ksize()`. Repository: rG LLVM

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

2022-02-08 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I can build and boot with this. Nice! :) One issue I see is in instruction sequence ordering. Looking at the end of `__startup_64` without the feature enabled, everything looks "normal": 31 c0 xor%eax,%eax 5b pop%rbx 41 5e pop%r14 41

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-30 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Is https://bugs.llvm.org/show_bug.cgi?id=50322 and dup of https://bugs.llvm.org/show_bug.cgi?id=23280 ? (Can both be closed?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109967/new/ https://reviews.llvm.org/D109967

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-27 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Yeah, I can confirm that many cases are improved with this patch (others are more sensitive and depend on the __bos behavior I mentioned). With the 17 kernel FORTIFY self-tests, all 17 fail without this patch. With this patch, 9 start passing. Nice! CHANGES SINCE LAST

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-27 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I'm setting up to test this patch (thank you!) using my current kernel FORTIFY improvements. Right now I have a bunch of compile-time behavior selftests written:

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-17 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Does this address https://bugs.llvm.org/show_bug.cgi?id=50322 ? (I assume this is a new version of https://reviews.llvm.org/D92657 ?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109967/new/ https://reviews.llvm.org/D109967

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-25 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. The kernel's [[ https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through | stance ]] on switch statements reads: | | All switch/case blocks must end in one of: break; fallthrough; continue; goto ; return

[PATCH] D68720: Support -fstack-clash-protection for x86

2020-08-28 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Ah! Yes, I see it now. Thanks and sorry for the noise! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

2020-08-27 Thread Kees Cook via Phabricator via cfe-commits
kees reopened this revision. kees added a comment. This revision is now accepted and ready to land. Sorry if I missed something here, but why is this marked as "Closed"? It seems like the feature has still not landed (i.e. it got reverted). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-06-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D80791#2120503 , @nickdesaulniers wrote: > Might someone wish to disable PAC/BTI on an individual function, while having > it on for the rest? I guess that would mean you can't call that function > indirectly? It would mean

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-06-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Specifically, this appears to be a legitimate bug, found by the warnings: https://bugs.llvm.org/show_bug.cgi?id=46258 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80791/new/ https://reviews.llvm.org/D80791

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-06-29 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Should the per-function analysis warning actually be removed? That seems like a helpful check to catch a different form of bad behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80791/new/

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

2019-08-15 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. For latest version see https://reviews.llvm.org/D64838 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63260/new/ https://reviews.llvm.org/D63260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65629: cfi-icall: Allow the jump table to be optionally made non-canonical.

2019-08-09 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Just FYI, I can confirm a happily running arm64 kernel with CFI enabled built with this patch series. The C wrappers aren't needed and CFI is still triggering on mismatches: [ 106.656280] lkdtm: Performing direct entry CFI_FORWARD_PROTO [ 106.657307] lkdtm: Calling

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Nick points out that "REQUIRES: x86-registered-target" is likely not needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 ___ cfe-commits mailing list

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-06-01 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: test/Analysis/asm-goto.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s 2>&1 | FileCheck %s + Based on Nathan's comments in another thread, it seems like this needs to be: ``` // REQUIRES:

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D56571#1386973 , @kees wrote: > Not sure if this is the fault of the LLVM half or the Clang half, but I'm > seeing mis-compilations in the current patches (llvm > ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 >

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. I reduced the C code to this: volatile int mystery = 0; static int noinline demo(int klen) { int err; int len; err = mystery * klen; if (err) return err; if (len > klen) len =

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-06 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Not sure if this is the fault of the LLVM half or the Clang half, but I'm seeing mis-compilations in the current patches (llvm ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 .185490 and clang 01879634f01bdbfac4636ebe03b68e85b20cd664