vsk (Vedant Kumar)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 8 2015, 10:26 AM (115 w, 2 d)

Recent Activity

Today

vsk updated the diff for D37544: [ubsan] Skip alignment checks which are folded away.
  • Tighten up lit test.
Fri, Sep 22, 10:54 AM
vsk added a comment to D37544: [ubsan] Skip alignment checks which are folded away.

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.

Fri, Sep 22, 10:53 AM

Wed, Sep 20

vsk accepted D37924: Add section headers to SpecialCaseLists.

Thanks

Wed, Sep 20, 6:06 PM
vsk added a comment to D38107: [Coverage] Add an option to emit limited coverage info.

Thanks for taking a look. I'd like to give Alex and others a chance to read over this (different timezones), and am happy to change the name of the cl::opt.

Wed, Sep 20, 5:31 PM
vsk created D38107: [Coverage] Add an option to emit limited coverage info.
Wed, Sep 20, 4:09 PM

Tue, Sep 19

vsk updated the diff for D38066: [cmake] Add an option to build llvm with IR PGO.
  • Make it possible to use LLVM_ENABLE_IR_PGO and LLVM_BUILD_INSTRUMENTED together. This will make it possible to use the Apple PGO cmake cache with IR PGO.
Tue, Sep 19, 7:25 PM
vsk updated the summary of D38066: [cmake] Add an option to build llvm with IR PGO.
Tue, Sep 19, 7:04 PM
vsk updated the diff for D38066: [cmake] Add an option to build llvm with IR PGO.
  • Use -fprofile-generate=<data-dir> instead of a hidden frontend option. I tested this by building llvm-config with LLVM_BUILD_INSTRUMENTED and LLVM_ENABLE_IR_PGO -- both workflows lgtm.
Tue, Sep 19, 7:04 PM
vsk added inline comments to D38066: [cmake] Add an option to build llvm with IR PGO.
Tue, Sep 19, 6:21 PM
vsk created D38066: [cmake] Add an option to build llvm with IR PGO.
Tue, Sep 19, 5:50 PM
vsk updated the diff for D37544: [ubsan] Skip alignment checks which are folded away.
  • Use a better test case. This one was lifted from an actual example in X86CallingConv.h:86.
Tue, Sep 19, 12:02 PM

Thu, Sep 14

vsk added a comment to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.

Actually could you add a blurb about this to docs/ReleaseNotes? I had to fix some old code in lldb and stuff we have internally. It'd be useful to add a sentence or two about how to upgrade old callsites.

Thu, Sep 14, 1:38 PM
vsk added a comment to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
In D37563#865541, @vsk wrote:

It'd be nice to add in "expected 3 redirects" to the assert bodies.

Sorry, missed this comment somehow. If we add a message, it should also cover the case of zero redirects. Something along the lines of "expected 0 or 3 redirects". I have a slight concern that the message would basically repeat the condition resulting in a sort of tautology (assert((Redirects.empty() || Redirects.size() == 3) && "expected 0 or 3 redirects");). But if you still find this useful, I can add a message.

Thu, Sep 14, 9:28 AM

Wed, Sep 13

vsk added a comment to D37820: [BinaryFormat] Teach identify_magic about Tapi files..

LGTM, but it'd be good to get a second +1 from someone more familiar with libObject internals.

Wed, Sep 13, 12:09 PM

Tue, Sep 12

vsk added a comment to D37781: [test] Enable LeakSanitizer on 64-bit Darwin ASan llvm builds.

LGTM, thanks!

Tue, Sep 12, 5:10 PM
vsk added inline comments to D37597: [ubsan] Function Sanitizer: Don't require writable text segments.
Tue, Sep 12, 5:06 PM
vsk added inline comments to D37597: [ubsan] Function Sanitizer: Don't require writable text segments.
Tue, Sep 12, 4:02 PM
vsk updated the diff for D37597: [ubsan] Function Sanitizer: Don't require writable text segments.

Address review feedback.

Tue, Sep 12, 4:01 PM
vsk created D37777: [Driver] Disable uwtable by default in -ffreestanding mode.
Tue, Sep 12, 3:41 PM
vsk added a comment to D32842: Specify which sanitizers are covered by a sanitizer blacklist.

@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?

Tue, Sep 12, 1:53 PM
vsk added a comment to D32842: Specify which sanitizers are covered by a sanitizer blacklist.

@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 to apply blacklists selectively.

Tue, Sep 12, 12:55 PM
vsk updated the diff for D32842: Specify which sanitizers are covered by a sanitizer blacklist.
  • Rebase to ToT.
Tue, Sep 12, 12:52 PM
vsk added inline comments to D37597: [ubsan] Function Sanitizer: Don't require writable text segments.
Tue, Sep 12, 12:15 PM

Mon, Sep 11

vsk closed D37642: [pp-trace] Update skipped source ranges in tests.

Thanks, r312948

Mon, Sep 11, 2:36 PM
vsk updated the diff for D37646: [ubsan-minimal] Enable on Darwin.
  • Simplify, per review feedback
Mon, Sep 11, 1:29 PM
vsk added inline comments to D37646: [ubsan-minimal] Enable on Darwin.
Mon, Sep 11, 1:24 PM

Fri, Sep 8

vsk added a comment to D37617: Debug info for variables whose type is shrinked to bool fix.

Could you include an IR-based test case?

Fri, Sep 8, 5:03 PM
vsk added a comment to D37564: Update users of llvm::sys::ExecuteAndWait etc..

Looks good to me.

Fri, Sep 8, 4:55 PM
vsk added inline comments to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.
Fri, Sep 8, 4:54 PM
vsk added a comment to D37563: Convenience/safety fix for llvm::sys::Execute(And|No)Wait.

It'd be nice to add in "expected 3 redirects" to the assert bodies.

Fri, Sep 8, 4:48 PM
vsk added a comment to D37470: [analyzer] Handle ObjC messages conservatively in CallDescription.

Can you add a test?

Fri, Sep 8, 4:38 PM
vsk created D37649: [Driver] Support ubsan-minimal on Darwin.
Fri, Sep 8, 4:17 PM
vsk updated the diff for D37646: [ubsan-minimal] Enable on Darwin.
  • Actually enable testing on Darwin. Depends on D37649
Fri, Sep 8, 4:17 PM
vsk added a comment to D37646: [ubsan-minimal] Enable on Darwin.

Hm, actually the tests are just unsupported. We'll need a small driver change to enable testing.

Fri, Sep 8, 4:02 PM
vsk created D37647: [ubsan-minimal] Document the new runtime.
Fri, Sep 8, 3:56 PM
vsk created D37646: [ubsan-minimal] Enable on Darwin.
Fri, Sep 8, 3:56 PM
vsk created D37642: [pp-trace] Update skipped source ranges in tests.
Fri, Sep 8, 2:39 PM
vsk added a dependent revision for D36642: [Lexer] Report more precise skipped regions (PR34166): D37642: [pp-trace] Update skipped source ranges in tests.
Fri, Sep 8, 2:39 PM
vsk added a reviewer for D36642: [Lexer] Report more precise skipped regions (PR34166): arphaman.
Fri, Sep 8, 2:39 PM
vsk updated the diff for D36642: [Lexer] Report more precise skipped regions (PR34166).
  • 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 tests.
Fri, Sep 8, 2:39 PM
vsk added inline comments to D36642: [Lexer] Report more precise skipped regions (PR34166).
Fri, Sep 8, 1:33 PM

Thu, Sep 7

vsk updated the diff for D37597: [ubsan] Function Sanitizer: Don't require writable text segments.

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. Truncating to Int32Ty should be possible, but causes dyld to crash, and also requires an extra sign-extension.
Thu, Sep 7, 6:57 PM
vsk added a comment to D37597: [ubsan] Function Sanitizer: Don't require writable text segments.

Thanks for your comments!

Thu, Sep 7, 5:03 PM
vsk resigned from D27608: Update LLVM documentation to allow for easier creation of documentation in multiple formats (in particular CHM format).

Unfortunately this review has stalled. I need to take it off my queue.

Thu, Sep 7, 3:51 PM
vsk updated the summary of D37598: [ubsan] Enable -fsanitize=function on Darwin.
Thu, Sep 7, 3:31 PM
vsk added a dependent revision for D37597: [ubsan] Function Sanitizer: Don't require writable text segments: D37598: [ubsan] Enable -fsanitize=function on Darwin.
Thu, Sep 7, 3:30 PM
vsk created D37597: [ubsan] Function Sanitizer: Don't require writable text segments.
Thu, Sep 7, 3:30 PM
vsk created D37598: [ubsan] Enable -fsanitize=function on Darwin.
Thu, Sep 7, 3:30 PM
vsk updated the diff for D36813: [Coverage] Build sorted and unique segments.

Address review feedback.

Thu, Sep 7, 11:27 AM
vsk added a comment to D36813: [Coverage] Build sorted and unique segments.

Thanks for your comments, I'll update the patch shortly.

Thu, Sep 7, 11:26 AM

Wed, Sep 6

vsk updated the summary of D37545: [ubsan] Defer pointer type checks, then try to skip the redundant ones.
Wed, Sep 6, 5:35 PM
vsk created D37545: [ubsan] Defer pointer type checks, then try to skip the redundant ones.
Wed, Sep 6, 5:30 PM
vsk created D37544: [ubsan] Skip alignment checks which are folded away.
Wed, Sep 6, 5:27 PM
vsk created D37543: [ubsan] Add helpers to decide when null/vptr checks are required. NFC..
Wed, Sep 6, 5:26 PM
vsk created D37542: [ubsan] Save a ptrtoint when emitting alignment checks.
Wed, Sep 6, 5:24 PM
vsk updated the diff for D36813: [Coverage] Build sorted and unique segments.

Address review feedback.

Wed, Sep 6, 11:08 AM
vsk added a comment to D36813: [Coverage] Build sorted and unique segments.

Thanks for your comments, I'll update the patch shortly.

Wed, Sep 6, 11:08 AM

Tue, Sep 5

vsk added inline comments to D37495: llvm-isel-fuzzer: Handle a subset of backend flags in the executable name.
Tue, Sep 5, 4:15 PM

Fri, Sep 1

vsk added a comment to D36813: [Coverage] Build sorted and unique segments.

Yes, if it's possible, I'd like to see all the patches (one way or another), just to have the whole picture.

Fri, Sep 1, 10:47 AM
vsk created D37387: [Coverage] Report errors when reading malformed source regions.
Fri, Sep 1, 10:44 AM

Thu, Aug 31

vsk added a comment to D36813: [Coverage] Build sorted and unique segments.

The patch doesn't apply to the current tip clearly. And after applying it manually and fix compilation errors, I have two failing tests for llvm-cov: tools/llvm-cov/showLineExecutionCounts.cpp and tools/llvm-cov/threads.c, both of which fail with "Coverage segments not unique or sorted" assertion.

Thu, Aug 31, 9:44 AM

Wed, Aug 30

vsk accepted D37111: [llvm-cov] Read in function names for filtering from a text file..

Thanks!

Wed, Aug 30, 10:36 AM

Tue, Aug 29

vsk accepted D36723: [llvm-cov] Allow hiding instantiation/region coverage from summary tables.

Thanks!

Tue, Aug 29, 2:28 PM
vsk added a comment to D36813: [Coverage] Build sorted and unique segments.

Friendly ping.

Tue, Aug 29, 1:07 PM

Mon, Aug 28

vsk accepted D36810: Minimal runtime for UBSan..

Thanks for working on this!

Mon, Aug 28, 1:56 PM
vsk added inline comments to D36810: Minimal runtime for UBSan..
Mon, Aug 28, 1:45 PM

Fri, Aug 25

vsk added inline comments to D36810: Minimal runtime for UBSan..
Fri, Aug 25, 6:24 PM
vsk added inline comments to D36810: Minimal runtime for UBSan..
Fri, Aug 25, 1:57 PM
vsk added inline comments to D37111: [llvm-cov] Read in function names for filtering from a text file..
Fri, Aug 25, 10:39 AM

Thu, Aug 24

vsk added inline comments to D37111: [llvm-cov] Read in function names for filtering from a text file..
Thu, Aug 24, 10:28 AM

Aug 23 2017

vsk accepted D35271: Fix printing policy for AST context loaded from file.

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.

Aug 23 2017, 10:33 AM · Restricted Project
vsk added inline comments to D36492: [time-report] Add preprocessor timer.
Aug 23 2017, 10:19 AM
vsk accepted D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass.

LGTM!

Aug 23 2017, 9:37 AM

Aug 22 2017

vsk added a comment to D35271: Fix printing policy for AST context loaded from file.

Thanks for adding the test! This is looking good.

Aug 22 2017, 11:24 AM · Restricted Project
vsk added a comment to D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass.

Nice! I'd like to get your thoughts on two candidate ergonomic changes:

Aug 22 2017, 10:43 AM

Aug 21 2017

vsk added a comment to D35271: Fix printing policy for AST context loaded from file.
In 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 any way to finalize an ASTInfoCollector after ReadAST happens (or ASTReaderListeners in general)?

I can look into this but would prefer to do so in a different patch, as this would require refactoring beyond this simple bug fix. Would it be okay to land this patch as-is?

Aug 21 2017, 11:47 AM · Restricted Project
vsk added a comment to D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass.

Would it be more convenient to extend ErrorInfo instead of creating a new diagnostic wrapper? E.g one benefit of passing around Expected<T>'s is that there's some assurance the diagnostics will be reported.

Aug 21 2017, 11:36 AM

Aug 18 2017

vsk added a dependent revision for D36813: [Coverage] Build sorted and unique segments: D36901: [Coverage] Add an expensive test for the segment builder.
Aug 18 2017, 2:54 PM
vsk created D36901: [Coverage] Add an expensive test for the segment builder.
Aug 18 2017, 2:54 PM
vsk added a comment to D36810: Minimal runtime for UBSan..

ubsan and suitable on production? I don't understand this phrasing, sounds to me like oxymoron.

Aug 18 2017, 11:06 AM
vsk added a comment to D36810: Minimal runtime for UBSan..

Good questions.

The primary motivation is security. I want this to be something that I can be confident does not add any new vulnerabilities to the program. The current implementation demangles potentially untrusted strings, writes to arbitrary files, and in general contains too much code of mixed quality.

In D36810#844863, @vsk wrote:

Have you explored the possibility of reducing the overhead of the regular runtime to an acceptable level?

  • If it's too big, we can disable any heavy weight features you identify at compile-time.

Not worried about that.

  • If it allocates or prints too much, we can add in a custom allocator or printing strategy.
  • If it requires too much metadata to be inserted into user programs, we can devise a new, smaller encoding for the metadata. Or simply teach the runtime handlers to accept nullptr. This should also achieve the binary size savings you mentioned.

Both metadata and extra code size to set up the call frame. UBSan checks are very common, and each instruction counts. If we are going to allow nullptr in the arguments, might as well implement _minimal variants.

Aug 18 2017, 10:55 AM

Aug 17 2017

vsk added inline comments to D36642: [Lexer] Report more precise skipped regions (PR34166).
Aug 17 2017, 6:20 PM
vsk updated the diff for D36642: [Lexer] Report more precise skipped regions (PR34166).
  • Address Eli's comment.
Aug 17 2017, 6:20 PM
vsk added inline comments to D36813: [Coverage] Build sorted and unique segments.
Aug 17 2017, 5:43 PM
vsk updated the diff for D36813: [Coverage] Build sorted and unique segments.
  • Address two comments by @ikudrin (split out an NFC change, emit better segments when handling 0-length regions).
  • I tested the updated patch in the same way, and was able to prepare a coverage report for clang without issue.
Aug 17 2017, 5:43 PM
vsk updated subscribers of D36810: Minimal runtime for UBSan..

Have you explored the possibility of reducing the overhead of the regular runtime to an acceptable level?

Aug 17 2017, 4:37 PM
vsk updated subscribers of D36847: [Support] Add reentrant start/stop Timer methods.

I tried this a while back: https://reviews.llvm.org/D20735

Aug 17 2017, 2:13 PM
vsk added a comment to D36274: Introduce FuzzMutate library.

This looks good to me!

Aug 17 2017, 1:13 PM

Aug 16 2017

vsk created D36813: [Coverage] Build sorted and unique segments.
Aug 16 2017, 7:01 PM
vsk accepted D36492: [time-report] Add preprocessor timer.

LGTM.

Aug 16 2017, 11:59 AM

Aug 15 2017

vsk accepted D36765: [llvmlab] Update llvmlab fetch documentation.
Aug 15 2017, 1:45 PM · Restricted Project
vsk added inline comments to D36274: Introduce FuzzMutate library.
Aug 15 2017, 11:27 AM
vsk added a comment to D36603: Enable profile on NetBSD.

I've reverted the commit on demand.

I was trying to restore the setup for check-profile after upgrading the sources.. and I hit new failures.

http://www.netbsd.org/~kamil/llvm/check-profile

I have new failures in sanitizers too.. Maybe my SVN checkout was in a faulty revision. For now I will move on and focus on sanitizers.

Aug 15 2017, 8:45 AM · Restricted Project

Aug 14 2017

vsk updated the diff for D36642: [Lexer] Report more precise skipped regions (PR34166).

Thanks for the review. I've updated the patch so that we do better with "#\" directives.

Aug 14 2017, 7:42 PM
vsk added a comment to D36723: [llvm-cov] Allow hiding instantiation/region coverage from summary tables.

I think it makes sense to hide instantiation coverage by default. Unless you're specifically looking for unused specializations (perhaps not the common case), it can be overwhelming.

Aug 14 2017, 6:58 PM
vsk added a comment to D36648: [PGO] Add support for profile dumping dir relocation.

Lgtm, although I think the line-wrapping on the added RUN lines needs to be corrected.

Aug 14 2017, 12:21 AM

Aug 13 2017

vsk added a comment to D36603: Enable profile on NetBSD.
In D36603#839356, @vsk wrote:

I think it would be better to XFAIL the test from the onset so that people reading the source are aware of the issue. That way, if for any reason this can't make its way back on your priority list, the build will remain in good shape.

This does not affect the build, only one test in both archs.

Aug 13 2017, 11:39 PM · Restricted Project

Aug 11 2017

vsk created D36642: [Lexer] Report more precise skipped regions (PR34166).
Aug 11 2017, 7:41 PM
vsk added a comment to D36274: Introduce FuzzMutate library.

Very cool. I've just done an initial pass and will need to take a closer look. The main piece of feedback I've got is that adding more docs would really clarify the code. I've pointed out some specific areas which could use doxygen comments. A small writeup in docs/Fuzzing would also be great -- maybe that makes more sense as a separate patch?

Aug 11 2017, 5:08 PM
vsk added a comment to D36603: Enable profile on NetBSD.

I think it would be better to XFAIL the test from the onset so that people reading the source are aware of the issue. That way, if for any reason this can't make its way back on your priority list, the build will remain in good shape.

Aug 11 2017, 10:45 AM · Restricted Project