8302865: Illegal bytecode for break from if with instanceof pattern matching condition

Reviewed-by: vromero
This commit is contained in:
Jan Lahoda 2023-06-21 09:15:48 +00:00
parent 67fbd87378
commit a15db1a56c
6 changed files with 195 additions and 68 deletions
src/jdk.compiler/share/classes/com/sun/tools/javac/comp
test/langtools/tools/javac/patterns

@ -1447,12 +1447,7 @@ public class Attr extends JCTree.Visitor {
public void visitDoLoop(JCDoWhileLoop tree) {
attribStat(tree.body, env.dup(tree));
attribExpr(tree.cond, env, syms.booleanType);
if (!breaksOutOf(tree, tree.body)) {
//include condition's body when false after the while, if cannot get out of the loop
MatchBindings condBindings = matchBindings;
condBindings.bindingsWhenFalse.forEach(env.info.scope::enter);
condBindings.bindingsWhenFalse.forEach(BindingSymbol::preserveBinding);
}
handleLoopConditionBindings(matchBindings, tree, tree.body);
result = null;
}
@ -1466,19 +1461,10 @@ public class Attr extends JCTree.Visitor {
} finally {
whileEnv.info.scope.leave();
}
if (!breaksOutOf(tree, tree.body)) {
//include condition's bindings when false after the while, if cannot get out of the loop
condBindings.bindingsWhenFalse.forEach(env.info.scope::enter);
condBindings.bindingsWhenFalse.forEach(BindingSymbol::preserveBinding);
}
handleLoopConditionBindings(condBindings, tree, tree.body);
result = null;
}
private boolean breaksOutOf(JCTree loop, JCTree body) {
preFlow(body);
return flow.breaksOutOf(env, loop, body, make);
}
public void visitForLoop(JCForLoop tree) {
Env<AttrContext> loopEnv =
env.dup(env.tree, env.info.dup(env.info.scope.dup()));
@ -1503,13 +1489,52 @@ public class Attr extends JCTree.Visitor {
finally {
loopEnv.info.scope.leave();
}
if (!breaksOutOf(tree, tree.body)) {
//include condition's body when false after the while, if cannot get out of the loop
condBindings.bindingsWhenFalse.forEach(env.info.scope::enter);
condBindings.bindingsWhenFalse.forEach(BindingSymbol::preserveBinding);
handleLoopConditionBindings(condBindings, tree, tree.body);
}
/**
* Include condition's bindings when false after the loop, if cannot get out of the loop
*/
private void handleLoopConditionBindings(MatchBindings condBindings,
JCStatement loop,
JCStatement loopBody) {
if (condBindings.bindingsWhenFalse.nonEmpty() &&
!breaksTo(env, loop, loopBody)) {
addBindings2Scope(loop, condBindings.bindingsWhenFalse);
}
}
private boolean breaksTo(Env<AttrContext> env, JCTree loop, JCTree body) {
preFlow(body);
return flow.breaksToTree(env, loop, body, make);
}
/**
* Add given bindings to the current scope, unless there's a break to
* an immediately enclosing labeled statement.
*/
private void addBindings2Scope(JCStatement introducingStatement,
List<BindingSymbol> bindings) {
if (bindings.isEmpty()) {
return ;
}
var searchEnv = env;
while (searchEnv.tree instanceof JCLabeledStatement labeled &&
labeled.body == introducingStatement) {
if (breaksTo(env, labeled, labeled.body)) {
//breaking to an immediately enclosing labeled statement
return ;
}
searchEnv = searchEnv.next;
introducingStatement = labeled;
}
//include condition's body when false after the while, if cannot get out of the loop
bindings.forEach(env.info.scope::enter);
bindings.forEach(BindingSymbol::preserveBinding);
}
public void visitForeachLoop(JCEnhancedForLoop tree) {
Env<AttrContext> loopEnv =
env.dup(env.tree, env.info.dup(env.info.scope.dup()));
@ -2242,8 +2267,7 @@ public class Attr extends JCTree.Visitor {
afterIfBindings = condBindings.bindingsWhenFalse;
}
afterIfBindings.forEach(env.info.scope::enter);
afterIfBindings.forEach(BindingSymbol::preserveBinding);
addBindings2Scope(tree, afterIfBindings);
result = null;
}

@ -287,7 +287,7 @@ public class Flow {
}
}
public boolean breaksOutOf(Env<AttrContext> env, JCTree loop, JCTree body, TreeMaker make) {
public boolean breaksToTree(Env<AttrContext> env, JCTree breakTo, JCTree body, TreeMaker make) {
//we need to disable diagnostics temporarily; the problem is that if
//"that" contains e.g. an unreachable statement, an error
//message will be reported and will cause compilation to skip the flow analysis
@ -295,10 +295,10 @@ public class Flow {
//related errors, which will allow for more errors to be detected
Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
try {
SnippetBreakAnalyzer analyzer = new SnippetBreakAnalyzer();
SnippetBreakToAnalyzer analyzer = new SnippetBreakToAnalyzer(breakTo);
analyzer.analyzeTree(env, body, make);
return analyzer.breaksOut();
return analyzer.breaksTo();
} finally {
log.popDiagnosticHandler(diagHandler);
}
@ -1909,52 +1909,21 @@ public class Flow {
}
}
class SnippetBreakAnalyzer extends AliveAnalyzer {
private final Set<JCTree> seenTrees = new HashSet<>();
private boolean breaksOut;
class SnippetBreakToAnalyzer extends AliveAnalyzer {
private final JCTree breakTo;
private boolean breaksTo;
public SnippetBreakAnalyzer() {
}
@Override
public void visitLabelled(JCTree.JCLabeledStatement tree) {
seenTrees.add(tree);
super.visitLabelled(tree);
}
@Override
public void visitWhileLoop(JCTree.JCWhileLoop tree) {
seenTrees.add(tree);
super.visitWhileLoop(tree);
}
@Override
public void visitForLoop(JCTree.JCForLoop tree) {
seenTrees.add(tree);
super.visitForLoop(tree);
}
@Override
public void visitForeachLoop(JCTree.JCEnhancedForLoop tree) {
seenTrees.add(tree);
super.visitForeachLoop(tree);
}
@Override
public void visitDoLoop(JCTree.JCDoWhileLoop tree) {
seenTrees.add(tree);
super.visitDoLoop(tree);
public SnippetBreakToAnalyzer(JCTree breakTo) {
this.breakTo = breakTo;
}
@Override
public void visitBreak(JCBreak tree) {
breaksOut |= (super.alive == Liveness.ALIVE &&
!seenTrees.contains(tree.target));
super.visitBreak(tree);
breaksTo |= breakTo == tree.target && super.alive == Liveness.ALIVE;
}
public boolean breaksOut() {
return breaksOut;
public boolean breaksTo() {
return breaksTo;
}
}

@ -231,6 +231,67 @@ public class BindingsTest1 {
throw new AssertionError();
}
{
L: {
while (!(o1 instanceof String s)) {
break L;
}
s.length();
}
}
{
L: {
for (; !(o1 instanceof String s); ) {
break L;
}
s.length();
}
}
{
int j = 0;
L: while (j++ < 2)
if (!(o1 instanceof String s)) {
break L;
}
}
{
int j = 0;
L: for (; j++ < 2; )
if (!(o1 instanceof String s)) {
break L;
}
}
{
//"s" in the outter scope does not flow out of the if, but
//variables inside a lambda or anonymous or local class may:
L: if (!(o1 instanceof String s)) {
Runnable r = () -> {
NESTED: {
if (!(o1 instanceof String n)) {
break NESTED;
}
n.length();
}
};
break L;
}
}
switch (0) {
case 0:
if (!(o1 instanceof String s)) {
break;
}
s.length();
}
//binding in an anonymous class:
if (!(invokeOnce("") instanceof String s)) {
throw new AssertionError();

@ -247,5 +247,62 @@ public class BindingsTest2 {
s = "";
}
}
{
LBL1: LBL2: if (!(o1 instanceof String s)) {
break LBL1;
}
System.err.println(s);
}
{
LBL1: LBL2: if (!(o1 instanceof String s)) {
break LBL2;
}
System.err.println(s);
}
{
LBL1: LBL2: if (o1 instanceof String s) {
} else {
break LBL1;
}
System.err.println(s);
}
{
LBL1: LBL2: if (o1 instanceof String s) {
} else {
break LBL2;
}
System.err.println(s);
}
{
switch (0) {
case 0:
if (!(o1 instanceof String s)) {
break;
}
}
s.length();
}
{
int j = 0;
L: while (j++ < 2)
if (!(o1 instanceof String s)) {
break L;
}
s.length();
}
{
int j = 0;
L: for (; j++ < 2; )
if (!(o1 instanceof String s)) {
break L;
}
s.length();
}
}
}

@ -43,11 +43,16 @@ BindingsTest2.java:179:13: compiler.err.cant.resolve.location: kindname.variable
BindingsTest2.java:196:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:204:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:212:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:221:17: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:231:17: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:241:17: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:247:17: compiler.err.cant.assign.val.to.var: final, s
BindingsTest2.java:255:32: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:262:32: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:270:32: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:278:32: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:287:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:296:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:305:13: compiler.err.cant.resolve.location: kindname.variable, s, , , (compiler.misc.location: kindname.class, BindingsTest2, null)
BindingsTest2.java:135:17: compiler.err.unreachable.stmt
BindingsTest2.java:160:17: compiler.err.unreachable.stmt
BindingsTest2.java:185:17: compiler.err.unreachable.stmt
52 errors
BindingsTest2.java:241:17: compiler.err.unreachable.stmt
57 errors

@ -104,6 +104,17 @@ public class BreakAndLoops extends ComboInstance<BreakAndLoops> {
shouldPass = true;
} else if (innerLoop.supportsAnonymousBreak && brk == Break.BREAK) {
shouldPass = true;
} else if (outterLabel == OutterLabel.LABEL && brk == Break.BREAK_LABEL && outterLoop != OutterLoop.NONE) {
shouldPass = switch(mainLoop) {
case WHILE, FOR -> true;
case DO_WHILE -> switch (innerLoop) {
case WHILE, FOR, FOR_EACH -> true;
//the statement following the do-while is unreachable:
case BLOCK, DO_WHILE, NONE -> {
yield false;
}
};
};
} else {
shouldPass = false;
}