New issue
Advanced search Search tips

Issue 830017 link

Starred by 1 user
Status: Fixed
Owner:
pwnall@chromium.org
Closed: Apr 2018
Cc:
p...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Many test failures on Linux CFI bot

Project Member Reported by taku...@chromium.org, Apr 6 2018 
https://ci.chromium.org/buildbot/chromium.memory/Linux%20CFI/6983

Numerous tests failing with the below trace.
pwnall@, could crrev.com/c/987696 be the cause?

../../third_party/googletest/src/googlemock/include/gmock/gmock-generated-nice-strict.h:169:11: runtime error: control flow integrity check for type 'testing::NiceMock<(anonymous namespace)::MockAutofillAgent>' failed during base-to-derived cast (vtable address 0x000000e5c050)
0x000000e5c050: note: vtable is of type '(anonymous namespace)::MockAutofillAgent'
 00 00 00 00  20 9e 4f 0f 00 00 00 00  70 d9 fb 05 00 00 00 00  80 d9 fb 05 00 00 00 00  90 d9 fb 05
              ^
    #0 0x67b7c45 in testing::internal::NiceMockBase<(anonymous namespace)::MockAutofillAgent>::NiceMockBase() ./../../third_party/googletest/src/googlemock/include/gmock/gmock-generated-nice-strict.h:169:11
    #1 0x67b74a1 in testing::NiceMock<(anonymous namespace)::MockAutofillAgent>::NiceMock<content::RenderFrame*, autofill::TestPasswordAutofillAgent*&, autofill::TestPasswordGenerationAgent*&, service_manager::BinderRegistryWithArgs<>*>(content::RenderFrame*&&, autofill::TestPasswordAutofillAgent*&&&, autofill::TestPasswordGenerationAgent*&&&, service_manager::BinderRegistryWithArgs<>*&&) ./../../third_party/googletest/src/googlemock/include/gmock/gmock-generated-nice-strict.h:106:3
    #2 0x67b7359 in ChromeRenderViewTest::SetUp() ./../../chrome/test/base/chrome_render_view_test.cc:129:25
    #3 0x708bd8c in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2487:3
    #4 0x708c63c in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2667:11
    #5 0x708cdb1 in testing::TestCase::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2785:28
    #6 0x7093a92 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5047:43
    #7 0x709371d in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:4663:10
    #8 0x9af7053 in base::TestSuite::Run() ./../../base/test/test_suite.cc:275:16
    #9 0x998e5c1 in ChromeTestSuiteRunner::RunTestSuite(int, char**) ./../../chrome/test/base/chrome_test_launcher.cc:66:38
    #10 0xa27fd7d in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) ./../../content/public/test/test_launcher.cc:625:31
    #11 0x998e9cf in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) ./../../chrome/test/base/chrome_test_launcher.cc:171:10
    #12 0x998e53b in main ./../../chrome/test/base/browser_tests_main.cc:36:10
    #13 0x7f3cd2457f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0
    #14 0x5c4a029 in _start ??:0:0

Comment 1 by pwnall@chromium.org, Apr 6 2018 

I suspect this CL is what causes the bot failure. I haven't debugged CFI stuff before, so I have no idea how it works. I'll revert the CL, and I'd appreciate some pointers to docs on what & how to test before relanding.

Comment 2 by pwnall@chromium.org, Apr 6 2018

Components: Tests Internals
The revert CL is https://crrev.com/c/1000414

I am getting it through CQ because I landed another CL that impacts tests after it. The other CL passed CQ on its own, so reverting this one on its own should be fine, but I'd like to be sure I don't break all trees.

Comment 3 by taku...@chromium.org, Apr 6 2018

Cc: p...@chromium.org
Thanks. I also know nothing about CFI.
I found this
https://www.chromium.org/developers/testing/control-flow-integrity
and cc-ing pcc@ (from  issue 701937 ).

Comment 5 by p...@chromium.org, Apr 6 2018 

The corresponding internal change is cl/156157936

The CFI Linux bot enables various checks for errors involving casts and virtual calls. That change appears to introduce undefined behaviour, which triggers the CFI failure. I will leave a comment there.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 7 2018 

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

commit bd78d300314bbaa01ab0f4c3c2b1893bbcefaf50
Author: Victor Costan <pwnall@chromium.org>
Date: Sat Apr 07 02:02:05 2018

Revert "Roll src/third_party/googletest/src/ a325ad2db..82febb8ea (47 commits)"

This reverts commit 23f0c0829f997f16fe4bfdcd0ad7fdd388cf3895.

Reason for revert: Probably broke Linux CFI bot.  https://crbug.com/830017 

Original change's description:
> Roll src/third_party/googletest/src/ a325ad2db..82febb8ea (47 commits)
> 
> https://chromium.googlesource.com/external/github.com/google/googletest.git/+log/a325ad2db5de..82febb8eafc0
> 
> This CL also aliases <tr1/tuple> types used by googletest to C++11
> <tuple> types, to prepare for gradually transitioning the Chromium
> codebase from std::tr1::tuple to std::tuple.
> 
> $ git log a325ad2db..82febb8ea --date=short --no-merges --format='%ad %ae %s'
> 2018-04-05 misterg Merging gMock, 2
> 2018-04-05 misterg formatting
> 2018-04-05 misterg And more MCVS warnings
> 2018-04-05 misterg fixing MCVS warn
> 2018-04-05 misterg Have to wait for this one
> 2018-04-05 misterg Have to wait for this one
> 2018-04-05 misterg Merging matchers test
> 2018-04-05 misterg Merging matchers test
> 2018-04-04 misterg bad cut/paste
> 2018-04-04 misterg More on MSVC warning C4503, decorated name length exceeded
> 2018-04-04 misterg More on MSVC warning C4503, decorated name length exceeded
> 2018-04-04 misterg Address MSVC warning C4503, decorated name length exceeded, name was truncated
> 2018-04-04 misterg Fixing build break on MSVC
> 2018-04-04 misterg merging gmock matchers 1
> 2018-04-04 fo40225 fix build break on locale windows
> 2018-04-04 misterg Tweaking https://github.com/google/googletest/pull/1523 to exclude nacl
> 2018-04-03 misterg Upstreaming, cl 191344765
> 2018-04-03 misterg merging port, cont. 191443078
> 2018-04-03 misterg merging, cont - 2
> 2018-04-03 misterg merging gtest-port.h , 191439094
> 2018-04-03 misterg merging, just comments format
> 2018-04-03 misterg testing, merge
> 2018-04-03 misterg Testing, gtest-port.h merge
> 2018-04-02 misterg merging gtest-port.h, again - 1
> 2018-03-29 gennadiycivil Include OSX builds back into PR builds
> 2018-03-29 gennadiycivil Revert "merging gtest-port 1 of N"
> 2018-03-29 gennadiycivil Revert "merging gtest-port, 2"
> 2018-03-29 leissa typo
> 2018-03-29 misterg merging gtest-port, 2
> 2018-03-28 leissa provide alternative for DebugBreak()
> 2018-03-27 misterg merging gtest-port 1 of N
> 2018-03-26 misterg merges 1
> 2018-03-26 misterg merges, gtest
> 2018-03-26 misterg merging gmock-matchers.h 3
> 2018-03-26 misterg merging gmock-matchers.h 2
> 2018-03-26 misterg Upstreaming FloatingEq2Matcher,
> 2018-03-23 misterg Merging gmock-matchers.h -2
> 2018-03-23 misterg gmock-matchers merging -2
> 2018-03-22 misterg merging, gmock -1
> 2018-03-22 misterg reverting gtest_list_tests_unittest.py
> 2018-03-22 gennadiycivil Update appveyor.yml
> 2018-03-22 misterg more merges
> 2018-03-22 misterg more merges
> 2018-03-21 misterg More merges
> 2018-03-16 misterg cl 189032107, again
> 2018-03-16 misterg cl 189032107
> 2018-03-15 misterg merge, again, IsRecursiveContainer
> 
> Created with:
>   roll-dep src/third_party/googletest/src
> 
> Bug:  829773 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.win:win_chrome_official
> Change-Id: Ic865698379d3626a5165f81b56104223ad1558d9
> Reviewed-on: https://chromium-review.googlesource.com/987696
> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548853}

TBR=thakis@chromium.org,chcunningham@chromium.org,pwnall@chromium.org

Bug:  830017 ,  829773 
Change-Id: I25807b133014e7816f6790080fce964aaaa65168
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.win:win_chrome_official
Reviewed-on: https://chromium-review.googlesource.com/1000414
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549014}
[modify] https://crrev.com/bd78d300314bbaa01ab0f4c3c2b1893bbcefaf50/DEPS
[modify] https://crrev.com/bd78d300314bbaa01ab0f4c3c2b1893bbcefaf50/media/filters/ffmpeg_glue_unittest.cc
[modify] https://crrev.com/bd78d300314bbaa01ab0f4c3c2b1893bbcefaf50/third_party/googletest/BUILD.gn
[delete] https://crrev.com/736e26fa2b93318635af46024207ffe55b1f2390/third_party/googletest/custom/gtest/internal/custom/gtest-port.h

Comment 7 by shimazu@chromium.org, Apr 9 2018

Status: Fixed (was: Assigned)
As far as I can see https://ci.chromium.org/buildbot/chromium.memory/Linux%20CFI/ , it gets green after https://chromium-review.googlesource.com/#/c/1000895/.
I think this can be marked as fixed.

Comment 8 by pwnall@chromium.org, Apr 9 2018

Labels: -Pri-1 -Sheriff-Chromium Pri-2
Status: Assigned (was: Fixed)
I removed the Chromium sheriff label from the list. However, the bug is not fixed -- googletest still has undefined behavior, and we won't be able to do another roll until we fix it.

Keeping this bug assigned to me until I get a fix in googletest that allows us to roll past commit fe402c27790ff1cc9a7e17c5d0aea4ebe7fd8a71.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2018 

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

commit 1e0821894f595b201b4d58e540404da7d09a0214
Author: Victor Costan <pwnall@chromium.org>
Date: Thu Apr 12 21:06:47 2018

Roll src/third_party/googletest/src/ 7e5f90d37..b640d8743 (50 commits)

This roll includes 1324e2d706d739217cceae361259a5cc01d1ff41 which fixes
the undefiend behavior breaking the Linux CFI bot introduced in
fe402c27790ff1cc9a7e17c5d0aea4ebe7fd8a71.

https://chromium.googlesource.com/external/github.com/google/googletest.git/+log/7e5f90d3780d..b640d8743d85

$ git log 7e5f90d37..b640d8743 --date=short --no-merges --format='%ad %ae %s'
2018-04-09 costan Remove multiple inheritance from "unintesting call" mock classes.
2018-04-11 misterg ...merging
2018-04-11 misterg merging
2018-04-11 misterg Upstream cl/192179348
2018-04-11 misterg merging
2018-04-11 misterg ..and this should be it
2018-04-11 misterg more
2018-04-11 misterg pizza work, cont
2018-04-11 misterg osx pizzas, cont
2018-04-11 misterg fixing osx pizza
2018-04-10 misterg merging
2018-04-10 misterg merging
2018-04-10 misterg merging
2018-04-10 misterg RE-Doing the merge, this time with gcc on mac in the PR so I can catch errors before merging the PR
2018-04-10 gennadiycivil Include gcc on mac into PR matrix
2018-04-09 gennadiycivil Revert "gmock actions 2"
2018-04-09 misterg ... and this
2018-04-09 misterg this should be it
2018-04-09 misterg yet more
2018-04-09 misterg formatting
2018-04-09 misterg tuning
2018-04-09 misterg tuning
2018-04-09 misterg more
2018-04-09 misterg cont
2018-04-09 misterg msvc
2018-04-09 misterg more msvc
2018-04-09 misterg msvc 14
2018-04-09 misterg testing msvc again
2018-04-09 misterg More msvc 14
2018-04-09 misterg And also silence for MSVS14
2018-04-09 misterg preproc syntax ( I can never remember it)
2018-04-09 misterg syntax
2018-04-09 misterg cont.
2018-04-09 misterg continued
2018-04-06 costan Sync gmock-generated-nice-strict.h.pump with gmock-generated-nice-strict.h.
2018-04-06 misterg more mcvs fixing
2018-04-06 misterg linkage, fixing MSVC
2018-04-06 misterg fixing MSVC
2018-04-06 misterg more warnings
2018-04-06 misterg more warnings
2018-04-06 misterg more MSVC warnings
2018-04-06 misterg warnings
2018-04-06 misterg cont - 2
2018-04-06 misterg cont
2018-04-06 misterg more warnings
2018-04-06 misterg deal with MSVC warn, cont 1
2018-04-06 misterg Cont. deal with MCVS warnings
2018-04-06 misterg Deal with MCVS warnings
2018-04-06 misterg merging gmock-actions 2
2018-04-05 misterg Merging gMock, 2

Created with:
  roll-dep src/third_party/googletest/src

Bug:  830017 ,  829773 
Change-Id: I2a20f5e12c1b0475e15c98251f9c3100247e14b9
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.win:win_chrome_official;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1004440
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550359}
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/DEPS
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/media/filters/ffmpeg_glue_unittest.cc
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/third_party/googletest/BUILD.gn

Comment 10 by pwnall@chromium.org, Apr 17 2018

Status: Fixed (was: Assigned)
The fix seems to have settled
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e0821894f595b201b4d58e540404da7d09a0214

commit 1e0821894f595b201b4d58e540404da7d09a0214
Author: Victor Costan <pwnall@chromium.org>
Date: Thu Apr 12 21:06:47 2018

Roll src/third_party/googletest/src/ 7e5f90d37..b640d8743 (50 commits)

This roll includes 1324e2d706d739217cceae361259a5cc01d1ff41 which fixes
the undefiend behavior breaking the Linux CFI bot introduced in
fe402c27790ff1cc9a7e17c5d0aea4ebe7fd8a71.

https://chromium.googlesource.com/external/github.com/google/googletest.git/+log/7e5f90d3780d..b640d8743d85

$ git log 7e5f90d37..b640d8743 --date=short --no-merges --format='%ad %ae %s'
2018-04-09 costan Remove multiple inheritance from "unintesting call" mock classes.
2018-04-11 misterg ...merging
2018-04-11 misterg merging
2018-04-11 misterg Upstream cl/192179348
2018-04-11 misterg merging
2018-04-11 misterg ..and this should be it
2018-04-11 misterg more
2018-04-11 misterg pizza work, cont
2018-04-11 misterg osx pizzas, cont
2018-04-11 misterg fixing osx pizza
2018-04-10 misterg merging
2018-04-10 misterg merging
2018-04-10 misterg merging
2018-04-10 misterg RE-Doing the merge, this time with gcc on mac in the PR so I can catch errors before merging the PR
2018-04-10 gennadiycivil Include gcc on mac into PR matrix
2018-04-09 gennadiycivil Revert "gmock actions 2"
2018-04-09 misterg ... and this
2018-04-09 misterg this should be it
2018-04-09 misterg yet more
2018-04-09 misterg formatting
2018-04-09 misterg tuning
2018-04-09 misterg tuning
2018-04-09 misterg more
2018-04-09 misterg cont
2018-04-09 misterg msvc
2018-04-09 misterg more msvc
2018-04-09 misterg msvc 14
2018-04-09 misterg testing msvc again
2018-04-09 misterg More msvc 14
2018-04-09 misterg And also silence for MSVS14
2018-04-09 misterg preproc syntax ( I can never remember it)
2018-04-09 misterg syntax
2018-04-09 misterg cont.
2018-04-09 misterg continued
2018-04-06 costan Sync gmock-generated-nice-strict.h.pump with gmock-generated-nice-strict.h.
2018-04-06 misterg more mcvs fixing
2018-04-06 misterg linkage, fixing MSVC
2018-04-06 misterg fixing MSVC
2018-04-06 misterg more warnings
2018-04-06 misterg more warnings
2018-04-06 misterg more MSVC warnings
2018-04-06 misterg warnings
2018-04-06 misterg cont - 2
2018-04-06 misterg cont
2018-04-06 misterg more warnings
2018-04-06 misterg deal with MSVC warn, cont 1
2018-04-06 misterg Cont. deal with MCVS warnings
2018-04-06 misterg Deal with MCVS warnings
2018-04-06 misterg merging gmock-actions 2
2018-04-05 misterg Merging gMock, 2

Created with:
  roll-dep src/third_party/googletest/src

Bug:  830017 ,  829773 
Change-Id: I2a20f5e12c1b0475e15c98251f9c3100247e14b9
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.win:win_chrome_official;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1004440
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550359}
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/DEPS
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/media/filters/ffmpeg_glue_unittest.cc
[modify] https://crrev.com/1e0821894f595b201b4d58e540404da7d09a0214/third_party/googletest/BUILD.gn
►

Sign in to add a comment