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

Issue 681290 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Crash in base::debug::DebugBreak

Project Member Reported by ClusterFuzz, Jan 14 2017

Issue description

Cc: bcwh...@chromium.org nyerramilli@chromium.org
Labels: Test-Predator-Wrong-CLs M-57
Predator and CL did not provide any possible suspects.

requesting //src/base/debug/OWNERS  - bcwhite@ could you please check and help.
Cc: -bcwh...@chromium.org
Owner: infe...@chromium.org
DebugBreak called by StaticLocalVerifier in third_party/WebKit/Source/wtf/, which was called by LLVMFuzzerInitialize in libFuzzer.  Assigning to owner of libFuzzer.

Comment 3 by aarya@google.com, Jan 19 2017

Owner: lukasza@chromium.org
Status: Assigned (was: Untriaged)
lukasza@chromium.org is the author of this fuzzer.
Status: Started (was: Assigned)
I am guessing that that the call to WTF::Partitions::initialize(nullptr) should happen earlier in the constructor of ScopedUnittestsEnvironmentSetup.
REPRO (at c9e8aae171ee):

$ cat out/libfuzzer/args.gn 
use_libfuzzer = true
is_asan = true
is_debug = true
enable_nacl = false
use_goma = true

$ ninja -C out/libfuzzer mhtml_parser_fuzzer
...

$ out/libfuzzer/mhtml_parser_fuzzer 
[0119/102451.828408:FATAL:Partitions.h(58)] Check failed: s_initialized. 
#0 0x00000046f401 __interceptor_backtrace
#1 0x7feddb842a4a base::debug::StackTrace::StackTrace()
#2 0x7feddb9ea140 logging::LogMessage::~LogMessage()
#3 0x7fedcf70eb78 WTF::Partitions::fastMallocPartition()
#4 0x7fedcf7107c6 WTF::Partitions::fastMalloc()
#5 0x7fedcf73c3fe WTF::ThreadSpecific<>::operator new()
#6 0x7fedcf73becd WTF::wtfThreadData()
#7 0x7fedcf739fea WTF::currentThread()
#8 0x0000004fab56 StaticLocalVerifier::StaticLocalVerifier()
#9 0x0000004fa0fa LLVMFuzzerInitialize
#10 0x000000517407 fuzzer::FuzzerDriver()
#11 0x00000059f99f main
#12 0x7fedc5d2af45 __libc_start_main
#13 0x000000423b85 <unknown>

Cc: csharrison@chromium.org
FWIW, this regression bisects to 6deb801a3e4c... Use TLS storage for thread ids in wtf/ThreadingPThreads
Hm, does this repro if your fuzzer uses blink::InitializeBlinkFuzzTest?

The problem is that after my patch, DEFINE_STATIC_LOCAL cannot be used until threading is initialized.

Another option is to change the calls to currentThread() in the static local verifier to use internal::currentThreadSyscall() which uses the old (slow) behavior.
lukasza: let me know if you would like to pass ownership, as I am certainly to blame for this one.
csharisson@ - thanks for the offer.

I didn't previously know about blink::InitializeBlinkFuzzTest from r415634 and now that I learned about this function, I am starting to dislike blink::ScopedUnittestsEnvironmentSetup that I've introduced in r412335.  Maybe ScopedUnittestsEnvironmentSetup should be deleted and initialization code put back into RunAllTests.cpp (given that InitializeBlinkFuzzTest is called from multiple fuzzers and ScopedUnittestsEnvironmentSetup is only called from the single mhtml fuzzer).

Alternatively, maybe RunAllTests.cpp and fuzzers can still share the initialization code (maybe blink::InitializeBlinkFuzzTest should be changed to just create a leaked ScopedUnittestsEnvironmentSetup instance)?

WDYT?
Components: Blink>Internals
I'm not sure, it's possible that InitializeBlinkFuzzTest is too heavyweight for platform unit tests. I don't know if there is any other reason why it doesn't use content::SetUpBlinkTestEnvironment.
Cc: dcheng@chromium.org haraken@chromium.org tasak@chromium.org alexclarke@chromium.org
Let me try adding an arbitrarily picked subset of people who have touched TestingPlatformSupport.cpp and RunAllTests.cpp (to see if they have any feedback on structuring test initialization for fuzzers and unittests).

I think the easiest (and probably reasonable) way forward would be to:

1) Switch mhtml_parser_fuzzer to call InitializeBlinkFuzzTest (instead of instantiating ScopedUnittestsEnvironmentSetup in a DEFINE_STATIC_LOCAL).

2) Keep RunAllTests.cpp using ScopedUnittestsEnvironmentSetup (even though it is potentially slightly overcomplicated / overarchitected at this point, since RunAllTests.cpp is the only user).

FWIW, #1 seems to work just fine in https://crrev.com/2645043003.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 23 2017

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

commit 608d39b1e2e2666c2a37512452be51309709362e
Author: lukasza <lukasza@chromium.org>
Date: Mon Jan 23 19:45:53 2017

Use InitializeBlinkFuzzTest instead of ScopedUnittestsEnvironmentSetup.

Since r443701, DEFINE_STATIC_LOCAL cannot be used until threading is
initialized.  This CL stops using DEFINE_STATIC_LOCAL and
ScopedUnittestsEnvironmentSetup from MHTMLFuzzer.cpp and instead uses
the initialization helper used by all the other fuzzers -
InitializeBlinkFuzzTest.

BUG= 681290 

Review-Url: https://codereview.chromium.org/2645043003
Cr-Commit-Position: refs/heads/master@{#445438}

[modify] https://crrev.com/608d39b1e2e2666c2a37512452be51309709362e/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/608d39b1e2e2666c2a37512452be51309709362e/third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 23 2017

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

commit af533915964a542005b59457bbf27fcd20972590
Author: csharrison <csharrison@chromium.org>
Date: Mon Jan 23 20:53:28 2017

Ensure DEFINE_STATIC_LOCAL can be used before wtf threading is initialized

This patch just makes StaticLocalVerifier make thread id syscalls rather
than using TLS lookups.

BUG= 681290 

Review-Url: https://codereview.chromium.org/2652723002
Cr-Commit-Position: refs/heads/master@{#445479}

[modify] https://crrev.com/af533915964a542005b59457bbf27fcd20972590/third_party/WebKit/Source/wtf/StdLibExtras.h
[modify] https://crrev.com/af533915964a542005b59457bbf27fcd20972590/third_party/WebKit/Source/wtf/Threading.h

Project Member

Comment 14 by ClusterFuzz, Jan 24 2017

ClusterFuzz has detected this issue as fixed in range 445411:445545.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5924545941995520

Fuzzer: libfuzzer_mhtml_parser_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x03e900001902
Crash State:
  base::debug::DebugBreak
  StaticLocalVerifier::StaticLocalVerifier
  LLVMFuzzerInitialize
  
Sanitizer: address (ASAN)

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=443630:443718
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan_debug&range=445411:445545

Minimized Testcase (0.00 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97Hu6AeGU8C5k1KgDcbBNXmxVUSsJc-_5GXtv6i9n_nz2VCOJHWpF54D6ISBLx5wUuhT2n44Hy76EWhZgzUDpN0pqpMVHcEA9bBCcvH0l_CLhIS00jIjh4ot20Xi_lElBbUCVawi8K8vVwpPa4IqgEb_l5-B8RQ2TTUgJO6ZsjHKSi27dm2pFvDsW1UjLihg3EOO7snEifpG7NLculCvqEhStZ3zLnzxmKJTrQ18tLJVnRpBGDlterPzT_mwTI39Kx-EYn6g4q3egqFZ8lB3fMaIIauT5IfeP8CFMXS3Ifr6H2Ys8FfMljMO1OsdkiWQGTPlit0FOYdDP2w9gWaygiZxhfweHTa8Ic0Ga83r4zTw_hbwzI?testcase_id=5924545941995520

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 15 by ClusterFuzz, Jan 24 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 5924545941995520 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment