[PATCH] D38567: [config] Warn when POSIX_C_SOURCE breaks threading support on Darwin

vsk added a comment. I'm not sure how to test the warning against anything but the macOS SDK. When I tried, I hit a -Wincompatible-sysroot issue. I can leave those changes out of this patch if we want to be more conservative. https://reviews.llvm.org/D38567 _

[PATCH] D38567: [config] Warn when POSIX_C_SOURCE breaks threading support on Darwin

vsk created this revision. Prior to macOS 10.13 and iOS 11, defining POSIX_C_SOURCE before including resulted in hard-to-understand errors. That definition causes a bunch of important definitions from the system headers to be skipped, so users see failures like "can't find mach_port_t". This pat

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

vsk planned changes to this revision. vsk added a comment. In https://reviews.llvm.org/D38210#887635, @pcc wrote: > Wouldn't we get false positives if there is an indirect call in C++ code that > calls into C code (or vice versa)? Ah, right, I'm surprised I didn't hit that while testing. > I

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

vsk added a reviewer: arphaman. vsk added a comment. Ping. https://reviews.llvm.org/D38210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

vsk closed this revision. vsk added a comment. Landed as r314749 https://reviews.llvm.org/D37542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38441: [compiler-rt] [cmake] Add a separate CMake var to control profile runtime

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rL LLVM https://reviews.llvm.org/D38441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[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] 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 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] 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] 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 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] 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] 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 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] D35271: Fix printing policy for AST context loaded from file

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! This seems like a pretty straightforward bug fix. Since it's not my usual area maybe it'd be worth waiting a day or so for more feedback. https://reviews.llvm.org/D35271 __

[PATCH] D36492: [time-report] Add preprocessor timer

vsk added inline comments. Comment at: lib/Lex/Preprocessor.cpp:746 void Preprocessor::Lex(Token &Result) { + llvm::TimeRegion(PPOpts->getTimer()); + MatzeB wrote: > modocache wrote: > > erik.pilkington wrote: > > > Doesn't this just start a timer and immediate

[PATCH] D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D36969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D35271: Fix printing policy for AST context loaded from file

vsk added a comment. Thanks for adding the test! This is looking good. Comment at: unittests/Frontend/ASTUnitTest.cpp:32 +llvm::SmallString<256> Dir; +ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("astunit-test", Dir)); +TestDir = Dir.str(); If

[PATCH] D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass

vsk added a comment. Nice! I'd like to get your thoughts on two candidate ergonomic changes: Comment at: unittests/Basic/DiagnosticTest.cpp:81 + llvm::Expected> Value = + llvm::make_error(PartialDiagnosticAt( + SourceLocation(), PartialDiagnostic(diag::err_cannot

[PATCH] D35271: Fix printing policy for AST context loaded from file

vsk added a comment. In https://reviews.llvm.org/D35271#847306, @jklaehn wrote: > In https://reviews.llvm.org/D35271#809159, @vsk wrote: > > > I wonder if it's possible to do away with the calls to 'updated()'... it > > seems strange that we initialize the same preprocessor repeatedly. Is there

[PATCH] D36969: [Basic] Add a DiagnosticOr type

vsk added a comment. Would it be more convenient to extend ErrorInfo instead of creating a new diagnostic wrapper? E.g one benefit of passing around Expected's is that there's some assurance the diagnostics will be reported. Repository: rL LLVM https://reviews.llvm.org/D36969 ___

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

vsk added inline comments. Comment at: lib/Lex/PPDirectives.cpp:570 + // We'll warn about reaching the end of file later. + if (C == '\0' || C == '\r' || C == '\n') +break; efriedma wrote: > This doesn't really handle backslash-escaped newlines

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

vsk updated this revision to Diff 111596. vsk marked an inline comment as done. vsk added a comment. - Address Eli's comment. https://reviews.llvm.org/D36642 Files: include/clang/Lex/Preprocessor.h lib/Lex/PPDirectives.cpp test/CoverageMapping/preprocessor.c test/Index/skipped-ranges.c

[PATCH] D36492: [time-report] Add preprocessor timer

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

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

vsk updated this revision to Diff 13. vsk added a comment. Thanks for the review. I've updated the patch so that we do better with "#\" directives. https://reviews.llvm.org/D36642 Files: include/clang/Lex/Preprocessor.h lib/Lex/PPDirectives.cpp test/CoverageMapping/preprocessor.c t

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

vsk created this revision. This patch teaches the preprocessor to report more precise source ranges for code that is skipped due to conditional directives. The new behavior includes the '#' from the opening directive and the full text of the line containing the closing directive in the skipped

[PATCH] D36492: [RFC][time-report] Add preprocessor timer

vsk added subscribers: mzolotukhin, vsk. vsk added a comment. Thanks for working on this. Collecting better timing information in the frontend sgtm. It's cheap to do, and we can use the information to guide our efforts re: attacking the compile-time problem. Feel free to add me to future timing

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

vsk accepted this revision. vsk added a comment. Thanks, lgtm. Repository: rL LLVM https://reviews.llvm.org/D36250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

vsk added a comment. I tested this out on clang's ItaniumMangle.cpp and the crash is resolved. Comment at: lib/CodeGen/CoverageMappingGen.cpp:723 + this->Visit(Child); +handleFileExit(getEnd(E)); + This is doing the right thing. I think it should just

[PATCH] D35925: [Coverage] Precise region termination with deferred regions

vsk added a comment. Reverted in r310154, because llvm-cov isn't doing a good job of displaying the new regions properly. I'll re-land this after llvm-cov is fixed. https://reviews.llvm.org/D35925 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D35925: [Coverage] Precise region termination with deferred regions

vsk closed this revision. vsk added a comment. Committed in r310010 https://reviews.llvm.org/D35925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rL LLVM https://reviews.llvm.org/D36250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D36112: [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

vsk marked an inline comment as done. vsk added a comment. In https://reviews.llvm.org/D36112#828891, @arphaman wrote: > That makes sense. It's kinda weird not to report the `null`, but I guess it > makes sense if the `null` sanitiser is off. It is kinda weird, but any such diagnostic would fi

[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

vsk added a comment. In https://reviews.llvm.org/D34801#828382, @efriedma wrote: > I'm going to look over the overall algorithm one more time to make sure this > direction makes sense. The current code for ending regions on > break/continue/return/noreturn doesn't really work well, Maybe it's

[PATCH] D35925: [Coverage] Precise region termination with deferred regions

vsk updated this revision to Diff 109142. vsk marked 2 inline comments as done. vsk edited the summary of this revision. vsk added a comment. Thanks for the review! I'll hold off on landing this until the llvm-cov changes are in, so that people don't hit the issue where unexpected line execution

[PATCH] D36096: [ubsan] Make the 'vptr check disabled' warning more helpful

vsk abandoned this revision. vsk added a comment. Abandoned in favor of https://reviews.llvm.org/D36112, since it turns out -fsanitize=null,vptr doesn't work for some use cases. https://reviews.llvm.org/D36096 ___ cfe-commits mailing list cfe-commi

[PATCH] D36112: [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

vsk created this revision. In r309007, I made -fsanitize=null a hard prerequisite for -fsanitize=vptr. I did not see the need for the two checks to have separate null checking logic for the same pointer. I expected the two checks to either always be enabled together, or to be mutually compatibl

[PATCH] D35849: [UBSan] Provide default blacklist filename for UBSan

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. This lgtm, we shouldn't defer this until https://reviews.llvm.org/D32842 is done, as that may take a while. https://reviews.llvm.org/D35849 ___ cfe-co

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

vsk added a comment. In https://reviews.llvm.org/D32842#826383, @shenhan wrote: > Ping? Can we make a decision on this? > I've this simple one D35849: [UBSan] Provide default blacklist filename for > UBSan which, depending on this, shall be > discarded or mov

[PATCH] D36096: [ubsan] Make the 'vptr check disabled' warning more helpful

vsk created this revision. If -fsanitize=vptr is passed without -fsanitize=null being specified, it will say: warning: implicitly disabling vptr sanitizer because null checking wasn't enabled (try specifying -fsanitize=null or -fsanitize=undefined) Otherwise if the vptr check is enabled and

[PATCH] D34590: [ubsan] Diagnose invalid uses of builtins (clang)

vsk edited reviewers, added: eugenis; removed: efriedma. vsk added a comment. Ping. https://reviews.llvm.org/D34590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35925: [Coverage] Precise region termination with deferred regions

vsk added a comment. In https://reviews.llvm.org/D35925#823978, @arphaman wrote: > This is awesome! > > I noticed in the sample output that llvm-cov is now forced to print some new > region markers because the terminator introduces a new region on the same > line, e.g. > > |// CHECK-LABEL: _Z

[PATCH] D35925: [Coverage] Precise region termination with deferred regions

vsk created this revision. The current coverage implementation doesn't handle region termination very precisely. Take for example an `if' statement with a `return': void f() { if (true) { return; // The `if' body's region is terminated here. } // This line gets the same covera

[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

vsk added a comment. Hi Eli, are you waiting on something before landing this? Repository: rL LLVM https://reviews.llvm.org/D34801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35736: [ubsan] -fsanitize=vptr now requires -fsanitize=null, update tests

vsk closed this revision. vsk added a comment. Thanks! This landed in r309008. https://reviews.llvm.org/D35736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35735: [ubsan] Null-check pointers in -fsanitize=vptr (PR33881)

vsk marked an inline comment as done. vsk added a comment. I made the suggested test changes and updated the release notes: r309007 Repository: rL LLVM https://reviews.llvm.org/D35735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D35849: [UBSan] Provide default blacklist filename for UBSan

vsk added a reviewer: vsk. vsk added a comment. This won't do the right thing if more than one sanitizer with a default blacklist is enabled. It's also problematic that a default blacklist for one sanitizer can blacklist code for a different sanitizer. See: https://reviews.llvm.org/D32043 https

[PATCH] D35735: [ubsan] Null-check pointers in -fsanitize=vptr (PR33881)

vsk updated this revision to Diff 107741. vsk marked an inline comment as done. vsk added a comment. - Drop 'REQUIRES: asserts'. https://reviews.llvm.org/D35735 Files: docs/UndefinedBehaviorSanitizer.rst include/clang/Basic/DiagnosticDriverKinds.td include/clang/Basic/DiagnosticGroups.td

[PATCH] D35729: [Frontend] - Mark some ASTUnit methods as const

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

[PATCH] D35736: [ubsan] -fsanitize=vptr now requires -fsanitize=null, update tests

vsk created this revision. Herald added a subscriber: kubamracek. See: https://bugs.llvm.org/show_bug.cgi?id=33881 Depends on https://reviews.llvm.org/D35735 https://reviews.llvm.org/D35736 Files: test/ubsan/TestCases/TypeCheck/PR33221.cpp test/ubsan/TestCases/TypeCheck/vptr-corrupted-vtab

[PATCH] D35735: [ubsan] Null-check pointers in -fsanitize=vptr (PR33881)

vsk created this revision. The instrumentation generated by -fsanitize=vptr does not null check a user pointer before loading from it. This causes crashes in the face of UB member calls (this=nullptr), i.e it causes user programs to crash only after UBSan is turned on. The fix is to make run-time

[PATCH] D32047: [Driver] Add support for default UBSan blacklists

vsk added a comment. I'd like to address the current problems with sanitizer blacklists before moving on to this patch: https://reviews.llvm.org/D32842 The summary is that the entries in a blacklist file apply to all sanitizers, resulting in potential false negatives when multiple default black

[PATCH] D35338: Add the -fdestroy-globals flag

vsk added a subscriber: bruno. vsk added a comment. That seems like a nice win and I like the convenience of this approach. That said I've just remembered that there's a thread on cfe-dev about this: [RFC] Suppress C++ static destructor registration I don't think a consensus was reached. From wh

[PATCH] D35212: [Index] Prevent canonical decl becoming nullptr

vsk added a comment. LGTM, thank you! (Let me know if you need someone to commit this for you.) https://reviews.llvm.org/D35212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35212: [Index] Prevent canonical decl becoming nullptr

vsk added a comment. Take a look at the tests in clang/test/Index/Core. You should be able to write a test which uses c-index-test to dump the symbols in this source. https://reviews.llvm.org/D35212 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D35271: Fix printing policy for AST context loaded from file

vsk added a comment. I wonder if it's possible to do away with the calls to 'updated()'... it seems strange that we initialize the same preprocessor repeatedly. Is there any way to finalize an ASTInfoCollector after ReadAST happens (or ASTReaderListeners in general)? https://reviews.llvm.org/

[PATCH] D35338: Add the -fdestroy-globals flag

vsk added a comment. This is interesting. Do you have any results/metrics to share (e.g some any binary size reduction for projects you've looked at)? Comment at: lib/Frontend/CompilerInvocation.cpp:585 Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases); + Opt

[PATCH] D35212: [Index] Prevent canonical decl becoming nullptr

vsk added subscribers: arphaman, vsk. vsk added a comment. Do you have a test case? https://reviews.llvm.org/D35212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35385: [Driver] Darwin: Link in the profile runtime archive first

vsk created this revision. While building a project with code coverage enabled, we can link in dependencies which export a weak definition of __llvm_profile_filename. After r306710, linking in the profiling runtime could pull in a weak definition of this symbol from a dependency, instead of from

[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)

vsk added a comment. In https://reviews.llvm.org/D34121#808486, @dtzWill wrote: > In https://reviews.llvm.org/D34121#806978, @vsk wrote: > > > @dtzWill do you have any further comments on this one? > > > > I'd like to get another 'lgtm' before committing, and it'd be nice to get > > this in befo

[PATCH] D34301: [Sema] Make sure the definition of a referenced virtual function is emitted when it is final

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

[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)

vsk added a comment. @dtzWill do you have any further comments on this one? I'd like to get another 'lgtm' before committing, and it'd be nice to get this in before llvm 5.0 branches (7/19). FWIW we've been living on this for a few weeks internally without any issues: https://github.com/apple/s

[PATCH] D34301: [Sema] Make sure the definition of a referenced virtual function is emitted when it is final

vsk added a comment. I suggest changing the API for 'canDevirtualizeCall' a bit but this is looking great overall. The code movement and test case changes look good. Comment at: include/clang/AST/DeclCXX.h:1902 + bool canDevirtualizeCall(const Expr *Base, bool IsAppleKext, +

[PATCH] D34981: RecursiveASTVisitor should visit the nested name qualifiers in a template specialisation

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. I haven't touched any AST code but this looks good to me: it's in line with what's done in TraverseRecordHelper and the test case is comprehensive. Comment at: unittests/Tooling/R

[PATCH] D34680: clang-cl crashes with -fprofile-instr-use flag

vsk added a comment. Thanks for the stack trace. Clang shouldn't ever be assigning counters to decls without bodies. As far as I can tell, this has only been happening when the microsoft ABI is in use, which may explain why we don't hit the issue more often. The fix required changing the way we

[PATCH] D34886: Add a fixit for -Wobjc-protocol-property-synthesis

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. lgtm with a nitpick. Comment at: include/clang/Sema/Sema.h:3351 + ObjCInterfaceDecl *IDecl, SourceRange AtEnd); + void DefaultSynthesizeProperti

[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks for the patch! LGTM. Comment at: lib/CodeGen/CoverageMappingGen.cpp:686 +// FIXME: a break in a switch should terminate regions for all preceding +// case statements

[PATCH] D34680: clang-cl crashes with -fprofile-instr-use flag

vsk added a comment. Do you know which function clang was processing when it crashed? That would help us find a test case. https://reviews.llvm.org/D34680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D34591: [ubsan] Diagnose invalid uses of builtins (compiler-rt)

vsk created this revision. Herald added subscribers: dberris, kubamracek. Compiler-rt changes and tests to go along with: https://reviews.llvm.org/D34590 https://reviews.llvm.org/D34591 Files: lib/ubsan/ubsan_checks.inc lib/ubsan/ubsan_handlers.cc lib/ubsan/ubsan_handlers.h lib/ubsan/ub

[PATCH] D34590: [ubsan] Diagnose invalid uses of builtins (clang)

vsk created this revision. On some targets, passing zero to the clz() or ctz() builtins has undefined behavior. I ran into this issue while debugging UB in __hash_table from libcxx: the bug I was seeing manifested itself differently under -O0 vs -Os, due to a UB call to clz() (see: libcxx/r304617)

[PATCH] D34563: [ubsan] Disable the object-size check at -O0

vsk updated this revision to Diff 103778. vsk added a comment. Add a diagnostic for users who explicitly turn the object-size check on at -O0, and tighten up the test a bit. https://reviews.llvm.org/D34563 Files: include/clang/Basic/DiagnosticDriverKinds.td lib/Driver/SanitizerArgs.cpp t

[PATCH] D34563: [ubsan] Disable the object-size check at -O0

vsk created this revision. This is motivated by the thread: [cfe-dev] Disabling ubsan's object size check at -O0 I think the driver is the best place to disable a sanitizer check at particular optimization levels. Doing so in the frontend is messy, and makes it really hard to test IR generation

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

vsk added a comment. In https://reviews.llvm.org/D34299#788908, @arphaman wrote: > Ok, so now the null check `return.sloc.load` won't call the checker in > compiler-rt and so the program won't `abort` and won't hit the `unreachable`. > I have one question tough: > > This patch changes the behav

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

vsk added inline comments. Comment at: lib/CodeGen/CGStmt.cpp:1035 +assert(ReturnLocation.isValid() && "No valid return location"); +Builder.CreateStore(Builder.CreateBitCast(SLocPtr, Int8PtrTy), +ReturnLocation); filcab wrote: > C

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

vsk updated this revision to Diff 103632. vsk marked an inline comment as done. vsk added a comment. Handle functions without return statements correctly (fixing an issue pointed out by @arphaman). Now, the instrumentation always checks that we have a valid return location before calling the run

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

vsk added a comment. In https://reviews.llvm.org/D34299#788379, @filcab wrote: > In https://reviews.llvm.org/D34299#788152, @vsk wrote: > > > The source locations aren't constants. The ubsan runtime uses a bit inside > > of source location structures as a flag. When an issue is diagnosed at a >

[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)

vsk updated this revision to Diff 103606. vsk added a comment. Fix a typo introduced in emitArraySubscriptGEP while refactoring /*isSubtraction=false*/ to CodeGenFunction::NotSubtraction, and add CHECK lines which catch the issue. https://reviews.llvm.org/D34121 Files: lib/CodeGen/CGExpr.cp

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

vsk added a comment. In https://reviews.llvm.org/D34299#787795, @arphaman wrote: > It looks like if we have a function without the `return` (like the sample > below), we will pass in a `0` as the location pointer. This will prevent a > report of a runtime error as your compiler-rt change ignore

[PATCH] D34301: [Sema] Make sure the definition of a referenced virtual function is emitted when it is final

vsk added a comment. It looks Comment at: lib/Sema/SemaExpr.cpp:14715 +if (Method->isVirtual() && !(Method->hasAttr() || + Method->getParent()->hasAttr())) OdrUse = false; Do you think it makes sense to eliminate all c

<    1   2   3   4   5   6   7   >