dvyukov (Dmitry Vyukov)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 6 2012, 2:31 AM (245 w, 22 h)

Recent Activity

Tue, Aug 8

dvyukov added a comment to D36465: [RFC] Change cmp instrumentation to distinguish comparisons with const operands.
In D36465#835650, @kcc wrote:

I am fine with breaking the API. Whoever uses this API will need to add a tiny bit of code to work with the new compiler (and that code will be compatible with the old version).
It's better to break the API this way than to introduce the flags or weak aliases, etc

Please update the documentation in this patch. (mention that these functions are emitted by clang >= 2017-08-XX)

Make sure to run 'ninja check-fuzzer'. Most likely it will break due to the API change, fix it in libFuzzer (in a simple way, by calling __sanitizer_cov_trace_cmpN)

Find all uses of __sanitizer_cov_trace_cmp1 in projects/compiler-rt/lib/sanitizer_common and update those files.

Tue, Aug 8, 11:13 AM
dvyukov added a comment to D36465: [RFC] Change cmp instrumentation to distinguish comparisons with const operands.

Kostya, Dima,

Do we need to put this code behind a flag (probably yes, if we don't want to break the API for the existing users)?
Any other comments?

Tue, Aug 8, 7:51 AM

Tue, Aug 1

dvyukov accepted D36192: [tsan] Fix format string in WriteMemoryProfile.
Tue, Aug 1, 11:08 PM · Restricted Project

Sun, Jul 23

dvyukov accepted D35690: [Sanitizers] TSan allocator set errno on failure..
Sun, Jul 23, 12:58 AM

Thu, Jul 20

dvyukov added inline comments to D35690: [Sanitizers] TSan allocator set errno on failure..
Thu, Jul 20, 10:41 PM
dvyukov added inline comments to D35690: [Sanitizers] TSan allocator set errno on failure..
Thu, Jul 20, 10:34 PM
dvyukov added inline comments to D35690: [Sanitizers] TSan allocator set errno on failure..
Thu, Jul 20, 10:49 AM

Jul 18 2017

dvyukov added a comment to D35555: [MIPS][TSAN] Fix test user_malloc.cc.

Running ninja check-tsan with this change on linux/amd64 gives me:
https://gist.githubusercontent.com/dvyukov/517dec28d8aecca5b3b8d6f94d0a0c9c/raw/adc41512c8cfdbf3022bfbc97aa75473c962b7bf/gistfile1.txt
This init sequence solves chicken and egg problem to large degree. This sequences served us well for years and I would feel nervous changing it even if no tests break on linux. I think it's better to do what asan does for dlsym (see AllocateFromLocalPool in asan sources), and I guess we need to move this code to sanitizer_common (not sure what msan does for this).

Jul 18 2017, 6:28 AM

Jul 14 2017

dvyukov committed rL308018: tsan: optimize sync clock memory consumption.
tsan: optimize sync clock memory consumption
Jul 14 2017, 4:30 AM
dvyukov closed D35323: tsan: optimize sync clock memory consumption.

Submitted as 308018.

Jul 14 2017, 4:30 AM
dvyukov added inline comments to D35323: tsan: optimize sync clock memory consumption.
Jul 14 2017, 4:29 AM
dvyukov updated the diff for D35323: tsan: optimize sync clock memory consumption.
Jul 14 2017, 4:28 AM

Jul 12 2017

dvyukov created D35323: tsan: optimize sync clock memory consumption.
Jul 12 2017, 12:55 PM
dvyukov committed rL307786: tsan: remove some clock-related stats.
tsan: remove some clock-related stats
Jul 12 2017, 5:55 AM
dvyukov committed rL307785: tsan: refactor SyncClock code.
tsan: refactor SyncClock code
Jul 12 2017, 5:51 AM
dvyukov committed rL307784: tsan: prepare clock for future changes.
tsan: prepare clock for future changes
Jul 12 2017, 5:45 AM
dvyukov committed rL307781: tsan: s/-1/kInvalidTid/.
tsan: s/-1/kInvalidTid/
Jul 12 2017, 5:37 AM
dvyukov committed rL307780: tsan: give debug names to dense allocators.
tsan: give debug names to dense allocators
Jul 12 2017, 5:34 AM
dvyukov committed rL307778: tsan: don't create sync objects on acquire-load.
tsan: don't create sync objects on acquire-load
Jul 12 2017, 5:28 AM
dvyukov committed rL307777: tsan: add another test for clock growth.
tsan: add another test for clock growth
Jul 12 2017, 5:25 AM
dvyukov committed rL307776: tsan: add test for __tsan_java_find.
tsan: add test for __tsan_java_find
Jul 12 2017, 5:24 AM

Jul 10 2017

dvyukov added a comment to D35217: Fix PR33732.

Please go over other sanitizer APIs and check if we have any other i8/16/32 args. dfsan?

Jul 10 2017, 12:22 PM
dvyukov edited reviewers for D35098: [asan] For iOS/AArch64, if the dynamic shadow doesn't fit, restrict the VM space, added: alekseyshl, vitalybuka; removed: dvyukov.
Jul 10 2017, 12:59 AM · Restricted Project
dvyukov accepted D35157: [tsan] Add support for running TSan tests on iOS simulator and devices.

LGTM with a nit

Jul 10 2017, 12:56 AM · Restricted Project
dvyukov added inline comments to D35154: [tsan] Add a max VM address check for Darwin/AArch64.
Jul 10 2017, 12:52 AM · Restricted Project
dvyukov accepted D35154: [tsan] Add a max VM address check for Darwin/AArch64.
Jul 10 2017, 12:50 AM · Restricted Project
dvyukov accepted D35147: [tsan] Add a mapping for Darwin/AArch64.
Jul 10 2017, 12:49 AM · Restricted Project
dvyukov accepted D35134: [tsan] Add comments for the bool argument of ThreadIgnoreBegin/ThreadIgnoreSyncBegin, NFC..
Jul 10 2017, 12:46 AM · Restricted Project
dvyukov accepted D35143: [tsan] Port setjmp/longjmp assembly to Darwin/AArch64.

LGTM with a nit

Jul 10 2017, 12:46 AM · Restricted Project

Jul 5 2017

dvyukov accepted D35016: [tsan] Use pthread_sigmask instead of sigprocmask to block signals in a thread on Darwin.
Jul 5 2017, 11:04 AM · Restricted Project
dvyukov added inline comments to D35016: [tsan] Use pthread_sigmask instead of sigprocmask to block signals in a thread on Darwin.
Jul 5 2017, 10:36 AM · Restricted Project

Jun 22 2017

dvyukov added a comment to D34451: [RFC] Add KMSAN instrumentation to MSan pass.

Evgenii, please take a look at actual pass changes.

Jun 22 2017, 1:57 AM · Restricted Project
dvyukov added inline comments to D34451: [RFC] Add KMSAN instrumentation to MSan pass.
Jun 22 2017, 1:56 AM · Restricted Project
dvyukov added inline comments to D34451: [RFC] Add KMSAN instrumentation to MSan pass.
Jun 22 2017, 1:45 AM · Restricted Project
dvyukov added a comment to D34451: [RFC] Add KMSAN instrumentation to MSan pass.

I know, not the feedback you was looking for :)

Jun 22 2017, 1:28 AM · Restricted Project

Jun 13 2017

dvyukov committed rL305281: tsan: fix reading of mutex flags.
tsan: fix reading of mutex flags
Jun 13 2017, 2:38 AM
dvyukov committed rL305273: tsan: fix pedantic warnings.
tsan: fix pedantic warnings
Jun 13 2017, 12:10 AM

May 25 2017

dvyukov added a comment to D25509: tsan: intercept libatomic.

I don't have time to figure out how to support Mac. Filed https://github.com/google/sanitizers/issues/816 so that this is not lost.

May 25 2017, 2:28 AM
dvyukov accepted D33286: Don't require ThreadState to be contained within tls on all platforms.

LGTM with a nit

May 25 2017, 12:51 AM

May 24 2017

dvyukov accepted D33521: [sanitizer] Pair atomic acquire with release in BlockingMutex::Unlock.
May 24 2017, 11:59 PM
dvyukov added inline comments to D33286: Don't require ThreadState to be contained within tls on all platforms.
May 24 2017, 2:04 AM
dvyukov added a comment to D33454: [sanitizer] Change the 32-bit Primary AllocateRegion to reduce fragmentation.

looks good to me

May 24 2017, 1:41 AM
dvyukov added a comment to D33454: [sanitizer] Change the 32-bit Primary AllocateRegion to reduce fragmentation.

I am not certain about the Windows impact of this change

May 24 2017, 1:41 AM
dvyukov added a comment to D33454: [sanitizer] Change the 32-bit Primary AllocateRegion to reduce fragmentation.

FWIW I've solved a similar problem in tcmalloc by trimming blocks either on left or on right. Namely, if heap grows up we trim on right; if heap grows down we trim on left. But I don't see how it is better than this solution.

May 24 2017, 1:37 AM
dvyukov added inline comments to D33325: [sanitizer] Avoid possible deadlock in child process after fork.
May 24 2017, 1:19 AM · Restricted Project

May 23 2017

dvyukov added a comment to D33286: Don't require ThreadState to be contained within tls on all platforms.

I think that's because we use _shadow_ memory to store pointer to ThreadState. See cur_thread in tsan_platform_mac.cc. We probably need to skip that slot or something.

May 23 2017, 8:17 AM
dvyukov added inline comments to D33325: [sanitizer] Avoid possible deadlock in child process after fork.
May 23 2017, 7:32 AM · Restricted Project
dvyukov added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

I don't completely understand this. allocator_fork_no_hang.cc is broken and deserves deadlocking. Why are we trying to fix this program only under sanitizers rather than detecting and reporting the bug? Sanitizers are meant to be less forgiving than production environment. Why are we trying to conceal the bug?

Yes, the testcase deserves deadlocking (malloc is not async signal safe). And the actual bug is reported here: https://bugzilla.gnome.org/show_bug.cgi?id=738620.
The motivation for "fixing" this in sanitizers is that most of modern allocators (Glibc, tcmalloc, jemalloc) are actually fork safe, so why not to do the same in sanitizers. And despite the fact that calling malloc after multi-threaded fork is not allowed, many people still would expect their code to work (since it works with Glibc/tcmalloc/jemalloc etc).

May 23 2017, 7:29 AM · Restricted Project
dvyukov added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

I don't completely understand this. allocator_fork_no_hang.cc is broken and deserves deadlocking. Why are we trying to fix this program only under sanitizers rather than detecting and reporting the bug? Sanitizers are meant to be less forgiving than production environment. Why are we trying to conceal the bug?

May 23 2017, 6:18 AM · Restricted Project
dvyukov accepted D33286: Don't require ThreadState to be contained within tls on all platforms.

Thanks!

May 23 2017, 12:22 AM

May 22 2017

dvyukov added a comment to D33286: Don't require ThreadState to be contained within tls on all platforms.

Yes, please do this only for Darwin. These things are extremely fragile, it's nice to be able to rely at least on some things. If we change it to if, it will silently break on other platforms and we will not notice.
You can either move some code to platform* files, or merely provide a flag in platform* files and use it here.

May 22 2017, 12:59 AM

May 10 2017

dvyukov added a comment to D33007: [scudo] Use our own combined allocator.

Looks fine to me. Aleksey, do you want to take a look?

May 10 2017, 2:30 PM

May 8 2017

dvyukov added inline comments to D32971: [scudo] CRC32 optimizations.
May 8 2017, 1:50 PM
dvyukov added inline comments to D32971: [scudo] CRC32 optimizations.
May 8 2017, 12:54 PM
dvyukov added inline comments to D32971: [scudo] CRC32 optimizations.
May 8 2017, 12:47 PM
dvyukov added inline comments to D32971: [scudo] CRC32 optimizations.
May 8 2017, 12:04 PM

May 5 2017

dvyukov edited reviewers for D32895: [ASAN] Insert call to __asan_init and load of dynamic shadow address in correct order, added: alekseyshl; removed: dvyukov.
May 5 2017, 7:29 AM · Restricted Project

May 3 2017

dvyukov accepted D31630: [tsan] Detect races on modifying accesses in Swift code.

Thanks!
I think this now provides a solid foundation for future extensions.

May 3 2017, 4:17 AM · Restricted Project

May 2 2017

dvyukov added a comment to D31395: [Asan] Disable inline instrumentation for MMU-less targets.

One more thing: now that D31592 is landed, would you mind if we do the same for Tsan?

May 2 2017, 9:46 AM · Restricted Project
dvyukov added a comment to D31395: [Asan] Disable inline instrumentation for MMU-less targets.

Dmitry, with your permission I'm going to update and commit this patch as it seems to be useful regardless of which way we support sanitizers on MMU-less platforms.

May 2 2017, 9:40 AM · Restricted Project
dvyukov committed rL301927: tsan: allow fast large MemoryRangeSet on non-Windows Go.
tsan: allow fast large MemoryRangeSet on non-Windows Go
May 2 2017, 8:28 AM
dvyukov accepted D32705: [compiler-rt] move tsan's Android __get_tls() to sanitizer_common.

Thanks!

May 2 2017, 8:07 AM
dvyukov added inline comments to D32705: [compiler-rt] move tsan's Android __get_tls() to sanitizer_common.
May 2 2017, 4:56 AM

May 1 2017

dvyukov added a comment to D31593: [Asan] Introduce the concept of physical shadow memory.

This patch introduces a class of shadow pointers designed to be the interface to physical shadow memory. The idea behind shadow pointers is to encapsulate the physical memory mechanics so that:

  • sequential accesses to the same shadow page do not cause extra virtual-to-physical address translations, thus providing acceptable performance in the MMU-less mode and
  • sanitizer developers are not forced to take care of the organization of physical memory when dealing with shadow accesses.

    This patch intentionally includes (not-yet-committed) D31395 so one could apply and test it if necessary. Once generally accepted, I will remove those common parts and update the patch.
May 1 2017, 3:58 AM · Restricted Project
dvyukov committed rL301795: tsan: support linker init flag in __tsan_mutex_destroy.
tsan: support linker init flag in __tsan_mutex_destroy
May 1 2017, 3:14 AM

Apr 29 2017

dvyukov added inline comments to D32649: [scudo] Add Android support.
Apr 29 2017, 2:07 PM
dvyukov accepted D32382: [tsan] Track external tags in thread traces.

One last thing. Otherwise LGTM.

Apr 29 2017, 12:52 PM · Restricted Project
dvyukov added inline comments to D32649: [scudo] Add Android support.
Apr 29 2017, 9:39 AM

Apr 27 2017

dvyukov accepted D32440: [scudo] Move thread local variables into their own files.

LGTM on my side, but wait for Aleksey if he has more comments

Apr 27 2017, 10:17 AM
dvyukov added inline comments to D32440: [scudo] Move thread local variables into their own files.
Apr 27 2017, 3:25 AM

Apr 26 2017

dvyukov added a comment to D32440: [scudo] Move thread local variables into their own files.

otherwise looks good

Apr 26 2017, 11:59 AM

Apr 25 2017

dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

The memory/threading model of Swift defines races in terms of accesses: Starting a new "modifying" access requires that there are no outstanding accesses. And "access" here means the formal duration over which the variable is accessed, e.g. the whole function call for inout arguments. This is all pretty new and still being worked on: https://github.com/apple/swift/blob/master/docs/OwnershipManifesto.md.

Then let's register report header for external tags.

What do you mean by that? Can you explain?

Apr 25 2017, 9:43 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

ThreadSanitizer: race on Swift array
ThreadSanitizer: race on Swift map

Unfortunately, we won't be able to tell the type of the variable (at least not now). I've updated the patch to say "race on Swift variable", but that's not really exact and it's not describing the problem well. Swift is getting exact threading rules that are very different from C/C++ and I'm trying to focus on making the reports readable and understandable by Swift users.

Is there a way we could still special-case the Swift reports? I understand the desire to keep the API generic, but I think we need to make sure the users understand the report. E.g. "Swift access race" much better describes what the problem is, and users words that define the Swift threading model.

Humm... what is so special about Swift? Isn't it that concurrent accesses to some objects are a bug? I would expect that it is, and then it's no different from C/C++ variables, C++ containers, Go variables and containers, Java containers, file descriptors and everything else.

I don't understand why saying race or data race on something is confusing or not understand-able. What's ambiguous/obscure/wrong about it?
On the other hand I have problems understanding meaning of "Swift access race". Does it mean "race during access to a Swift object"? If so it's semantically equivalent to "race on Swift object".

The memory/threading model of Swift defines races in terms of accesses: Starting a new "modifying" access requires that there are no outstanding accesses. And "access" here means the formal duration over which the variable is accessed, e.g. the whole function call for inout arguments. This is all pretty new and still being worked on: https://github.com/apple/swift/blob/master/docs/OwnershipManifesto.md.

Apr 25 2017, 9:20 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

ThreadSanitizer: race on Swift array
ThreadSanitizer: race on Swift map

Unfortunately, we won't be able to tell the type of the variable (at least not now). I've updated the patch to say "race on Swift variable", but that's not really exact and it's not describing the problem well. Swift is getting exact threading rules that are very different from C/C++ and I'm trying to focus on making the reports readable and understandable by Swift users.

Is there a way we could still special-case the Swift reports? I understand the desire to keep the API generic, but I think we need to make sure the users understand the report. E.g. "Swift access race" much better describes what the problem is, and users words that define the Swift threading model.

Apr 25 2017, 5:35 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

Also, could swift runtime register a tag at startup? Or performance overhead is a concern?

It's quite hard to make the runtime call the registration at process startup, because it would need to dynamically figure out whether we're running with TSan or not, plus we'd need to add an indirection to each access (to read the value of the tag). I'd really prefer to keep the value as a well-defined constant in TSan.

Apr 25 2017, 5:23 AM · Restricted Project
dvyukov added inline comments to D32382: [tsan] Track external tags in thread traces.
Apr 25 2017, 5:21 AM · Restricted Project

Apr 23 2017

dvyukov added reviewers for D32408: [ASAN] Add interceptor for __longjmp_chk: alekseyshl, vitalybuka.
Apr 23 2017, 11:11 PM
dvyukov accepted D32402: Add missing acquire_load to call_once overload..
Apr 23 2017, 10:06 AM

Apr 22 2017

dvyukov accepted D32384: [tsan] Include __tsan_external_* API from a header file instead of declaring them manually.
Apr 22 2017, 2:09 AM · Restricted Project
dvyukov accepted D32383: [tsan] Remove the extra word "object" from description of external races.
Apr 22 2017, 1:59 AM · Restricted Project

Apr 21 2017

dvyukov accepted D32365: [sanitizer] Cache SizeClassForTransferBatch in the 32-bit local cache.
Apr 21 2017, 12:12 PM
dvyukov accepted D32360: [tsan] Refactor __tsan_external_read/__tsan_external_write to avoid code duplication.
Apr 21 2017, 10:41 AM · Restricted Project
dvyukov accepted D32359: [tsan] Track external API accesses as 1-byte accesses (instead of 8-byte).
Apr 21 2017, 10:30 AM · Restricted Project
dvyukov added a comment to D32310: [scudo] Bypass Quarantine if its size is set to 0.

which will be checked in due time.

Apr 21 2017, 10:27 AM
dvyukov accepted D32358: [tsan] Publish the TSan external API in tsan_interface.h.
Apr 21 2017, 10:24 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

Hi Kuba,

I've seen a bunch of changes from you and they all are marked in my inbox. The past 1.5 weeks I was travelling and next week I am OOO so won't be able to review them. Just wanted to let you know.

I am back. Slowly working on backlog.

Apr 21 2017, 10:17 AM · Restricted Project
dvyukov added inline comments to D31689: [tsan] Summary should point to "responsible caller" for external races.
Apr 21 2017, 7:11 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

We have 3 potential users for the external API (Go, Java, races on fd, potentially more in future).

Apr 21 2017, 6:48 AM · Restricted Project
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

I don't like that we sprinkle swift-specific bits throughout the code when the external API was meant to be general enough to cover all such cases.
We have the special tag for swift, so it really seems to me that it should be the only swift-specific bit here and the rest must be handled by the external API on general basis. We have 3 potential users for the external API (Go, Java, races on fd, potentially more in future). So I would be really interested in making the external API general and flexible enough to support all of them, rather than later sprinkle Go/Java/fd/etc-specific bits throughout the code again later.

Apr 21 2017, 6:42 AM · Restricted Project
dvyukov accepted D31553: [tsan] Ignore memory accesses for libignored modules for "external" races.
Apr 21 2017, 6:05 AM · Restricted Project
dvyukov added inline comments to D31684: [sanitizer] Fix various issues reported by Clang Static Analyzer [NFC].
Apr 21 2017, 5:21 AM · Restricted Project
dvyukov accepted D31734: [tsan] Add a test for "external" API that checks the dup suppression is based on the caller PC.
Apr 21 2017, 4:45 AM · Restricted Project
dvyukov accepted D31449: [tsan] Don't report bugs from interceptors called from libignored modules.
Apr 21 2017, 4:45 AM · Restricted Project
dvyukov added a comment to D31729: [tsan] Mark "responsible frames" in stack traces to show which frame contains the race.

I am not completely follow motivation for this. Is it meant for user? Or for any automated tools?
For automated tools we already have SUMMARY line. User must be proficient enough to understand what happens. And in the end this is not specific to libc, and the guilty frame is not necessary the one just above libc. Consider a race in std::vector, it's probably not std::vector code that is guilty. And this extends just to any library, consider you have race in libfoo, but it's actually your code that misuses libfoo. We don't have enough info to provide precise signal. What we provide here is a weak, potentially incorrect signal.

Apr 21 2017, 4:33 AM · Restricted Project
dvyukov accepted D32310: [scudo] Bypass Quarantine if its size is set to 0.

If we actually plan to use such configuration, does it make sense to check header for corruption when we reallocate a block (in allocate)? That will give us at least some windows for UAF detection.

Apr 21 2017, 1:46 AM

Apr 20 2017

dvyukov accepted D32299: [scudo] Remove GetActuallyAllocatedSize calls from the fast path.
Apr 20 2017, 10:35 AM
dvyukov accepted D32242: [scudo] Minor changes and refactoring.

LGTM with a nit

Apr 20 2017, 6:16 AM

Apr 19 2017

dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

Hi Kuba,

I've seen a bunch of changes from you and they all are marked in my inbox. The past 1.5 weeks I was travelling and next week I am OOO so won't be able to review them. Just wanted to let you know.

Apr 19 2017, 1:03 PM · Restricted Project
dvyukov added a comment to D31947: [scudo] Android support groundwork.

@dvyukov would it be acceptable to get this patch out in its current form and use this as a base for the further improvement you suggested?
Namely: improve contention, potential use of the tsan TLS slot, rework thread/global initialization.

Apr 19 2017, 11:32 AM

Apr 18 2017

dvyukov added a comment to D31947: [scudo] Android support groundwork.

First of all, do you have any benchmarks? Single-threaded, multi-threaded with/without contention. How do profiles look like?

I have shared with Aleksey some Flame profiles, I can send that to you as well. This ended up with the couple of optimizations he recently did.

Apr 18 2017, 8:44 AM