New issue
Advanced search Search tips

Issue 860533 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 848035



Sign in to add a comment

MSAN doesn't work with SwiftShader's JIT compiled code

Project Member Reported by sugoi@chromium.org, Jul 5

Issue description

Chromium's memory sanitizer doesn't work with JIT compiled code, so
we have to skip blitReactor when using MSAN, otherwise most uses of
glReadPixels() will cause MSAN to detect a false use-of-uninitialized-value,
since it doesn't instrument the inside of JIT compiled code.

This causes multiple failures when running cc_unittests with SwiftShader if MSAN is enabled and potentially other failures in other tests.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/551478a9b8be146edaf4bcc7639ed77752d1650f

commit 551478a9b8be146edaf4bcc7639ed77752d1650f
Author: Alexis Hetu <sugoi@google.com>
Date: Fri Jul 06 11:53:55 2018

Fix for MSAN issues

Chromium's memory sanitizer doesn't work with JIT compiled code, so
we have to skip blitReactor when using MSAN, otherwise most uses of
glReadPixels() will cause MSAN to detect a false use-of-uninitialized-value,
since it doesn't instrument the inside of JIT compiled code.

 Bug chromium:848035   chromium:860533 

Change-Id: Idfa194ce0fcd41eb7acf7868cbcebfc04d598f5b
Reviewed-on: https://swiftshader-review.googlesource.com/19808
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

[modify] https://crrev.com/551478a9b8be146edaf4bcc7639ed77752d1650f/src/Renderer/Blitter.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/871c1b8610aabbb722b0bca64b5d6c8f68ef4f26

commit 871c1b8610aabbb722b0bca64b5d6c8f68ef4f26
Author: Alexis Hetu <sugoi@google.com>
Date: Fri Jul 06 15:04:53 2018

Roll SwiftShader e1fa9ea..551478a

https://swiftshader.googlesource.com/SwiftShader.git/+log/e1fa9ea..551478a

BUG= chromium:848035   chromium:860533 

TBR=kbr@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng;luci.chromium.try:android_optional_gpu_tests_rel

Change-Id: I0eadf45001e70a37f14e4b2c4a4bbabfedcbfb2a
Reviewed-on: https://chromium-review.googlesource.com/1127843
Commit-Queue: Alexis Hétu <sugoi@chromium.org>
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572974}
[modify] https://crrev.com/871c1b8610aabbb722b0bca64b5d6c8f68ef4f26/DEPS

Cc: -capn@chromium.org sugoi@chromium.org
Components: Internals>GPU>SwiftShader
Owner: capn@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/3639df1403505d385cd6bdd6f8ccdf5926b4efa5

commit 3639df1403505d385cd6bdd6f8ccdf5926b4efa5
Author: Nicolas Capens <nicolas.capens@gmail.com>
Date: Fri Jul 06 20:55:53 2018

Support 64-bit call targets.

The x86-64 call instruction only supports 32-bit IP-relative direct
calls or 64-bit indirect calls. So handle 64-bit direct calls by
storing the value into a register and making an indirect call.

 Bug chromium:860533 

Change-Id: I3781d1e8a489ce9ab7c17b098ffe830cae62477e
Reviewed-on: https://swiftshader-review.googlesource.com/19828
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

[modify] https://crrev.com/3639df1403505d385cd6bdd6f8ccdf5926b4efa5/third_party/subzero/src/IceTargetLoweringX8664.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/6a990f8b1a930336eee5b2af2319fae11b967d4e

commit 6a990f8b1a930336eee5b2af2319fae11b967d4e
Author: Nicolas Capens <capn@google.com>
Date: Fri Jul 06 20:55:53 2018

Support MSAN with Subzero JIT compiled code.

MemorySanitizer doesn't automatically work with dynamically generated
code or inline assembly, since it can't instrument it to know what
memory it touches. We can help it by marking all the memory that is
written to by Reactor with the Subzero back-end as initialized by
calling __msan_unpoison.

Note that writes to memory don't guarantee proper initialization. It
could be copying or writing other uninitialized values.

See also https://sites.google.com/a/chromium.org/dev/developers/testing/memorysanitizer

 Bug chromium:860533 

Change-Id: Idf64e43c6ab9b8f71f64723fc7e3653f6ea2fb30
Reviewed-on: https://swiftshader-review.googlesource.com/19789
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

[modify] https://crrev.com/6a990f8b1a930336eee5b2af2319fae11b967d4e/src/Common/Memory.cpp
[modify] https://crrev.com/6a990f8b1a930336eee5b2af2319fae11b967d4e/src/Reactor/SubzeroReactor.cpp
[modify] https://crrev.com/6a990f8b1a930336eee5b2af2319fae11b967d4e/src/Renderer/Blitter.cpp

Blocking: 848035
Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b9a275b081bf97a3b4cf06bfd2f73fbe7d69020

commit 9b9a275b081bf97a3b4cf06bfd2f73fbe7d69020
Author: Nicolas Capens <capn@chromium.org>
Date: Mon Jul 09 13:44:03 2018

Roll SwiftShader 551478a..6a990f8

https://swiftshader.googlesource.com/SwiftShader.git/+log/551478a..6a990f8

BUG= chromium:833229 ,  chromium:824522 ,  chromium:860533 

TBR=kbr@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng;luci.chromium.try:android_optional_gpu_tests_rel

Change-Id: I6ed8a81614a509c703f47be4f8fdb74168381010
Reviewed-on: https://chromium-review.googlesource.com/1128266
Commit-Queue: Nicolas Capens <capn@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573290}
[modify] https://crrev.com/9b9a275b081bf97a3b4cf06bfd2f73fbe7d69020/DEPS

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/6a6692214c26bc135841ead1cf9170d48bd1ed02

commit 6a6692214c26bc135841ead1cf9170d48bd1ed02
Author: Nicolas Capens <capn@google.com>
Date: Mon Jul 09 14:48:14 2018

Remove invalid assert.

Subzero x86-64 call instruction operands can be 64-bit now and require
a register.

 Bug chromium:860533 

Change-Id: I4446be34f57ef73e24718252210bdaa81995c8be
Reviewed-on: https://swiftshader-review.googlesource.com/19848
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Corentin Wallez <cwallez@google.com>

[modify] https://crrev.com/6a6692214c26bc135841ead1cf9170d48bd1ed02/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h

Sign in to add a comment