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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG8c4a65b9b2ca: [ubsan] Check implicit casts in ObjC for-in statements (authored by vsk). Repository: rG LLVM Github Mono

[PATCH] D83592: [Parser] Add comment to skipped regions

vsk added a comment. @zequanwu thanks for working on this. Instead of threading CommentSkipped through PPCallbacks, wdyt of taking advantage of the existing CommentHandler interface? To simplify things, we can add a static method like 'CoverageMappingGen::setupPreprocessorCallbacks(Preprocessor

[PATCH] D83514: [Lexer] Fix missing coverage line after #endif

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/D83514/new/ https://reviews.llvm.org/D83514

[PATCH] D82547: [Debugify] Expose debugify (original mode) as CC1 option

vsk requested changes to this revision. vsk added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/BackendUtil.cpp:855 +class ClangCustomPassManager : public legacy::PassManager { +public: Please factor out OptCustom

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

vsk updated this revision to Diff 273815. vsk added a comment. Use the `IsKindOfClass` CallArgList when emitting the check, and add a runtime test to ensure that an objc-cast diagnostic is not emitted on correct code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

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

vsk marked 2 inline comments as done. vsk added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:1860 + CGM.getObjCRuntime().GetClass(*this, InterfaceTy->getDecl()); + Args.add(RValue::get(Cls), C.getObjCClassType()); + llvm::Value *IsClass = -

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

vsk updated this revision to Diff 271508. vsk added a comment. Herald added projects: clang, Sanitizers. Herald added a subscriber: Sanitizers. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 Files:

[PATCH] D81122: Reland: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

vsk added a subscriber: phosek. vsk added a comment. In D81122#2074213 , @keith wrote: > FYI I actually removed that piece this morning since I felt like since this > now supports `-path-equivalence=.,foo` which is the "expected" behavior from > lldb, th

[PATCH] D81122: Reland: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

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

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

vsk marked an inline comment as done. vsk added inline comments. Comment at: test/CodeGenObjCXX/os_log.mm:3 +// RUN: -fexceptions -fcxx-exceptions -O1 | FileCheck %s + +// Check that no EH cleanup is emitted around the call to __os_log_helper. ahatanak wrote: >

[PATCH] D80463: Debug Info: Mark os_log helper functions as artificial

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

[PATCH] D79967: Fix debug info for NoDebug attr

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. @yaxunl thanks, this patch lgtm. @dblaikie I've kicked off a thread on cfe-dev about the topics you brought up ("Design discussion re: DW_TAG_call_site support in clang") and cc'd you. CHANGES SIN

[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, this lgtm. If you could split this up into two commits before landing it, I'd appreciate it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D793

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

vsk added a comment. @manojgupta I apologize for not catching this earlier. The issue should really be fixed by 636665331bbd4c369a9f33c4d35fb9a863c94646 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ http

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

vsk added a comment. In D73534#1933988 , @djtodoro wrote: > In D73534#1933985 , @djtodoro wrote: > > > Oh sorry, I thought it all has been fixed, since all the tests pass. > > > > We should revert the D75036

[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] 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] 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] D75615: Revert "[CGBlocks] Improve line info in backtraces containing *_helper_block"

vsk added inline comments. Comment at: clang/test/CodeGenObjC/debug-info-blocks.m:24 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]] -// CHECK: ret void, !dbg ![[DESTROY_LINE]] +// CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: ![[COPY_SP:[0-9]+]])

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG99317124e1c7: [Coverage] Revise format to reduce binary size (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D69471?vs=245756&id=247396#toc Repository: rG LLVM Github Monorepo C

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

vsk updated this revision to Diff 245756. vsk added a comment. This revision is now accepted and ready to land. Get rid of multiple inheritance in the coverage::accessors namespace. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69471/new/ https://reviews.llvm.org/D69471 Files: clang/

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

vsk planned changes to this revision. vsk added a comment. @rnk Thanks for chasing this down. I'll update the function record structs to use free functions instead of multiple inheritance. I don't plan on getting rid of the awkward method calls at this point. The coverage reader is still templa

[PATCH] D74813: [RFC] Add hash of block contents to function block names

vsk added a comment. In D74813#1883241 , @alexbdv wrote: > As for making it default - would rather have this under a flag as hashing the > block contents does have some overhead and I imagine this feature wouldn't be > beneficial in most scenarios. Also,

[PATCH] D74813: [RFC] Add hash of block contents to function block names

vsk edited reviewers, added: ahatanak, erik.pilkington, dexonsmith; removed: vsk. vsk added a comment. At a high level the idea sounds good to me. For out OSes, symbol name instability results in serious performance regressions as this invalidates text/data orderfiles. Why shouldn’t this be on

[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] 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] 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] 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 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] 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] 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 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] 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] 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] 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] D71491: [ubsan] Check implicit casts in ObjC for-in statements

vsk updated this revision to Diff 238393. vsk marked an inline comment as done. 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/CodeGen/CGObj

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

vsk closed this revision. vsk added a comment. Yes, this landed in 568db780bb7267651a902da8e85bc59fc89aea70 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69970/new/ https://reviews.llvm.org/D69970 ___

[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] 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 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] 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 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 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 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] 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] 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] 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 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] 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 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] 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: 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] 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] 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] 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] 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] 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 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] 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 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] 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 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] 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] 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] 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] 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] 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] 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 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] 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] 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. @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 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] 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. 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] 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] 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] 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

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 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] 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] 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] D68733: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

vsk added a comment. In D68733#1723734 , @liaoyuke wrote: > In D68733#1702609 , @vsk wrote: > > > Thanks, lgtm! > > > > In PR43614 I mentioned adding an extra argument to llvm-cov to specify the > > base directory.

[PATCH] D69137: [profile] Do not cache __llvm_profile_get_filename result

This revision was automatically updated to reflect the committed changes. Closed by commit rG937241b0d9e8: [profile] Do not cache __llvm_profile_get_filename result (authored by vsk). Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Repository: rG LLV

[PATCH] D68733: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm! In PR43614 I mentioned adding an extra argument to llvm-cov to specify the base directory. On second thought, the existing `-path-equivalence` option should make that unnecessary.

[PATCH] D67774: [Mangle] Add flag to asm labels to disable '\01' prefixing

vsk updated this revision to Diff 221801. vsk added a comment. - Add a comment describing where non-literal labels are used. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67774/new/ https://reviews.llvm.org/D67774 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/A

[PATCH] D67774: [Mangle] Add flag to asm labels to disable '\01' prefixing

vsk updated this revision to Diff 221645. vsk retitled this revision from "[Mangle] Add flag to asm labels to disable global prefixing" to "[Mangle] Add flag to asm labels to disable '\01' prefixing". vsk edited the summary of this revision. vsk added a comment. - Address latest round of comment

[PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk added inline comments. Comment at: clang/lib/AST/Mangle.cpp:130 + return; +} + rjmccall wrote: > This is actually backwards, right? A literal label is one that doesn't get > the global prefix and therefore potentially needs the `\01` prefix to > s

[PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk updated this revision to Diff 221391. vsk marked 3 inline comments as done. vsk added a comment. - Address latest review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67774/new/ https://reviews.llvm.org/D67774 Files: clang/include/clang/Basic/Attr.td clang/include/cla

[PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

vsk updated this revision to Diff 221066. vsk retitled this revision from "[Mangle] Check ExternalASTSource before adding prefix to asm label names" to "[Mangle] Add flag to asm labels to disable global prefixing". vsk edited the summary of this revision. vsk added a comment. Thanks for your fee

[PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

vsk created this revision. vsk added reviewers: teemperor, jasonmolenda, rjmccall. LLDB may synthesize decls using asm labels. These decls cannot have a mangle different than the one specified in the label name. I.e., the mangle-suppression prefix should not be added. Fixes an expression evaluati

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

vsk added a comment. > In D67122#1659721 , @vsk wrote: > >> Still think this looks good. Have you tried running this on the llvm test >> suite, or some other interesting corpus? Would be curious to see any >> pre/post patch numbers. > > > There were 1.

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

vsk added a comment. Still think this looks good. Have you tried running this on the llvm test suite, or some other interesting corpus? Would be curious to see any pre/post patch numbers. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4703-4720 +// 2) The sign of the diff

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

vsk added a subscriber: dtzWill. vsk added a comment. Thanks, this is looking pretty good. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4538 + llvm::Value * /*OffsetOverflows*/> +EmitGEPOffsetInBytes(Value *BasePtr, Value *GEPVal, + llvm::L

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

vsk added inline comments. Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177 ``-fsanitize=undefined``. + - ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset`` + and ``pointer-overflow``. lebedev.ri wrote: > vsk wrote: > >

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

vsk added inline comments. Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177 ``-fsanitize=undefined``. + - ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset`` + and ``pointer-overflow``. Why does this need to be a separat

[PATCH] D66493: [NewPM] Run ubsan-coroutines test under the legacy pass manager only

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/D66493/new/ https://reviews.llvm.org/D66493

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

vsk added a comment. In D44672#1637891 , @leonardchan wrote: > It seems that this test leads to an `UNREACHABLE` under the new pass manager > (can check this by adding `-fexperimental-new-pass-manager` to the test. I > think this is because the passes fo

[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

<    1   2   3   4   5   6   7   >