[PATCH] D45588: Start reserving x18 by default on Android targets.

This revision was automatically updated to reflect the committed changes. Closed by commit rC340889: Start reserving x18 by default on Android targets. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D45588?vs=142448&id

[PATCH] D51905: Front-end of the implementation of the interleaving algorithm

pcc added a subscriber: cfe-commits. pcc added a reviewer: rsmith. pcc added inline comments. Comment at: clang/lib/CodeGen/CGCXXABI.h:53 + // interleaved layout is decided. + bool EnableVTableInterleaving; + Why does this need to be stored separately on the CG

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

pcc added a comment. Can you update this patch please? Then I will commit. https://reviews.llvm.org/D50372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

This revision was automatically updated to reflect the committed changes. Closed by commit rC341989: Introduce the VTable interleaving scheme to the CFI design documentation (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D50372?vs=164964&id=164967#toc Repos

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

pcc added a subscriber: cfe-commits. pcc added inline comments. Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3215 // Emit the standard library with external linkage. llvm::GlobalVariable::LinkageTypes Linkage; if (IsStdLib) `IsStdLib` will always be false a

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

pcc added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:211-224 + std::string ErrorMsg1; + EngineBuilder builder1(std::move(Owner1)); + builder1.setMArch(MArch); + builder1.setMCPU(getCPUStr()); + builder1.setMAttrs(getFeatureList()); + b

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D49109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. Sorry, I noticed that this patch is missing a test case. Can you add one please? https://reviews.llvm.org/D49109 ___ cfe-commits mailing list

[PATCH] D49704: Fix typo in test/CodeGen/Mips/dins.ll

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D49704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D49109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49745: Generate fundamental type info with weak linkage

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. As I mentioned in https://bugs.llvm.org/show_bug.cgi?id=38281 I don't see a good reason to do this. Also extern_weak is not an appropriate linkage for definitions, only declarations. htt

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

pcc added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:147 + builder.setUseOrcMCJITReplacement(false); + builder.setMCJITMemoryManager(make_unique()); + builder.setOptLevel(OLvl); morehouse wrote: > This uses `llvm:make_uni

[PATCH] D42725: IRGen: Move vtable load after argument evaluation.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324286: IRGen: Move vtable load after argument evaluation. (authored by pcc, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42725?vs=132094&i

[PATCH] D10831: Attach attribute "trap-func-name" to IR function

pcc added a comment. Herald added subscribers: llvm-commits, mehdi_amini. Maybe I'm missing something, but I don't see why we need this attribute. Couldn't clang have been changed to implement `-ftrap-function` by generating a call to the trap function instead of emitting an `llvm.trap` intrinsi

[PATCH] D42680: [ThinLTO] Ignore object files with no ThinLTO modules if -fthinlto-index= is set

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGen/thinlto_backend.ll:27 +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc +; RUN: llvm-nm

[PATCH] D42503: libcxx: Unbreak external thread library configuration.

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX325723: libcxx: Unbreak external thread library configuration. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D42503?vs=131

[PATCH] D43579: [libcxx] Do not include the C math.h header before __config

pcc added inline comments. Comment at: include/math.h:1499 +// has previously been included. +#if defined(_LIBCPP_MSVCRT) && defined(_USE_MATH_DEFINES) +#include_next Nit: you don't actually need the ` && defined(_USE_MATH_DEFINES)` part. https://reviews.llvm.

[PATCH] D33525: [ThinLTO] Migrate ThinLTOBitcodeWriter to the new PM.

pcc added inline comments. Comment at: clang/test/CodeGen/thin_link_bitcode.c:5 +// RUN: %clang_cc1 -o %t -flto=thin -fexperimental-new-pass-manager -fthin-link-bitcode=%t.newpm -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s +// RUN: llvm-bcanalyzer -dump %t.newpm | FileCheck

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

pcc added a comment. Have you considered writing the regular LTO summaries unconditionally if `-flto` was specified? That was how I imagined that the interface would look. Also, how were you planning to expose the reference graph to the linker? I gather from your message that you are teaching y

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

pcc added a comment. In https://reviews.llvm.org/D34156#779661, @tobiasvk wrote: > In https://reviews.llvm.org/D34156#779270, @pcc wrote: > > > Have you considered writing the regular LTO summaries unconditionally if > > `-flto` was specified? That was how I imagined that the interface would >

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

pcc added a comment. In https://reviews.llvm.org/D34156#780714, @mehdi_amini wrote: > In https://reviews.llvm.org/D34156#780570, @tobiasvk wrote: > > > - Change patch to always emit a module summary for regular LTO > > > > I don't see any real downside of having a summary given the marginal tim

[PATCH] D34233: [CFI] Add ability to explicitly link classes

pcc added a comment. Before looking at this patch in detail, can you please explain the need for this feature? How difficult would it be to change your code to avoid relying on invalid casts? https://reviews.llvm.org/D34233 ___ cfe-commits mailing

[PATCH] D34156: [LTO] Enable module summary emission by default for regular LTO

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. Please confirm that we can still self host with full LTO now that https://reviews.llvm.org/D33922 has landed. LGTM otherwise. Thanks! https://reviews.llvm.org/D34156 ___

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

pcc created this revision. Herald added subscribers: eraman, inglorion. https://reviews.llvm.org/D34546 Files: clang/docs/ThinLTO.rst Index: clang/docs/ThinLTO.rst === --- clang/docs/ThinLTO.rst +++ clang/docs/ThinLTO.rst @@ -126

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

pcc added inline comments. Comment at: clang/docs/ThinLTO.rst:160 + value of 0 forces the scan to occur. The default is every 20 minutes. + Clang Bootstrap mehdi_amini wrote: > Why do we document linker-specific option in clang docs? Is it usual? It does seem a

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

This revision was automatically updated to reflect the committed changes. Closed by commit rL306125: docs: Add documentation for the ThinLTO cache pruning policy string. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D34546?vs=103682&id=103743#toc Repository: rL LLVM h

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

pcc created this revision. pcc added reviewers: EricWF, mclow.lists. As a result of this change, the basic_stringbuf constructor that takes a mode ends up leaving __hm_ set to 0, causing the comparison "__hm_ - __str_.data() < __noff" in seekoff() to succeed, which caused the function to incorrect

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

pcc added a comment. Would it be possible to fix this by stripping the noexcept specifiers from both the function type used in the check and the one that is embedded in the prefix data? The downside is that we won't catch the case where the caller has a noexcept specifier and the callee doesn't

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

pcc updated this revision to Diff 127609. pcc added a comment. - Make __hm const https://reviews.llvm.org/D41319 Files: libcxx/include/sstream libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp Index: libcxx/test/std/input.output/string.streams/str

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

This revision was automatically updated to reflect the committed changes. pcc marked an inline comment as done. Closed by commit rL321124: libcxx: Fix for basic_stringbuf::seekoff() after r320604. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D41319?vs=1276

[PATCH] D43805: Optionally use nameless IR types

pcc added a comment. Why is this a driver flag? This seems like it ought to be a cc1-only flag to me. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D43805: Optionally use nameless IR types

pcc added a comment. > If the backend will be changed so that it will not depend on IR type names Are you referring to https://reviews.llvm.org/D43199? If so it seems to me that this should be a cc1 flag that defaults to whether `-flto=thin` is passed. In any case it seems like a bad idea to de

[PATCH] D47567: [wip] Implement CFI for indirect calls via a member function pointer.

pcc created this revision. pcc added a reviewer: vlad.tsyrklevich. Herald added a subscriber: mgrang. Similarly to CFI on virtual and indirect calls, this implementation tries to use program type information to make the checks as precise as possible. The basic way that it works is as follows, whe

[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.

pcc created this revision. pcc added reviewers: tejohnson, dblaikie. Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, mehdi_amini. https://reviews.llvm.org/D47597 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGen/thinlto-split-dwarf.c llvm/include/llvm/LTO/Co

[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.

This revision was automatically updated to reflect the committed changes. Closed by commit rL333677: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto… (authored by pcc, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D46700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

pcc added inline comments. Comment at: lib/Driver/ToolChains/CommonArgs.cpp:435 + CmdArgs.push_back( + Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy)); + CmdArgs.push_back( You no longer need to pass `objcopy=`, see D47091. https

[PATCH] D47567: [wip] Implement CFI for indirect calls via a member function pointer.

pcc updated this revision to Diff 151090. pcc added a comment. Herald added subscribers: steven_wu, kubamracek, mehdi_amini. - Address TODOs https://reviews.llvm.org/D47567 Files: clang/docs/ControlFlowIntegrity.rst clang/docs/LTOVisibility.rst clang/include/clang/Basic/Sanitizers.def c

[PATCH] D47567: Implement CFI for indirect calls via a member function pointer.

pcc updated this revision to Diff 151272. pcc added a comment. - Add some more documentation to ControlFlowIntegrity.rst https://reviews.llvm.org/D47567 Files: clang/docs/ControlFlowIntegrity.rst clang/docs/LTOVisibility.rst clang/include/clang/Basic/Sanitizers.def clang/lib/CodeGen/CGC

[PATCH] D48155: Teach Clang to emit address-significance tables.

pcc created this revision. pcc added reviewers: echristo, Bigcheese, rsmith. By default, we emit an address-significance table on all ELF targets when the integrated assembler is enabled. The emission of an address-significance table can be controlled with the -faddrsig and -fno-addrsig flags. De

[PATCH] D48155: Teach Clang to emit address-significance tables.

pcc updated this revision to Diff 151282. pcc added a comment. - Add some additional driver tests https://reviews.llvm.org/D48155 Files: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/CodeGenOptions.def clang/lib/CodeGen/BackendUtil.cpp clang/lib/Driver/ToolChains/Cl

[PATCH] D48206: IRgen: Mark aliases of ctors and dtors as unnamed_addr.

pcc created this revision. pcc added a reviewer: rsmith. This is not only semantically correct but ensures that they will not be marked as address-significant once https://reviews.llvm.org/D48155 lands. https://reviews.llvm.org/D48206 Files: clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/Cod

[PATCH] D48206: IRgen: Mark aliases of ctors and dtors as unnamed_addr.

This revision was automatically updated to reflect the committed changes. Closed by commit rC334982: IRgen: Mark aliases of ctors and dtors as unnamed_addr. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D48206?vs=151455&id=151787#toc Repository: rC Clang

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

pcc added inline comments. Comment at: lib/Driver/ToolChains/CommonArgs.cpp:423 +llvm::sys::path::native(Dwo_Dir, DwoDir); +llvm::sys::path::append(DwoDir, Twine(Output.getFilename()) + "_dwo"); +CmdArgs.push_back( I think that if I pass `-gsplit-dwar

[PATCH] D48454: Ignore blacklist when generating __cfi_check_fail.

pcc added a comment. __cfi_check_fail certainly seems like a special case in that its behaviour is controlled only by flags and not the blacklist. Maybe a simpler fix would be to add this to the top of `EmitCfiCheckFail`? SanOpts = CGM.getLangOpts().Sanitize; https://reviews.llvm.org/D48454

[PATCH] D48454: Ignore blacklist when generating __cfi_check_fail.

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52857: [clang-query] Add non-exclusive output API

pcc added a comment. I'm not really involved in clang-query development these days, so this is more of a drive-by comment. It wasn't really obvious to me what the wording of the help text for `set enable-output` meant: > Set whether to enable content non-exclusively. I figured out from the na

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. The reason why LTO unit is always enabled is so that you can link translation units compiled with `-fsanitize=cfi` and/or `-fwhole-program-vtables` against translation units compiled witho

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

pcc added a comment. In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote: > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote: > > > In https://reviews.llvm.org/D53524#1271357, @pcc wrote: > > > > > The reason why LTO unit is always enabled is so that you can link > > > trans

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

pcc added a comment. In https://reviews.llvm.org/D53524#1279288, @tejohnson wrote: > In https://reviews.llvm.org/D53524#1276038, @pcc wrote: > > > In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote: > > > > > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote: > > > > > > > In

[PATCH] D38430: Enable -pie and --enable-new-dtags by default on Android.

pcc added inline comments. Herald added a subscriber: llvm-commits. Comment at: cfe/trunk/lib/Driver/ToolChains/Linux.cpp:814 +bool Linux::isPIEDefault() const { + return (getTriple().isAndroid() && !getTriple().isAndroidVersionLT(16)) || + getSanitizerArgs().requiresPIE

[PATCH] D38430: Enable -pie and --enable-new-dtags by default on Android.

pcc added inline comments. Comment at: cfe/trunk/lib/Driver/ToolChains/Linux.cpp:814 +bool Linux::isPIEDefault() const { + return (getTriple().isAndroid() && !getTriple().isAndroidVersionLT(16)) || + getSanitizerArgs().requiresPIE(); eugenis wrote: > pcc

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">, + Flags<[CC1Opt

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">, + Flags<[CC1Opt

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

pcc created this revision. pcc added a reviewer: eugenis. Herald added subscribers: llvm-commits, cryptoad. Repository: rL LLVM https://reviews.llvm.org/D54330 Files: clang/lib/Driver/SanitizerArgs.cpp clang/test/Driver/fsanitize.c Index: clang/test/Driver/fsanitize.c ===

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

This revision was automatically updated to reflect the committed changes. Closed by commit rL346526: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal… (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D54330?vs=173367&id=173368#toc

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

pcc added inline comments. Comment at: docs/LTOVisibility.rst:9 unit's *LTO unit* is the subset of the linkage unit that is linked together -using link-time optimization; in the case where LTO is not being used, the -linkage unit's LTO unit is empty. Each linkage unit has only a

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

pcc added a comment. In https://reviews.llvm.org/D46403#1291697, @filcab wrote: > Sorry to ressurect this review, but I have a few questions: > > - What kind of functions fail? > - Are there bugzillas to track these? > - How can a compiler expect to have blacklists for "all" its CFI clients? > (

[PATCH] D54453: Remove myself as owner of clang-query.

snail-mail address (S). -N: Peter Collingbourne -E: pe...@pcc.me.uk -D: clang-query - N: Manuel Klimek E: kli...@google.com D: clang-rename, all parts of clang-tools-extra not covered by someone else Index: clang-tools-extra/CODE_

[PATCH] D54453: Remove myself as owner of clang-query.

xtra https://reviews.llvm.org/D54453 Files: CODE_OWNERS.TXT Index: CODE_OWNERS.TXT === --- CODE_OWNERS.TXT +++ CODE_OWNERS.TXT @@ -12,10 +12,6 @@ E: aa...@aaronballman.com D: clang-query -N: Peter Collingbourne -E: pe...@pcc.m

[PATCH] D54735: Driver: SCS is compatible with every other sanitizer.

pcc created this revision. pcc added a reviewer: eugenis. Herald added a subscriber: llvm-commits. Because SCS relies on system-provided runtime support, we can use it together with any other sanitizer simply by linking the runtime for the other sanitizer. Repository: rL LLVM https://reviews.

[PATCH] D54735: Driver: SCS is compatible with every other sanitizer.

This revision was automatically updated to reflect the committed changes. Closed by commit rC347282: Driver: SCS is compatible with every other sanitizer. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D54735?vs=174709&id=174714#toc Repository: rC Clang

[PATCH] D54805: [Driver] Use --push/pop-state with Sanitizer link deps

pcc added a comment. Unfortunately it looks like the Android NDK uses some ancient version of gold that doesn't support `--push-state`, so we probably can't rely on being able to use it. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/17287/steps/run%20lit%20tests%20%5Bi6

[PATCH] D55159: Lex: Fix bug in __BASE_FILE__ implementation.

pcc created this revision. pcc added a reviewer: rsmith. Herald added a subscriber: llvm-commits. __BASE_FILE__ previously expanded to "" instead of the name of the main source file in files included using -include. Also added test coverage for __BASE_FILE__ since there doesn't seem to be any.

[PATCH] D55229: [COFF, ARM64] Make -flto-visibility-public-std a driver and cc1 flag

pcc added a comment. The reason why it has that name is that it was originally added to support the logic here: http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGVTables.cpp#991 It looks like it was later repurposed as a "was /MT passed to the driver" flag when the logic was added to mark runt

[PATCH] D55246: AST: Relax an assertion in constexpr constructor evaluation.

pcc created this revision. pcc added a reviewer: rsmith. Herald added a subscriber: llvm-commits. It's possible for the base type specified in a constructor to be qualified via sugar, so the existing assertion was too strict. Relax it so that we only check that the unqualified types are the same.

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: include/clang/Driver/ToolChain.h:355 + virtual LangOptions::TrivialAutoVarInitKind + GetDefaultTrivialAutoVarInit() const { +return LangOptions::TrivialAutoVarInitKind::Uninitialized; I wouldn't introduce this function

[PATCH] D59034: Delete x86_64 ShadowCallStack support

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. Thanks for cleaning this up. Comment at: clang/docs/ShadowCallStack.rst:23 +to have critical performance and security deficiencies--it was removed in +LLVM 9.0.

[PATCH] D59254: [RFC] Implementation of Clang randstruct

pcc added a comment. This needs a test under `test/CodeGen` at least. Comment at: clang/include/clang/AST/RecordFieldReorganizer.h:54 + std::seed_seq Seq; + std::default_random_engine rng; +}; I don't think we can use `default_random_engine` for this because

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

pcc created this revision. pcc added reviewers: jfb, glider. Herald added a project: clang. Otherwise the object may have an incorrect size due to tail padding. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59446 Files: clang/lib/CodeGen/CGDecl.cpp clang/test/CodeGenCXX/a

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

This revision was automatically updated to reflect the committed changes. Closed by commit rL356328: CodeGen: Preserve packed attribute in constStructWithPadding. (authored by pcc, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: ht

[PATCH] D59724: IRGen: Remove StructorType; thread GlobalDecl through more code. NFCI.

pcc created this revision. pcc added reviewers: rsmith, rjmccall. Herald added a subscriber: jdoerfert. Herald added a project: clang. This should make it easier to add more structor variants. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59724 Files: clang/include/clang/AS

[PATCH] D59724: IRGen: Remove StructorType; thread GlobalDecl through more code. NFCI.

This revision was automatically updated to reflect the committed changes. Closed by commit rL356822: IRGen: Remove StructorType; thread GlobalDecl through more code. NFCI. (authored by pcc, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

pcc added inline comments. Comment at: lld/COFF/Chunks.cpp:47 +namespace { +// This class exists just for the purpose of calculating the expected size of rnk wrote: > ruiu wrote: > > rnk wrote: > > > ruiu wrote: > > > > rnk wrote: > > > > > ruiu wrote: > > > >

[PATCH] D60059: [Driver] implement -feverything

pcc added a comment. In D60059#1450320 , @lebedev.ri wrote: > In D60059#1450252 , @yuxuanchen1997 > wrote: > > > > enable all runtime sanitizers for extra safety > > > > Brilliant idea. But I wonder how that could

[PATCH] D60548: Variable auto-init: also auto-init alloca

pcc added a comment. I probably wouldn't do anything about suppressing init on alloca for now, but if we did do something I think I'd be in favour of the separate builtin for uninitialized alloca. I also considered the alternative of allowing `__attribute__((uninitialized))` to appear on a func

[PATCH] D60807: Test for Oz fail

pcc added a comment. Interesting, it looks like the difference here is almost entirely due to different inlining decisions. And according to the tool I posted here: http://lists.llvm.org/pipermail/llvm-dev/2019-April/131811.html in the few functions in which we made the same inlining decisions,

[PATCH] D58321: Support for relative vtables

pcc added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:531 +/// \brief Whether the class uses the relative C++ vtable ABI. +unsigned IsRelativeCXXABI : 1; + rjmccall wrote: > Should we proactively generalize this as a "CXXABIVariant" enu

[PATCH] D58321: [WIP] Support for relative vtables

pcc added a comment. Can we start with a patch that just exposes a flag that enables the relative ABI unconditionally, and remove all the platform ABI compatibility stuff? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58321/new/ https://reviews.ll

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

pcc added a comment. Thanks. Once we apply this fix to other platforms this would seem to fix PR17633. We may also want to change the ubsan function signature in order to avoid bad pointer reads in case of version mismatches. Comment at: lib/CodeGen/CodeGenFunction.cpp:434 +

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

pcc added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:463 + auto *PCRelAsInt = + Builder.CreatePtrToInt(EncodedAddr, IntPtrTy, "encoded_addr.int"); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy, "func_addr.int"); pcc wrote: > Mayb

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

pcc added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:445 + auto *GV = new llvm::GlobalVariable(CGM.getModule(), Addr->getType(), + /*isConstant=*/false, + llvm::GlobalValue::PrivateLinkag

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: lib/CodeGen/CodeGenFunction.cpp:461 + ? EncodedAddr + : Builder.CreateSExt(EncodedAddr, IntPtrTy); + auto

[PATCH] D37787: Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

pcc created this revision. Also add some missing driver tests for the regular clang driver. https://reviews.llvm.org/D37787 Files: clang/include/clang/Driver/Options.td clang/test/Driver/whole-program-vtables.c Index: clang/test/Driver/whole-program-vtables.c =

[PATCH] D37787: Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313169: Driver: Make -fwhole-program-vtables a core option so it can be used from clang… (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D37787?vs=114960&id=115082#toc Repository:

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. Here's a standalone reproducer for the problem: $ cat exe.cpp #include std::function f(); int main() { f()(); } $ cat dso.cpp #include __attribute__((visibility("default"))) std::function f() { return [](){}; } $ ~/l3/ra-cmake/bin/clang+

[PATCH] D62636: Driver, IRGen: Set partitions on GlobalValues according to -fsymbol-partition flag.

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

[PATCH] D62636: Driver, IRGen: Set partitions on GlobalValues according to -fsymbol-partition flag.

This revision was automatically updated to reflect the committed changes. pcc marked an inline comment as done. Closed by commit rL362829: Driver, IRGen: Set partitions on GlobalValues according to -fsymbol-partition… (authored by pcc, committed by ). Herald added a project: LLVM. Herald added a s

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xAAAAAAAA

pcc added a comment. The problem with `0x` on 32-bit is that it is likely to be a valid address. When I discussed this with JF I proposed a pointer initialization of `0x` which he agreed to. This value is very likely to trap when accessed (due to accesses likely wrapping to zer

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF

pcc added a comment. In D64597#1582944 , @hubert.reinterpretcast wrote: > In D64597#1581605 , @pcc wrote: > > > The problem with `0x` on 32-bit is that it is likely to be a valid > > address. > > > > When

[PATCH] D64458: add -fthinlto-index= option to clang-cl

pcc added a comment. I think it's a little unfortunate that we're continuing to go down the road of letting users pass more flags to the ThinLTO backend action, but I won't stand in the way here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64458

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. I think it would be up to the libc++ maintainers to approve the patch. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list cfe

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. In D48680#1587967 , @ldionne wrote: > @pcc In your reproducer, what is `~/l3/ra-cmake/bin/clang++`? That's just clang built from trunk at the time that I posted my comment. > Are you able to reproduce without `-fuse-ld=lld`? I'm try

[PATCH] D64843: hwasan: Initialize the pass only once.

pcc created this revision. pcc added a reviewer: eugenis. Herald added subscribers: cfe-commits, hiraditya. Herald added projects: clang, LLVM. This will let us instrument globals during initialization. This required making the new PM pass a module pass, which should still provide access to analys

[PATCH] D64843: hwasan: Initialize the pass only once.

pcc added a comment. IIUC the reason was to work around the lack of analysis availability from module passes, which is no longer a concern with the new PM. I would personally prefer to see the other sanitizers re-architected this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D64843: hwasan: Initialize the pass only once.

This revision was automatically updated to reflect the committed changes. Closed by commit rL366379: hwasan: Initialize the pass only once. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D64843?vs=210227&id=210429#toc Repository: rL LLVM CHANGES SINCE LA

[PATCH] D64843: hwasan: Initialize the pass only once.

pcc added a comment. The instrumentation for globals is coming in a separate patch. It seems more important to achieve implementation simplicity for the sanitizer passes rather than data locality because they make fundamental changes to the IR that touch the whole module (unlike other passes th

[PATCH] D65009: [LTO] Don't mark regular LTO units with EnableSplitLTOUnit

pcc added a comment. Sorry, just realized this. If I do clang++ -c -flto a.cpp # "split" clang++ -c -flto=thin b.cpp -fwhole-program-vtables # non-split clang++ a.o b.o this should fail, right? If I'm not mistaken, this patch series will cause this to succeed. I think we need to change th

[PATCH] D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65009/new/ https://reviews.llvm.org/D65009 ___ cfe-commits mailing

[PATCH] D65029: [Driver] Support for disabling sanitizer runtime linking

pcc added subscribers: filcab, pcc. pcc added a comment. It's also worth asking whether `-nodefaultlibs` would work for this use case as @filcab suggested on D64547 . Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65029/new/ https

<    1   2   3   4   5   6   7   8   >