[PATCH] D44103: Adding additional UNSUPPORTED platform in libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp

vsk added a comment. Is the test expected to pass with clang-902? If not, then it'd be best to use "apple-clang-9" instead of "apple-clang-9.0". Repository: rCXX libc++ https://reviews.llvm.org/D44103 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D44069: Test Driver sanitise, unsupported on OpenBSD

vsk requested changes to this revision. vsk added inline comments. This revision now requires changes to proceed. Comment at: test/Driver/fsanitize.c:392 +// RUN %clang -target i386-pc-openbsd -fsanitize=undefined %s -### 2>&1 | FileCheck --check-prefix=CHECK_UBSAN-OPENBSD +// C

[PATCH] D44069: Test Driver sanitise, unsupported on OpenBSD

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm provided the bots are happy. https://reviews.llvm.org/D44069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

vsk added a comment. Herald added a subscriber: christof. @mclow.lists is this still fine to commit? I can land it and watch the bots, if you'd like. https://reviews.llvm.org/D41976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

vsk added a comment. Thanks @tvanslyke and @mclow.lists, landed as r327064. Repository: rCXX libc++ https://reviews.llvm.org/D41976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44069: Test Driver sanitise, unsupported on OpenBSD

vsk added a comment. The new changes look fine. Do you need someone to commit this for you? https://reviews.llvm.org/D44069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47720: [DebugInfo] Inline for without DebugLocation

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. As is the case with https://reviews.llvm.org/D47097, this helps preserve more scope information after SROA. LGTM. Thanks! Repository: rC Clang https://reviews.llvm.org/D47720

[PATCH] D53231: [Sema] Fix PR38987: keep end location of a direct initializer list

vsk added a comment. Thanks for looking into this! Do you know why the InitializationKind doesn't contain the right brace locations in this scenario? Should it? Repository: rC Clang https://reviews.llvm.org/D53231 ___ cfe-commits mailing list cf

[PATCH] D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Comment at: test/CoverageMapping/macros.c:60 +// CHECK-NEXT: func6 +void func6(unsigned count) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+4]]:2 = #0

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

vsk added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2871 + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); Why shouldn't this always b

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

vsk added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2871 + auto *DI = CGF.getDebugInfo(); + SourceLocation Loc = DI ? DI->getLocation() : SourceLocation(); + auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc); aprantl wrote: > vsk wrote:

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

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

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

vsk added a comment. In https://reviews.llvm.org/D52034#1268277, @calixte wrote: > @vsk, gcc guys are ok for -fprofile-filter-files and > -fprofile-exclude-files, are you ok with that ? That sounds fine to me. > Should these options prefixed by -Xclang or not ? I don't think they should be.

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

vsk added a comment. In https://reviews.llvm.org/D52034#1246379, @calixte wrote: > I reported a bug for gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87442 Thank you! > @vsk I'd like to add documentation in Docs/UsersManual.rst, but I've no idea > on what's a good place for this (I look f

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. What are the advantages of a generalized expect_noreturn attribute, vs. a narrower attribute or intrinsic? The expect_noreturn semantics do not provide strong guarantees, and are not really orthogonal from the pre-existing cold attribute. In particular, expect_noreturn does

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1369767 , @yln wrote: > In D56624#1369635 , @vsk wrote: > > > What are the advantages of a generalized expect_noreturn attribute, vs. a > > narrower attribute or intrinsic? The expect

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1369940 , @yln wrote: > Note that all of this currently only matters when compiling with > `-fsanitize=unreachable`. The following discussion is within the context of > the current implementation: UBSan removes the `noretur

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370243 , @eugenis wrote: > > Because "expect_noreturn" calls are allowed to return, the compiler must > > behave as they could. In particular, this means that unpoisoning the stack > > before expect_noreturn calls (given t

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370280 , @eugenis wrote: > > Wouldn’t it be preferable to unpoison the stack inside of maybe_longjmp, > > once the opaque condition can be checked? > > Sure, but that's not always possible. That's why we have interceptors.

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370607 , @yln wrote: > In D56624#1370579 , @eugenis wrote: > > > Maybe the frontend should insert __asan_handle_noreturn whenever ASan is > > enabled, and then ASan would not care ab

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk created this revision. vsk added reviewers: chandlerc, t.p.northover, tejohnson, hiraditya. Herald added a subscriber: mehdi_amini. In order for the hot/cold splitting pass to graduate out of experimental status, users need some way to safely enable it. The current method of passing `-mllvm -

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk updated this revision to Diff 183661. vsk added a comment. - Fix a think-o in a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ https://reviews.llvm.org/D57265 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/CC1Options.td clang

[PATCH] D57278: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. Is it necessary to remove visitation of 'noreturn' call sites from ASan? I'd expect that to break non-C frontends which emit noreturn calls (rust/swift?). Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57278/new/ https://reviews.llvm.org/D5727

[PATCH] D57278: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk accepted this revision. vsk added a comment. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57278/new/ https://reviews.llvm.org/D57278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D54195: Fix linker option for -fprofile-arcs -ftest-coverage

vsk added a comment. Thanks for the patch. Would you mind uploading a diff with additional context (e.g. git diff -U1)? Phab doesn't render it. Repository: rC Clang https://reviews.llvm.org/D54195 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D53231: [Sema] Fix PR38987: keep end location of a direct initializer list

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. The history seems complicated. I think it'd be really useful to sort out why getParenOrBraceRange() couldn't give the right result, but I'd be happy to see this land first to address the crash. Re

[PATCH] D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end

vsk added a comment. Friendly ping -- @orivej were you still looking for more feedback? If not, do you still need someone to land this patch on your behalf? Repository: rC Clang https://reviews.llvm.org/D53244 ___ cfe-commits mailing list cfe-co

[PATCH] D54195: Fix linker option for -fprofile-arcs -ftest-coverage

vsk added a comment. @jessicah Please ping this review if you need someone to commit this patch on your behalf. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54195/new/ https://reviews.llvm.org/D54195 ___ cfe-commit

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

vsk created this revision. vsk added reviewers: ributzka, steven_wu, calixte, marco-c. Make sure that symbols needed to implement runtime support for gcov are exported when using an export list on Darwin. Without the clang driver exporting these symbols, the linker hides them, resulting in tapi v

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

vsk marked an inline comment as done. vsk added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1037 if (hasExportSymbolDirective(Args)) { -addExportedSymbol(CmdArgs, "___llvm_profile_filename"); -addExportedSymbol(CmdArgs, "___llvm_profile_raw_vers

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

vsk added inline comments. Comment at: clang/test/Profile/cxx-abc-deleting-dtor.cpp:28 +// FIXME: Should we emit coverage info for deleting dtors? They do contain +// conditional branches. LLVM IR PGO will insrument them just fine, though. + Probably not. IIUC th

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks! Lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58691/new/ https://reviews.llvm.org/D58691

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

vsk added a comment. I'm confused by this wording re: comdats in the LangRef: "All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key". Why can multiple global objects with the same comdat key end up in the final obj

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

vsk added a comment. In D58737#1412846 , @rnk wrote: > Oops, forgot to respond to this... > > In D58737#1412734 , @vsk wrote: > > > I'm confused by this wording re: comdats in the LangRef: "All global > > objects t

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk added a comment. In D57265#1393814 , @fedor.sergeev wrote: > In D57265#1393453 , @vsk wrote: > > > > I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is > > > soemthing that refers to the

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk updated this revision to Diff 192210. vsk edited the summary of this revision. vsk added a comment. Herald added subscribers: jdoerfert, dexonsmith, steven_wu, eraman. Use a function attribute to implement the -fsplit-cold-code option. This removes some complexity from the old/new PMs and ens

[PATCH] D60108: [os_log] Mark os_log_helper `nounwind`

vsk created this revision. vsk added reviewers: ahatanak, JDevlieghere. Herald added subscribers: jdoerfert, dexonsmith. Allow the optimizer to remove unnecessary EH cleanups surrounding calls to os_log_helper, to save some code size. As a follow-up, it might be worthwhile to add a BasicNoexcept

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ https://reviews.llvm.org/D57265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58033: Add option for emitting dbg info for call site parameters

vsk added a comment. I think a small extension to `test/CodeGenCXX/dbg-info-all-calls-described.cpp` for the 'Supported: DWARF4 + -femit_param_entry_values' case would be appropriate here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

vsk added a comment. Thanks for working on this! Comment at: lib/CodeGen/CGObjCMac.cpp:1983 + else +GV->setSection("__OBJC,__cstring_object,regular,no_dead_strip"); + Are constant NSStrings within the shared cache ever dirtied? I was under the impression

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

vsk added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:3961 + // linkage so that the linker preserves the symbol name. + llvm::GlobalValue::LinkageTypes LT = ExplicitDataSegment || Section.empty() + ? llvm::GlobalValue::Intern

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

vsk added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:6039 +LT = llvm::GlobalValue::PrivateLinkage; + } + I see some minor variant of this logic repeated a few times. Wdyt of defining a 'getLinkageTypeForObjCMetadata(CGM, Section)' helper to cons

[PATCH] D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61454/new/ https://reviews.llvm.org/D61454 ___ cfe-commits

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

vsk added inline comments. Comment at: test/CodeGen/code-coverage-filter.c:4 +// RUN: %clang_cc1 -emit-llvm -femit-coverage-data -coverage-exclude=.*\\.h %s -o - \ +// RUN:| FileCheck -check-prefix=NOH %s +// RUN: %clang_cc1 -emit-llvm -femit-coverage-data -coverage-filter=.

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

vsk added inline comments. Comment at: include/clang/Driver/CC1Options.td:236 +def coverage_exclude_EQ : Joined<["-"], "coverage-exclude=">, + Alias; def coverage_exit_block_before_body : Flag<["-"], "coverage-exit-block-before-body">, marco-c wrote: > calixte

[PATCH] D52064: [Sema] Add a note pointing to the first use of an implicit capture

vsk added a comment. @rsmith, friendly ping. I'm not in a rush on this one, but I know you wanted to see an improvement in lambda capture diagnostics. https://reviews.llvm.org/D52064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ https://reviews.llvm.org/D57265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM, thanks. It might make sense to add a predicate to SanitizerSet to make it easier to check when ASan is enabled, either pre-commit or as a follow-up. Repository: rG LLVM Github Monorepo CHA

[PATCH] D57722: [Clang][NCF] Sanitizer options: helpers to check if {Kernel, Hardware}ASan is enabled

vsk added reviewers: eugenis, kcc. vsk added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.h:89 + /// * Hardware Kernel ASan + bool hasASanOrKASanOrHWASanOrHWKASan() const { +return hasASanOrKASan() || hasHWASanOrHWKASan(); Sorry to bike

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk added reviewers: fedor.sergeev, philip.pfaffe. vsk added a comment. Thanks @hiraditya. I'd also like to get a +1 from someone who's worked on the NewPM infrastructure extensively, just to make sure those changes look ok. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ http

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk updated this revision to Diff 185642. vsk added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ https://reviews.llvm.org/D57265 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/CC1Options.td clang/lib/CodeGen/BackendUt

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

vsk marked an inline comment as done. vsk added a comment. > I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is > soemthing that refers to the construction of one particular pipeline, not to > pipeline-building in general. It should be an argument passed down through > th

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

vsk added subscribers: dblaikie, vsk. vsk added a comment. I think this could roughly double the memory utilization of the writer, which might be problematic because the number of records to write can be high. (@dblaikie did some work on reducing memory usage in this area, he might have hard da

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

vsk added reviewers: bricci, rsmith. vsk added a comment. + rsmith, + bricci for review. I was under the impression that space inside VarDecl was quite constrained. Pardon the likely naive question, but: is there any way to make the representation more compact (maybe sneak a bit into ParmVarDec

[PATCH] D37542: [ubsan] Save a ptrtoint when emitting alignment checks

vsk created this revision. The alignment check emits a ptrtoint instruction which can be reused in the call to the diagnostic handler. https://reviews.llvm.org/D37542 Files: lib/CodeGen/CGExpr.cpp test/CodeGen/catch-undef-behavior.c Index: test/CodeGen/catch-undef-behavior.c =

[PATCH] D37543: [ubsan] Add helpers to decide when null/vptr checks are required. NFC.

vsk created this revision. Adding these helpers will make a planned change simpler: [ubsan] Defer pointer type checks, then try to skip the redundant ones https://reviews.llvm.org/D37543 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h Index: lib/CodeGen/CodeGenFunction.h =

[PATCH] D37544: [ubsan] Skip alignment checks which are folded away

vsk created this revision. Don't emit alignment checks which the IR constant folder throws away. I've tested this out on X86FastISel.cpp. While this doesn't decrease end-to-end compile-time significantly, it results in 122 fewer type checks (1% reduction) overall, without adding any real complexi

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

vsk created this revision. This change will make it possible to use -fsanitize=function on Darwin and possibly on other platforms. It fixes an issue with the way RTTI is stored into function prologue data. On Darwin, addresses stored in prologue data can't require run-time fixups and must be PC-r

[PATCH] D37598: [ubsan] Enable -fsanitize=function on Darwin

vsk created this revision. Depends on https://reviews.llvm.org/D37597 https://reviews.llvm.org/D37598 Files: docs/UndefinedBehaviorSanitizer.rst lib/Driver/ToolChains/Darwin.cpp test/Driver/fsanitize.c Index: test/Driver/fsanitize.c ==

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

vsk added a comment. Thanks for your comments! Comment at: lib/CodeGen/CodeGenFunction.cpp:434 +llvm::Constant *Addr) { + if (!CGM.getTriple().isOSDarwin()) +return Addr; pcc wrote: > I think you can just do this

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

vsk updated this revision to Diff 114303. vsk edited the summary of this revision. vsk added a comment. Address review feedback: - Enable this change on all platforms. - Bump the function signature version. - Store the indirect RTTI pointer as an IntPtrTy. This saves a "ptrtoint" instruction. Tr

[PATCH] D36642: [Lexer] Report more precise skipped regions (PR34166)

vsk added inline comments. Comment at: test/Index/skipped-ranges.c:23 // RUN: env CINDEXTEST_SHOW_SKIPPED_RANGES=1 c-index-test -test-annotate-tokens=%s:1:1:16:1 %s | FileCheck %s -// CHECK: Skipping: [5:2 - 6:7] -// CHECK: Skipping: [8:2 - 12:7] -// CHECK: Skipping: [14:2 - 20

[PATCH] D36642: [Lexer] Report more precise skipped regions (PR34166)

vsk updated this revision to Diff 114433. vsk added a comment. Herald added subscribers: kbarton, nemanjai. - Add an 'EndifLoc' parameter to the SourceRangeSkipped callback so that indexing clients can preserve their existing behavior. - I'll submit a follow-up patch which updates the pp-trace te

[PATCH] D37642: [pp-trace] Update skipped source ranges in tests

vsk created this revision. Herald added subscribers: kbarton, nemanjai. Depends on https://reviews.llvm.org/D36642 https://reviews.llvm.org/D37642 Files: pp-trace/PPCallbacksTracker.cpp pp-trace/PPCallbacksTracker.h test/pp-trace/pp-trace-conditional.cpp test/pp-trace/pp-trace-macro.cpp

[PATCH] D37647: [ubsan-minimal] Document the new runtime

vsk created this revision. https://reviews.llvm.org/D37647 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst Index: docs/UndefinedBehaviorSanitizer.rst === --- docs/UndefinedBehaviorSanitizer.rst +++ docs/Undefin

[PATCH] D37649: [Driver] Support ubsan-minimal on Darwin

vsk created this revision. Make it possible to use the minimal ubsan runtime on Darwin. https://reviews.llvm.org/D37649 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/sanitizer-ld.c Index: test/Driver/sanitizer-ld.c ===

[PATCH] D37470: [analyzer] Handle ObjC messages conservatively in CallDescription

vsk added a comment. Can you add a test? Repository: rL LLVM https://reviews.llvm.org/D37470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37564: Update users of llvm::sys::ExecuteAndWait etc.

vsk added a comment. Looks good to me. https://reviews.llvm.org/D37564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37642: [pp-trace] Update skipped source ranges in tests

vsk closed this revision. vsk added a comment. Thanks, r312948 https://reviews.llvm.org/D37642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

vsk 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"); vsk wrote: > pcc

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

vsk updated this revision to Diff 114882. vsk added a comment. - Rebase to ToT. https://reviews.llvm.org/D32842 Files: include/clang/AST/ASTContext.h include/clang/Basic/LangOptions.h include/clang/Basic/SanitizerBlacklist.h include/clang/Driver/SanitizerArgs.h lib/AST/ASTContext.cpp

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

vsk added a comment. @eugenis I gave the idea of annotating blacklist entries with a list of sanitizers they apply to some thought. It would be a nice follow-up to this change, but would depend on it. As an initial step, I think we should move forward with this patch, since it makes it possible

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

vsk added a comment. In https://reviews.llvm.org/D32842#868505, @vlad.tsyrklevich wrote: > @vsk Why don't I take a look at implementing the blacklist selection methods > @eugenis mentioned on top of this change now so that we can skip ahead and > merge something everyone is satisfied with? Fe

[PATCH] D37777: [Driver] Disable uwtable by default in -ffreestanding mode

vsk created this revision. Herald added subscribers: kristof.beyls, aemerson. We make the same decision when compiling the kernel or kexts -- we should do this in -ffreestanding mode as well to avoid size regressions in a potentially large set of firmware projects. It's still possible to get uwta

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

vsk updated this revision to Diff 114932. vsk marked 8 inline comments as done. vsk added a comment. Address review feedback. https://reviews.llvm.org/D37597 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/TargetInfo.cpp test/Cod

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

vsk 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

vsk marked 4 inline comments as done. vsk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:461 + ? EncodedAddr + : Builder.CreateSExt(EncodedAddr, IntPtrTy); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy,

[PATCH] D37544: [ubsan] Skip alignment checks which are folded away

vsk updated this revision to Diff 115879. vsk added a comment. - Use a better test case. This one was lifted from an actual example in X86CallingConv.h:86. https://reviews.llvm.org/D37544 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-suppress-checks.cpp Index: test/CodeGenCXX/ubsan

[PATCH] D37544: [ubsan] Skip alignment checks which are folded away

vsk added a comment. Sorry, I see the issue now. The pre-patch IR looked like "br i1 true, label %continue, label %diagnose", so there was no alignment check, per-se. https://reviews.llvm.org/D37544 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D37544: [ubsan] Skip alignment checks which are folded away

vsk updated this revision to Diff 116374. vsk added a comment. - Tighten up lit test. https://reviews.llvm.org/D37544 Files: lib/CodeGen/CGExpr.cpp test/CodeGenCXX/ubsan-suppress-checks.cpp Index: test/CodeGenCXX/ubsan-suppress-checks.cpp ==

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

vsk created this revision. The function sanitizer relies on RTTI to check callee types, but this scheme doesn't work well in languages without the ODR. This patch introduces a simple, best-effort function type encoding which can be used when RTTI isn't available. In this scheme, function types ar

[PATCH] D38211: [ubsan] Test -fsanitize=function with a C program

vsk created this revision. Herald added a subscriber: kubamracek. Depends on https://reviews.llvm.org/D38210 https://reviews.llvm.org/D38211 Files: test/ubsan/TestCases/TypeCheck/Function/function-c.c Index: test/ubsan/TestCases/TypeCheck/Function/function-c.c ==

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

vsk updated this revision to Diff 116453. vsk added a comment. - Remove some noisy changes. https://reviews.llvm.org/D38210 Files: docs/UndefinedBehaviorSanitizer.rst lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h tes

[PATCH] D62623: Reduce memory consumption of coverage dumps

vsk added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1393 + size_t CoverageMappingSize = 0; + for (auto &S : CoverageMappings) { +CoverageMappingSize += S.size(); It doesn't look like the CoverageMappings std::vector is needed at

[PATCH] D62623: Reduce memory consumption of coverage dumps

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1393 + size_t CoverageMappingSize = 0; + for (auto &S : CoverageMappings) { +CoverageMappingSize += S.size(); -

[PATCH] D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC

vsk reopened this revision. vsk added a comment. This revision is now accepted and ready to land. Sorry about that, reopening per Eric's comment. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64540/new/ https://reviews.llvm.org/D64540 __

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

vsk added a comment. Looks reasonable to me -- mind waiting for another +1 on this to be safe? @eugenis, any thoughts? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61479/new/ https://reviews.llvm.org/D61479 ___ cfe-commits mailing list cf

[PATCH] D65116: [Driver] Set the default win32-macho debug format to DWARF

vsk created this revision. vsk added reviewers: ahatanak, JDevlieghere, friss. Herald added a subscriber: dexonsmith. rdar://53267670 https://reviews.llvm.org/D65116 Files: clang/lib/Driver/ToolChains/MSVC.h clang/test/Misc/win32-macho.c Index: clang/test/Misc/win32-macho.c ==

[PATCH] D65426: [Coverage] Hide coverage for regions with incorrect end locations (PR39942)

vsk created this revision. vsk added reviewers: rnk, phosek, manojgupta. ... instead of crashing. The real bug here is due to clang generating coverage for a conditional operator expanded from a macro. The location of the end of the conditional expression is not correct. The actual fix for this r

[PATCH] D65428: Remove cache for macro arg stringization

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65428/new/ https://reviews.llvm.org/D65428

[PATCH] D65426: [Coverage] Hide coverage for regions with incorrect end locations (PR39942)

vsk abandoned this revision. vsk added a comment. Thanks Reid! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65426/new/ https://reviews.llvm.org/D65426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44672/new/ https://reviews.llvm.org/D44672

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

vsk added a comment. Looks reasonable to me. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3457 + // the interface type. + if (const auto *OMD = dyn_cast_or_null(D)) { +auto *ID = dyn_cast_or_null(CD); Return early on '! dyn_cast'? It doesn't look like 'D

[PATCH] D53578: [CodeGen] Fix clang test for gcov profiling (follow-up of D51974)

vsk resigned from this revision. vsk added a comment. Herald added a project: clang. (Clearing my review queue - I believe the parent revision D51974 has been accepted, perhaps @marco-c can take a look) Repository: rC Clang CHANGES SINCE LAST ACTION https

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

vsk requested changes to this revision. vsk added a comment. This revision now requires changes to proceed. Herald added a project: clang. (Marking this to reflect my comment from 3/20/18 to clear my review queue) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D4467

[PATCH] D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC

vsk created this revision. vsk added reviewers: djtodoro, aprantl. Replace a `llvm::Function *` parameter with a bool, which seems harder to set to the wrong value by accident. https://reviews.llvm.org/D64540 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h Index:

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

vsk added a comment. This looks reasonable to me. I'd prefer to disallow -fsanitize=function in combination with -fsanitize-minimal-runtime, as I'm not sure it's appropriate for the minimal runtime to handle SANITIZER_NON_UNIQUE_TYPEINFO incorrectly. If this has already been discussed elsewhere

[PATCH] D55658: Avoid Decl::getASTContext in hot paths where possible, Part 1

vsk added a comment. Thanks for working on this :). It’d be interesting to see some pre/post patch compile time numbers on CTMark. Naive/strawman alternative here, but: what’s the impact on peak RSS and compile time of just storing an ASTContext pointer in Decl/DeclContext? If it’s not out of

[PATCH] D55658: Avoid Decl::getASTContext in hot paths where possible, Part 1

vsk added a comment. In D55658#1332249 , @riccibruno wrote: > In D55658#1332240 , @vsk wrote: > > > Thanks for working on this :). It’d be interesting to see some pre/post > > patch compile time numbers on CTMark.

[PATCH] D55843: [CodeGen] Handle mixed-width ops in mixed-sign mul-with-overflow lowering

vsk created this revision. vsk added reviewers: efriedma, tstellar, dtzWill. The special lowering for __builtin_mul_overflow introduced in r320902 fixed an ICE seen when passing mixed-sign operands to the builtin. This patch extends the special lowering to cover mixed-width, mixed-sign operands.

<    1   2   3   4   5   6   7   >