Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/b49206ded97c4eaac7c273ce004d840a0185d40e (ThreadDataTable: Change global linked list to per-Isolate hash map.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Not sure what I'm supposed to do here. The "reproducer testcase" is an empty file. How can I reproduce?

Stab in the dark: Could this be because some code, somewhere, makes assumptions about the offsets of member variables of Isolate, which changed because I added new fields? In particular, I believe `base::Mutex thread_data_table_mutex_` is now the first field in the class, whereas previously `base::Atomic32 id_` was the first field. Does something somewhere assume that an isolate pointer can be reinterpret_casted as base::Atomic32 to directly access id_? Could that code possibly then write to said pointer, which would corrupt the mutex, leading to the crash seen here?

This seems outlandish but I'm having trouble understanding why else locking this mutex would crash.

The reason my change added new members to the front of the class is because they were previously declared static, and I simply removed the "static" keyword without moving the declaration. It turns out these declarations appear before all other member fields. Perhaps thread_data_table_mutex_ and thread_data_table_ should be moved to the end of the class?

If my silly theory above is correct then this will fix it: https://chromium-review.googlesource.com/c/v8/v8/+/1036478

(If not, then at least it makes the code cleaner...)

Michael and Michael, do you know how we could reproduce this failure?

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5a9b1d5bc8c51357505f1d613c1b0d3b0ecc88c2

commit 5a9b1d5bc8c51357505f1d613c1b0d3b0ecc88c2
Author: Kenton Varda <kenton@cloudflare.com>
Date: Tue May 01 18:53:00 2018

Cleanup: Move thread_data_table_ to end of Isolate class.

In b49206ded9 I changed thread_data_table_ and thread_data_table_mutex_ from
static members to regular class member variables. To do this, I only deleted
the `static` keyword and left the declarations where they were. This was a
little odd in that all of the dynamic class members are declared together in
one place, but now these two new members weren't next to the rest. Making it
a little bit weirder is the fact that these two new members actually ended up
being the first members of the class, since the exsiting dynamic members were
declared later.

This change merely moves these two members down to the end of the dynamic
member variable list, where they probably should have gone.

Bug:  chromium:837477 

Change-Id: If993935cc56c8026bb7331493ed657c42ba06ac7
Reviewed-on: https://chromium-review.googlesource.com/1036478
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52902}
[modify] https://crrev.com/5a9b1d5bc8c51357505f1d613c1b0d3b0ecc88c2/src/isolate.h

@clusterfuzz folks: Why's there no repro file downloadable? The test seems to run a file 15403aada2bf54d76923bc724dc5f089c5a700d096d814a92808a722aefuzz-0

*** Bulk Edit ***
M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. 

If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.

How do I tell ClusterFuzz to retry? It hasn't tested since before yesterday's patch.

Is it possible that the test case is legitimately an empty file? (This would strongly suggest that there's some bug in the fuzzer setup. Maybe it's been corrupting the first few bytes of Isolate objects all along, but no one ever noticed before because those bytes didn't contain anything important.)

Yeah something is fishy with clusterfuzz here. I doubt that it is corrupting the Isolate, but the empty repro doesn't help :[

Looks like this regressed in 68, changing labels.

I think Mac buider is having some troubles these days. That might be the reason why CF still hasn't recognized the fix.

Also, the issue might be a build problem specific for Mac. We had somewhat similar stuff in the past. I can take a look if this doesn't get resolved in the next few days.

Re 19: Why mac builder? Clusterfuzz was able to reproduce, has a link to the test case, shows a last-tested revision. But there is no test case to download. This seems like a pure clusterfuzz-side problem.

Filed https://crbug.com/840250 for tracking the issue with the missing download.

Did anybody check if this reproduces with an empty test case? (see issue 840250). Can somebody with a mac try this out?

I.e. us this cmd line:
javascript_parser_proto_fuzzer -timeout=25 -rss_limit_mb=2048 -runs=100 empty-file

With asan options set to:
ASAN_OPTIONS = redzone=64:strict_string_check=1:strict_memcmp=1:allow_user_segv_handler=0:allocator_may_return_null=1:handle_sigfpe=1:handle_sigbus=1:detect_stack_use_after_return=1:alloc_dealloc_mismatch=0:print_scariness=1:max_uar_stack_size_log=16:quarantine_size_mb=256:detect_odr_violation=0:handle_sigill=1:allocator_release_to_os_interval_ms=500:coverage=0:use_sigaltstack=1:fast_unwind_on_fatal=1:detect_leaks=0:print_summary=1:handle_abort=1:check_malloc_usable_size=0:detect_container_overflow=0:symbolize=0:handle_segv=1

mmoroz-macpro:libfuzzer-mac-release-553975 mmoroz$ touch empty
mmoroz-macpro:libfuzzer-mac-release-553975 mmoroz$ ASAN_OPTIONS=redzone=64:strict_string_check=1:strict_memcmp=1:allow_user_segv_handler=0:allocator_may_return_null=1:handle_sigfpe=1:handle_sigbus=1:detect_stack_use_after_return=1:alloc_dealloc_mismatch=0:print_scariness=1:max_uar_stack_size_log=16:quarantine_size_mb=256:detect_odr_violation=0:handle_sigill=1:allocator_release_to_os_interval_ms=500:coverage=0:use_sigaltstack=1:fast_unwind_on_fatal=1:detect_leaks=0:print_summary=1:handle_abort=1:check_malloc_usable_size=0:detect_container_overflow=0:symbolize=0:handle_segv=1 ./javascript_parser_proto_fuzzer -timeout=25 -rss_limit_mb=2048 -runs=100 empty
INFO: Seed: 3306138094
INFO: Loaded 1 modules   (379010 guards): 379010 [0x10f987f08, 0x10fafa110), 
./javascript_parser_proto_fuzzer: Running 1 inputs 100 time(s) each.
Running: empty
AddressSanitizer:DEADLYSIGNAL
=================================================================
==17067==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005680 (pc 0x7fff5227434c bp 0x7ffee1ad42f0 sp 0x7ffee1ad4278 T0)
==17067==The signal is caused by a READ memory access.
SCARINESS: 20 (wild-addr-read)
    #0 0x7fff5227434b  (/usr/lib/system/libsystem_pthread.dylib:x86_64+0x134b)
    #1 0x10ebcba42  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x100aa0a42)
    #2 0x10ebdaa51  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x100aafa51)
    #3 0x10e177587  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x10004c587)
    #4 0x10e1773e2  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x10004c3e2)
    #5 0x10e19857b  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x10006d57b)
    #6 0x10e1797b5  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x10004e7b5)
    #7 0x10e17f194  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x100054194)
    #8 0x10e1a9d31  (/Users/mmoroz/Downloads/libfuzzer-mac-release-553975/./javascript_parser_proto_fuzzer:x86_64+0x10007ed31)
    #9 0x7fff51f5e014  (/usr/lib/system/libdyld.dylib:x86_64+0x1014)

==17067==Register values:
rax = 0x000000010faffc00  rbx = 0x00007ffee1ad4280  rcx = 0x000000000005c53a  rdx = 0x000000010fcffc00  
rdi = 0x0000000000005680  rsi = 0x0000000115701800  rbp = 0x00007ffee1ad42f0  rsp = 0x00007ffee1ad4278  
 r8 = 0x0000000115701000   r9 = 0x00000fffffffffff  r10 = 0x0000000119eb63e0  r11 = 0x00007fff52274992  
r12 = 0x0000000115701ea0  r13 = 0x0000000115701e80  r14 = 0x0000000115701e80  r15 = 0x0000100022ae03d0  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/system/libsystem_pthread.dylib:x86_64+0x134b) 
==17067==ABORTING
Abort trap: 6

And the stacktrace when run with symbolize=1 in ASAN_OPTIONS:

==18029==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005680 (pc 0x7fff5227434c bp 0x7ffee7cf32f0 sp 0x7ffee7cf3278 T0)
==18029==The signal is caused by a READ memory access.
SCARINESS: 20 (wild-addr-read)
    #0 0x7fff5227434b in _pthread_key_global_init (libsystem_pthread.dylib:x86_64+0x134b)
    #1 0x1089aca42 in v8::internal::Isolate::FindOrAllocatePerThreadDataForThisThread() isolate.cc:202
    #2 0x1089bba51 in v8::internal::Isolate::Enter() isolate.cc:3187
    #3 0x107f58587 in TestOneProtoInput(javascript_parser_proto_fuzzer::Source const&) javascript_parser_proto_fuzzer.cc:48
    #4 0x107f583e2 in LLVMFuzzerTestOneInput javascript_parser_proto_fuzzer.cc:46
    #5 0x107f7957b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:517
    #6 0x107f5a7b5 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) FuzzerDriver.cpp:280
    #7 0x107f60194 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:703
    #8 0x107f8ad31 in main FuzzerMain.cpp:20
    #9 0x7fff51f5e014 in start (libdyld.dylib:x86_64+0x1014)

This turns out to be what I've been afraid of. Not a real bug, but an issue with build configuration:

  $ nm javascript_parser_proto_fuzzer | egrep LLVM
  000000010004c2f0 t _LLVMFuzzerTestOneInput

LLVMFuzzerInitialize is not linked in on Mac, and thus not getting called. We've had a similar issue in the past:
- https://bugs.chromium.org/p/chromium/issues/detail?id=693760
- https://bugs.chromium.org/p/chromium/issues/detail?id=754124

I've been trying to fix that in a universal way, but none of my suggestions either in Chromium or LLVM were accepted. The only thing I've been allowed to do was to avoid using LLVMFuzzerInitialize, unless it's really needed (e.g. argv/argc are needed). In that case, LLVMFuzzerInitialize needs to be defined in the same file as LLVMFuzzerTestOneInput.

In case of this particular fuzz target, that doesn't seem possible, as LLVMFuzzerTestOneInput is defined in a common header: https://cs.chromium.org/chromium/src/third_party/libprotobuf-mutator/src/src/libfuzzer/libfuzzer_macro.h?gsn=DEFINE_BINARY_PROTO_FUZZER&l=56

At the same time, I see that the fuzz target needs argv/argc for the initialization. Which means I don't see a good solution here except of disabling this fuzz target on Mac.

I have one idea though, let me try that and I'll post an update.

I've uploaded a fix: https://chromium-review.googlesource.com/c/chromium/src/+/1050505

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

commit 80ee67f4c34215cdd3f627de5584c0cfdf9fb11f
Author: Max Moroz <mmoroz@chromium.org>
Date: Tue May 08 17:15:26 2018

Add visibility attributes to LLVMFuzzerInitialize on Mac in JS parser proto fuzzer.

That prevents linker on Mac from dead-stripping the function, as it's actually used
but is being called indirectly using dlsym().

Bug:  837477 
Change-Id: Ic02066132cba3e1f9deb977219b4f3ec13207991
Reviewed-on: https://chromium-review.googlesource.com/1050505
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556844}
[modify] https://crrev.com/80ee67f4c34215cdd3f627de5584c0cfdf9fb11f/testing/libfuzzer/fuzzers/javascript_parser_proto_fuzzer.cc

Is this fixed?

Yes, it should be fixed, but I'd wait for ClusterFuzz to verify that and update the bug.

ClusterFuzz has detected this issue as fixed in range 556812:557044.

Detailed report: https://clusterfuzz.com/testcase?key=6135427984261120

Fuzzer: libFuzzer_javascript_parser_proto_fuzzer
Job Type: mac_libfuzzer_chrome_asan
Platform Id: mac

Crash Type: UNKNOWN READ
Crash Address: 0x000000005680
Crash State:
  _pthread_key_global_init
  v8::internal::Isolate::FindOrAllocatePerThreadDataForThisThread
  v8::internal::Isolate::Enter
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=553059:553281
Fixed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=556812:557044

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6135427984261120

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

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

ClusterFuzz testcase 6135427984261120 is verified as fixed, so closing issue as verified.

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

The NextAction date has arrived: 2018-05-14

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot