[PATCH] D151388: [HWASan] use hwasan linker for Android 14+

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151388/new/ https://reviews.llvm.org/D151388 ___

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

pcc added a comment. In D149215#4303197 , @tejohnson wrote: > In D149215#4303149 , @pcc wrote: > >>> Adds an LTO option >> >> Usual question: does it need to be an option? Could the allocator expose a >> symbol

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

pcc added a comment. > Adds an LTO option Usual question: does it need to be an option? Could the allocator expose a symbol such as `__malloc_hot_cold` that the linker could check for in the symbol table? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129700: [clang] Don't emit type tests for dllexport/import classes

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129700/new/ https://reviews.llvm.org/D129700 ___

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

This revision was automatically updated to reflect the committed changes. Closed by commit rG201fdef40dd6: libclang: Pass Clang install directory to driver via argv[0]. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D146497?vs=506832=507525#toc Repository: rG LLVM

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

pcc added inline comments. Comment at: clang/tools/libclang/CIndex.cpp:4022 + llvm::sys::path::append(ClangPath, "bin"); + llvm::sys::path::append(ClangPath, "clang"); + pcc wrote: > aaron.ballman wrote: > > I suspect this doesn't matter *too* much, but... on

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

pcc added inline comments. Comment at: clang/tools/libclang/CIndex.cpp:4022 + llvm::sys::path::append(ClangPath, "bin"); + llvm::sys::path::append(ClangPath, "clang"); + aaron.ballman wrote: > I suspect this doesn't matter *too* much, but... on Windows,

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

pcc created this revision. pcc added reviewers: bkramer, aaron.ballman. Herald added subscribers: mikhail.ramalho, arphaman. Herald added a project: All. pcc requested review of this revision. Herald added a project: clang. Various driver features, such as the sysroot path detection for Android

[PATCH] D129700: [clang] Don't emit type tests for dllexport/import classes

pcc added a comment. Can't the code in CodeGenModule::HasHiddenLTOVisibility be moved here instead of duplicating it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129700/new/ https://reviews.llvm.org/D129700

[PATCH] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144035/new/ https://reviews.llvm.org/D144035 ___

[PATCH] D144035: [hwasan] Ensure hwasan aliases do not have ODR linkage

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. Passes shouldn't be replacing aliases with aliasees in general, see D66606 . The right fix is to fix SCEV to not do that. Repository: rG LLVM Github

[PATCH] D143634: [ModuleUtils] Assert correct linkage and visibility of structors' COMDAT key

pcc added a comment. I've always considered that this should be fixed by extending the resolution-based LTO API to also include resolutions for comdats. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143634/new/ https://reviews.llvm.org/D143634

[PATCH] D139395: Add CFI integer types normalization

pcc accepted this revision. pcc added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D139395: Add CFI integer types normalization

pcc accepted this revision. pcc added a comment. LGTM with nits Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1694 + getCXXABI().getMangleContext().mangleTypeName( + T, Out, !!getCodeGenOpts().SanitizeCfiICallNormalizeIntegers); + Is the !! necessary

[PATCH] D139395: Add CFI integer types normalization

pcc added a comment. In D139395#4066948 , @samitolvanen wrote: > Thanks for the patch, Ramon. This looks like a reasonable approach to me, and > just for reference, here appears to be the corresponding rustc change: > >

[PATCH] D139395: Add CFI integer types normalization

pcc added a comment. In D139395#3990772 , @rcvalle wrote: > I elaborated on the reasons why not use a generalized encoding in the design > document in the tracking issue > https://github.com/rust-lang/rust/issues/89653. The tl;dr; is that it will >

[PATCH] D139395: Add CFI integer types normalization

pcc added a reviewer: pcc. pcc added a comment. A high level question is whether we want to base the cross-language encoding on Itanium at all. Itanium has concepts such as substitutions that will complicate the implementation in other languages. Encoding pointee types can also lead to

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

pcc added a comment. In D138337#3972138 , @samitolvanen wrote: > In D138337#3972009 , @pcc wrote: > >> Can't this be implicit if LTO is being used? > > I would prefer to keep this behind a flag (similarly to

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

pcc added a comment. Can't this be implicit if LTO is being used? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits mailing list

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

pcc added a comment. Does that DR apply retroactively to C++17? I get the impression that "Status: C++20" means that the issue was only fixed in C++20, which would make this well-formed with `-std=c++17`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135421: Driver: Change default Android linker to lld.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8d9c4a7425d9: Driver: Change default Android linker to lld. (authored by pcc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135421: Driver: Change default Android linker to lld.

pcc added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135421/new/ https://reviews.llvm.org/D135421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135421: Driver: Change default Android linker to lld.

pcc created this revision. pcc added reviewers: srhines, rprichard, danalbert. Herald added subscribers: danielkiss, cryptoad. Herald added a project: All. pcc requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. The clang distributed with the

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

pcc added a comment. Ping^2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120862/new/ https://reviews.llvm.org/D120862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM > Yes, but not indirectly called from C/C++ but rather from compiler-generated > code right? That's presumably why this patch + D116130 > worked for

[PATCH] D119296: KCFI sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6694 - if (isExternallyVisible(T->getLinkage())) { + if (isExternallyVisible(T->getLinkage()) || !OnlyExternal) { std::string OutName; It would be better to have a separate

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. Okay, it seems reasonable enough to have the `[[clang::lto_visibility_public]]` attribute override the `--lto-whole-program-visibility` flag. What I'm not sure about though is whether

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

pcc added a comment. In D127876#3586154 , @aeubanks wrote: > In D127876#3586134 , @pcc wrote: > >> This diverges from the documented behavior of >> `-lto-whole-program-visibility`. The flag is meant to give all

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. This diverges from the documented behavior of `-lto-whole-program-visibility`. The flag is meant to give all classes hidden LTO visibility, but now it only does that to some of them.

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

pcc added inline comments. Comment at: llvm/tools/gold/gold-plugin.cpp:44-52 // FIXME: remove this declaration when we stop maintaining Ubuntu Quantal and // Precise and Debian Wheezy (binutils 2.23 is required) #define LDPO_PIE 3 #define LDPT_GET_SYMBOLS_V3 28 //

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

pcc added a comment. In D120727#3512177 , @paulkirth wrote: > Hi, we're seeing some failures in Fuchsia's Clang CI. Our runtimes build > seems to be unable to find `cxxabi.h`. > > The failing bot can be found here: >

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1843 + + auto *FTRTTIProxy = new llvm::GlobalVariable( + TheModule, Addr->getType(), ychen wrote: > pcc wrote: > > Are these proxy variables necessary? I think that now that we have

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1843 + + auto *FTRTTIProxy = new llvm::GlobalVariable( + TheModule, Addr->getType(), Are these proxy variables necessary? I think that now that we have custom code generation for

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

pcc added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120862/new/ https://reviews.llvm.org/D120862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D119296: KCFI sanitizer

pcc added a comment. In D119296#3467905 , @samitolvanen wrote: > In D119296#3466027 , @joaomoreira > wrote: > >> I played a little bit with kcfi and here are some thoughts: >> >> - under -Os I saw functions

[PATCH] D119296: KCFI sanitizer

pcc added a comment. > Note that if additional data has been injected between the KCFI > type identifier and the start of the function, e.g. by using > -fpatchable-function-entry, the offset in bytes must be specified > using -fsanitize-kcfi-offset= to avoid errors. The offset > must be the same

[PATCH] D85649: [AArch64] PAC/BTI code generation for LLVM generated functions

pcc added a comment. Herald added a project: All. In D85649#2281439 , @chill wrote: > It looks like the only value that makes sense is `Error` - any other policy > (existing or not) would potentially lead to meaningfully different code > generated with

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

pcc created this revision. pcc added a reviewer: rsmith. Herald added a project: All. pcc requested review of this revision. Herald added a project: clang. Fixes pr54158. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D120862 Files: clang/lib/Sema/SemaOverload.cpp

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

pcc added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116774/new/ https://reviews.llvm.org/D116774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG82e5f951fd6e: AST: Move __va_list tag back to std conditionally on AArch64. (authored by pcc). Changed prior to commit:

[PATCH] D116773: AST: Make getEffectiveDeclContext() a member function of ItaniumMangleContextImpl. NFCI.

This revision was automatically updated to reflect the committed changes. Closed by commit rG18ead23385a4: AST: Make getEffectiveDeclContext() a member function of… (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D116773?vs=398007=409733#toc Repository: rG LLVM Github

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

pcc marked 2 inline comments as done. pcc added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116774/new/ https://reviews.llvm.org/D116774 ___ cfe-commits mailing list

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

pcc updated this revision to Diff 409716. pcc added a comment. Use isARM() etc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116774/new/ https://reviews.llvm.org/D116774 Files: clang/lib/AST/ASTContext.cpp clang/lib/AST/ItaniumMangle.cpp

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

pcc added a comment. In D115844#3321235 , @ychen wrote: > In D115844#3321190 , @pcc wrote: > >> On the bug you have: >> >> define internal fastcc void >> @_Z4callIiE4taskv.resume(%_Z4callIiE4taskv.Frame*

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

pcc added a comment. On the bug you have: define internal fastcc void @_Z4callIiE4taskv.resume(%_Z4callIiE4taskv.Frame* noalias nonnull align 8 dereferenceable(24 ) %FramePtr) #1 prologue <{ i32, i32 }> <{ i32 846595819, i32 trunc (i64 sub (i64 ptrtoint (i8** @1 to i64), i64 ptrtoint

[PATCH] D119367: [HWASan] Allow no_sanitize(..) and change metadata passing.

pcc added a comment. In addition to the .bc support we will also need support for reading/writing .ll files. Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1721 -static DenseSet getExcludedGlobals(Module ) { - NamedMDNode *Globals =

[PATCH] D119296: KCFI sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3168 + -1); + llvm::Value *Test = Builder.CreateICmpEQ(Builder.CreateLoad(HashPtr), Hash); + llvm::BasicBlock *ContBB = createBasicBlock("kcfi.cont"); pcc wrote: > samitolvanen wrote: > >

[PATCH] D119296: KCFI sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3168 + -1); + llvm::Value *Test = Builder.CreateICmpEQ(Builder.CreateLoad(HashPtr), Hash); + llvm::BasicBlock *ContBB = createBasicBlock("kcfi.cont"); samitolvanen wrote: > pcc wrote: > >

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

pcc updated this revision to Diff 407227. pcc retitled this revision from "AST: Move __va_list tag to the top level on ARM architectures." to "AST: Move __va_list tag back to std conditionally on AArch64.". pcc edited the summary of this revision. pcc added a comment. Make it conditional

[PATCH] D119296: KCFI sanitizer

pcc added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3168 + -1); + llvm::Value *Test = Builder.CreateICmpEQ(Builder.CreateLoad(HashPtr), Hash); + llvm::BasicBlock *ContBB = createBasicBlock("kcfi.cont"); We considered a scheme like this

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118171/new/ https://reviews.llvm.org/D118171 ___

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

pcc added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:842 + if (!Args.hasArg(options::OPT_shared)) +SharedRuntimes.push_back("hwasan-preinit"); } morehouse wrote: > eugenis wrote: > > pcc wrote: > > > Shouldn't it be

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

pcc added a comment. What guarantee do we have that the `__hwasan_init` .preinit_array entry will be placed before the user's .preinit_array entries? Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:842 + if (!Args.hasArg(options::OPT_shared)) +

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

pcc added a comment. I'm not aware of any of those places causing an actual problem though? The AST isn't a stable interface, and __builtin_dump_struct is for debugging purposes only. I would expect debug info consumers to be able to handle __va_list in the global namespace as this is the

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

pcc added a comment. In D104830#3225370 , @jrtc27 wrote: > Ping? Sorry for the delay, D116774 should fix this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

pcc created this revision. pcc added reviewers: rsmith, eugenis, jrtc27. Herald added a subscriber: kristof.beyls. pcc requested review of this revision. Herald added a project: clang. In post-commit feedback on D104830 Jessica Clarke pointed out that

[PATCH] D116773: AST: Make getEffectiveDeclContext() a member function of ItaniumMangleContextImpl. NFCI.

pcc created this revision. pcc added reviewers: eugenis, rsmith, jrtc27. pcc requested review of this revision. Herald added a project: clang. In an upcoming change we are going to need to access mangler state from the getEffectiveDeclContext() function. Therefore, make it a member function of

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108479/new/ https://reviews.llvm.org/D108479 ___

[PATCH] D115015: CodeGen: Strip exception specifications from function types in CFI type names.

pcc created this revision. pcc added a reviewer: eugenis. pcc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With C++17 the exception specification has been made part of the function type, and therefore part of mangled type names.

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

pcc added a comment. Agreed that this should probably be done in the mangler, I'll try to take a look next week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/ https://reviews.llvm.org/D104830

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc added a comment. In D108479#3129577 , @rjmccall wrote: > In D108479#3129571 , @jrtc27 wrote: > >> For CHERI there's the added complication that descriptors and trampolines >> can exist for security reasons

[PATCH] D113738: [LTO] Allow passing -Os/-Oz as the optimization level

pcc added a comment. Please see D63976 where we rejected a similar change in favor of just letting this be controllable at compile time. To the extent that the pass pipeline is affected by the size optimization level, I think we should change the passes so

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc added subscribers: rjmccall, rsmith. pcc added a comment. (Adding back @rsmith, @rjmccall.) In D108479#3113035 , @samitolvanen wrote: > In D108479#3112492 , @rjmccall > wrote: > >> You could also make this

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc added a comment. > Maybe it should even be semantically restricted to require a constant decl > reference as its operand? I think we should do this. Maybe it should be named something like `__builtin_symbol_address` if we're intending for this to have an effect with PAuth as well?

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc requested changes to this revision. pcc added subscribers: rjmccall, rsmith. pcc added a comment. This revision now requires changes to proceed. In D108479#3108360 , @ardb wrote: > I would argue that the existing __builtin_addressof() should absorb

[PATCH] D112761: cfi-icall: Add -fsanitize-cfi-promotion-aliases

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. I asked @samitolvanen out-of-band to check whether this really needs a flag since it seems like there could be some underlying issue that needs to be resolved so that we can do this

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/APValue.cpp:133 +void APValue::LValueBase::setNoCFIValue(bool NoCFI) { + if (is()) { +Local.NoCFIValue = NoCFI; Remove braces

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

pcc added subscribers: thomasanderson, thakis. pcc added a comment. I believe that Chromium uses it (or at least it did at the time that I added this, and Chromium has since switched to using libc++ on Windows). I don't work on Chromium much anymore but perhaps @thakis or @thomasanderson can

[PATCH] D107850: [asan] Implemented intrinsic for the custom calling convention similar used by HWASan for X86.

pcc added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1640 +def int_asan_check_memaccess : + Intrinsic<[],[llvm_ptr_ty, llvm_i8_ty, llvm_i8_ty], kstoimenov wrote: > eugenis wrote: > > vitalybuka wrote: > > > pcc wrote: > > > > eugenis

[PATCH] D107850: [asan] Implemented intrinsic for the custom calling convention similar used by HWASan for X86.

pcc added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1640 +def int_asan_check_memaccess : + Intrinsic<[],[llvm_ptr_ty, llvm_i8_ty, llvm_i8_ty], eugenis wrote: > vitalybuka wrote: > > kstoimenov wrote: > > > vitalybuka wrote: > > > >

[PATCH] D108223: gn build: Build libclang.so on ELF platforms.

This revision was automatically updated to reflect the committed changes. Closed by commit rGb2e77cd095a6: gn build: Build libclang.so and libLTO.so on ELF platforms. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D108223?vs=366966=367318#toc Repository: rG LLVM Github

[PATCH] D108223: gn build: Build libclang.so on ELF platforms.

pcc added inline comments. Comment at: llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn:16 - # For now, make libclang a static lib there. - libclang_target_type = "static_library" -} else { thakis wrote: > probably should update these too: > > ``` > % rg

[PATCH] D108223: gn build: Build libclang.so on ELF platforms.

pcc updated this revision to Diff 366966. pcc added a comment. Herald added subscribers: ormris, steven_wu, hiraditya. Also libLTO.so Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108223/new/ https://reviews.llvm.org/D108223 Files:

[PATCH] D108223: gn build: Build libclang.so on ELF platforms.

pcc created this revision. pcc added a reviewer: thakis. pcc requested review of this revision. Herald added a project: LLVM. This requires changing the ELF build to enable -fPIC, consistent with other platforms. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108223 Files:

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

pcc accepted this revision. pcc added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104058/new/ https://reviews.llvm.org/D104058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D94355: [Passes] Add relative lookup table converter pass

pcc added inline comments. Herald added a subscriber: ormris. Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:73 + return false; + +// If an operand in the lookup table is not dso_local, In the version of the patch that you committed,

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe655e74a318e: AST: Create __va_list in the std namespace even in

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

pcc updated this revision to Diff 354130. pcc added a comment. Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/ https://reviews.llvm.org/D104830 Files: clang/lib/AST/ASTContext.cpp clang/test/CodeGen/aarch64-varargs.c

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

pcc created this revision. pcc added reviewers: rsmith, eugenis. pcc requested review of this revision. Herald added a project: clang. This ensures that the mangled type names match between C and C++, which is significant when using -fsanitize=cfi-icall. Ideally we wouldn't have created this

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGen/thinlto-cfi-icall-static-inline-asm.c:3 + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -flto=thin -fsanitize=cfi-icall

[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103845/new/ https://reviews.llvm.org/D103845 ___

[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

pcc added a comment. Thanks for tracking this down. It seems best to change the Printf call to add the newline, since otherwise it looks like you'd be adding a spurious newline to the other callers of `RenderFrame` on Fuchsia. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

pcc added a comment. In D103845#2806211 , @leonardchan wrote: > In D103845#2804441 , @pcc wrote: > >> This isn't how the output looks on Android. Are you sure this isn't a >> Fuchsia-specific bug in the output

[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. This isn't how the output looks on Android. Are you sure this isn't a Fuchsia-specific bug in the output formatting? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

pcc added a comment. Isn't the bug here that module hashing is using `hash_code`? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

pcc added a comment. This warning seems to have a lot of false positives on things like reference arguments that are used as output parameters. For example here is a small sample of output from a stage2 build of part of LLVM: In file included from ../llvm/lib/BinaryFormat/Minidump.cpp:9:

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

pcc added a comment. This causes us to reject the following (reduced from AOSP): int sprintf(char* __s, const char* __fmt, ...) __attribute__((__format__(printf, 2, 3))) ; int sprintf(char* dest, const char* format) __attribute__((overloadable))

[PATCH] D63908: hwasan: Improve precision of checks using short granule tags.

pcc added inline comments. Comment at: compiler-rt/trunk/lib/hwasan/hwasan_checks.h:76 +#endif + return *(u8 *)(ptr | (kShadowAlignment - 1)) == ptr_tag; +} xiangzhangllvm wrote: > Hello @pcc I think here seems some problem, the ptr is user passing point, >

[PATCH] D91466: [WIP][clang][Fuchsia] Support HWASan for Fuchsia

pcc added a comment. How does Zircon handle tagged addresses in syscalls? Are they handled equivalently to Linux's tagged address ABI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91466/new/ https://reviews.llvm.org/D91466

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

pcc added a comment. I agree with @MaskRay that this should be a binutils-specific option. The flag `-mlinker-version` seems to have been designed around macOS-specific assumptions i.e. there is a single linker (ld64) and that the linker and assembler are not version coupled. Having this

[PATCH] D90424: AArch64: Use SBFX instead of UBFX to extract address granule in outlined HWASan checks.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc9b1a2b41dca: AArch64: Use SBFX instead of UBFX to extract address granule in outlined HWASan… (authored by pcc). Repository: rG LLVM Github

[PATCH] D90422: AArch64: Switch to x20 as the shadow base register for outlined HWASan checks.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3859fc653fb4: AArch64: Switch to x20 as the shadow base register for outlined HWASan checks. (authored by pcc). Repository: rG LLVM Github

[PATCH] D46791: Make -gsplit-dwarf generally available

pcc added a comment. Correct, clang no longer uses objcopy for this as of D47093 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46791/new/ https://reviews.llvm.org/D46791 ___ cfe-commits mailing list

[PATCH] D90422: AArch64: Switch to x20 as the shadow base register for outlined HWASan checks.

pcc added a comment. For the kernel I measured a small regression in boot time (with a version of this change that uses x20 for the v1 checks as well since the kernel doesn't use short granules yet) -- from 6.65s to 6.70s or 0.8%. But that's a fraction of the size gains which were 4% for

[PATCH] D90424: AArch64: Use SBFX instead of UBFX to extract address granule in outlined HWASan checks.

pcc created this revision. pcc added reviewers: eugenis, hctim. Herald added subscribers: cfe-commits, danielkiss, hiraditya, kristof.beyls. Herald added projects: clang, LLVM. pcc requested review of this revision. In a kernel (or in general in environments where bit 55 of the address is set)

[PATCH] D90422: AArch64: Switch to x20 as the shadow base register for outlined HWASan checks.

pcc created this revision. pcc added reviewers: eugenis, hctim. Herald added subscribers: Sanitizers, cfe-commits, danielkiss, hiraditya, kristof.beyls. Herald added projects: clang, Sanitizers, LLVM. pcc requested review of this revision. >From a code size perspective it turns out to be better

[PATCH] D89766: Driver: Add integer sanitizers to trapping group automatically.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc5acd3490b79: Driver: Add integer sanitizers to trapping group automatically. (authored by pcc). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D89766: Driver: Add integer sanitizers to trapping group automatically.

pcc created this revision. pcc added reviewers: jfb, eugenis. Herald added a project: clang. pcc requested review of this revision. In D86000 we added a new sanitizer to the integer group without adding it to the trapping group. This broke usage of

[PATCH] D87717: [docs] Update ControlFlowIntegrity.rst.

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87717/new/ https://reviews.llvm.org/D87717 ___

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

pcc added a comment. Hi, thanks for getting started on upstreaming this! But I was wondering: shouldn't this be the *last* patch? I was imagining that you would first upstream support for the `-fptrauth-*` flags, and then at the end you would add an `arm64e` subarch that turns them on by

  1   2   3   4   >