[PATCH] D75615: Revert "[CGBlocks] Improve line info in backtraces containing *_helper_block"

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

[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

This revision was automatically updated to reflect the committed changes. Closed by commit rGd889d1efefe9: [profile] Add a mode to continuously sync counter updates to a file (authored by vsk). Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Changed pr

[PATCH] D69699: [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

vsk added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69699/new/ https://reviews.llvm.org/D69699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

vsk created this revision. vsk added reviewers: aprantl, djtodoro, dblaikie. Currently, clang emits subprograms for declared functions when the target debugger or DWARF standard is known to support entry values (DW_OP_entry_value & the GNU equivalent). Expand this behavior to include debuggers th

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

vsk updated this revision to Diff 227774. vsk edited the summary of this revision. vsk added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Add coverage for a callee without a subprogram in llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll. CHANGES SIN

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

vsk added a comment. In D69743#1732967 , @dblaikie wrote: > What's the extra DWARF that's emitted from the backend? Is there a > corresponding LLVM code change/test, or is the functionality there but > dormant in this particular case? (& already running/

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5c8ec4baa2c: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (authored by vsk). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk created this revision. vsk added reviewers: aprantl, djtodoro, dblaikie. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Currently, clang emits subprograms for declared functions when the target debugger or DWARF standard is known to support entry values (DW_OP_entry_val

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk added a comment. In D69970#1737814 , @dblaikie wrote: > The failure I am investigating from the original commit of this at Google > probably isn't related to the assertion failure that caused the revert of > this patch/being addressed by this recommi

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

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Comment at: clang/test/CodeGenObjC/debug-info-objc-property-dwarf5.m:26 +// CHECK: ![[DECL:[0-9]+]] = !DISubprogram(name: "-[Foo setBar:]", +// CHECK-SAME: scope: ![

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk added a comment. @dblaikie do you need more time to investigate? fwiw I didn't uncover any new failures in a stage2 build with this patch applied. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69970/new/ https://reviews.llvm.org/D69970 ___

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk added a comment. @dblaikie thanks for chasing that down & the detailed explanation. It's unfortunate that introducing call site info affects the line table. The impact of the change seems somewhat neutral (the additional line 0 locations aren't //wrong//, exactly), so I'm not sure it's wort

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk updated this revision to Diff 229215. vsk added a comment. Don't emit declaration subprograms for functions with reserved names. There may not be much benefit from referencing these functions from call site tags (e.g. instead of relying on call site info to work out the arguments passed to

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

vsk added a comment. In D69970#1746103 , @dblaikie wrote: > In D69970#1745038 , @vsk wrote: > > > Don't emit declaration subprograms for functions with reserved names. There > > may not be much benefit from referen

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

vsk added a comment. I’d expect only the entry values test to fail. Is that the one? Could you just mark it as skipped (there are examples of how to do this for inline tests, see the skipIf decorator) and file a bugzilla PR for me to look at? CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D69586: [profile] Support online merging with continuous sync mode

This revision was automatically updated to reflect the committed changes. Closed by commit rG2492b5a12550: [profile] Support online merging with continuous sync mode (authored by vsk). Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Repository: rG LL

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

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

[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

vsk marked an inline comment as done. vsk added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:377 +#if defined(__Fuchsia__) || defined(_WIN32) + PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows."); +#else // defined(__Fuchsia_

[PATCH] D69586: [profile] Support online merging with continuous sync mode

vsk marked an inline comment as done. vsk added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:419 + * the open file object \p File. */ +static int writeProfileWithFileObject(const char *Filename, FILE *File) { + setProfileFile(File);

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

vsk added a comment. Could you add a regression test? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70537/new/ https://reviews.llvm.org/D70537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

vsk created this revision. vsk added reviewers: hans, rnk, efriedma. vsk edited the summary of this revision. Emit a gap region beginning where the switch body begins. This sets line execution counts in the areas between non-overlapping cases to 0. This does not resolve an outstanding issue with

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

vsk planned changes to this revision. vsk marked an inline comment as done. vsk added a comment. I'll write something up in the coverage mapping docs. Briefly, though, with this change there should be a gap region that covers the entire switch body (starting from the '{' in 'switch {', and termi

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

vsk requested changes to this revision. vsk added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1437 +if (isa(V)) + continue; + I see that we don't attempt to handle `VarTemplateSpec

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

vsk updated this revision to Diff 230714. vsk added a comment. - Add some documentation about clang internals. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 Files: clang/docs/SourceBasedCodeCoverage.rst clang/lib/CodeGen/CoverageMappingGen

[PATCH] D69740: [profile] Support counter relocation at runtime

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm with a small cleanup. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:674 + if (isRuntimeCounterRelocationEnabled()) { +Type *Int64Ty = Type::getInt64T

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

vsk updated this revision to Diff 231724. vsk added a comment. Add test case from PR44011. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 Files: clang/docs/SourceBasedCodeCoverage.rst clang/lib/CodeGen/CoverageMappingGen.cpp clang/test/Co

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

vsk added inline comments. Comment at: clang/test/CodeGenCXX/pr42710.cpp:4 +// RUN: %clang %s -DTYPE=int -emit-llvm -S -g -o - -std=c++17 +// expected-no-diagnostics + Could you validate the debug info for the struct members in the `int` case? (It may be simpler

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

vsk marked an inline comment as done. vsk added inline comments. Comment at: clang/docs/SourceBasedCodeCoverage.rst:330 +last case ends). This gap region has a zero count: this causes "gap" areas in +between case statements, which contain no executable code, to appear uncovered.

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

This revision was automatically updated to reflect the committed changes. Closed by commit rG859bf4d2bea2: [Coverage] Emit a gap region to cover switch bodies (authored by vsk). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D70571?vs=231724&id=231964#toc Repo

[PATCH] D69471: [Coverage] Revise format to reduce binary size

This revision was automatically updated to reflect the committed changes. Closed by commit rGe18531595bba: [Coverage] Revise format to reduce binary size (authored by vsk). Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Repository: rG LLVM Github Mo

[PATCH] D69471: [Coverage] Revise format to reduce binary size

vsk reopened this revision. vsk marked an inline comment as done. vsk added a comment. This revision is now accepted and ready to land. I reverted this due to failures on Windows that I did not encounter in local testing. I suspect that there's an error in the coverage parsing logic, as the same

[PATCH] D71042: WIP: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk created this revision. vsk added reviewers: aprantl, dblaikie. When `CodeGenFunction::FinishFunction()` is passed an empty end location the function does not only contain 'simple' return statements, a cleanup which involves a call may be popped. As there is no debug location set (due to the

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk updated this revision to Diff 232373. vsk retitled this revision from "WIP: [DebugInfo] Ensure fallback artificial location is available for cleanups" to "[DebugInfo] Ensure fallback artificial location is available for cleanups". vsk edited the summary of this revision. vsk added a comment.

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk added a comment. In D71042#1771040 , @aprantl wrote: > This will definitely work; Once you got the testcase, it might be good to > check whether there is a more targeted root cause that we could fix and > assert on EndLoc here. I believe the root c

[PATCH] D71084: Set a source location for Objective-C accessor stubs

vsk added a comment. Shall we add in the assert we discussed in CFG::FinishFunction, as an additional test? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71084/new/ https://reviews.llvm.org/D71084 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm with a minor change. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:384 + else +assert(EndLoc.isValid() && "no location for inlineable cleanup calls");

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk added inline comments. Comment at: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm:16 + +// [[LOC]] = !DILocation(line: 0 + Heads-up: this location should be different once https://reviews.llvm.org/D71084 lands, which I assume this depends on. CHA

[PATCH] D71084: Set a source location for Objective-C accessor stubs

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71084/new/ https://reviews.llvm.org/D71084 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

vsk added a comment. This is causing a failure in debuginfo-tests: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4450/testReport/junit/debuginfo-tests/llgdb-tests/block_var_m/ Assertion failed: (EndLoc.isValid() && "no location for inlineable cleanup calls") Repository: rG LLVM Gi

[PATCH] D69471: [Coverage] Revise format to reduce binary size

vsk added a comment. Sorry for the delay here. An update: I tested this patch with a sanitized build on a Linux install, but could not reproduce the "malformed coverage data" errors seen on the Windows bots. At this point, it seems likely that there is a bug in CoverageMappingReader (this would

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk created this revision. vsk added reviewers: rjmccall, jfb, arphaman, delcypher. Herald added subscribers: llvm-commits, dexonsmith. Herald added a project: LLVM. Check that the implicit cast from `id` used to construct the element variable in an ObjC for-in statement is valid. This check is i

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk updated this revision to Diff 234136. vsk marked 3 inline comments as done. vsk added a comment. Address review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 Files: clang/docs/UndefinedBehaviorSanitizer.rst clang/include/clan

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:953 + SanitizerKind::ImplicitConversion | SanitizerKind::Nullability | + SanitizerKind::LocalBounds | SanitizerKind::ObjCCast; if (getTriple().getArch() == llvm::Triple::x86 || del

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk updated this revision to Diff 234139. vsk added a comment. Avoid a static initializer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 Files: clang/docs/UndefinedBehaviorSanitizer.rst clang/include/clang/Basic/Sanitizers.def clang/lib/

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk updated this revision to Diff 234142. vsk added a comment. Ignore an objc-cast report at a given SourceLocation after it's been reported once. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 Files: clang/docs/UndefinedBehaviorSanitizer.rs

[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm. Separately, if you have the bandwidth to test out the new '%c%m' mode (https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program / https://reviews.llvm.org/D68

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29 +const char *__ubsan::getObjCClassName(ValueHandle Pointer) { +#if defined(__APPLE__) + // We need to query the ObjC runtime for some information, but do not want delcypher wrote: >

[PATCH] D75175: [CallSiteInfo] Enable the call site info only for -g + optimizations

vsk added a comment. Sorry for the delay here. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:793 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values); +Opts.EmitCallSiteInfo = true; + } djtodoro wrote: > vsk wrote: > > I don

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

vsk added a comment. @awpandey @SouraVX this is still breaking the stage2 clang self-host with -gdwarf-5, see https://bugs.llvm.org/show_bug.cgi?id=45166. Do you have time to take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ h

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

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

[PATCH] D74355: [ubsan] Null-check TypeLoc before using it

vsk created this revision. vsk added reviewers: erik.pilkington, delcypher. Herald added a subscriber: dexonsmith. Null-check a TypeLoc before casting it to a FunctionTypeLoc. This fixes a crash in -fsanitize=nullability-return. rdar://59263039 https://reviews.llvm.org/D74355 Files: clang/li

[PATCH] D74355: [ubsan] Null-check and adjust TypeLoc before using it

vsk updated this revision to Diff 243672. vsk retitled this revision from "[ubsan] Null-check TypeLoc before using it" to "[ubsan] Null-check and adjust TypeLoc before using it". vsk edited the summary of this revision. vsk added a comment. - Check adjusted return type per Erik's offline feedback

[PATCH] D74355: [ubsan] Null-check and adjust TypeLoc before using it

This revision was automatically updated to reflect the committed changes. Closed by commit rG8b81ebfe7eba: [ubsan] Null-check and adjust TypeLoc before using it (authored by vsk). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

vsk added a comment. Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from llvm/lib/Transforms/Utils/CodeExtractor.cpp? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 __

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

vsk added a comment. Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, as we get an assert in CodeExtractor without the workaround. (side note: please don't take my comments here as blocking, I just wanted to see if we could delete some cruft) Repository: rG LLVM G

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

vsk added inline comments. Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1408 - // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor. - bool NeedWorkaroundForOpenMPIRBuilderBug = - OldSP && OldSP->getRetainedNodes()->isTemporary(); - -

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk added subscribers: erik.pilkington, ahatanak. vsk added a comment. + Erik and Akira for IRgen expertise. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D69471: [Coverage] Revise format to reduce binary size

vsk updated this revision to Diff 244790. vsk added a comment. Rebase. Sorry this hasn't seen any update: at this point in our release cycle I've had to put this on the back burner. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69471/new/ https://reviews.llvm.org/D69471 Files: clan

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

vsk updated this revision to Diff 241019. vsk marked 2 inline comments as done. vsk edited the summary of this revision. vsk added a comment. Rebase, address Florian's feedback. Apologies for the delay here. I haven't had time to push this forward, but I have also been waiting to get an addition

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

vsk added a comment. The "Cannot represent a difference across sections" error is almost certainly related to -fsanitize=function. Time permitting, could you file a separate PR for that and attach the IR? It'd be great to know whether the issue reproduces without coroutines involved. Also, cou

[PATCH] D44915: [coroutines] Fix invalid source range in co_await call expressions.

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

[PATCH] D44964: Change order of libclang_rt.profile link for freebsd

vsk added a comment. Could you add a test in test/Driver/instrprof-ld.c? Repository: rC Clang https://reviews.llvm.org/D44964 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45178: Fixes errors caused by https://reviews.llvm.org/D44960

vsk added a comment. I'd like to test this out a bit more. Could you wait one more day? Repository: rC Clang https://reviews.llvm.org/D45178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D45178: Fixes errors caused by https://reviews.llvm.org/D44960

vsk accepted this revision. vsk added a comment. Lgtm, thank you. If you don't have commit access, let us know, and somebody will commit this on your behalf. Repository: rC Clang https://reviews.llvm.org/D45178 ___ cfe-commits mailing list cfe-

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk created this revision. Instead of only setting a non-zero debug location on the return instruction in *_helper_block functions, set a proper location on all instructions within these functions. Pick the start location of the block literal expr for maximum clarity. The debugger does not step i

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk updated this revision to Diff 120342. vsk added a comment. - Simplify the checks by not hardcoding a line number. https://reviews.llvm.org/D39310 Files: lib/CodeGen/CGBlocks.cpp test/CodeGenObjC/debug-info-blocks.m Index: test/CodeGenObjC/debug-info-blocks.m ==

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1651 StartFunction(FD, C.VoidTy, Fn, FI, args); - // Create a scope with an artificial location for the body of this function. - auto AL = ApplyDebugLocation::CreateArtificial(*this); + ApplyDebugLocation NL{*t

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk added inline comments. Comment at: test/CodeGenObjC/debug-info-blocks.m:13 -// rdar://problem/14386148 -// Test that we don't emit bogus line numbers for the helper functions. -// Test that we do emit scope info for the helper functions. +// rdar://problem/32907581 +// Test

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk updated this revision to Diff 120461. vsk marked 8 inline comments as done. vsk added a comment. - Tighten test case to show the dbg loc on return instructions. https://reviews.llvm.org/D39310 Files: lib/CodeGen/CGBlocks.cpp test/CodeGenObjC/debug-info-blocks.m Index: test/CodeGenObjC

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk added inline comments. Comment at: test/CodeGenObjC/debug-info-blocks.m:20 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]] // CHECK-NOT: ret // CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]] aprantl wrote: > aprantl wrote: > > vsk wrote: > > > aprantl wro

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

vsk added a comment. Thanks! Comment at: test/CodeGenObjC/debug-info-blocks.m:20 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]] // CHECK-NOT: ret // CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]] aprantl wrote: > vsk wrote: > > aprantl wrote: > > > aprantl

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk created this revision. Lifting from Bob Wilson's notes: The hash value that we compute and store in PGO profile data to detect out-of-date profiles does not include enough information. This means that many significant changes to the source will not cause compiler warnings about the profile bei

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk planned changes to this revision. vsk added a comment. Apart from Alex's point, this patch is missing a compatibility test for v5 of the profile format, and is missing coverage for the new entries in `HashType`. Comment at: lib/CodeGen/CodeGenPGO.cpp:186 +if (Hash.getH

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk updated this revision to Diff 121224. vsk marked 2 inline comments as done. vsk edited the summary of this revision. vsk added a comment. - Handle loop nesting, conditions, and out-of-order control flow. - Improve test coverage. Add a format compatibility test, and check that functions which

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk updated this revision to Diff 122368. vsk edited the summary of this revision. vsk added a comment. - Consider logical nots and some binary comparison operators in the hash, per an offline conversation with @bogner https://reviews.llvm.org/D39446 Files: lib/CodeGen/CodeGenPGO.cpp test/

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:244 + DEFINE_NESTABLE_TRAVERSAL(CXXTryStmt) + DEFINE_NESTABLE_TRAVERSAL(CXXCatchStmt) + arphaman wrote: > What about `ObjCAtTryStmt`? I thought about this, but: we don't place a counter on ObjC a

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

vsk added a comment. Thanks for the review Alex. There hasn't been any more feedback so I'll commit this soon. https://reviews.llvm.org/D39446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D27410: Always issue vtables when generating coverage instrumentation

vsk added inline comments. Comment at: llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp:3 +// RUN: FileCheck %s < %t +// CHECK: @_ZTV8Category = linkonce_odr unnamed_addr constant {{.*}}, comdat, + ahatanak wrote: > I'm not sure I understood the purpose

[PATCH] D27410: Always issue vtables when generating coverage instrumentation

vsk added inline comments. Comment at: llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp:3 +// RUN: FileCheck %s < %t +// CHECK: @_ZTV8Category = linkonce_odr unnamed_addr constant {{.*}}, comdat, + vsk wrote: > ahatanak wrote: > > I'm not sure I understo

[PATCH] D28131: [libcxx] Fix PR31402: map::__find_equal_key has undefined behavior.

vsk added a comment. LGTM. I'm not sure what to do for a test. Have you tried checking the IR for the affected object file in tablegen at '-O2 -fsanitize=undefined'? If there's an unconditional call to the ubsan handler, maybe the tablegen source could be used to find a small reproducer? http

[PATCH] D30949: [CodeGen] Add an option to enable LLVM IR linting

vsk created this revision. LLVM has a nifty linter which checks for some common kinds of unusual or undefined behavior by doing some basic IR-level static analysis. Add a CC1 option to clang which enables this analysis. Having the linter available through clang could be a useful debugging tool.

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk added a comment. Hi Eli, thanks for your feedback :). In https://reviews.llvm.org/D30283#702085, @efriedma wrote: > I'm not sure we actually want to skip these checks for DeclRefExps. I mean, > you can rely on the backend to correctly align a local variable (assuming the > stack is correc

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk added a comment. In https://reviews.llvm.org/D30283#707007, @efriedma wrote: > It's possible to misalign a global definition by misaligning its section with > a linker script... but we can probably ignore that possibility. The current implementation does ignore this case. > It's very easy

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk updated this revision to Diff 92572. vsk added a comment. Add a test which shows that we don't regress alignment-checking when accessing extern variables. https://reviews.llvm.org/D30283 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprCXX.cpp lib/CodeGen/CodeGenFunction.cpp test/Co

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk updated this revision to Diff 92732. vsk added a comment. Per Eli's comment: test that we don't regress alignment-checking for extern globals which aren't arrays. I verified that for this case, there is not functional change. However, there *is* a somewhat surprising IR change even at -O0,

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk updated this revision to Diff 95053. vsk edited the summary of this revision. vsk added a comment. Eli pointed out that it's possible to make alignment checks for extern globals work better (llvm.org/PR32630). One solution depends on emitting alignment checks on bases of member expressions w

[PATCH] D32043: [Driver] Load all necessary default sanitizer blacklists

vsk created this revision. This patch removes a limitation which causes us to load at most one default sanitizer blacklist when multiple sanitizers are enabled. E.g if asan + cfi are enabled, and default blacklists for both sanitizers are present, we would only load one of the blacklists. The new

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

vsk created this revision. Clang should look for a default UBSan blacklist in its resource dir. Depends on https://reviews.llvm.org/D32043. https://reviews.llvm.org/D32047 Files: lib/Driver/SanitizerArgs.cpp test/Driver/Inputs/resource_dir/ubsan_blacklist.txt test/Driver/fsanitize-blackl

[PATCH] D30283: [ubsan] Reduce alignment checking of C++ object pointers

vsk added a comment. Thanks for the review! Comment at: test/CodeGenCXX/ubsan-suppress-checks.cpp:25 +// LAMBDA: and i64 %[[THISINT2]], 3, !nosanitize +// LAMBDA: call void @__ubsan_handle_type_mismatch + efriedma wrote: > LAMBDA-NOT: call void @__ubsan

[PATCH] D32144: [Coverage] Don't emit mappings for functions in dependent contexts (fixes PR32679)

vsk created this revision. The coverage implementation marks functions which won't be emitted as 'deferred', so that it can emit empty coverage regions for them later (once their linkages are known). Functions in dependent contexts are an exception: if there isn't a full instantiation of a functi

[PATCH] D28867: [Profile] Warn about out-of-date profiles only when there are mismatches

vsk created this revision. Clang warns that a profile is out-of-date if it can't find a profile record for any function in a TU. This warning is now noisy because llvm can dead-strip functions with profiling instrumentation. Only emit the out-of-date warning if there is an actual record mismatch.

[PATCH] D28867: [Profile] Warn about out-of-date profiles only when there are mismatches

vsk added a comment. Ah ok, so it sounds like a better approach would be to split the missing record message into a separate off-by-default warning. I don't have the time to update this diff this week, but will shoot for the next. https://reviews.llvm.org/D28867

[PATCH] D29234: [ubsan] Sanity-check shift amounts before truncation (fixes PR27271)

vsk created this revision. Ubsan does not report UB shifts in some cases where the shift exponent needs to be truncated to match the type of the shift base. We perform a range check on the truncated shift amount, leading to false negatives. Fix the issue (PR27271) by performing the range check on

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

vsk created this revision. C requires the operands of arithmetic expressions to be promoted if their types are smaller than an int. Ubsan emits overflow checks when this sort of type promotion occurs, even if there is no way to actually get an overflow with the promoted type. This patch teaches c

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

vsk marked an inline comment as done. vsk added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:72 + if (const auto *UO = dyn_cast(Op.E)) +return IsPromotedInteger(UO->getSubExpr()); + efriedma wrote: > Checking isPromotableIntegerType doesn't work

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

vsk updated this revision to Diff 86739. vsk marked an inline comment as done. vsk added a comment. - Per Eli's comment: check that integers are actually widened, instead of incorrectly assuming they are always widened. I added some test cases for this. - Address the 'fixme' regarding multiplicat

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

vsk updated this revision to Diff 86746. vsk edited the summary of this revision. vsk added a comment. - Remove a stale test case in unsigned-promotion.c. https://reviews.llvm.org/D29369 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGen/compound-assign-overflow.c test/CodeGen/ubsan-promote

[PATCH] D29437: [ubsan] Detect signed overflow UB in remainder operations

vsk created this revision. Teach ubsan to diagnose remainder operations which have undefined behavior due to signed overflow. My copy of the C11 spec draft (6.5.5.6) says that: if the quotient a/b is representable, (a/b)*b + a%b shall equal a; otherwise, the behavior of both a/b and a%b is und

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

vsk added a comment. In https://reviews.llvm.org/D29369#664366, @regehr wrote: > Out of curiosity, how many of these superfluous checks are not subsequently > eliminated by InstCombine? I don't have numbers from a benchmark prepped. Here's what we get with the 'ubsan-promoted-arith.cpp' test

[PATCH] D29437: [ubsan] Detect signed overflow UB in remainder operations

vsk added a comment. In https://reviews.llvm.org/D29437#664379, @regehr wrote: > Does this check need to be sensitive to the dialect of C/C++ that the user > asked for? I know that it used to be the case that the standard could be read > either way for this case, but as you observe it is now un

<    1   2   3   4   5   6   7   >