Skip to content

Commit

Permalink
Prevent errorneous floating point expression optimization.
Browse files Browse the repository at this point in the history
Negations of floating point comparison expressions where incorrectly
transformed changing its value if operands where NaNs.

Comparison operations on floating point numbers do not statisfy arithmetic
laws like  !(a > b) == (a <= b).

Bug: #9463
Bug-Link: #9463
Change-Id: Id8a5c4e060410ab513eea5c27168eb367d645c2b
  • Loading branch information
rluble committed Jan 28, 2017
1 parent 382766e commit cc2d0ea
Show file tree
Hide file tree
Showing 6 changed files with 328 additions and 317 deletions.
4 changes: 3 additions & 1 deletion dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java
Expand Up @@ -18,6 +18,7 @@
import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.SourceInfo;


import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.List; import java.util.List;


/** /**
Expand All @@ -27,8 +28,9 @@ public class JBlock extends JStatement {


private final List<JStatement> statements = new ArrayList<JStatement>(); private final List<JStatement> statements = new ArrayList<JStatement>();


public JBlock(SourceInfo info) { public JBlock(SourceInfo info, JStatement... statements) {
super(info); super(info);
this.statements.addAll(Arrays.asList(statements));
} }


/** /**
Expand Down
19 changes: 10 additions & 9 deletions dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
Expand Up @@ -202,10 +202,10 @@ public void endVisit(JBinaryOperation x, Context ctx) {


switch (op) { switch (op) {
case AND: case AND:
maybeReplaceMe(x, Simplifier.and(x), ctx); maybeReplaceMe(x, Simplifier.simplifyAnd(x), ctx);
break; break;
case OR: case OR:
maybeReplaceMe(x, Simplifier.or(x), ctx); maybeReplaceMe(x, Simplifier.simplifyOr(x), ctx);
break; break;
case BIT_XOR: case BIT_XOR:
simplifyXor(lhs, rhs, ctx); simplifyXor(lhs, rhs, ctx);
Expand Down Expand Up @@ -315,12 +315,12 @@ public void endVisit(JBlock x, Context ctx) {


@Override @Override
public void endVisit(JCastOperation x, Context ctx) { public void endVisit(JCastOperation x, Context ctx) {
maybeReplaceMe(x, Simplifier.cast(x), ctx); maybeReplaceMe(x, Simplifier.simplifyCast(x), ctx);
} }


@Override @Override
public void endVisit(JConditional x, Context ctx) { public void endVisit(JConditional x, Context ctx) {
maybeReplaceMe(x, Simplifier.conditional(x), ctx); maybeReplaceMe(x, Simplifier.simplifyConditional(x), ctx);
} }


@Override @Override
Expand All @@ -340,7 +340,7 @@ public void endVisit(JDoStatement x, Context ctx) {


// If false, replace do with do's body // If false, replace do with do's body
if (!booleanLiteral.getValue()) { if (!booleanLiteral.getValue()) {
if (Simplifier.isEmpty(x.getBody())) { if (JjsUtils.isEmptyBlock(x.getBody())) {
ctx.removeMe(); ctx.removeMe();
} else { // Unless it contains break/continue statements } else { // Unless it contains break/continue statements
FindBreakContinueStatementsVisitor visitor = new FindBreakContinueStatementsVisitor(); FindBreakContinueStatementsVisitor visitor = new FindBreakContinueStatementsVisitor();
Expand Down Expand Up @@ -429,7 +429,8 @@ public void endVisit(JForStatement x, Context ctx) {
*/ */
@Override @Override
public void endVisit(JIfStatement x, Context ctx) { public void endVisit(JIfStatement x, Context ctx) {
maybeReplaceMe(x, Simplifier.ifStatement(x, getCurrentMethod()), ctx); JType methodReturnType = getCurrentMethod() != null ? getCurrentMethod().getType() : null;
maybeReplaceMe(x, Simplifier.simplifyIfStatement(x, methodReturnType), ctx);
} }


/** /**
Expand Down Expand Up @@ -651,7 +652,7 @@ public void endVisit(JPrefixOperation x, Context ctx) {
} }


if (x.getOp() == JUnaryOperator.NOT) { if (x.getOp() == JUnaryOperator.NOT) {
maybeReplaceMe(x, Simplifier.not(x), ctx); maybeReplaceMe(x, Simplifier.simplifyNot(x), ctx);
return; return;
} else if (x.getOp() == JUnaryOperator.NEG) { } else if (x.getOp() == JUnaryOperator.NEG) {
maybeReplaceMe(x, simplifyNegate(x, x.getArg()), ctx); maybeReplaceMe(x, simplifyNegate(x, x.getArg()), ctx);
Expand Down Expand Up @@ -708,9 +709,9 @@ public void endVisit(JTryStatement x, Context ctx) {
} }


// Compute properties regarding the state of this try statement // Compute properties regarding the state of this try statement
boolean noTry = Simplifier.isEmpty(x.getTryBlock()); boolean noTry = JjsUtils.isEmptyBlock(x.getTryBlock());
boolean noCatch = catchClauses.size() == 0; boolean noCatch = catchClauses.size() == 0;
boolean noFinally = Simplifier.isEmpty(x.getFinallyBlock()); boolean noFinally = JjsUtils.isEmptyBlock(x.getFinallyBlock());


if (noTry) { if (noTry) {
// 2) Prune try statements with no body. // 2) Prune try statements with no body.
Expand Down
10 changes: 10 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
Expand Up @@ -453,6 +453,16 @@ static void synthesizeStaticInitializerChain(
.put(JStringLiteral.class, LiteralTranslators.STRING_LITERAL_TRANSLATOR) .put(JStringLiteral.class, LiteralTranslators.STRING_LITERAL_TRANSLATOR)
.build(); .build();


/**
* Return true if the statement is an empty block.
*/
public static boolean isEmptyBlock(JStatement stmt) {
if (stmt == null) {
return true;
}
return (stmt instanceof JBlock && ((JBlock) stmt).getStatements().isEmpty());
}

private enum LiteralTranslators { private enum LiteralTranslators {
BOOLEAN_LITERAL_TRANSLATOR() { BOOLEAN_LITERAL_TRANSLATOR() {
@Override @Override
Expand Down

0 comments on commit cc2d0ea

Please sign in to comment.