New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 795332 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression

Blocking:
issue 771365



Sign in to add a comment

gl_unittests failure on Linux Debug NVidia bot due to CFI-safe refactor

Project Member Reported by ericrk@chromium.org, Dec 15 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Dec 15 2017

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

commit 2fce2f6270fcbefb94fca2e227f5db192bbec7f0
Author: Eric Karl <ericrk@chromium.org>
Date: Fri Dec 15 18:15:51 2017

Revert "[cfi-icall] Refactor GL g_driver_* to be CFI-safe"

This reverts commit a71b54d85c18694c4c4ec34392f87c7d5c8a8f27.

Reason for revert: Breaking the Linux Debug NVidia bot. See  crbug.com/795332  for more details.

Original change's description:
> [cfi-icall] Refactor GL g_driver_* to be CFI-safe
> 
> Control Flow Integrity [1] indirect call (cfi-icall) checking can not
> verify that dynamically resolved function pointers call their intended
> function. To account for this, we place the function pointers in the
> various g_driver_* structs in ProtectedMemory, a wrapper for keeping
> variables in read-only memory except for when they are initialized.
> Once they are set they can not be tampered with since the memory is set
> back to read-only.
> 
> The generated bindings in gl_bindings_autogen_*.cc are not aware of the
> fact that their implementation is held in protected memory, so instead
> of using base::UnsanitizedCfiCall() we mark the individual function
> stubs with __attribute__((no_sanitize("cfi-icall"))) to disable icall
> checks on the generated bindings since their function pointer calls
> are routed through protected memory.
> 
> [1] https://www.chromium.org/developers/testing/control-flow-integrity
> 
> Bug: 771365
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I6bbf1b9a4122e1d00f6d2a365ae3730b4bb95a73
> Reviewed-on: https://chromium-review.googlesource.com/770252
> Commit-Queue: Peter Collingbourne <pcc@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Peter Collingbourne <pcc@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524318}

TBR=dcheng@chromium.org,dalecurtis@chromium.org,kbr@chromium.org,sandersd@chromium.org,pcc@chromium.org,vtsyrklevich@chromium.org

Change-Id: Ib4a1eae5e9f01f5e9ddbb1fcc5c66fc0b91f1015
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 771365,  795332 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/830173
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524411}
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/base/compiler_specific.h
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/gpu/command_buffer/service/passthrough_program_cache.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/gpu/command_buffer/service/texture_definition.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/media/gpu/dxva_video_decode_accelerator_win.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/tools/cfi/blacklist.txt
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/egl_api_unittest.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/generate_bindings.py
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings.h
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings_autogen_egl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings_autogen_gl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings_autogen_glx.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings_autogen_osmesa.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_bindings_autogen_wgl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_context_glx_unittest.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_context_wgl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_egl_api_implementation.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_fence.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_fence_egl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_glx_api_implementation.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_osmesa_api_implementation.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_surface_glx.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_surface_wgl.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/gl_wgl_api_implementation.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/glx_api_unittest.cc
[modify] https://crrev.com/2fce2f6270fcbefb94fca2e227f5db192bbec7f0/ui/gl/wgl_api_unittest.cc

Comment 2 by ericrk@chromium.org, Dec 15 2017

A bit more info. Errors from the builder look like:

[2/46] EGLInitializationDisplaysTest.NoExtensions (0 ms)
[ RUN      ] GLXApiTest.DisabledExtensionBitTest
[8126:8126:1214/205702.556506:5873041330:FATAL:protected_memory.h(189)] Check failed: ptr >= ProtectedMemoryStart && ptr_end <= ProtectedMemoryEnd. 
#0 0x7ff6976a69dd base::debug::StackTrace::StackTrace()
#1 0x7ff6976a4e1c base::debug::StackTrace::StackTrace()
#2 0x7ff69772beba logging::LogMessage::~LogMessage()
#3 0x00000030256e base::AutoWritableMemory::AutoWritableMemory()
#4 0x00000030a79a base::AutoWritableMemory::Create<>()
#5 0x00000030a42d gl::GLXApiTest::SetUp()
#6 0x000000383c1e testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#7 0x000000376462 testing::internal::HandleExceptionsInMethodIfSupported<>()
#8 0x000000360984 testing::Test::Run()
#9 0x00000036134d testing::TestInfo::Run()
#10 0x000000361dbc testing::TestCase::Run()
#11 0x00000036da7c testing::internal::UnitTestImpl::RunAllTests()
#12 0x000000383d2e testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#13 0x000000377b22 testing::internal::HandleExceptionsInMethodIfSupported<>()
#14 0x00000036d68e testing::UnitTest::Run()
#15 0x00000050d0e1 RUN_ALL_TESTS()
#16 0x00000050a0d2 base::TestSuite::Run()
#17 0x0000003112fd _ZN4base8internal13FunctorTraitsIMNS_9TestSuiteEFivEvE6InvokeIPN12_GLOBAL__N_111GlTestSuiteEJEEEiS4_OT_DpOT0_
#18 0x000000311244 _ZN4base8internal12InvokeHelperILb0EiE8MakeItSoIRKMNS_9TestSuiteEFivEJPN12_GLOBAL__N_111GlTestSuiteEEEEiOT_DpOT0_
#19 0x0000003111f5 _ZN4base8internal7InvokerINS0_9BindStateIMNS_9TestSuiteEFivEJNS0_17UnretainedWrapperIN12_GLOBAL__N_111GlTestSuiteEEEEEEFivEE7RunImplIRKS5_RKNSt3__15tupleIJS9_EEEJLm0EEEEiOT_OT0_NSG_16integer_sequenceImJXspT1_EEEE
#20 0x00000031113c _ZN4base8internal7InvokerINS0_9BindStateIMNS_9TestSuiteEFivEJNS0_17UnretainedWrapperIN12_GLOBAL__N_111GlTestSuiteEEEEEEFivEE3RunEPNS0_13BindStateBaseE
#21 0x00000051dcbd _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#22 0x00000051e847 base::(anonymous namespace)::LaunchUnitTestsInternal()
#23 0x00000051e6b5 base::LaunchUnitTests()
#24 0x000000310a90 main
#25 0x7ff696442f45 __libc_start_main
#26 0x0000002b302a _start
[3/46] GLXApiTest.DisabledExtensionBitTest (CRASHED)


Full error text here:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.gpu%2FLinux_Debug__NVIDIA_%2F87017%2F%2B%2Frecipes%2Fsteps%2Fgl_unittests_on_NVIDIA_GPU_on_Linux_on_Ubuntu%2F0%2Fstdout

Additional build failures can be seen here:
https://ci.chromium.org/buildbot/chromium.gpu/Linux%20Debug%20%28NVIDIA%29/87054
https://ci.chromium.org/buildbot/chromium.gpu/Linux%20Debug%20%28NVIDIA%29/87053
https://ci.chromium.org/buildbot/chromium.gpu/Linux%20Debug%20%28NVIDIA%29/87052

Full list (at time of posting):
https://ci.chromium.org/buildbot/chromium.gpu/Linux%20Debug%20%28NVIDIA%29/?limit=200

Comment 3 by kbr@chromium.org, Dec 15 2017

Blocking: 771365
Note: unfortunately we don't have the capacity to run the GPU tests on the debug trybots, but usually the release trybots, including linux_chromium_rel_ng, would catch these failures since they build with dcheck_always_on=true.

Per https://cs.chromium.org/chromium/src/base/memory/protected_memory.h?sq=package:chromium&l=28 it's not clear to me why this was only caught on the Debug bot. Could some of the bots be misconfigured and not running lld?

The gl_unittests failure on the Linux Intel 630 bot is similar too: https://ci.chromium.org/buildbot/chromium.gpu.fyi/Linux%20Release%20%28Intel%20HD%20630%29/1553
Cc: kcc@chromium.org
I've taken a look and this is occurring because of a peculiarity of how ProtectedMemory works with the component build and how the GL unittests are structured. With component builds every component has it's own separate protected memory section, so AutoWritableMemory::Create can only check against its own PM section. In this case, the unit tests under ui/gl/ are being compiled into gl_ or gpu_unittests but the writable memory section is actually under libgl_wrapper.so so this failure occurs. This happens because users of g_driver_* happen to know about its implementation as a ProtectedMemory container--the right way to implement this might be to hide that implementation so downstream users don't have to deal with ProtectedMemory at all, fixing this issue and simplifying the implementation. I'll take a look to see what that looks like.
Status: WontFix (was: Assigned)
Closing for now as this change will need to be rewritten entirely.

Sign in to add a comment