GrConfigConversionEffect::TestForPreservingPMConversions() causing MSAN Failure in unit tests |
|||
Issue description
Chrome Version: 60
OS: Linux, Win10
WebKit Linux Trusty MSAN fails with:
CanvasRenderingContext2DTest.GetImageDataDisablesAcceleration
==17429==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x9a21142 in GrConfigConversionEffect::TestForPreservingPMConversions(GrContext*) third_party/skia/src/gpu/effects/GrConfigConversionEffect.cpp:188:17
#1 0x984332f in validPMUPMConversionExists third_party/skia/src/gpu/GrContext.cpp:860:38
#2 0x984332f in GrContextPriv::readSurfacePixels(GrSurfaceContext*, int, int, int, int, GrPixelConfig, SkColorSpace*, void*, unsigned long, unsigned int) third_party/skia/src/gpu/GrContext.cpp:461:0
#3 0x98f57ba in GrSurfaceContext::readPixels(SkImageInfo const&, void*, unsigned long, int, int, unsigned int) third_party/skia/src/gpu/GrSurfaceContext.cpp:49:36
#4 0x9c95999 in SkImage_Gpu::onReadPixels(SkImageInfo const&, void*, unsigned long, int, int, SkImage::CachingHint) const third_party/skia/src/image/SkImage_Gpu.cpp:215:20
#5 0xc1feeb8 in blink::ImageBuffer::GetImageData(blink::Multiply, blink::IntRect const&, WTF::ArrayBufferContents&) const third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp:404:13
#6 0x10fb0f2e in blink::BaseRenderingContext2D::getImageData(int, int, int, int, blink::ExceptionState&) third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1635:16
#7 0x418b3ce in blink::CanvasRenderingContext2DTest_GetImageDataDisablesAcceleration_Test::TestBody() third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:940:18
#8 0x846115b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2455:12
#9 0x846115b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2471:0
#10 0x846487a in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2653:11
#11 0x8466279 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2771:28
#12 0x8486a73 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4648:43
#13 0x84858f1 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2455:12
#14 0x84858f1 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4256:0
#15 0x913c2a5 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
#16 0x913c2a5 in base::TestSuite::Run() base/test/test_suite.cc:271:0
#17 0xc0a2f0 in (anonymous namespace)::runHelper(base::TestSuite*) third_party/WebKit/Source/web/tests/RunAllTests.cpp:48:27
#18 0x9142235 in Run base/callback.h:80:12
#19 0x9142235 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, int, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:216:0
#20 0x9141a66 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:458:10
#21 0xc0a117 in main third_party/WebKit/Source/web/tests/RunAllTests.cpp:70:10
#22 0x7fe5307c8f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287:0
#23 0x76009f in _start ??:0:0
,
Jun 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c16bed3a00e56c1ebf2c4e83d703b761951ef9b commit 2c16bed3a00e56c1ebf2c4e83d703b761951ef9b Author: huangs <huangs@google.com> Date: Sun Jun 11 18:32:06 2017 [Sheriffing] Disable CanvasRenderingContext2DTest.GetImageDataDisablesAcceleration. TBR=junov@chromium.org Bug: 732140 Change-Id: I95acb83bf7e2d62fa559c618b8e32a893c19bd69 Reviewed-on: https://chromium-review.googlesource.com/530467 Commit-Queue: Samuel Huang <huangs@chromium.org> Reviewed-by: Samuel Huang <huangs@chromium.org> Cr-Commit-Position: refs/heads/master@{#478535} [modify] https://crrev.com/2c16bed3a00e56c1ebf2c4e83d703b761951ef9b/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
,
Jun 15 2017
This problem stems form the fact that the unit test sets up a fake graphics context, so readPixels is just a stub that leaves pixels uninitialized. I think GrConfigConversionEffect::TestForPreservingPMConversions() should detect that GrGLTestInterface is being used and just return false in that case, without performing the test. Or perhaps GrGLCreateNullInterface.cpp should override readPixels with an implementation that fills the destination buffer with zeroes.
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34394dc6fb5ce9a6c351589deff9236fc0564d65 commit 34394dc6fb5ce9a6c351589deff9236fc0564d65 Author: Justin Novosad <junov@chromium.org> Date: Fri Jun 16 01:48:26 2017 Disable CanvasRenderingContext2DTest.GetImageDataDisablesAcceleration only for MSAN Replace earlier test suppression with a more specific suppression, so that we continue to get coverage from this test. TBR=huangs@chromium.org Bug: 732140 Change-Id: I2df3e97a1bc919e5effe845a179d81c947d2900f Reviewed-on: https://chromium-review.googlesource.com/537854 Reviewed-by: Justin Novosad <junov@chromium.org> Commit-Queue: Justin Novosad <junov@chromium.org> Cr-Commit-Position: refs/heads/master@{#479894} [modify] https://crrev.com/34394dc6fb5ce9a6c351589deff9236fc0564d65/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
,
Jun 22 2017
Skia would like to remove the null interface. Is there a mock command buffer implementation that could be used instead?
,
Jul 25
|
|||
►
Sign in to add a comment |
|||
Comment 1 by hua...@chromium.org
, Jun 11 2017MSAN claim that in GrConfigConversionEffect::TestForPreservingPMConversions(), if (firstRead[kSize * y + x] != secondRead[kSize * y + x]) { uses uninitialized value. To diagnose this, in I filled srcData with 0xDEADBEEF, and verified that indeed the firstRead[] and secondRead[] are unaltered by the test. Further tracing suggests that down the line, GrGLTestInterface::readPixels() gets called, which is a NO-OP. So the test really just compares uninit value with uninit values, and not very effective. Test system claims that culprit is somewhere in: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20MSAN/builds/1280 However, it seems the problem of test occurred before this. I'm going to disable the test to remove noise.