New issue
Advanced search Search tips

Issue 815537 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Threadsafe death tests fail on Android

Project Member Reported by pwnall@chromium.org, Feb 24 2018

Issue description

Google Test's default style for death tests has recently (01/2018) switched from "fast" to "threadsafe". Thread-safe death tests have been used internally at Google for a while, and are proven to be a better default. We should try to switch to them in Chrome.

Currently, running death tests on Android fails. Example error from base_unittests:

[ RUN      ] OutOfMemoryDeathTest.AlignedAlloc
../../base/process/memory_unittest.cc:202: Failure
Death test: { SetUpInDeathAssert(); value_ = base::AlignedAlloc(test_size_, 8); }
    Result: died but not with expected error.
  Expected: Out of memory
Actual msg:
[  DEATH   ] execve(_, ...) in / failed: No such file or directory
[  FAILED  ] OutOfMemoryDeathTest.AlignedAlloc (16 ms)

Example runs:
https://ci.chromium.org/buildbot/tryserver.chromium.android/android_n5x_swarming_rel/363675
https://ci.chromium.org/buildbot/tryserver.chromium.android/linux_android_rel_ng/491300

In order to avoid falling behind on Google Test updates, I've switched Android death tests back to "fast" via a #define that the Google Test maintainers have graciously agreed to support. This bug tracks switching from "fast" to "threadsafe" on Android.
 

Comment 1 by pwnall@chromium.org, Feb 24 2018

Summary: Threadsafe death tests fail on Android (was: Threadsafe death tests crash on Android)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/655a49d1a7385c14f9822a39673970a05b41a00c

commit 655a49d1a7385c14f9822a39673970a05b41a00c
Author: Manoj Gupta <manojgupta@google.com>
Date: Tue Mar 13 04:03:06 2018

llvm-next Update to r326829.

Update llvm-next to current crosstool version.
In addition, add a few cherry picks to fix issues
related to Linxu kernel:
1. PLT relocations: r327198.
2. Increased stack usage: r327192 and r327229.

Note: Does not affect the current llvm used in Chrome OS.

BUG= chromium:817628 
BUG= chromium:820140 
BUG=chromium:815537

TEST=USE="llvm-next" sudo emerge llvm lld works.

Change-Id: I436d578030775b3ffbccb246dbd43394452e67a3
Reviewed-on: https://chromium-review.googlesource.com/957790
Trybot-Ready: Manoj Gupta <manojgupta@chromium.org>
Commit-Queue: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[add] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-devel/llvm/files/cherry/8fdc88794b44e70bdb93c6cf04baf3c1e3251d8b.patch
[delete] https://crrev.com/bd99bff64464733038e8b5cdc488fd9f4941aea6/sys-devel/llvm/files/cherry/98079e294f718c14d25ccf30ab2b1938780ffe4d.patch
[rename] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-devel/lld/lld-6.0_pre321490-r3.ebuild
[delete] https://crrev.com/bd99bff64464733038e8b5cdc488fd9f4941aea6/sys-devel/llvm/files/cherry/30f9051d7d8b6f56c8149fd1bdcc714285f77527.patch
[delete] https://crrev.com/bd99bff64464733038e8b5cdc488fd9f4941aea6/sys-devel/llvm/files/cherry/16748767563bb9bcb1e1c3e42c35d44924d464d0.patch
[rename] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-devel/llvm/llvm-6.0_pre321490_p20180131-r9.ebuild
[add] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-devel/llvm/files/cherry/824eedb9eb4888575924b1ed80c4250dddd5b59b.patch
[delete] https://crrev.com/bd99bff64464733038e8b5cdc488fd9f4941aea6/sys-devel/llvm/files/cherry/284236c047631c8b0eabac3ddd3d0c95253f4361.patch
[rename] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-libs/compiler-rt/compiler-rt-6.0_pre321490-r5.ebuild
[delete] https://crrev.com/bd99bff64464733038e8b5cdc488fd9f4941aea6/sys-devel/llvm/files/cherry/ffaaef68dbab0e872c0e6013836170bb78705a81.patch
[add] https://crrev.com/655a49d1a7385c14f9822a39673970a05b41a00c/sys-devel/llvm/files/cherry/c28eb6d02c5cedd40b02aa7c496f13a71763312e.patch

FWIW, Bionic code appears to workaround this by supplying the full path. See 
AddPathSeparatorInTestProgramPath() in https://android.googlesource.com/platform/bionic/+/master/tests/gtest_main.cpp

I tried to port this into my own test main, but I only managed to produce a new run time error. 



Components: Infra>Client>Chrome
More context: I'm adding a death test must run in a multithreaded environment. I'm explicitly setting the "threadsafe" value in the test, because I otherwise observe a 1% timeout flake. 

This issue prevents me from running the test on Android. 
Seems many death tests actually aren't running on Android at all. Most tests use the macros from base/test/gtest_util.h. These explicitly do-nothing on android - that decision definitely predates this particular bug. 
https://cs.chromium.org/chromium/src/base/test/gtest_util.h?l=30&gsn=EXPECT_DCHECK_DEATH

This explains how existing tests that explicitly use the "threadsafe" flag seemingly pass on android. Example:
https://cs.chromium.org/chromium/src/base/task/post_task_unittest.cc?rcl=78ffbbca287d3a022e120cce116e497448eca518&l=190

Android is not excluded if you use the EXPECT_DEATH_IF_SUPPORTED macro from gtest.
https://cs.chromium.org/chromium/src/third_party/googletest/src/googletest/include/gtest/gtest-death-test.h?rcl=2e68926a9d4929e9289373cd49e40ddcb9a628f7&l=331

Sign in to add a comment