Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nashorn parser included in the artifact is broken #20

Closed
voronaam opened this issue Jun 20, 2018 · 2 comments
Closed

Nashorn parser included in the artifact is broken #20

voronaam opened this issue Jun 20, 2018 · 2 comments
Assignees

Comments

@voronaam
Copy link

Reproducer:

$ cat jspart2.js 
load("nashorn:parser.js");
var json = parse("print('hello')");
print(JSON.stringify(json));

$ js --jvm jspart2.js 
ReferenceError: TestNashorn is not defined
    at <js> parse(nashorn:parser.js:24:734-744)
    at <js> :program(jspart2.js:2:38-60)

Cause:
https://github.com/graalvm/graaljs/blob/master/graal-js/src/com.oracle.truffle.js.runtime/src/com/oracle/truffle/js/runtime/resources/parser.js#L24

A reference to a test class which is not part of the distributed artifact.

@wirthi wirthi self-assigned this Jun 21, 2018
@wirthi
Copy link
Member

wirthi commented Jun 21, 2018

Hi Aleksey,

thanks for trying Graal JavaScript and reporting this issue. You are right, this is broken. We will fix that in a future release.

Best, Christian

@wirthi
Copy link
Member

wirthi commented Jul 6, 2018

Hi Aleksey,

this has been fixed in our codebase and is part of GraalVM RC3. Note however that you need to specify an additional flag, --nashorn-compat.

$ js --jvm --nashorn-compat issue_20.js 
{"type":"Program","body":[{"type":"ExpressionStatement","expression":{"type":"CallExpression","callee":{"type":"Identifier","name":"print"},"arguments":[{"type":"Literal","value":"hello"}]}}]}

@wirthi wirthi closed this as completed Jul 6, 2018
graalvmbot pushed a commit that referenced this issue Sep 1, 2021
Original commit message:

    Merged: Squashed multiple commits.

    Merged: [test] Make finding build directory more flexible
    Revision: 4f015e85faf1d64466eafd897d1d59b1d77071f3

    Merged: [test] Use the correct precedence for choosing the build directory
    Revision: 7b24b13981e411602fc77db1305d0ae034a92fd8

    Merged: [test] Add fallback to legacy output directory
    Revision: bf3adea58aab3d21e36e23c60e1e0bbc994cd5b8

    Merged: [gcmole] Fix gcmole after property change
    Revision: c87bdbcf0d1d8f8bcc927f6b364d27e72c22736d

    Merged: [test] Overhaul mode processing in test runner
    Revision: 608b732d141689e8e10ee918afc8ed1fae1ab80c

    Merged: [test] Switch to flattened json output
    Revision: 373a9a8cfc8db3ef65fcdca0ec0c4ded9e4acc89

    BUG=chromium:1132088,v8:10893
    NOTRY=true
    NOTREECHECKS=true
    [email protected]

    Change-Id: I3c1de04ca4fe62e36da29e706a20daec0b3d4d98
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461745
    Reviewed-by: Liviu Rau <[email protected]>
    Commit-Queue: Michael Achenbach <[email protected]>
    Cr-Commit-Position: refs/branch-heads/8.6@{#20}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@d724820

PR-URL: nodejs/node#38275
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
graalvmbot pushed a commit that referenced this issue Sep 1, 2021
Original commit message:

    Merged: [codegen] Skip invalid optimization in tail calls

    Preparing for tail call is usually done by emitting the gap moves and
    then moving the stack pointer to its new position. An optimization
    consists in moving the stack pointer first and transforming some of the
    moves into pushes. In the attached case it looks like this (arm):

    138  add sp, sp, #40
    13c  str r6, [sp, #-4]!
    140  str r6, [sp, #-4]!
    144  str r6, [sp, #-4]!
    148  str r6, [sp, #-4]!
    14c  str r6, [sp, #-4]!
    ...
    160  vldr d1, [sp - 4*3]

    The last line is a gap reload, but because the stack pointer was already
    moved, the slot is now below the stack pointer. This is invalid and
    triggers this DCHECK:

    Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
    Debug check failed: 0 <= offset (0 vs. -12).

    A comment already explains that we skip the optimization if the gap
    contains stack moves to prevent this, but the code only checks for
    non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
    "source.IsAnyStackSlot()":

    108  vldr d1, [sp + 4*2]
    ...
    118  str r0, [sp, #+36]
    11c  str r0, [sp, #+32]
    120  str r0, [sp, #+28]
    124  str r0, [sp, #+24]
    128  str r0, [sp, #+20]
    ...
    134  add sp, sp, #20

    TBR=​[email protected]

    (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)

    Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
    Bug: chromium:1137608
    No-Try: true
    No-Presubmit: true
    No-Tree-Checks: true
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
    Reviewed-by: Thibaud Michaud <[email protected]>
    Commit-Queue: Thibaud Michaud <[email protected]>
    Cr-Commit-Position: refs/branch-heads/8.6@{#42}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@8c725f7

PR-URL: nodejs/node#38275
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants