New issue
Advanced search Search tips

Issue 5483 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: Bug

Blocking:
issue 1569



Sign in to add a comment

Crash while reporting exception inside Function inside a module.

Project Member Reported by neis@chromium.org, Oct 6 2016

Issue description

Executing 'Function("foo")()' as a module currently crashes in messages.cc (when gathering the stack trace).
 

Comment 1 by neis@chromium.org, Oct 6 2016

This is due to the module's "function name" being a Symbol.  FormatEvalOrigin calls ToString on that, which throws. And the caller of FormatEvalOrigin doesn't account for an exception.  A quick and dirty fix is the following:

diff --git a/src/messages.cc b/src/messages.cc
index cc6349d..66f366f 100644
--- a/src/messages.cc
+++ b/src/messages.cc
@@ -333,7 +333,8 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
 
   Handle<Object> eval_from_function_name =
       handle(EvalFromFunctionName(isolate, script), isolate);
-  if (eval_from_function_name->BooleanValue()) {
+  if (eval_from_function_name->BooleanValue() &&
+      !eval_from_function_name->IsSymbol()) {
     Handle<String> str;
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, str, Object::ToString(isolate, eval_from_function_name),

Let me know what you think.
BTW, the idea that I mentioned elsewhere is also related to this.

Comment 2 by adamk@chromium.org, Oct 6 2016

Cc: -adamk@chromium.org neis@chromium.org
Owner: adamk@chromium.org
Status: Started (was: Assigned)
Going to give a shot at fixing this by not setting the name to a Symbol.

Comment 3 by caitp@chromium.org, Oct 6 2016

Sounds like SetFunctionName step 4 is being ignored by something? https://tc39.github.io/ecma262/#sec-setfunctionname

Comment 4 by adamk@chromium.org, Oct 6 2016

This is about the "name" field of the SharedFunctionInfo representing a module, not the name of an actual function.

Comment 5 by ca...@igalia.com, Oct 6 2016

it's not necessarily clear that there's a distinction, since that's what the FunctionName accessor uses anyways.

Comment 6 by adamk@chromium.org, Oct 6 2016

The point is this isn't actually exposed to script. The failure here is in a very strange part of the code (it's not even the stack trace formatting stuff, it's for passing back to the embedder from what I can tell).
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 7 2016

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

commit 2d4871c143d916ce3c5f047d8722b0f854089652
Author: adamk <adamk@chromium.org>
Date: Fri Oct 07 22:29:02 2016

[modules] Give Module an internal [hash] field

This allows us to stop using a Symbol, set as the name of the Module's
SharedFunctionInfo, as our storage for a hash.

As part of this, centralize the code for generating a random, non-zero
hash code in one place (there were previously two copies of this code,
and I needed to call it from a third file).

BUG= v8:5483 
TBR=jochen@chromium.org

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

[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/factory.cc
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/heap/heap.cc
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/isolate.cc
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/isolate.h
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/objects-debug.cc
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/objects-inl.h
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/objects.cc
[modify] https://crrev.com/2d4871c143d916ce3c5f047d8722b0f854089652/src/objects.h

Comment 8 by adamk@chromium.org, Oct 7 2016

Status: Fixed (was: Started)
Labels: Priority-2

Sign in to add a comment