Fix some assertion failures on Fuchsia

Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core snapshot.
 - Move depfile resolution so it happens outside of the tag handler.

Issue #29458

R=asiva@google.com, johnmccutchan@google.com, zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/06080dae96b5c3d727f071d564d50810c628ac7c

[rmacnak, 2017-04-25 23:07:55]

rmacnak@google.com changed reviewers:
+ asiva@google.com, johnmccutchan@google.com, zra@google.com

[Cutch, 2017-04-26 14:31:48]

LGTM.

The loader assertion failures are odd.

[zra, 2017-04-26 15:52:20]

I'm not sure about this change. I think when we get a test bot set up we'll
definitely want our debug asserts enabled, but I'm not sure how much value
they'll add for folks working on other parts of Fuchsia.

Maybe the change we'd really like is making it easier to enable the asserts for
our own local development.

https://codereview.chromium.org/2837873005/diff/1/runtime/bin/isolate_data.h
File runtime/bin/isolate_data.h (right):

https://codereview.chromium.org/2837873005/diff/1/runtime/bin/isolate_data.h#...
runtime/bin/isolate_data.h:71: #ifndef TARGET_OS_FUCHSIA
#if !defined(TARGET_OS_FUCHSIA)

[siva, 2017-04-26 16:30:56]

I think we figured this out last night, the asserts are hit on other hosts too.

Loader::ResolveAsFilePath needs to be adjusted to expect a loader to already be
initialized.

[rmacnak, 2017-04-26 20:07:45]

Description was changed from

==========
Enable VM asserts in Fuchsia and Flutter debug builds.
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Suppress loader asserts on Fuchsia.
==========

to

==========
Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Move depfile resolution so it happens outside of the tag handler.
==========

[rmacnak, 2017-04-26 20:09:00]

On 2017/04/26 15:52:20, zra wrote:
> I'm not sure about this change. I think when we get a test bot set up we'll
> definitely want our debug asserts enabled, but I'm not sure how much value
> they'll add for folks working on other parts of Fuchsia.
> 
> Maybe the change we'd really like is making it easier to enable the asserts
for
> our own local development.

I'm still getting assertions failures trying to run our test harness, so I've
backed out enabling debug by default from this CL.

[rmacnak, 2017-04-26 20:10:15]

On 2017/04/26 20:09:00, rmacnak wrote:
> On 2017/04/26 15:52:20, zra wrote:
> > I'm not sure about this change. I think when we get a test bot set up we'll
> > definitely want our debug asserts enabled, but I'm not sure how much value
> > they'll add for folks working on other parts of Fuchsia.
> > 
> > Maybe the change we'd really like is making it easier to enable the asserts
> for
> > our own local development.
> 
> I'm still getting assertions failures trying to run our test harness, so I've
> backed out enabling debug by default from this CL.

Reusing the loader leads to pthreads error re-entering the loader's monitor.
I've instead moved the dependency resolution to happen after script loading,
instead of as each dependency is encountered.

[zra, 2017-04-26 20:25:01]

lgtm

https://codereview.chromium.org/2837873005/diff/40001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/2837873005/diff/40001/runtime/bin/loader.cc#n...
runtime/bin/loader.cc:295: 
two newlines between functions.

[rmacnak, 2017-04-26 20:34:06]

https://codereview.chromium.org/2837873005/diff/40001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/2837873005/diff/40001/runtime/bin/loader.cc#n...
runtime/bin/loader.cc:295: 
On 2017/04/26 20:25:01, zra wrote:
> two newlines between functions.

Done.

[rmacnak, 2017-04-26 20:38:34]

Description was changed from

==========
Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Move depfile resolution so it happens outside of the tag handler.
==========

to

==========
Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Move depfile resolution so it happens outside of the tag handler.

Issue #29458
==========

[siva, 2017-04-26 22:30:07]

lgtm

[rmacnak, 2017-04-26 22:34:37]

Description was changed from

==========
Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Move depfile resolution so it happens outside of the tag handler.

Issue #29458
==========

to

==========
Fix some assertion failures on Fuchsia
 - Fix longjmp asserts to work with -fsanitize=safe-stack.
 - Remove outdated asserts in gen_snapshot assuming generation of a core
snapshot.
 - Move depfile resolution so it happens outside of the tag handler.

Issue #29458

R=asiva@google.com, johnmccutchan@google.com, zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/06080dae96b5c3d727f071d564d50810c628ac7c
==========

[rmacnak, 2017-04-26 22:34:38]

Committed patchset #4 (id:60001) manually as
06080dae96b5c3d727f071d564d50810c628ac7c (presubmit successful).