Page MenuHomePhabricator

dvyukov (Dmitry Vyukov)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 6 2012, 2:31 AM (435 w, 6 d)

Recent Activity

Today

dvyukov added a comment to D99434: [TSAN] Honor acquire failure mode on AtomicCAS.

Not directly related to the change but we could also episodically fail weak CASes to test fmo case better.

Wed, Apr 14, 2:44 AM · Restricted Project
dvyukov added a comment to D99434: [TSAN] Honor acquire failure mode on AtomicCAS.

Also I wonder if we could first evaluate CAS and then use either mo or fmo accordingly, so that we don't over-synchronize on failure if mo is stronger than fmo.

Wed, Apr 14, 2:42 AM · Restricted Project
dvyukov added inline comments to D99434: [TSAN] Honor acquire failure mode on AtomicCAS.
Wed, Apr 14, 1:40 AM · Restricted Project

Yesterday

dvyukov accepted D72549: [TSan] Register threads created via pthread_create_from_mach_thread().
Tue, Apr 13, 11:42 PM · Restricted Project, Restricted Project
dvyukov added a comment to D100142: [compiler-rt][aarch64] Add PAC-RET/BTI support to TSan..

I am not much knowledgeable about PAC and would otherwise leave this to somebody else.

Tue, Apr 13, 8:00 AM

Mar 9 2021

dvyukov closed D98238: [sanitizers] Enable runtime vma for mips64 in buildgo.sh.

Merged in https://github.com/llvm/llvm-project/commit/373e1681c970d7e7462164defb98098b7bc79946

Mar 9 2021, 2:47 AM · Restricted Project
dvyukov committed rG373e1681c970: [sanitizers] Enable runtime vma for mips64 in buildgo.sh (authored by dvyukov).
[sanitizers] Enable runtime vma for mips64 in buildgo.sh
Mar 9 2021, 2:46 AM
dvyukov accepted D98238: [sanitizers] Enable runtime vma for mips64 in buildgo.sh.
Mar 9 2021, 2:42 AM · Restricted Project
dvyukov closed D98231: [sanitizers] fix wrong enum of memory_order for mips.

Merged in https://github.com/llvm/llvm-project/commit/85801b4c68ee4f1c7b9f2a33162d852b577fe536

Mar 9 2021, 2:30 AM · Restricted Project
dvyukov committed rG85801b4c68ee: [sanitizers] fix wrong enum of memory_order for mips (authored by dvyukov).
[sanitizers] fix wrong enum of memory_order for mips
Mar 9 2021, 2:29 AM

Mar 8 2021

dvyukov accepted D98231: [sanitizers] fix wrong enum of memory_order for mips.

Hi Meng,

Mar 8 2021, 10:31 PM · Restricted Project

Feb 22 2021

dvyukov accepted D96762: [sanitizers] fix test failing for -Wunused-command-line-argument.

This is fine with me.

Feb 22 2021, 9:38 AM · Restricted Project
dvyukov reopened D96762: [sanitizers] fix test failing for -Wunused-command-line-argument.
Feb 22 2021, 9:38 AM · Restricted Project

Feb 19 2021

dvyukov added a comment to D96762: [sanitizers] fix test failing for -Wunused-command-line-argument.

Mkay, they I guess we need to revert. Or, Joachim, do you have any ideas for an alternative fix? We could whitelist/blacklist flags from $EXTRA_CFLAGS... I can't think of anything better atm.

Feb 19 2021, 12:39 AM · Restricted Project

Feb 17 2021

dvyukov closed D96874: tsan: fix mmap_lots test.

Committed in https://github.com/llvm/llvm-project/commit/fb19400d4e4c421ce61907bf92a92be8d937e584

Feb 17 2021, 10:04 AM
dvyukov committed rGfb19400d4e4c: tsan: fix mmap_lots test (authored by dvyukov).
tsan: fix mmap_lots test
Feb 17 2021, 10:03 AM
dvyukov requested review of D96874: tsan: fix mmap_lots test.
Feb 17 2021, 8:47 AM

Feb 16 2021

dvyukov closed D96697: tsan: don't leave unmapped hole in non-app memory.

Committed in https://github.com/llvm/llvm-project/commit/0984b8de0b0d5d178a8e6e5de1eb89f29493a89e

Feb 16 2021, 11:38 PM
dvyukov committed rG0984b8de0b0d: tsan: don't leave unmapped hole in non-app memory (authored by dvyukov).
tsan: don't leave unmapped hole in non-app memory
Feb 16 2021, 11:37 PM
dvyukov accepted D96762: [sanitizers] fix test failing for -Wunused-command-line-argument.

How about passing the CMAKE_C_FLAGS into the script? Because of ccache, I use the -Wno-unused-command-line-argument flags anyways.

Feb 16 2021, 11:19 PM · Restricted Project
dvyukov added a comment to D96762: [sanitizers] fix test failing for -Wunused-command-line-argument.

What is the unused argument? Maybe we should remove the unused argument instead?

Feb 16 2021, 2:00 AM · Restricted Project

Feb 15 2021

dvyukov requested review of D96697: tsan: don't leave unmapped hole in non-app memory.
Feb 15 2021, 3:12 AM

Dec 14 2020

dvyukov accepted D86377: [tsan] Use large address space mapping on Apple Silicon Macs.

Gosh, I somehow missed all pings... I don't know how it happened.
Please add vitalybuka@ for future tsan changes, having several people may help to avoid delays.
I don't see any issues with the change.

Dec 14 2020, 2:27 AM · Restricted Project

Dec 9 2020

dvyukov accepted D92846: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases.

The code looks reasonable to me. I see it only affects kernel, so assuming you booted kernel, we should be fine.
I can rubber-stamp it, but if you want more meaningful review, please wait for Alex or Nick.

Dec 9 2020, 4:28 AM · Restricted Project, Restricted Project

Dec 8 2020

dvyukov added a comment to D92846: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases.

Have you checked if there is already a function that does this? Frequently there is :)
Some functions that look similar based on names:
stripPointerCasts
stripPointerCastsAndOffsets
stripPointerCastsAndAliases
canonicalizeAlias

Dec 8 2020, 8:25 AM · Restricted Project, Restricted Project

Dec 3 2020

dvyukov edited reviewers for D92521: [tsan] Fix build failure with _FORTIFY_SOURCE, added: vitalybuka; removed: dvyukov.
Dec 3 2020, 1:35 AM · Restricted Project
dvyukov edited reviewers for D92519: [tsan] Use REAL macro when calling intercepted function, added: vitalybuka; removed: dvyukov.
Dec 3 2020, 1:34 AM · Restricted Project

Nov 18 2020

dvyukov accepted D91684: [tsan] Add pthread_cond_clockwait interceptor.
Nov 18 2020, 1:23 AM · Restricted Project

Oct 30 2020

dvyukov closed D90435: [TSAN] add Go race detector support for macOS/ARM64.

Merged in https://github.com/llvm/llvm-project/commit/00da38ce2d36c07f12c287dc515d37bb7bc410e9
thanks

Oct 30 2020, 11:43 AM · Restricted Project
dvyukov committed rG00da38ce2d36: tsan: add Go race detector support for macOS/ARM64 (authored by dvyukov).
tsan: add Go race detector support for macOS/ARM64
Oct 30 2020, 11:43 AM
dvyukov added a comment to D90435: [TSAN] add Go race detector support for macOS/ARM64.

Cherry, do you have commit access? Or you need somebody to merge it?

Oct 30 2020, 1:39 AM · Restricted Project
dvyukov accepted D90435: [TSAN] add Go race detector support for macOS/ARM64.
Oct 30 2020, 1:37 AM · Restricted Project
dvyukov closed D89940: [sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets.

Merged as: https://github.com/llvm/llvm-project/commit/26c1ced41c262bb87619cfa8a03c1879c63fb5f7
Thanks!

Oct 30 2020, 1:26 AM · Restricted Project
dvyukov committed rG26c1ced41c26: [sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets (authored by dvyukov).
[sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets
Oct 30 2020, 1:25 AM

Oct 26 2020

dvyukov closed D90130: Add mips64 support in lib/tsan/go/buildgo.sh.

Landed in https://github.com/llvm/llvm-project/commit/5cad535ccfebf9b41a57cf2788d8de7a765f7f35

Oct 26 2020, 4:20 AM · Restricted Project, Restricted Project, Restricted Project
dvyukov committed rG5cad535ccfeb: tsan: add mips64 support in lib/tsan/go/buildgo.sh (authored by dvyukov).
tsan: add mips64 support in lib/tsan/go/buildgo.sh
Oct 26 2020, 4:20 AM
dvyukov accepted D90130: Add mips64 support in lib/tsan/go/buildgo.sh.

Looks good to me.
Do you want me to merge this change?

Oct 26 2020, 12:42 AM · Restricted Project, Restricted Project, Restricted Project
dvyukov added a comment to D90130: Add mips64 support in lib/tsan/go/buildgo.sh.

This now looks like a patch for some other change :)

Oct 26 2020, 12:39 AM · Restricted Project, Restricted Project, Restricted Project
dvyukov accepted D89940: [sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets.
Oct 26 2020, 12:19 AM · Restricted Project
dvyukov added inline comments to D90130: Add mips64 support in lib/tsan/go/buildgo.sh.
Oct 26 2020, 12:16 AM · Restricted Project, Restricted Project, Restricted Project

Oct 24 2020

dvyukov added a comment to D89940: [sanitizer] Use __atomic_load/store() built-ins for generic 32-bit targets.

Looks good with nits.
Please also upload with full file context, I wonder what's before that "} else {" on line 80.

Oct 24 2020, 3:52 AM · Restricted Project

Oct 1 2020

dvyukov accepted D88616: Avoid getpid call after kill.

Thanks Dmitry, the problem isn't that the getpid is wrong per se, the scenario is:

  • ptraced program self stops with a SIGSTOP either by raise, kill, ..
  • ptracing program attaches... and issues PTRACE_SYSCALL to resume the ptraced program until the next syscall,
  • normally this would be some expected syscall, with tsan it will always be the getpid here.

As we'd like tests to pass with tsan, then we either teach the code to ignore getpid when running with tsan or avoid the interception using assembly - both of which are somewhat ugly workarounds.
While adding a test is attractive the harnessing of a parent/child process plus variants for OS and architecture, mean test code would dwarf what is a somewhat straightforward change.

Oct 1 2020, 11:39 PM · Restricted Project

Sep 30 2020

dvyukov added a comment to D88616: Avoid getpid call after kill.

We are querying own PID, I don't understand how it can be wrong. We definitely did not kill itself if we are still running :)
Please add a test that exposes the problem. Will also help to understand what scenario we are fixing.

Sep 30 2020, 11:29 PM · Restricted Project

Aug 13 2020

dvyukov added a comment to D85841: [tsan] Respect no_huge_pages_for_shadow..

I wonder if it would pay to selectively apply this for the parts of the shadow that correspond to shared library mappings.
Otherwise we are losing the benefits of huge pages on the heap, which should have a pretty dense shadow.
@dvyukov

Aug 13 2020, 4:13 AM · Restricted Project

Aug 7 2020

dvyukov added a reviewer for D85512: [TSan] Use max reduction to aggreagate StatThreadMaxAlive: vitalybuka.
Aug 7 2020, 3:55 AM · Restricted Project

Jul 30 2020

dvyukov accepted D84859: Disable getauxval for Go.
Jul 30 2020, 12:15 PM · Restricted Project
dvyukov added a reviewer for D84859: Disable getauxval for Go: vitalybuka.
Jul 30 2020, 1:45 AM · Restricted Project

Jul 24 2020

dvyukov edited reviewers for D84570: [tsan] Fix the open and open64 interceptors to have correct declarations (variadic functions), added: vitalybuka; removed: dvyukov.
Jul 24 2020, 9:46 PM · Restricted Project

Jul 20 2020

dvyukov added a comment to D83625: [TSan] Optimize handling of racy address.

Joachim suggested cherry-picking this (https://reviews.llvm.org/rG7358a1104a02) to the 11.x branch. Dmitry, what do you think?

Jul 20 2020, 5:41 AM · Restricted Project

Jul 16 2020

dvyukov added a comment to D83949: [TSan] Do not compound reads and writes when separated by atomics.

In practice, it probably doesn't really matter because T0 would probably load acquire spin on B or something better, and it'd be all in separate basic blocks.

Jul 16 2020, 8:12 AM · Restricted Project
dvyukov accepted D83867: [TSan] Add option for emitting compound read-write instrumentation.

LGTM on my side

Jul 16 2020, 7:21 AM · Restricted Project
dvyukov added a comment to D83949: [TSan] Do not compound reads and writes when separated by atomics.

What would be an example of code where it matters?

Jul 16 2020, 7:19 AM · Restricted Project
dvyukov added inline comments to D83867: [TSan] Add option for emitting compound read-write instrumentation.
Jul 16 2020, 2:41 AM · Restricted Project
dvyukov added a comment to D83625: [TSan] Optimize handling of racy address.

No idea why it broke now. THe change to tsan_test_util_posix.cpp looks fine to me.

Jul 16 2020, 2:04 AM · Restricted Project

Jul 15 2020

dvyukov added a comment to D57158: [RFC] [KASAN] Instrument globals.

Alex, is this still relevant? If not, please close.

Jul 15 2020, 11:51 PM
dvyukov removed a reviewer for D58108: [sanitizer]: fix warnings reported by SVACE static analyzer.: dvyukov.
Jul 15 2020, 11:51 PM · Restricted Project, Restricted Project
dvyukov added a comment to D83625: [TSan] Optimize handling of racy address.

Right, it can do more harm.

What do you think about comparing the local stack against the two hashes in racy_stacks?
I would propably have this defaulting to false unless we find it convenient for the typical usecase.

Jul 15 2020, 8:31 AM · Restricted Project
dvyukov added a comment to D83625: [TSan] Optimize handling of racy address.

I agree, that the patch changes the transitive suppression of reports. Unfortunately, this would bring back the overhead of reconstructing the remote stack trace.

As a follow-up patch I was thinking about adding a suppress_equal_location flag.
I would only compare the local stack trace with previously reported stack traces in racy_stacks.
Initially I was thinking about suppress_equal_pc, but especially for wrapper functions, the top pc is function pointer of the wrapper.

Jul 15 2020, 3:42 AM · Restricted Project
dvyukov accepted D83625: [TSan] Optimize handling of racy address.

Looks good to me.
You have access, right? If so, please land it.

Jul 15 2020, 3:40 AM · Restricted Project

Jul 14 2020

dvyukov added inline comments to D83625: [TSan] Optimize handling of racy address.
Jul 14 2020, 5:19 AM · Restricted Project
dvyukov added a comment to D83625: [TSan] Optimize handling of racy address.

Hi Joachim,

Jul 14 2020, 5:18 AM · Restricted Project

Jun 8 2020

dvyukov committed rGfcf6ae2f070e: tsan: add OpenBSD support for Go (authored by dvyukov).
tsan: add OpenBSD support for Go
Jun 8 2020, 8:14 AM
dvyukov closed D80469: Add OpenBSD support in lib/tsan/go/buildgo.sh.

Landed in https://github.com/llvm/llvm-project/commit/fcf6ae2f070eba73074b6ec8d8281e54d29dbeeb
Thanks.

Jun 8 2020, 8:11 AM · Restricted Project

May 28 2020

dvyukov accepted D80469: Add OpenBSD support in lib/tsan/go/buildgo.sh.
May 28 2020, 10:59 PM · Restricted Project
dvyukov committed rG0969541ffcb2: tsan: disable java_finalizer2 test on darwin (authored by dvyukov).
tsan: disable java_finalizer2 test on darwin
May 28 2020, 2:08 AM
dvyukov added a comment to D80474: tsan: fix false positives in AcquireGlobal.

Looks like the test added here is failing on Darwin. Can you please look into it.

May 28 2020, 2:08 AM

May 27 2020

dvyukov added a comment to D80474: tsan: fix false positives in AcquireGlobal.

But even if we store just 1 element per thread, won't it require storing potentially infinite amount of snapshots for all previous versions?

We only run a single GlobalAcquire at a time, so we should only need to store a snapshot for a single version in each thread. Once the GlobalAcquire has collected the snapshot from each thread, it is no longer necessary.

If I am reading this correctly a thread can advance arbitrary number of version since last_version.

In practice, I don't think a thread will ever advance more than a single version at a time, because all threads are advanced in lockstep.

May 27 2020, 1:36 PM
dvyukov committed rGd24dd2b279ff: tsan: fix test in debug mode (authored by dvyukov).
tsan: fix test in debug mode
May 27 2020, 1:04 PM
dvyukov committed rG4408eeed0ff1: tsan: fix false positives in AcquireGlobal (authored by dvyukov).
tsan: fix false positives in AcquireGlobal
May 27 2020, 7:34 AM
dvyukov added a comment to D80474: tsan: fix false positives in AcquireGlobal.

Or GlobalAcquire could do some kind of multi-pass O(N^2) operation that will consider all values in all thread clocks, but this looks too expensive and I very hard to prove to be correct.

Taking inspiration from the distributed systems world, I think we could also do a two-pass O(n) solution by using an augmented copy-on-write scheme if we are willing to propagate a clock "version" through SyncClock operations (I'd use "epoch", but that's already taken). Imagine that each clock had an associated version. When releasing into a SyncClock, we set its version to sync_clock.version = max(sync_clock.version, thread_clock.version). When acquiring from a SyncClock, we set our ThreadClock to thread_clock.version = max(sync_clock.version, thread_clock.version). Whenever a ThreadClock's version changes, it snapshots its current clock state before making the corresponding modification.

We could then implement GlobalAcquire as:

last_version = cur_version;
next_version = cur_version + 1;
for th in thread:
    th.setVersionAndSnapshotIfNeeded(next_version);
for th in thread:
    acquiring_thr->clock.set(th, th.getClockSnapshotForVersion(last_version));

Writing this all out reveals that a ThreadClock's snapshot doesn't even need to be of the entire vector clock, just its own element at the time of the snapshot. Also, since AcquireGlobal is never run concurrently, a ThreadClock only needs to store its last snapshot. So I think this all could be accomplished with only 2 new u64s per ThreadClock (cur_version_ and last_snapshot_) and 1 new u64 per SyncClock (cur_version_).

This is still more complicated than what we have here though. I'm still fine with merging the global_acquire_ approach.

May 27 2020, 7:33 AM
dvyukov closed D80474: tsan: fix false positives in AcquireGlobal.

Merged in https://github.com/llvm/llvm-project/commit/4408eeed0ff191304121c11168aa1db861cccb97

May 27 2020, 7:33 AM

May 25 2020

dvyukov updated the diff for D80474: tsan: fix false positives in AcquireGlobal.
May 25 2020, 10:28 PM
dvyukov added a comment to D80474: tsan: fix false positives in AcquireGlobal.

My one concern is that the change revives the last_acquire_ optimization while still allowing vector clocks to reflect non-linearizable global states.

May 25 2020, 10:28 PM

May 24 2020

dvyukov added a reviewer for D80474: tsan: fix false positives in AcquireGlobal: vitalybuka.
May 24 2020, 5:20 AM

May 23 2020

dvyukov added a comment to D80474: tsan: fix false positives in AcquireGlobal.

Daniel, I am not asking to review (but if you want, you are welcome!). It's just a very interesting exercise in vector clock reasoning and a super tricky bug, so I thought you may be interested.

May 23 2020, 8:27 AM
dvyukov created D80474: tsan: fix false positives in AcquireGlobal.
May 23 2020, 8:27 AM
dvyukov updated subscribers of D80474: tsan: fix false positives in AcquireGlobal.
May 23 2020, 8:27 AM

May 15 2020

dvyukov committed rG151ed6aa38a3: [TSAN] Add option to allow instrumenting reads of reads-before-writes (authored by dvyukov).
[TSAN] Add option to allow instrumenting reads of reads-before-writes
May 15 2020, 7:34 AM
dvyukov closed D79983: [TSAN] Add option to allow instrumenting reads of reads-before-writes.

Merged as https://github.com/llvm/llvm-project/commit/151ed6aa38a3ec6c01973b35f684586b6e1c0f7e

May 15 2020, 7:33 AM · Restricted Project
dvyukov accepted D79983: [TSAN] Add option to allow instrumenting reads of reads-before-writes.
May 15 2020, 3:02 AM · Restricted Project
dvyukov added a comment to D79983: [TSAN] Add option to allow instrumenting reads of reads-before-writes.

Maybe this flag should turn off instrumentation of the "atomic" writes? Then the "reads-before-write" are not before writes anymore and will be naturally instrumented. And we also get faster execution. What do you think?

Do you mean putting some of KCSAN's logic to determine what are "atomic" writes here? For one, I don't think it's reliable because the logic in KCSAN may change again and we shouldn't make any code here be KCSAN-specific.

The other thing is that conflicts between "atomic write" and "plain read" will be missed.

Or did I misunderstand what you meant?

May 15 2020, 3:02 AM · Restricted Project
dvyukov added a comment to D79983: [TSAN] Add option to allow instrumenting reads of reads-before-writes.

Maybe this flag should turn off instrumentation of the "atomic" writes? Then the "reads-before-write" are not before writes anymore and will be naturally instrumented. And we also get faster execution. What do you think?

May 15 2020, 12:52 AM · Restricted Project

Apr 22 2020

dvyukov committed rG5a2c31116f41: [TSAN] Add optional support for distinguishing volatiles (authored by dvyukov).
[TSAN] Add optional support for distinguishing volatiles
Apr 22 2020, 8:40 AM
dvyukov closed D78554: [TSAN] Add optional support for distinguishing volatiles.

ninja check-tsan passed.
ninja check-all kills my self-isolation machine, so let's assume it works...
committed as: https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814

Apr 22 2020, 8:38 AM · Restricted Project

Apr 21 2020

dvyukov added a comment to D78554: [TSAN] Add optional support for distinguishing volatiles.

Any objections, anybody?

Apr 21 2020, 9:09 AM · Restricted Project
dvyukov accepted D78554: [TSAN] Add optional support for distinguishing volatiles.
Apr 21 2020, 9:09 AM · Restricted Project
dvyukov added a reviewer for D78554: [TSAN] Add optional support for distinguishing volatiles: Restricted Project.
Apr 21 2020, 9:09 AM · Restricted Project
dvyukov added a comment to D78554: [TSAN] Add optional support for distinguishing volatiles.

I am not opposed to adding this in general. Andrey did the same for KTSAN in gcc long time ago.
However, this feature will be present only in the newest clang and not in gcc. So what is the use story for kernel? Do we make KCSAN require the newest clang? I think majority of kernel developers use gcc.

I am planning to make the same change to gcc. And yes, it's a problem that it's only the newest versions that will support this. But, for now it's not urgent and no kernel dev is explicitly requesting this. But my feeling is that eventually we will need it, and we will be glad to have this option when that day comes.

Apr 21 2020, 9:09 AM · Restricted Project
dvyukov added a comment to D78554: [TSAN] Add optional support for distinguishing volatiles.

I am not opposed to adding this in general. Andrey did the same for KTSAN in gcc long time ago.
However, this feature will be present only in the newest clang and not in gcc. So what is the use story for kernel? Do we make KCSAN require the newest clang? I think majority of kernel developers use gcc.

Apr 21 2020, 6:27 AM · Restricted Project

Apr 16 2020

dvyukov accepted D78111: tsan: fixes to ThreadClock::releaseStoreAcquire.

You can now land this yourself, right?

Apr 16 2020, 2:49 AM

Apr 14 2020

dvyukov added a comment to D78111: tsan: fixes to ThreadClock::releaseStoreAcquire.

Please add a unit test for these bugs to tsan/tests/unit/tsan_clock_test.cpp and releaseStoreAcquire to the stress test in that file.

Apr 14 2020, 10:10 AM

Apr 11 2020

dvyukov committed rGc65e6079fc9b: tsan: add newline in test file (authored by dvyukov).
tsan: add newline in test file
Apr 11 2020, 2:38 AM
dvyukov committed rG1624be938dd2: tsan: fix leak of ThreadSignalContext memory mapping when destroying fibers (authored by dvyukov).
tsan: fix leak of ThreadSignalContext memory mapping when destroying fibers
Apr 11 2020, 1:34 AM
dvyukov committed rGefeb35e19563: tsan: disable ASLR in Go test on NetBSD (authored by dvyukov).
tsan: disable ASLR in Go test on NetBSD
Apr 11 2020, 1:34 AM
dvyukov added a comment to D76073: [compiler-rt][tsan] Fix: Leak of ThreadSignalContext memory mapping when destroying fibers..

I integrated your feedback and put a separate callback for just the clean up. To not do any further refactoring I placed it in OnFinished where also other resources of ThreadState are freed.
The cleanup accesses non of the previously freed resources and does not free any resources needed in the destructor called after it or the stat aggregation, which are the only functions accessing ThreadState afterwards.

PS: It is my very first time contributing to a bigger repository/project and I have only limited experience with big codebases. I really try to get this right and not cause any issues, maybe I was a bit over-eager with my refactoring. Thank you for all your patience and feedback :)

Apr 11 2020, 1:34 AM · Restricted Project, Restricted Project
dvyukov added a comment to D76073: [compiler-rt][tsan] Fix: Leak of ThreadSignalContext memory mapping when destroying fibers..

Re-merged as https://github.com/llvm/llvm-project/commit/1624be938dd2badf8297e63b6b330882b8023372

Apr 11 2020, 1:34 AM · Restricted Project, Restricted Project

Apr 10 2020

dvyukov committed rG87735b5b1d03: tsan: don't check libc dependency on NetBSD (authored by dvyukov).
tsan: don't check libc dependency on NetBSD
Apr 10 2020, 2:40 AM

Apr 9 2020

dvyukov accepted D77477: tsan: don't instrument __attribute__((naked)) functions.
Apr 9 2020, 2:40 AM · Restricted Project

Apr 7 2020

dvyukov committed rG2db63723a875: tsan: fix Go/ppc build (authored by dvyukov).
tsan: fix Go/ppc build
Apr 7 2020, 8:07 AM
dvyukov added a comment to D76073: [compiler-rt][tsan] Fix: Leak of ThreadSignalContext memory mapping when destroying fibers..

This is lots of changes. This makes me nervous and makes it harder for me approve this. Tsan is currently in maintenance mode with no official staffing and I don't remember most of this code well. Keeping changes "safer" makes it much easier to handle.
Say, you reorder thr->~ThreadState() and StatAggregate. And this has no explanation and is hidden in the code movement, so not even really visible. This makes me wonder why, and if you have given this enough consideration and are there other similar hidden changes...
Also moving all this ThreadSignalContext/SignalDesc/ucontext_buffer_t/768/129/65 into abstract logic layer makes me wince. It was specifically kept as "interceptors" stuff for a 8 years. I think the same can be achieved by providing a callback in tsan_interceptors_posix.cc with no major code movement.
If you restrict this to just fixing the bug, it will make both of us less scared of breaking things :)
Refactrorings may go in separate subsequent changes with own proper descriptions. This is much better for review, tests and these are also a unit of rollback. So if just a refactoring is rolled back, that's much better.

Apr 7 2020, 3:45 AM · Restricted Project, Restricted Project