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

2018-08-06 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. Please upload patches with context. `arc diff` will do this for you. Comment at: clang/docs/ControlFlowIntegrityDesign.rst:277 +Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables +

[PATCH] D50724: SafeStack: Disable Darwin support

2018-08-14 Thread Peter Collingbourne via Phabricator via cfe-commits
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/D50724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D50743: libcxx: Mark __temp_value::__temp_value as _LIBCPP_NO_CFI.

2018-08-15 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX339797: libcxx: Mark __temp_value::__temp_value as _LIBCPP_NO_CFI. (authored by pcc, committed by ). Herald added subscribers: cfe-commits, ldionne. Changed prior to commit: https://reviews.llvm.org/D

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

2018-08-15 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM except for a few spelling/grammar nits. Comment at: clang/docs/ControlFlowIntegrityDesign.rst:361 +it starts by allocating two work lists, one initialized with all the offset

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-21 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision. pcc added a reviewer: rnk. Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51049 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/addrsig.c Index: clang/test/Driver/addrsig.c =

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-22 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. With ELF it was around 0.1% for a self-host of Clang. I also checked the overhead with COFF for Chromium's base_unittests and it was around 0.2% without debug info. I think that's small enough that we can turn this on by default. Repository: rL LLVM https://reviews.llvm

[PATCH] D47050: MC: Change the streamer ctors to take an object writer instead of a stream. NFCI.

2018-05-18 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC332749: MC: Change the streamer ctors to take an object writer instead of a stream. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D47093: CodeGen, Driver: Start using direct split dwarf emission in clang.

2018-05-18 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision. pcc added reviewers: dblaikie, echristo. Herald added a subscriber: JDevlieghere. Fixes PR37466. Depends on https://reviews.llvm.org/D47091 https://reviews.llvm.org/D47093 Files: clang/include/clang/Driver/CC1Options.td clang/lib/CodeGen/BackendUtil.cpp clang/l

[PATCH] D46472: [HIP] Support offloading by linker script

2018-05-18 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp:1371-1388 + // machines. + LksStream << "/*\n"; + LksStream << " HIP Offload Linker Script\n"; + LksStream << " *** Automatically generated by Clang ***\n"; + LksStream << "*/\n"; +

[PATCH] D47089: CodeGen: Add a dwo output file argument to addPassesToEmitFile and hook it up to dwo output.

2018-05-21 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC332881: CodeGen: Add a dwo output file argument to addPassesToEmitFile and hook it up… (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.

[PATCH] D47093: CodeGen, Driver: Start using direct split dwarf emission in clang.

2018-05-21 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC332885: CodeGen, Driver: Start using direct split dwarf emission in clang. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D47093?vs=147609&id=147858#toc Repository:

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

2018-05-21 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. There were a bunch of them but the last one was https://reviews.llvm.org/D47093 which has already landed :) Probably all that needs to happen on this change is to replace the isLinux() check with isELF(). https://reviews.llvm.org/D46791

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision. pcc added reviewers: rnk, rsmith. Codebases that need to be compatible with the Microsoft ABI can enable this warning to avoid issues caused by the lack of a fixed ABI for incomplete member pointers. https://reviews.llvm.org/D47503 Files: clang/include/clang/Basic/D

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc updated this revision to Diff 149000. pcc added a comment. - Add some negative tests https://reviews.llvm.org/D47503 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaType.cpp clang/test/SemaCXX/warn-incomplete-member-pointers.cpp Index: clang/test/SemaCXX/w

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc updated this revision to Diff 149001. pcc added a comment. - One more negative test https://reviews.llvm.org/D47503 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaType.cpp clang/test/SemaCXX/warn-incomplete-member-pointers.cpp Index: clang/test/SemaCXX/wa

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:2451-2455 + // FIXME: This will cause errors if template instantiation fails. + if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) && + !Class->isDependentType() && + !Class->ge

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc updated this revision to Diff 149017. pcc added a comment. - Move the warning to RequireCompleteTypeImpl https://reviews.llvm.org/D47503 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaType.cpp clang/test/SemaCXX/warn-incomplete-member-pointers.cpp Index:

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7599-7604 + if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, + Loc) && + !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc updated this revision to Diff 149029. pcc added a comment. - Implement as -f flag https://reviews.llvm.org/D47503 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang

[PATCH] D47503: Sema: Add a flag for rejecting member pointers with incomplete base types.

2018-05-29 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC333498: Sema: Add a flag for rejecting member pointers with incomplete base types. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D47503?vs=149029&id=149031#toc Rep

[PATCH] D38210: [ubsan] Port the function sanitizer to C

2017-10-03 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. Wouldn't we get false positives if there is an indirect call in C++ code that calls into C code (or vice versa)? I think I'd prefer it if we came up with a precise encoding of function types that was independent of RTTI, and use it in all languages. One possibility would b

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: clang/test/Driver/sanitizer-ld.c:605 -// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}" -// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi -// CHECK-CFI-ANDROID-NOT: __cfi_check Why was this passing before? https://rev

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38913: [ubsan] Don't emit function signatures for virtual methods

2017-10-13 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Small nit on the first line of the commit message: it should probably mention non-static member functions, rather than virtual methods. https://reviews.llvm.org/D38913

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-23 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC340552: Driver: Enable address-significance tables by default when targeting COFF. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D51049?vs=161763&id=162232#toc Rep

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-23 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. Thanks, I'll take a look. Repository: rC Clang https://reviews.llvm.org/D51049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-23 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc reopened this revision. pcc added a comment. This revision is now accepted and ready to land. I received another report of breakage so I reverted in https://reviews.llvm.org/rC340579. Repository: rC Clang https://reviews.llvm.org/D51049 ___

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-23 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. https://reviews.llvm.org/D51199 fixes the above breakage as well as crbug.com/877235. Once it lands, I'll reland this change. Repository: rC Clang https://reviews.llvm.org/D51049 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

2018-08-24 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC340649: Reland r340552, "Driver: Enable address-significance tables by default when… (authored by pcc, committed by ). Repository: rC Clang https://reviews.llvm.org/D51049 Files: lib/Driver/ToolChai

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

2018-08-28 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-09-11 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-09-11 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-09-11 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-23 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-23 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-24 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-07-26 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-02-05 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-02-14 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-02-16 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-02-21 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-02-21 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-05-25 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-06-13 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-06-13 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-06-14 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-06-14 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-06-15 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-06-22 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-06-22 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-06-23 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-12-15 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2017-12-18 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-12-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2017-12-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-03-06 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-03-06 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-05-30 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-05-31 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-05-31 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-06-04 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-05 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-12 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-13 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-13 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-13 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-14 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-18 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-18 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-21 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-06-21 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-10-22 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-10-22 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-10-25 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-10-29 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2019-01-15 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2019-01-15 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2019-01-16 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2019-01-16 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-12 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-12 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision. pcc added reviewers: klimek, alexfh. Herald added a subscriber: llvm-commits. I haven't been involved with the project for years, so it's probably best for someone else to be the code owner. Repository: rL LLVM https://reviews.llvm.org/D54453 Files: clang-tools-e

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

2018-11-15 Thread Peter Collingbourne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE346998: Remove myself as owner of clang-query. (authored by pcc, committed by ). Changed prior to commit: https://reviews.llvm.org/D54453?vs=173776&id=174282#toc Repository: rCTE Clang Tools Extra

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

2018-11-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-19 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-11-21 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-11-30 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-12-03 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2018-12-03 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2018-12-03 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2019-03-06 Thread Peter Collingbourne via Phabricator via cfe-commits
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

2019-03-12 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2019-03-15 Thread Peter Collingbourne via Phabricator via cfe-commits
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.

2019-03-16 Thread Peter Collingbourne via Phabricator via cfe-commits
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

  1   2   3   4   >