Data race in sw::Renderer::draw |
||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6449486399537152 Fuzzer: inferno_layout_test_unmodified Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Data race READ 4 Crash Address: 0x7bb00000c70c Crash State: sw::Renderer::draw es2::Device::drawPrimitive es2::Context::drawArrays Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=463855:463874 Reproducer Testcase: https://clusterfuzz.com/download/AMIfv97lck6ZvT0Gfin9xm2XhxVCLpPmV4jz_i0n_IYGVUHaUQZvNs27UFSFf-iJmSVDBpz5OiIlS96BM7RT1Zur2_d7x_AetcDxibS2ic_RHmgi9Y5zZwpgBDpEzONS0TxfYfLZdnVgphdrdzq_iNe47UhvngZbCHteaQtcFxOe7qW82QNoio8j71Qe4nNCbDNiWrrDvQLO53iPol73_28p-UGTosQ3LmRQdlooc8iM8D8pf_PBbOMC5w-ohNWtdnscO3pSiz0Pawlz-TEDoj23_nW9EryuMtZhElkxosE9MkH6lRZQTV7-Nvdd8aY-H75H8EJQciKurmVgkcMPQeK3dDIL0mG4o10s0JGUYPHTULrcdTPoreM?testcase_id=6449486399537152 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Apr 12 2017
Thanks for the report. This is indeed related to my change. We've enabled the use of SwiftShader (OpenGL ES 2.0 implementation on the CPU) as a WebGL fallback on Linux (was already enabled on Windows). The threading code hasn't really changed in years, and we consistently pass the WebGL and dEQP conformance test suites. So to the best of my knowledge it's actually robust. That said, we implemented our own exponential back-off mutex lock, and it may rely on memory ordering guarantees offered by x86 but not necessarily other CPUs. Is it possible TSAN is detecting a false positive? I'll have a deeper look at the test case, but overall I'm not too worried since SwiftShader has been in use for several years in other projects.
,
Apr 13 2017
,
Apr 13 2017
Issue 710822 has been merged into this issue.
,
Apr 13 2017
Issue 710789 has been merged into this issue.
,
Apr 13 2017
Issue 710787 has been merged into this issue.
,
Apr 13 2017
As far as I can tell so far there could be a data race iff the code was running on a CPU with weak memory ordering. Specifically, data that was written by a first thread before our lock is acquired by a second thread could still have the old values when read by the second thread. A store-release of BackoffLock's "mutex" variable should fix the issue on CPUs with weak memory ordering. Since SwiftShader only supports x86 for now, and it has strong memory ordering, there shouldn't be any actual problem. What confuses me a bit is that TSAN can apparently detect a weak memory ordering issue on an architecture with strong memory ordering? Ironically the use of <atomic>, necessary to specify store-release semantics, is banned by Chromium: https://chromium-cpp.appspot.com/#blacklist_lib_list. But a quick code search reveals that it is in fact used in several places, so it's probably more of a precaution. Since x86 offers strong memory ordering, std::memory_order_release should have no effect anyway except for silencing TSAN.
,
Apr 15 2017
ClusterFuzz testcase 5361075651608576 is verified as fixed, so closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Apr 17 2017
,
Apr 17 2017
,
Apr 18 2017
Issue 711849 has been merged into this issue.
,
Apr 19 2017
The following revision refers to this bug: https://swiftshader.googlesource.com/SwiftShader.git/+/dc7759ccc3151a1aefefa9f86610e66f6fe9311c commit dc7759ccc3151a1aefefa9f86610e66f6fe9311c Author: Nicolas Capens <capn@google.com> Date: Wed Apr 19 15:30:28 2017 Fix potential data race in mutex lock implementation. Writing or reading an ordinary volatile boolean does not provide memory ordering guarantees. Use atomic<bool> with release and acquire semantics instead. On x86 this should have no impact on the generated code, since it architecturally offers strong memory ordering guarantees. Bug chromium:710753 Change-Id: I5aa2a6eeecfcd691a0aa8d06fd067f9d59ab13c3 Reviewed-on: https://swiftshader-review.googlesource.com/9328 Reviewed-by: Corentin Wallez <cwallez@google.com> Reviewed-by: Nicolas Capens <capn@google.com> Tested-by: Nicolas Capens <capn@google.com> [modify] https://crrev.com/dc7759ccc3151a1aefefa9f86610e66f6fe9311c/src/Common/MutexLock.hpp
,
Apr 19 2017
The following revision refers to this bug: https://swiftshader.googlesource.com/SwiftShader.git/+/dc7759ccc3151a1aefefa9f86610e66f6fe9311c commit dc7759ccc3151a1aefefa9f86610e66f6fe9311c Author: Nicolas Capens <capn@google.com> Date: Wed Apr 19 15:30:28 2017 Fix potential data race in mutex lock implementation. Writing or reading an ordinary volatile boolean does not provide memory ordering guarantees. Use atomic<bool> with release and acquire semantics instead. On x86 this should have no impact on the generated code, since it architecturally offers strong memory ordering guarantees. Bug chromium:710753 Change-Id: I5aa2a6eeecfcd691a0aa8d06fd067f9d59ab13c3 Reviewed-on: https://swiftshader-review.googlesource.com/9328 Reviewed-by: Corentin Wallez <cwallez@google.com> Reviewed-by: Nicolas Capens <capn@google.com> Tested-by: Nicolas Capens <capn@google.com> [modify] https://crrev.com/dc7759ccc3151a1aefefa9f86610e66f6fe9311c/src/Common/MutexLock.hpp
,
Apr 19 2017
The following revision refers to this bug: https://swiftshader.googlesource.com/SwiftShader.git/+/dc7759ccc3151a1aefefa9f86610e66f6fe9311c commit dc7759ccc3151a1aefefa9f86610e66f6fe9311c Author: Nicolas Capens <capn@google.com> Date: Wed Apr 19 15:30:28 2017 Fix potential data race in mutex lock implementation. Writing or reading an ordinary volatile boolean does not provide memory ordering guarantees. Use atomic<bool> with release and acquire semantics instead. On x86 this should have no impact on the generated code, since it architecturally offers strong memory ordering guarantees. Bug chromium:710753 Change-Id: I5aa2a6eeecfcd691a0aa8d06fd067f9d59ab13c3 Reviewed-on: https://swiftshader-review.googlesource.com/9328 Reviewed-by: Corentin Wallez <cwallez@google.com> Reviewed-by: Nicolas Capens <capn@google.com> Tested-by: Nicolas Capens <capn@google.com> [modify] https://crrev.com/dc7759ccc3151a1aefefa9f86610e66f6fe9311c/src/Common/MutexLock.hpp
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c9bd5f0202de1500d75c22654b2e22f0bc34312 commit 7c9bd5f0202de1500d75c22654b2e22f0bc34312 Author: capn <capn@chromium.org> Date: Wed Apr 19 17:28:33 2017 Roll SwiftShader 30385f0..dc7759c https://swiftshader.googlesource.com/SwiftShader.git/+log/30385f0..dc7759c - Fixes ThinLTO linking. - Fixes TSAN data race. BUG= chromium:686980 , chromium:710753 TBR=kbr@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2826133002 Cr-Commit-Position: refs/heads/master@{#465654} [modify] https://crrev.com/7c9bd5f0202de1500d75c22654b2e22f0bc34312/DEPS
,
Apr 25 2017
The following revision refers to this bug: https://swiftshader.googlesource.com/SwiftShader.git/+/35e0ba7753cab88905b428b65577669620798507 commit 35e0ba7753cab88905b428b65577669620798507 Author: Nicolas Capens <capn@google.com> Date: Tue Apr 25 17:04:23 2017 Use pthread mutexes on all Linux platforms. Bug chromium:710753 Change-Id: Ie74d86de027417579c9c6949aabad1c3ae89ebfc Reviewed-on: https://swiftshader-review.googlesource.com/9329 Reviewed-by: Corentin Wallez <cwallez@google.com> Reviewed-by: Nicolas Capens <capn@google.com> Tested-by: Nicolas Capens <capn@google.com> [modify] https://crrev.com/35e0ba7753cab88905b428b65577669620798507/src/Common/MutexLock.hpp
,
May 18 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/4c54802b8c32deb32093688a836a8c17cdfe8bc6 commit 4c54802b8c32deb32093688a836a8c17cdfe8bc6 Author: Nicolas Capens <capn@google.com> Date: Thu May 18 15:31:13 2017
,
Jun 14 2017
ClusterFuzz has detected this issue as fixed in range 479057:479065. Detailed report: https://clusterfuzz.com/testcase?key=6449486399537152 Fuzzer: inferno_layout_test_unmodified Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Data race READ 4 Crash Address: 0x7bb00000c70c Crash State: sw::Renderer::draw es2::Device::drawPrimitive es2::Context::drawArrays Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=463855:463874 Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479057:479065 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6449486399537152 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Sep 18 2017
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.
,
Aug 1
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by msrchandra@chromium.org
, Apr 12 2017Components: Internals>GPU>SwiftShader
Labels: Test-Predator-Wrong-CLs M-59
Owner: capn@chromium.org
Status: Assigned (was: Untriaged)