Issue 14495 in v8: mozilla/ecma/Date/15.9.5.4-1 starts flaking

ad… via monorail

unread,

Nov 30, 2023, 6:26:30 PM11/30/23

to v8-re...@googlegroups.com

Status: Untriaged
Owner: ----
Labels: Hotlist-Flake
Priority: 1
Type: Bug

New issue 14495 by ad...@chromium.org: mozilla/ecma/Date/15.9.5.4-1 starts flaking
https://bugs.chromium.org/p/v8/issues/detail?id=14495

Failing test: mozilla/ecma/Date/15.9.5.4-1
Failure link: https://cr-buildbucket.appspot.com/build/8762989031701842881
Link to Flako run: http://ci.chromium.org/b/8762960405494802913
Suspected commit: TBD

Crash type: Data race
READ 4

Crash state:
load
bytecode_or_interpreter_data
GetActiveBytecodeArray

Error summary:
==================
WARNING: ThreadSanitizer: data race (pid=1104455)
  Read of size 4 at 0x7e79001c25a8 by thread T14 (mutexes: read M0):
    #0 load src/objects/tagged-field-inl.h:138:20 (d8+0x834267) (BuildId: b81ffed24df1a89c)
    #1 bytecode_or_interpreter_data src/objects/code-inl.h:104:7 (d8+0x834267)
    #2 GetActiveBytecodeArray src/objects/shared-function-info-inl.h:740:27 (d8+0x834267)
    #3 v8::internal::Tagged<v8::internal::BytecodeArray> v8::internal::SharedFunctionInfo::GetBytecodeArray<v8::internal::LocalIsolate>(v8::internal::LocalIsolate*) const src/objects/shared-function-info-inl.h:724:10 (d8+0x834267)
    #4 v8::internal::SharedFunctionInfo::Inlineability v8::internal::SharedFunctionInfo::GetInlineability<v8::internal::LocalIsolate>(v8::internal::LocalIsolate*) const src/objects/shared-function-info-inl.h:351:7 (d8+0x1a777d2) (BuildId: b81ffed24df1a89c)
    #5 v8::internal::compiler::SharedFunctionInfoRef::GetInlineability(v8::internal::compiler::JSHeapBroker*) const src/compiler/heap-refs.cc:1668:26 (d8+0x1a7736f) (BuildId: b81ffed24df1a89c)
    #6 v8::internal::compiler::JSInliner::ReduceJSCall(v8::internal::compiler::Node*) src/compiler/js-inlining.cc:677:20 (d8+0x1b0ce3f) (BuildId: b81ffed24df1a89c)
    #7 v8::internal::compiler::JSInliningHeuristic::InlineCandidate(v8::internal::compiler::JSInliningHeuristic::Candidate const&, bool) src/compiler/js-inlining-heuristic.cc:710:42 (d8+0x1b05d5a) (BuildId: b81ffed24df1a89c)
    #8 v8::internal::compiler::JSInliningHeuristic::Finalize() src/compiler/js-inlining-heuristic.cc:306:33 (d8+0x1b067f0) (BuildId: b81ffed24df1a89c)
    #9 v8::internal::compiler::GraphReducer::ReduceNode(v8::internal::compiler::Node*) src/compiler/graph-reducer.cc:86:57 (d8+0x1a50f4b) (BuildId: b81ffed24df1a89c)
    #10 v8::internal::compiler::GraphReducer::ReduceGraph() src/compiler/graph-reducer.cc:97:36 (d8+0x1a514d4) (BuildId: b81ffed24df1a89c)
    #11 v8::internal::compiler::InliningPhase::Run(v8::internal::compiler::

Crash analysis hash: 586689189552046a9a32c81634ce3da8

-- 
You received this message because:
  1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

ad… via monorail

unread,

Nov 30, 2023, 6:32:50 PM11/30/23

to v8-re...@googlegroups.com

Updates:
	Components: Sandbox
	Owner: sa...@chromium.org
	Status: Assigned

Comment #1 on issue 14495 by ad...@chromium.org: mozilla/ecma/Date/15.9.5.4-1 starts flaking
https://bugs.chromium.org/p/v8/issues/detail?id=14495#c1

I suspect this is due to 9d6bd9e75a7350327121ae4f9d748c01886e374 (given that it's in SharedFunctionInfo::GetBytecodeArray).

I see similar failures on other TSAN bots & tests:
mjsunit/regress/regress-982 (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20debug/4015/overview)
mjsunit/regress/regress-1236303 (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20isolates/28324/overview)
mjsunit/regress/regress-99167 (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20no-concurrent-marking/17933/overview)

Git Watcher via monorail

unread,

Nov 30, 2023, 6:37:10 PM11/30/23

to v8-re...@googlegroups.com

Comment #2 on issue 14495 by Git Watcher: mozilla/ecma/Date/15.9.5.4-1 starts flaking
https://bugs.chromium.org/p/v8/issues/detail?id=14495#c2

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8/+/62eb26279630758daa6d4f7ae9f1cf872dfe87f5

commit 62eb26279630758daa6d4f7ae9f1cf872dfe87f5
Author: Adam Klein <ad...@chromium.org>
Date: Thu Nov 30 23:35:01 2023

Revert "Reland "[sandbox] Ensure bytecode is loaded via a trusted pointer""

This reverts commit 9d6bd9e75a7350327121ae4f9d748c01886e374c.

Reason for revert: causing flakiness on multiple TSAN bots
See https://crbug.com/v8/14495 for details

Original change's description:
> Reland "[sandbox] Ensure bytecode is loaded via a trusted pointer"
>
> This is a reland of commit 403b8120fadd2387900034b6d44da303d3916b6a
>
> Fixed incorrect type check in builtins-arm64.cc.
>
> Original change's description:
> > [sandbox] Ensure bytecode is loaded via a trusted pointer
> >
> > Currently, the routines loading the BytecodeArray from an SFI would
> > allow an attacker to use an attacker-controlled BytecodeArray by
> > clearing the trusted_function_data field and using the tagged
> > function_data field to point to a fake BytecodeArray inside the sandbox.
> > To prevent this, we now force the bytecode accessor routines to always
> > use the trusted pointer field.
> >
> > Bug: chromium:1472252
> > Change-Id: Ie39a0e6ba450db2fdccedcfc68e2b2f883156ee3
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5076895
> > Commit-Queue: Samuel Groß <sa...@chromium.org>
> > Reviewed-by: Igor Sheludko <ish...@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#91278}
>
> Bug: chromium:1472252
> Change-Id: I47c1318f78e507062e6be3ac841885132543ce88
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5077249
> Commit-Queue: Samuel Groß <sa...@chromium.org>
> Reviewed-by: Igor Sheludko <ish...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#91285}

Bug: chromium:1472252, v8:14495
Change-Id: I27d7bbe5564cc1be80637265767ec36b0b3aefd5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5078505
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Auto-Submit: Adam Klein <ad...@chromium.org>
Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#91287}

[modify] https://crrev.com/62eb26279630758daa6d4f7ae9f1cf872dfe87f5/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/62eb26279630758daa6d4f7ae9f1cf872dfe87f5/src/codegen/code-stub-assembler.cc
[modify] https://crrev.com/62eb26279630758daa6d4f7ae9f1cf872dfe87f5/src/objects/shared-function-info-inl.h
[modify] https://crrev.com/62eb26279630758daa6d4f7ae9f1cf872dfe87f5/src/builtins/x64/builtins-x64.cc

sa… via monorail

unread,

Dec 1, 2023, 5:17:21 AM12/1/23

to v8-re...@googlegroups.com

Comment #3 on issue 14495 by sa...@google.com: mozilla/ecma/Date/15.9.5.4-1 starts flaking
https://bugs.chromium.org/p/v8/issues/detail?id=14495#c3

This is a false-positive complaint from TSan: we use a release-store when writing the IndirectPointerTable, but only a relaxed load when loading it because we generally assume that dependent loads cannot be reordered (we can't reorder the load of a field of an object before the load of the pointer to the object from the table, etc.). I think the best fix is to just use an acquire load in this particular case, but if we see more of these complaints, we might want to always use an acquire load if TSan is enabled, or something along those lines.

Git Watcher via monorail

unread,

Dec 1, 2023, 5:44:14 AM12/1/23

to v8-re...@googlegroups.com

Comment #4 on issue 14495 by Git Watcher: mozilla/ecma/Date/15.9.5.4-1 starts flaking
https://bugs.chromium.org/p/v8/issues/detail?id=14495#c4

The following revision refers to this bug:
  

https://chromium.googlesource.com/v8/v8/+/619a24b9969864ab9c48ddaf2d6c747992995b1f

commit 619a24b9969864ab9c48ddaf2d6c747992995b1f
Author: Samuel Groß <sa...@chromium.org>
Date: Fri Dec 01 09:21:16 2023

Reland^2 "[sandbox] Ensure bytecode is loaded via a trusted pointer"

This is a reland of commit 9d6bd9e75a7350327121ae4f9d748c01886e374c

Fix TSan complaints by using an acquire load.

Change-Id: Iffe130aa046b13eee2eb7cd93d54e62d5896281a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5079305

Commit-Queue: Samuel Groß <sa...@chromium.org>
Reviewed-by: Igor Sheludko <ish...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#91294}

[modify] https://crrev.com/619a24b9969864ab9c48ddaf2d6c747992995b1f/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/619a24b9969864ab9c48ddaf2d6c747992995b1f/src/codegen/code-stub-assembler.cc
[modify] https://crrev.com/619a24b9969864ab9c48ddaf2d6c747992995b1f/src/objects/shared-function-info-inl.h
[modify] https://crrev.com/619a24b9969864ab9c48ddaf2d6c747992995b1f/src/builtins/x64/builtins-x64.cc

Jeffrey brown Chocola

unread,

Dec 1, 2023, 10:40:22 PM12/1/23

to v8-reviews

Reply all

Reply to author

Forward