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

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. SGTM; please make a note of this new diagnostic flag in clang/docs/ReleaseNotes.rst under `Improvements to Clang's diagnostics`. Comment at:

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:376 + if (getTriple().isMIPS()) { +const Multilib = GCCInstallation.getMultilib(); +Path = Path + "/libc" + Multilib.osSuffix(); nickdesaulniers wrote: > It might

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:363 const StringRef TripleStr = GCCInstallation.getTriple().str(); - const Multilib = GCCInstallation.getMultilib(); + std::string Path = (InstallDir + "/../../../../" +

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:360-361 // CSKY toolchains use different names for sysroot folder. if (!GCCInstallation.isValid()) return std::string(); // GCCInstallation.getInstallPath() =

[PATCH] D134702: [Clang] Don't warn if deferencing void pointers in unevaluated context

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134702/new/ https://reviews.llvm.org/D134702

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. It's not clear to me why Arch is special here. The path may be different, but normalizing the triple seems unnecessary. Blocking this until I'm confident that

[PATCH] D133574: [C2x] reject type definitions in offsetof

nickdesaulniers added a comment. In D133574#3816066 , @nathanchance wrote: > In D133574#3815951 , @aaron.ballman > wrote: > >> LGTM, thank you! Please don't land until you have some indication from the >>

[PATCH] D134461: [Clang] Warn when trying to deferencing void pointers in C

nickdesaulniers added a comment. In D134461#3815458 , @aaron.ballman wrote: > In D134461#3815298 , @nathanchance > wrote: > >> This warning is quite noisy for the Linux kernel due to a couple of places >>

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

nickdesaulniers added a comment. Is it worth contacting the package maintainer for LLVM+clang for Arch-Linux in regards to this patch? Comment at: clang/lib/Driver/Distro.cpp:210 + // Sometimes the OS can't be detected from the triplet due to ambiguity, for + // eg.

[PATCH] D134362: [clang] Fix interaction between asm labels and inline builtins

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Please make sure to mark comments as "Done" in phab to help reviewers skip over stale feedback. Comment at: clang/lib/CodeGen/CGExpr.cpp:5055-5059 +

[PATCH] D134362: [clang] Fix interaction between asm labels and inline builtins

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:5056-5057 +std::string FDInlineName; +if (FD->hasAttr()) + FDInlineName = FD->getAttr()->getLabel().str(); +else I think you could use: ``` if (auto *A =

[PATCH] D134362: [clang] Fix interaction between asm labels and inline builtins

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/asm-label-inline-builtins.c:18-20 +// CHECK-NOT: @vprintf( +// CHECK-NOT: @__vprintf_chk +// CHECK-NOT: @vprintf.inline( please be consistent wrt. the trailing open paren `(`. Let's add a

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

nickdesaulniers added a comment. Follow up fix: https://reviews.llvm.org/D133946 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110869/new/ https://reviews.llvm.org/D110869 ___ cfe-commits mailing list

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

nickdesaulniers added a comment. @rsmith can we get some guidance here? Has your opinion changed in the time since GCC has been shipping this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

nickdesaulniers accepted this revision. nickdesaulniers added a comment. For the Linux kernel, we're only building v5+ continuously. It looks like the Linux kernel supports v4+ and v3m (for Acorn Risc-PC (Intel StrongARM(R) SA-110)). We've never been able to build that target, and it's not

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. @aaron.ballman should review+accept this before you land it, but I'm satisfied with the result. Thank you @inclyc for working on this! Repository: rG LLVM Github Monorepo

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10133-10135 + if (ImplicitMatch == ArgType::NoMatchPedantic || + ImplicitMatch == ArgType::NoMatchTypeConfusion) +Match = ImplicitMatch; nickdesaulniers wrote: >

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10133-10135 + if (ImplicitMatch == ArgType::NoMatchPedantic || + ImplicitMatch == ArgType::NoMatchTypeConfusion) +Match = ImplicitMatch; Similarly, if `ImplicitMatch`

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10127-10129 +if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion && +ImplicitMatch != ArgType::NoMatchTypeConfusion && +!IsCharacterLiteralInt) There's

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added inline comments. Comment at: clang/lib/AST/FormatString.cpp:401 + if (const auto *BT = argTy->getAs()) { +if (!Ptr) { + switch (BT->getKind()) { inclyc wrote: > nickdesaulniers wrote: > > aaron.ballman wrote: > > >

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added a comment. In D132568#3747971 , @inclyc wrote: >> Do we want to encode that in `test_promotion` in >> `clang/test/Sema/format-strings.c`? Seems like tests on shorts are missing. > > Tests for short and char "incompatibility" could

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

nickdesaulniers added a comment. In D132568#3746551 , @aaron.ballman wrote: > Thank you for the patch, but it'd be really helpful to me as a reviewer if > you and @nickdesaulniers could coordinate so there's only one patch trying to > address #57102

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. Prefer D132568 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132266/new/ https://reviews.llvm.org/D132266

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

nickdesaulniers created this revision. nickdesaulniers added a reviewer: aaron.ballman. Herald added a subscriber: emaste. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similar to commit

[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation

nickdesaulniers added a comment. https://llvm-compile-time-tracker.com/compare.php?from=0299ebc1bdc9fca176ebcacd590c90dc77a47551=333771f3558c81390a0e5d715ad8d1e419050b66=cycles should be the link, once it gets picked up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation

This revision was automatically updated to reflect the committed changes. Closed by commit rG333771f3558c: [Sema] Avoid isNullPointerConstant invocation (authored by justinstitt, committed by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Awesome! This makes a measurable improvement in my Linux kernel build times! Thanks for the patch, and thanks @rtrieu for suggestions that will improve C++ code compilation times as well. I'll commit this for you since

[PATCH] D131416: [Clang][BinaryOperator] cache ICEKind

nickdesaulniers added inline comments. Comment at: clang/include/clang/AST/Expr.h:30 #include "clang/Basic/TypeTraits.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/APFloat.h" Please keep the list of `#include`s alphabetically sorted.

[PATCH] D131012: No longer place const volatile global variables in a read only section

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the patch. I've asked some of my colleagues who work on libabigail their thoughts on the implications of this change. Due to timezones, it may take some time to

[PATCH] D129954: [CodeGen][inlineasm] assume the flag output of inline asm is boolean value

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129954/new/ https://reviews.llvm.org/D129954 ___ cfe-commits

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. LGTM; thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130754/new/ https://reviews.llvm.org/D130754

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:9 // RUN: --check-prefixes=CHECK,CHECK-EXTERN +// RUN: %clang_cc1 -std=gnu2x -triple x86_64-linux-gnu %s -emit-llvm -o - \ +// RUN: -mfunction-return=thunk-extern -fprofile-arcs \

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG140bfdca60ae: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444743. nickdesaulniers added a comment. - resort langref, oops Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/ https://reviews.llvm.org/D129709 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added a comment. Thanks for the review! Comment at: clang/test/CodeGen/attr-function-return.c:9 // RUN: --check-prefixes=CHECK,CHECK-EXTERN +// RUN: %clang_cc1 -std=gnu2x -triple x86_64-linux-gnu %s -emit-llvm -o - \ +// RUN: -mfunction-return=thunk-extern

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444742. nickdesaulniers marked 2 inline comments as done. nickdesaulniers added a comment. - rename md idententifier, sort langref Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444725. nickdesaulniers added a comment. - rebase on top of D129691 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/ https://reviews.llvm.org/D129709 Files:

[PATCH] D129691: [clang][test] fix typo in fn attr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2792e7d37c4: [clang][test] fix typo in fn attr (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers updated this revision to Diff 444695. nickdesaulniers added a comment. - don't reset captured vars, that was a copy+paste mistake I made in the original patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129691/new/

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:107 +// CHECK-GCOV: @__llvm_gcov_init() unnamed_addr [[EXTERNGCOV]] +// CHECK-ASAN: @asan.module_ctor() [[EXTERNASAN:#[0-9]+]] +// CHECK-TSAN: @tsan.module_ctor() [[EXTERNTSAN:#[0-9]+]]

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers created this revision. nickdesaulniers added reviewers: MaskRay, void. Herald added subscribers: jsji, StephenFan, jdoerfert, pengfei, hiraditya. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added projects: clang, LLVM. Herald added

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:18 // CHECK: @keep2() [[KEEP:#[0-9]+]] [[gnu::function_return("keep")]] void keep2(void) {} probably don't want to reset `KEEP` Comment at:

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers created this revision. nickdesaulniers added reviewers: aaron.ballman, erichkeane, MaskRay. Herald added subscribers: jsji, StephenFan, pengfei. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:42 +// CHECK: @double_keep_thunk2() [[EXTERN]] +[[gnu::function_return("thunk-keep")]][[gnu::function_return("thunk-extern")]] +void double_keep_thunk2(void) {} I just

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers added a comment. In D129572#3645934 , @erichkeane wrote: > Our typical rule is to keep the 1st one I think, and reject the 2nd. But then the _codegen_ will differ from GCC. And we _want_ clang to be a drop in replacement, so differing

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers added a comment. In D129572#3645895 , @erichkeane wrote: > Did a post-commit review on the CFE changes, and all look OK to me. Thanks for the review! > That FIXME is a shame, we should see if we can fix that ASAP. We should AT >

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers added a comment. In D129572#3645660 , @nickdesaulniers wrote: > - add more links to commit message as they come in Oh, right, updating the commit message doesn't update the description in phab. Oops! Repository: rG LLVM Github

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2240d72f15f3: [X86] initial -mfunction-return=thunk-extern support (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers updated this revision to Diff 443986. nickdesaulniers added a comment. - add more links to commit message as they come in Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129572/new/ https://reviews.llvm.org/D129572 Files:

[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers created this revision. Herald added subscribers: jsji, jdoerfert, pengfei, hiraditya, mgorny. Herald added a reviewer: aaron.ballman. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald

[PATCH] D119296: KCFI sanitizer

nickdesaulniers accepted this revision. nickdesaulniers added a comment. I see you modified the mir parser & printer; consider adding a .mir test. Still LGTM. Might be nice to document the generated asm more for other compiler vendors to better understand the implementation, rather than having

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

nickdesaulniers added a comment. 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 created? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/

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

nickdesaulniers added a comment. In D126864#3564519 , @efriedma wrote: >> As for SOCK_MAXADDRLEN, that's a horrid hack, and the definition of struct >> sockaddr needs to change. :) > > Do we want some builtin define so headers can check if we're in >

[PATCH] D126137: [X86] Add support for `-mharden-sls=[none|all|return|indirect-jmp]`

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jsji. Thanks again Phoebe for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/

[PATCH] D126137: [X86] Add support for `-mharden-sls=[none|all|return|indirect-jmp]`

nickdesaulniers requested changes to this revision. nickdesaulniers added a subscriber: aaron.ballman. nickdesaulniers added a comment. This revision now requires changes to proceed. Let's get this rebased on top of D126511 . Comment at:

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:368 -Select straight-line speculation hardening scope +Select straight-line speculation hardening scope (AArch64/X86 only). must be 'all', 'none', 'retbr'(AArch64), 'blr'(AArch64),

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Nice work Phoebe; thank you very much for this patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added a comment. Diff 431607 boots for me in QEMU (triple checked that CONFIG_CC_HAS_SLS=y AND CONFIG_SLS=y were set this time ;) ). Comment at: clang/docs/ReleaseNotes.rst:371 +- Support ``-mharden-sls=all`` for X86. + pengfei wrote: >

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added a comment. In D126137#3530777 , @kristof.beyls wrote: > Therefore, I wonder if it wouldn't be better to name this -mharden-sls=retbr > for more consistency across architectures? I think it's best to maintain compatibility with

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added a comment. Thanks for the patch! > A tangential question is do we need these two options expect all? For the Linux kernel's use, no. But I think it would extremely simple to implement. Instead of having one `SubtargetFeature` (`FeatureHardenSlsAll`), you'd have two (say

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Don't forget to rebase this on top of https://reviews.llvm.org/D125421. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. pushed 45e01ce5fe6a5e4dc25ffdf626caa344fbcb93dd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG45e01ce5fe6a: [clang] Avoid suggesting typoed directives in `.S` files (authored by ken-matsui, committed by nickdesaulniers). Repository: rG

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. sorted, will commit soon Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727 ___ cfe-commits mailing list

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. Sorry, it looks like `arcanist` or my PHP runtime was auto updated on my host and has regressed. I need to sort that out first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. > I do not have permission to land this patch, so could you please do it on my > behalf? Will do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s + These three can be removed now as well. Clang will read `%s` as input without the need to read from

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + nickdesaulniers wrote: > nickdesaulniers wrote: > > Consider adding a test: > > > > ``` > > // RUN:

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + nickdesaulniers wrote: > Consider adding a test: > > ``` > // RUN: %clang_cc1 -fsyntax-only -verify

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:484 const SourceLocation ) const { + // If this is a `.S` file, treat unknown # directives as non-preprocessor + // directives. Also,

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + Consider adding a test: ``` // RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added a comment. In D124836#3507268 , @void wrote: > I'll split off the TableGen changes into a separate patch. It will supersede > those changes here, so it shouldn't delay other reviews here. :-) I'm referring to the changes to

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added a comment. I still think this would be easier to review if the `isArgumentRegister` tablegen changes were separated out into a distinct parent patch and then the existing x86 implementation updated to use, then this would rebased on top of as a child patch.

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

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Worth mentioning the deprecation plans in `clang/docs/ReleaseNotes.rst` and updaing the commit message+description on phab? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125167: [WIP] Fix member access of anonymous struct/union fields in C

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Consider adding an AST test; that was something that came up in my patch https://reviews.llvm.org/D95408. > I'm pushing back on the WG14 lists to see if this is a good

[PATCH] D95408: [Sema][C] members of anonymous struct inherit QualType

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. Herald added a project: All. Prefer https://reviews.llvm.org/D125167. Comment at: clang/test/AST/ast-dump-decl.c:132 +// CHECK: IndirectFieldDecl{{.*}} Test48755Const 'int' +// CHECK-NEXT: Field{{.*}}

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

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. It looks like clang still produces meaningful diagnostics from `-Wuninitialized` and `-Wsometimes-uninitialized` even with `-ftrivial-auto-var-init=zero`. Same for

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:776-778 + // For GPRs, we only care to clear out the 64-bit register. + if (MCRegister XReg = getRegisterOrZero(Reg)) +GPRsToZero.set(XReg);

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:682 +// The called routine is expected to preserve r19-r28 +// r29 and r30 are used as frame pointer and link register resp. +return 0; What happens

[PATCH] D119296: KCFI sanitizer

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. In D119296#3481573 , @joaomoreira wrote: > I'm not an expert on LLVM's pipeline, but it just feels a little awkward and >

[PATCH] D119296: KCFI sanitizer

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. (tentatively removing my +1 since this patch has changed quite a bit since my initial review) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124211: Add __builtin_call_kcfi_unchecked

nickdesaulniers added a comment. In D124211#3480709 , @samitolvanen wrote: > In D124211#3480652 , > @nickdesaulniers wrote: > >> Can you link to the lore thread on discussions around the builtin? > > There's no

[PATCH] D124211: Add __builtin_call_kcfi_unchecked

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:436 + + auto Call = dyn_cast_or_null(BuiltinCall->getArg(0)); + nickdesaulniers wrote: > `auto *Call =` >

[PATCH] D124211: Add __builtin_call_kcfi_unchecked

nickdesaulniers added a comment. Can you link to the lore thread on discussions around the builtin? Comment at: clang/lib/Sema/SemaChecking.cpp:436 + + auto Call = dyn_cast_or_null(BuiltinCall->getArg(0)); + `auto *Call =`

[PATCH] D119296: KCFI sanitizer

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. LGTM; it would be good if you could provide steps to test this on the Linux kernel. (i.e. what kernel patches are needed). Consider waiting for at least one additional

[PATCH] D119296: KCFI sanitizer

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2255-2260 + for (const char : Name) { +if (llvm::isAlnum(C) || C == '_' || C == '.') + continue; +return false; + } + return true; samitolvanen wrote: >

[PATCH] D119296: KCFI sanitizer

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:553-554 + if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) { +EmitKCFIConstants(); +ClearUnusedKCFIPrefixes(); + } This is the only call site for these two methods,

[PATCH] D119296: KCFI sanitizer

nickdesaulniers added inline comments. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp: + + const MCSectionELF = static_cast(TextSec); + unsigned Flags = ELF::SHF_LINK_ORDER; nickdesaulniers wrote: > or just `cast`. >

[PATCH] D119296: KCFI sanitizer

nickdesaulniers added a comment. Please add a test where calls to `@llvm.kcfi.check` produce `KCFI_CHECK` during instruction selection. (You can use `-stop-before=finalize-isel` to dump the IR prior to isel; this can stay a .ll test, I think, rather than .mir). Comment at:

[PATCH] D123874: [Clang][IA] support -generate-unused-section-symbols={yes|no}

nickdesaulniers created this revision. nickdesaulniers added a reviewer: MaskRay. Herald added subscribers: StephenFan, dexonsmith, hiraditya. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits,

[PATCH] D123649: Allow flexible array initialization in C++.

nickdesaulniers added a comment. For posterity, posting the link to the failure this caused in Linaro's TCWG's CI of kernel builds w/ clang: https://lore.kernel.org/llvm/906914372.14298.1650022522881@jenkins.jenkins/. I'm guessing D123826 fixes this?

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

nickdesaulniers added a comment. > if the developer uses default initialization for a randomized structure. Yeah, I suspect that @aaron.ballman @lebedev.ri @xbolva00 missed that this just extends the newly added opt-in `-frandomize-layout-seed=` (https://reviews.llvm.org/D121556). Why a

[PATCH] D123308: Handle a subtle hole in inline builtin handling

nickdesaulniers added a comment. Also, for the initial lines of commit messages, please try to make them less ambiguous about which parts of the project they touch. For example `[Clang][Fortify] drop inline decls when redeclared` is more descriptive should this get bucketed with other commits

[PATCH] D123308: Handle a subtle hole in inline builtin handling

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the quick fix! Comment at: clang/lib/CodeGen/CGExpr.cpp:4968-4969 // name to make it clear it's not the actual builtin. -if

[PATCH] D121556: [randstruct] Add randomize structure layout support

nickdesaulniers added a comment. In D121556#3427620 , @void wrote: > In D121556#3419184 , @aaron.ballman > wrote: > >> Generally I think things are looking pretty good, but there's still an open >> question and

[PATCH] D118052: [X86] Fix CodeGen Module Flag for -mibt-seal

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/X86/x86-cf-protection.c:5-16 +// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S -fcf-protection=branch -flto %s | FileCheck %s --check-prefix=NOIBTSEAL +// RUN: %clang -target

[PATCH] D122189: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5a2e56b70e2f: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement (authored by nickdesaulniers). Repository: rG LLVM

[PATCH] D122189: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement

nickdesaulniers updated this revision to Diff 417661. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - fix incorrect REQUIRES Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122189/new/

[PATCH] D122325: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. D122189 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122325/new/ https://reviews.llvm.org/D122325

<    1   2   3   4   5   6   7   8   9   10   >