Page MenuHomePhabricator

eugenis (Evgenii Stepanov)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 3 2012, 3:00 AM (409 w, 3 d)

Recent Activity

Fri, Aug 7

eugenis added inline comments to D85559: [MSAN] Reintroduce libatomic load/store instrumentation.
Fri, Aug 7, 4:03 PM · Restricted Project, Restricted Project, Restricted Project
eugenis added a comment to D85559: [MSAN] Reintroduce libatomic load/store instrumentation.

__libatomic_load might come at the end of the function, with no succeeding BB

Not exactly. It may come at the end of a BB.

Fri, Aug 7, 3:30 PM · Restricted Project, Restricted Project, Restricted Project
eugenis added inline comments to D83337: [MSAN] Instrument libatomic load/store calls.
Fri, Aug 7, 11:51 AM · Restricted Project, Restricted Project

Thu, Aug 6

eugenis added inline comments to D85227: [Draft][MSAN] Cache stack traces and chained origins.
Thu, Aug 6, 5:26 PM · Restricted Project, Restricted Project
eugenis committed rGaa57cabae2fc: [msan] Support %ms in scanf. (authored by eugenis).
[msan] Support %ms in scanf.
Thu, Aug 6, 2:50 PM
eugenis closed D85350: [msan] Support %ms in scanf..
Thu, Aug 6, 2:50 PM · Restricted Project
eugenis added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Are you seriously adding an attribute to literally every argument and return value? Why is this the right representation?

Thu, Aug 6, 2:31 PM · Restricted Project
eugenis added inline comments to D85350: [msan] Support %ms in scanf..
Thu, Aug 6, 1:56 PM · Restricted Project
eugenis updated the diff for D85350: [msan] Support %ms in scanf..

.

Thu, Aug 6, 1:55 PM · Restricted Project
eugenis committed rG189ba3db8653: Fix CFI issues in <future> (authored by eugenis).
Fix CFI issues in <future>
Thu, Aug 6, 12:05 PM
eugenis closed D82627: Fix CFI issues in <future>.
Thu, Aug 6, 12:05 PM · Restricted Project
eugenis accepted D85412: [AArch64][NFC] require aarch64 support for hwasan test.

LGTM

Thu, Aug 6, 11:11 AM · Restricted Project

Wed, Aug 5

eugenis added a comment to D82627: Fix CFI issues in <future>.

I can push this tomorrow unless anyone objects before then.

Wed, Aug 5, 4:12 PM · Restricted Project
eugenis added a comment to D85350: [msan] Support %ms in scanf..

The problem with "a" is that we do not always know what it means exactly (could be a float), and it's dangerous to dereference the pointer.

Wed, Aug 5, 1:32 PM · Restricted Project
eugenis requested review of D85350: [msan] Support %ms in scanf..
Wed, Aug 5, 12:44 PM · Restricted Project
eugenis committed rGf2c04239955a: [msan] Remove readnone and friends from call sites. (authored by eugenis).
[msan] Remove readnone and friends from call sites.
Wed, Aug 5, 10:41 AM
eugenis closed D85259: [msan] Remove readnone and friends from call sites..
Wed, Aug 5, 10:41 AM · Restricted Project
eugenis accepted D84509: Fix qsort() interceptor for FreeBSD.

LGTM

Wed, Aug 5, 10:32 AM · Restricted Project

Tue, Aug 4

eugenis requested review of D85259: [msan] Remove readnone and friends from call sites..
Tue, Aug 4, 4:50 PM · Restricted Project

Mon, Aug 3

eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

Right, that's what the isNoModRef check is for.

Mon, Aug 3, 1:23 PM · Restricted Project, Restricted Project
eugenis updated subscribers of D84940: [JumpThreading] Conditionally freeze its condition when unfolding select.
Mon, Aug 3, 11:07 AM · Restricted Project

Sat, Aug 1

eugenis committed rGdc3388b0209d: [msan] Respect no_huge_pages_for_shadow. (authored by eugenis).
[msan] Respect no_huge_pages_for_shadow.
Sat, Aug 1, 5:07 PM
eugenis closed D85061: [msan] Respect no_huge_pages_for_shadow..
Sat, Aug 1, 5:07 PM · Restricted Project

Fri, Jul 31

eugenis added a comment to D85061: [msan] Respect no_huge_pages_for_shadow..

Testing this would be pretty hard - we'd need to touch a bunch of memory pages, and then somehow wait for the huge page daemon to merge them (or not).

Fri, Jul 31, 5:28 PM · Restricted Project
eugenis requested review of D85061: [msan] Respect no_huge_pages_for_shadow..
Fri, Jul 31, 5:20 PM · Restricted Project
eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

You want to start scanning at the same point where poisoning is going to be inserted.

Fri, Jul 31, 3:45 PM · Restricted Project, Restricted Project
eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

The code currently scans from the alloca, rather than from the lifetime_start. This might make only searching in single BB pretty limiting, since afaict an alloca can be detached from its lifetime region.

Fri, Jul 31, 3:44 PM · Restricted Project, Restricted Project
eugenis accepted D85040: [MSAN] Instrument freeze instruction by clearing shadow.

LGTM

Fri, Jul 31, 3:42 PM · Restricted Project
eugenis accepted D84630: [StackSafety] Skip ambiguous lifetime analysis.

LGTM

Fri, Jul 31, 3:29 PM · Restricted Project
eugenis added inline comments to D85040: [MSAN] Instrument freeze instruction by clearing shadow.
Fri, Jul 31, 1:08 PM · Restricted Project
eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

In general, this implementation looks pretty complex and easy to get wrong. I'd prefer something along the lines of AArch64StackTagging::collectInitializers - directly calculate the offset for each store/load instruction. It might do some extra work with unrelated memory instructions, but probably not too much.

Fri, Jul 31, 11:58 AM · Restricted Project, Restricted Project
eugenis accepted D84510: [msan] Compile the libatomic.c test with a C compiler.
Fri, Jul 31, 10:19 AM · Restricted Project

Thu, Jul 30

eugenis accepted D84621: [NFC] Remove unused GetUnderlyingObject paramenter.

Seems fine to me.

Thu, Jul 30, 5:30 PM · Restricted Project
eugenis accepted D84617: [ValueTracking] Remove AllocaForValue parameter.
Thu, Jul 30, 5:29 PM · Restricted Project
eugenis accepted D84616: [NFC] Move findAllocaForValue into ValueTracking.h.
Thu, Jul 30, 5:28 PM · Restricted Project
eugenis added inline comments to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.
Thu, Jul 30, 3:31 PM · Restricted Project, Restricted Project
eugenis added a reviewer for D84961: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow: kcc.
Thu, Jul 30, 3:07 PM · Restricted Project
eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

This actually has very significant effects on some, but not all, benchmarks.

Running grep I observed ~8% decrease in binary with this patch. But clang sees little effect: <1%.

On a few different benchmarks from a benchmark suite, this also decreased runtime overhead by a significant amount: 8% for sha512, and 13% for qsort.

Thu, Jul 30, 2:45 PM · Restricted Project, Restricted Project
eugenis added inline comments to D84940: [JumpThreading] Conditionally freeze its condition when unfolding select.
Thu, Jul 30, 2:13 PM · Restricted Project
eugenis added inline comments to D84630: [StackSafety] Skip ambiguous lifetime analysis.
Thu, Jul 30, 1:30 PM · Restricted Project

Wed, Jul 29

eugenis added inline comments to D84630: [StackSafety] Skip ambiguous lifetime analysis.
Wed, Jul 29, 1:47 PM · Restricted Project

Tue, Jul 28

eugenis accepted D84509: Fix qsort() interceptor for FreeBSD.
Tue, Jul 28, 4:52 PM · Restricted Project
eugenis accepted D84740: [Sanitizers] Test including rpc/xdr.h requires sunrpc.

LGTM

Tue, Jul 28, 12:45 PM · Restricted Project

Mon, Jul 27

eugenis accepted D84652: [GWP-ASan] Crash handler API returns sizeof(collected trace).

LGTM

Mon, Jul 27, 10:14 AM · Restricted Project

Fri, Jul 24

eugenis added inline comments to D84509: Fix qsort() interceptor for FreeBSD.
Fri, Jul 24, 9:42 AM · Restricted Project
eugenis added a comment to D84510: [msan] Compile the libatomic.c test with a C compiler.

Does it help to replace %clangxx_msan with %clang_msan instead?

Fri, Jul 24, 9:29 AM · Restricted Project

Thu, Jul 23

eugenis accepted D84361: scudo: Interleave odd and even tags for adjacent blocks..

LGTM

Thu, Jul 23, 2:20 PM · Restricted Project
eugenis accepted D84446: [MSAN] Allow inserting array checks.

LGTM

Thu, Jul 23, 1:49 PM · Restricted Project
eugenis added a comment to D84446: [MSAN] Allow inserting array checks.

I don't believe this happens in C++ but wanted to add it for completeness's sake (or if some other LLVM based language might use it).

Thu, Jul 23, 12:53 PM · Restricted Project
eugenis added a comment to D84446: [MSAN] Allow inserting array checks.

Do you want to use this in eager checks when passing an array as a function argument (does that even happen in C++?) ?

Thu, Jul 23, 12:33 PM · Restricted Project

Wed, Jul 22

eugenis accepted D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}.

LGTM

Wed, Jul 22, 4:42 PM · Restricted Project
eugenis accepted D82680: MSAN: Allow emitting checks for struct types.

LGTM with 2 comments

Wed, Jul 22, 1:35 PM · Restricted Project
eugenis added inline comments to D83427: [MSAN] Update tests due to widespread eager checking.
Wed, Jul 22, 1:26 PM · Restricted Project, Restricted Project
eugenis added a comment to D83427: [MSAN] Update tests due to widespread eager checking.

The patch is missing context

Wed, Jul 22, 1:03 PM · Restricted Project, Restricted Project
eugenis added a comment to D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}.

Please add a test (instrumentation only, no need for a compiler-rt test).

Wed, Jul 22, 1:01 PM · Restricted Project
eugenis committed rGa305d2502948: asan_device_setup's wrapper scripts not handling args with spaces correctly (authored by agrieve2).
asan_device_setup's wrapper scripts not handling args with spaces correctly
Wed, Jul 22, 12:56 PM
eugenis closed D84237: asan_device_setup's wrapper scripts not handling args with spaces correctly.
Wed, Jul 22, 12:56 PM · Restricted Project
eugenis added a comment to D84237: asan_device_setup's wrapper scripts not handling args with spaces correctly.

Sure, no problem.
Thanks for the patch!

Wed, Jul 22, 12:51 PM · Restricted Project

Tue, Jul 21

eugenis accepted D83358: [Sanitizers] Add interceptor for xdrrec_create.

LGTM w/ nit

Tue, Jul 21, 4:15 PM · Restricted Project
eugenis accepted D84237: asan_device_setup's wrapper scripts not handling args with spaces correctly.

LGTM

Tue, Jul 21, 12:41 PM · Restricted Project

Mon, Jul 20

eugenis added a comment to D82680: MSAN: Allow emitting checks for struct types.

Please upload with context.

Mon, Jul 20, 9:16 PM · Restricted Project
eugenis accepted D83337: [MSAN] Instrument libatomic load/store calls.

LGTM

Mon, Jul 20, 9:16 PM · Restricted Project, Restricted Project
eugenis updated subscribers of D82627: Fix CFI issues in <future>.

They are "errors reported by Control Flow Integrity" - that part is true. This is valid C++, but it triggers false positive errors in CFI.
This is not without precedent - see https://github.com/llvm/llvm-project/commit/3e58a6a7, https://github.com/llvm/llvm-project/commit/f11c00d7 and more.
The fact that CFI is stricter that the wording of the standard has been discussed many times before, but the conclusion is always that these reports are useful.
This type of false positive is not common at all outside of libc++.
@pcc
@kcc

Mon, Jul 20, 9:16 PM · Restricted Project

Fri, Jul 17

eugenis added inline comments to D83337: [MSAN] Instrument libatomic load/store calls.
Fri, Jul 17, 11:15 AM · Restricted Project, Restricted Project
eugenis added a comment to D82627: Fix CFI issues in <future>.

Before the lifetime of an object has started but after the storage which the object will occupy has been allocated [...] any pointer that represents the address of the storage location where the object will be or was located may be used but only in limited ways. [...]
The program has undefined behavior if:
[...]

  • the pointer is used as the operand of a static_cast (8.2.9), except when the conversion is to pointer to cv void, or to pointer to cv void and subsequently to pointer to cv char, cv unsigned char, or cv std::byte (21.2.1),[...]

That's a good find. However, in this case, what we do is not a static_cast. It's a C-style cast from an unrelated type (std::aligned_storage<...>::type) to __base* (for example in the expression (__base*)&__buf_), which is actually equivalent to a reinterpret_cast unless I'm mistaken. Do you agree?

Fri, Jul 17, 9:31 AM · Restricted Project

Thu, Jul 16

eugenis added a comment to D82627: Fix CFI issues in <future>.

There is _LIBCPP_NO_CFI, if you prefer it that way.

Thu, Jul 16, 1:27 PM · Restricted Project
eugenis updated subscribers of D82627: Fix CFI issues in <future>.

@HAPPY
I'm not an expert, but I've heard that such casts are UB somewhere.
Looking at [basic.life] 6.8/6 in c++17

Thu, Jul 16, 1:26 PM · Restricted Project

Mon, Jul 13

eugenis accepted D82920: [MSAN] Implement experiemental vector reduction intrinsics.

LGTM

Mon, Jul 13, 4:23 PM · Restricted Project
eugenis accepted D81699: MemorySanitizer: Add option to insert init checks at call site.

LGTM with a nit

Mon, Jul 13, 3:54 PM · Restricted Project, Restricted Project

Fri, Jul 10

eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

I figured if it's using GEP then it's likely not going to be storing to the entire shadow. Bitcast is overwhelmingly common though (especially bitcast->lifetime.start, which I realize I need to handle).

Fri, Jul 10, 6:16 PM · Restricted Project, Restricted Project
eugenis added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

Allocas are often used through bitcast or GEP, we should handle them as well.

Fri, Jul 10, 4:35 PM · Restricted Project, Restricted Project
eugenis requested changes to D83499: [MSAN runtime] Add poison_stack function that also updates origin.

I don't think 1.5% is good enough to introduce new runtime functions for the off-by-default code path.

Fri, Jul 10, 11:39 AM · Restricted Project, Restricted Project

Jul 8 2020

eugenis added a comment to D83358: [Sanitizers] Add interceptor for xdrrec_create.

read also needs to be wrapped to do COMMON_INTERCEPTOR_UNPOISON_PARAM

Jul 8 2020, 1:00 PM · Restricted Project
eugenis added inline comments to D83358: [Sanitizers] Add interceptor for xdrrec_create.
Jul 8 2020, 12:47 PM · Restricted Project
eugenis accepted D82820: [InstCombine] Fix mismatched attribute lists for combined calls.

LGTM

Jul 8 2020, 11:58 AM · Restricted Project
eugenis accepted D83412: [LLVM] Accept `noundef` attribute in function definitions/calls.

LGTM

Jul 8 2020, 11:48 AM · Restricted Project
eugenis added a comment to D83412: [LLVM] Accept `noundef` attribute in function definitions/calls.

This needs to include tests for bitcode writing and parsing.

Jul 8 2020, 11:36 AM · Restricted Project

Jul 7 2020

eugenis added inline comments to D83358: [Sanitizers] Add interceptor for xdrrec_create.
Jul 7 2020, 5:07 PM · Restricted Project
eugenis added inline comments to D83358: [Sanitizers] Add interceptor for xdrrec_create.
Jul 7 2020, 5:02 PM · Restricted Project
eugenis added inline comments to D83337: [MSAN] Instrument libatomic load/store calls.
Jul 7 2020, 2:55 PM · Restricted Project, Restricted Project
eugenis added inline comments to D82820: [InstCombine] Fix mismatched attribute lists for combined calls.
Jul 7 2020, 2:39 PM · Restricted Project
eugenis added a comment to D82820: [InstCombine] Fix mismatched attribute lists for combined calls.

This code generates a libcall out of thin air. My intuition says the safest thing to do is to drop all call site attributes, because they generally specify something about how an attribute must be passed to the callee, and not a property of the value being passed, so there is no reason for the attribute lists on pow and on exp to have anything in common at all.

Jul 7 2020, 2:13 PM · Restricted Project

Jul 6 2020

eugenis added a comment to D83134: [asan] Disable fast unwinder on arm-linux-gnueabi with thumb.

Is unwinding actually broken on an all-clang, all-thumb system?

Jul 6 2020, 12:21 PM · Restricted Project

Jun 30 2020

eugenis added inline comments to D82920: [MSAN] Implement experiemental vector reduction intrinsics.
Jun 30 2020, 5:55 PM · Restricted Project
eugenis added inline comments to D82820: [InstCombine] Fix mismatched attribute lists for combined calls.
Jun 30 2020, 4:50 PM · Restricted Project
eugenis accepted D82897: [Sanitizers] Implement interceptors for msgsnd, msgrcv.

LGTM

Jun 30 2020, 4:18 PM · Restricted Project
eugenis added inline comments to D82820: [InstCombine] Fix mismatched attribute lists for combined calls.
Jun 30 2020, 1:03 PM · Restricted Project
eugenis added inline comments to D82897: [Sanitizers] Implement interceptors for msgsnd, msgrcv.
Jun 30 2020, 12:29 PM · Restricted Project

Jun 29 2020

eugenis added inline comments to D82680: MSAN: Allow emitting checks for struct types.
Jun 29 2020, 2:12 PM · Restricted Project

Jun 26 2020

eugenis added inline comments to D82680: MSAN: Allow emitting checks for struct types.
Jun 26 2020, 3:46 PM · Restricted Project
eugenis added a comment to D82316: [LangRef] Add `noundef` attribute to documentation.

I think, we could even remove the padding sentence and say "undefined or poison" bits. The latter seems to match our existing language. The former should be OK because a value of type { i8, i32 } has 8 + 32 bits. The fact that the store and alloca size is different does not mean the value has undefined bits. If people agree with this interpretation we could also add a sentence explaining this difference.

Jun 26 2020, 3:10 PM · Restricted Project
eugenis added a comment to D82627: Fix CFI issues in <future>.

Oh, it would be great to run libc++ tests with CFI on the buildbot, but it may be complicated, as CFI requires LTO.

Jun 26 2020, 10:55 AM · Restricted Project
eugenis added a comment to D82627: Fix CFI issues in <future>.

This change look right to me, but I defer to libc++ maintainers.

Jun 26 2020, 10:55 AM · Restricted Project

Jun 24 2020

eugenis added inline comments to D82424: sanitizers: Add interceptors for getproto{ent,byname,bynumber}_r.
Jun 24 2020, 2:40 PM · Restricted Project

Jun 23 2020

eugenis accepted D82398: [MSAN] Handle x86 {round,min,max}sd intrinsics.

LGTM

Jun 23 2020, 4:43 PM · Restricted Project, Restricted Project
eugenis added inline comments to D82398: [MSAN] Handle x86 {round,min,max}sd intrinsics.
Jun 23 2020, 3:38 PM · Restricted Project, Restricted Project
eugenis added a comment to D82411: sanitizers: Implement sig{and,or}set interceptors.

The sanitizer_common test seems excessive, but I'm inclined to take it anyway simply because it costs us very little to have it there.

Jun 23 2020, 3:05 PM · Restricted Project
eugenis added a reviewer for D82411: sanitizers: Implement sig{and,or}set interceptors: vitalybuka.
Jun 23 2020, 3:05 PM · Restricted Project
eugenis added a comment to D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Also, what's the plan to detect these cases in ubsan?

I don't think this has any practical impact on our goals with sanitizers. We should detect undefined behavior before it gets to the point of actually passing or returning an undef or poison value.

Jun 23 2020, 3:05 PM · Restricted Project, Restricted Project
eugenis added a comment to D82249: [HWASan] Disable GlobalISel/FastISel for HWASan Globals..

I'm OK with this as a workaround, but it would be more natural to detect the unsupported IR pattern in globalisel and fall back instead of disabling it entirely. Is it difficult to do for some reason?

Eh, it's not an unsupported IR pattern that's the problem, it's that the IR is lowered into adrp + add so that the add can be folded into a ldr/str as an offset. IMO on the scale of "painting over the problem vs. fixing it", this patch is 100% paint, forced fallback with MO_TAGGED is 80% paint for maybe 60% of the work of just fixing it.

I'm working on fixing this properly now that we know we don't need to make a less-risky, fast-to-deploy patch. If that all falls into place this patchset will just become obsolete anyway.

Jun 23 2020, 3:04 PM · Restricted Project, Restricted Project, Restricted Project