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

Issue 772057 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 1
Type: Bug-Regression



Sign in to add a comment

ProxyResolverV8* tests started failing under Fuchsia

Project Member Reported by w...@chromium.org, Oct 5 2017

Issue description

Last working build was:
https://build.chromium.org/p/chromium.fyi/builders/Fuchsia/builds/9894

but since:
https://build.chromium.org/p/chromium.fyi/builders/Fuchsia/builds/9895

the Fuchsia test-running bot has been red, due to failures in both content_unittests and net_unittests; content_unittests failures are exceptional ("some shards did not complete") but the net_unittests are all ProxyResolverV8*, which seems a plausible root-cause.

Build 9895 does include a V8 auto-roll.
 

Comment 1 by w...@chromium.org, Oct 6 2017

Components: Blink>JavaScript
We have a failing CHECK_EQ() due to |magic_number_| mismatch:

[00002.237] 02575.02725> [ RUN      ] ProxyResolverV8Test.BadReturnType
[00002.237] 02575.02725>
[00002.237] 02575.02725>
[00002.237] 02575.02725> #
[00002.237] 02575.02725> # Fatal error in ../../v8/src/snapshot/deserializer.cc, line 150
[00002.237] 02575.02725> # Check failed: magic_number_ == SerializedData::ComputeMagicNumber(external_reference_table_) (3235775431 vs. 3235775429).
[00002.237] 02575.02725> #

This appears to have regressed in the https://chromium-review.googlesource.com/c/chromium/src/+/701702 V8 auto-roll.

Comment 2 by w...@chromium.org, Oct 6 2017

Cc: eholk@chromium.org
Status: Started (was: Assigned)
Bisected the issue to https://chromium-review.googlesource.com/688481.

eholk: It looks like the V8_TRAP_HANDLER_SUPPORTED conditional in ExternalReferenceTable::AddReferences() is causing the snapshot magic numbers not to line-up - one of the values calculated presumably includes the extra entries while the other doesn't.  Any ideas why that would only show up under our Fuchsia build?



Comment 3 by w...@chromium.org, Oct 6 2017

Owner: eholk@chromium.org
IIUC the issue is that we're now building a mksnapshot binary under Linux, with V8_TRAP_HANDLER_SUPPORTED=1, but then loading the snapshot under Fuchsia, in code built with V8_TRAP_HANDLER_SUPPORTED=0.

That said, I'd then expect the Android net_unittests to be failing as well...

Comment 4 by eholk@chromium.org, Oct 6 2017

That sounds likely, that mksnapshot is built with V8_TRAP_HANDLER_SUPPORTED=1 but then used with V8_TRAP_HANDLER_SUPPORTED=0. Can we detect when building mksnapshot that this is targeting a Fuchsia build? If so, it'd make sense to compile it without trap handler support.

Comment 5 by w...@chromium.org, Oct 6 2017

Yes, the GN target_os will be fuchsia rather than Linux; I'm not sure how
we can propagate that into the V8 build so as to get a Linux binary with
Fuchsia V8 features though - do you know how we manage that for Android?

Comment 6 by eholk@chromium.org, Oct 6 2017

For Android I'm guessing v8_target_cpu is always arm or arm64, but if you did an x64 build of Android you'd run into the same problems.

Trap handlers are only supported on x64 at the moment.


Maybe the best thing to do is give mksnapshot a command line option to tell it not to include trap handler support. Then Fuchsia builds we just have to make sure to set that flag.

Comment 7 by w...@chromium.org, Oct 6 2017

Agreed offline w/ eholk@ to revert the change and get a cross-compilation-friendly version of the patch prepped instead.

(see the instructions at https://chromium.googlesource.com/chromium/src/+/master/docs/fuchsia_build_instructions.md for building & running Fuchsia test binaries :)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 9 2017

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

commit 527d55680f241c95b6ddb45dd12dfba341ef2f5e
Author: Eric Holk <eholk@chromium.org>
Date: Mon Oct 09 20:50:19 2017

[wasm] Add thread-in-wasm flag builtins for x64 target

This was causing trouble with Fuchsia, since mksnapshot was built and run on
Linux which supports trap handlers, while Fuchsia does not yet. This change
causes the external references to match between Fuchsia and Linux.

Bug:  chromium:772057 
Change-Id: I8e8f3539e3f5c4b798c364101ef2d16b5137f16d
Reviewed-on: https://chromium-review.googlesource.com/706109
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48408}
[modify] https://crrev.com/527d55680f241c95b6ddb45dd12dfba341ef2f5e/src/external-reference-table.cc

Comment 9 by w...@chromium.org, Oct 10 2017

Status: Fixed (was: Started)

Sign in to add a comment