8273039: JShell crashes when naming variable or method "abstract" or "strictfp"

[lahodaj]

When JShell processes int strictfp = 0; (which is obviously erroneous), it will speculativelly try to parse and attribute code along these lines:

package REPL;
import java.io.*;import java.math.*;import java.net.*;import java.nio.file.*;import java.util.*;import java.util.concurrent.*;import java.util.function.*;import java.util.prefs.*;import java.util.regex.*;import java.util.stream.*;
class $JShell$DOESNOTMATTER {
    public static Object do_it$() throws Throwable {
        return int strictfp = 0;
    }
}

This crashes Attr, as the strictfp will be wrapped in JCModifiers, wrapped inside an erroneous tree, and Attr does not handle modifiers.

The proposal is to let Attr "handle" the JCModifiers in an error-recovery situation.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8273039: JShell crashes when naming variable or method "abstract" or "strictfp"

Reviewers

• Vicente Romero (@vicente-romero-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5897/head:pull/5897
$ git checkout pull/5897

Update a local copy of the PR:
$ git checkout pull/5897
$ git pull https://git.openjdk.java.net/jdk pull/5897/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5897

View PR using the GUI difftool:
$ git pr show -t 5897

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5897.diff

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj The following labels will be automatically applied to this pull request:

• compiler
• kulla

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (8e49f434)

[vicente-romero-oracle]

question can't we report this error when the erroneous code is being parsed? adding this method to Attr makes me feel a bit uncomfortable, I can understand if this is the only / best way though

[lahodaj]

Parser will report the error, and will wrap the (presumed) modifiers into an erroneous tree, so the AST is reasonable. But JShell runs javac with --should-stop=at=FLOW, so the error in parser will not stop the processing, and when Attr will look into the erroneous tree, it will fail.

[vicente-romero-oracle]

I see, thanks for the clarification

[vicente-romero-oracle]

lgtm

[openjdk]

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8273039: JShell crashes when naming variable or method "abstract" or "strictfp"

Reviewed-by: vromero

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 425 new commits pushed to the master branch:

• b01f107: 8276252: java/nio/channels/Channels/TransferTo.java failed with OOM java heap space error
• 92d2176: 8273967: gtest os.dll_address_to_function_and_library_name_vm fails on macOS12
• a74a839: 8276571: C2: pass compilation options as structure
• c393ee8: 8276632: Use blessed modifier order in security-libs code
• 7023b3f: 8276628: Use blessed modifier order in serviceability code
• b933136: 8276641: Use blessed modifier order in jshell
• 0616d86: 8276635: Use blessed modifier order in compiler code
• d95299a: 8276634: Remove usePlainDatagramSocketImpl option from the test DatagramChannel/SendReceiveMaxSize.java
• 3c0faa7: 8276173: Clean up and remove unneeded casts in HeapDumper
• 323d201: 8275506: Rename allocated_on_stack to allocated_on_stack_or_embedded
• ... and 415 more: https://git.openjdk.java.net/jdk/compare/8f7a37c92f5713f7728f54d4a5924484a535e968...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[lahodaj]

/integrate

[openjdk]

Going to push as commit 2f20b0d.
Since your change was applied there have been 600 commits pushed to the master branch:

• f34f119: 8277213: CompileTask_lock is acquired out of order with MethodCompileQueue_lock
• 47564ca: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
• 2f0bde1: 8277102: Dubious PrintCompilation output
• 839033b: 8276976: Rename LIR_OprDesc to LIR_Opr
• 89b125f: 8275527: Refactor forward pointer access
• 36bd4a3: 8277404: Test VMDeprecatedOptions.java failing with Unable to create shared archive file
• 57eb864: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
• 8db0c36: 8277414: ProblemList runtime/CommandLine/VMDeprecatedOptions.java on windows-x64
• 03473b4: 8270874: JFrame paint artifacts when dragged from standard monitor to HiDPI monitor
• ce0f00f: 8276093: Improve naming in closures to iterate over card sets
• ... and 590 more: https://git.openjdk.java.net/jdk/compare/8f7a37c92f5713f7728f54d4a5924484a535e968...master

Your commit was automatically rebased without conflicts.

[openjdk]

@lahodaj Pushed as commit 2f20b0d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.