Skip to content
This repository was archived by the owner on Sep 26, 2020. It is now read-only.

Commit 4392b1b

Browse files
author
rluble@google.com
committed
Fixes a missing flow in DataflowOptimizer that might cause incorrect code.
The DataflowOptimizer was building an incorrect control flow graph in the case that a catch block ended in a throw (in all paths) and a finally block was also present. In this case the graph would be missing the flow from the end of the catch block to the finally block. An example snippet that would be optimized incorrectly is Object o = null; try { if (b) throw new Exception(); k++; } catch (Exception e) { o = e; throw e; } finally { if (o == null) { j++; } } where the optimizer would assume that o is always null at the finally block and thus transform the code into: Object o = null; try { if (b) throw new Exception(); k++; } catch (Exception e) { o = e; throw e; } finally { j++; } Fixes issue 8115. Change-Id: I608bd25157eb0c76ac771697ca70756751c10750 Review-Link: https://gwt-review.googlesource.com/#/c/2600/ git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11602 8db76d5a-ed1c-0410-87a9-c151d255dfc7
1 parent c00b01e commit 4392b1b

File tree

3 files changed

+75
-7
lines changed

3 files changed

+75
-7
lines changed

dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -946,13 +946,11 @@ public boolean visit(JTryStatement x, Context ctx) {
946946
// then the try statement completes abruptly for the same reason.
947947
for (List<Exit> exits : catchExits) {
948948
for (Exit e : exits) {
949-
// If the catch block completes normally, then the finally block is
949+
// If the catch block completes normally or abruptly, then the finally block is
950950
// executed.
951-
if (e.isNormal()) {
952-
addEdge(e, finallyNode);
953-
} else {
954-
// If the catch block completes abruptly for reason R, then the
955-
// finally block is executed. Then there is a choice:
951+
addEdge(e, finallyNode);
952+
if (!e.isNormal()) {
953+
// If the catch block completes abruptly for reason R there is a choice:
956954
for (Exit finallyExit : finallyExits) {
957955
if (finallyExit.isNormal()) {
958956
// If the finally block completes normally, then the try

dev/core/test/com/google/gwt/dev/jjs/impl/gflow/DataflowOptimizerTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,39 @@ public void testLinearStatements3() throws Exception {
5555
"int i; int j; foo(1);");
5656
}
5757

58+
/**
59+
* Test case for issue 8115 (http://code.google.com/p/google-web-toolkit/issues/detail?id=8115)
60+
* @throws Exception
61+
*/
62+
public void testCatchThrowExceptionFinally() throws Exception {
63+
addSnippetClassDecl("static boolean b;");
64+
addSnippetClassDecl("static int i;");
65+
addSnippetClassDecl("static int j;");
66+
67+
optimize("void", ""
68+
+ "Object o = null;"
69+
+ "try {"
70+
+ " if (b) throw new UncheckedException1();"
71+
+ "} catch (UncheckedException1 e) {"
72+
+ " o = e;"
73+
+ " throw e;"
74+
+ "} finally {"
75+
+ " if (o == null)"
76+
+ " j++;"
77+
+ "}").into(""
78+
79+
+ "Object o = null;"
80+
+ "try {"
81+
+ " if (b) throw new UncheckedException1();"
82+
+ "} catch (UncheckedException1 e) {"
83+
+ " o = e;"
84+
+ " throw e;"
85+
+ "} finally {"
86+
+ " if (o == null)" // the bug would cause this if to be optimized away as always true.
87+
+ " j++;"
88+
+ "}");
89+
}
90+
5891
public void testDeadIfBranch1() throws Exception {
5992
optimize("void",
6093
"int i = 1; if (i == 1) { int j = 2; } else { int j = 3; }").into(

dev/core/test/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilderTest.java

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,44 @@ public void testCatchThrowException1() throws Exception {
758758
);
759759
}
760760

761-
public void testCatchThrowUncatchedException() throws Exception {
761+
/**
762+
* Test case for issue 8115 (http://code.google.com/p/google-web-toolkit/issues/detail?id=8115)
763+
* @throws Exception
764+
*/
765+
public void testCatchThrowExceptionFinally() throws Exception {
766+
assertCfg("void",
767+
"try {",
768+
" if (b) throw uncheckedException2;",
769+
" k++;",
770+
"} catch (UncheckedException1 e) {",
771+
" throw e;",
772+
"} finally {",
773+
" j++;",
774+
"}").is(
775+
"BLOCK -> [*]",
776+
"TRY -> [*]",
777+
"BLOCK -> [*]",
778+
"STMT -> [*]",
779+
"READ(b) -> [*]",
780+
"COND (EntryPoint.b) -> [THEN=*, ELSE=1]",
781+
"STMT -> [*]",
782+
"READ(uncheckedException2) -> [*]",
783+
"THROW -> [2]",
784+
"1: STMT -> [*]",
785+
"READWRITE(k, null) -> [2]",
786+
"BLOCK -> [*]",
787+
"STMT -> [*]",
788+
"READ(e) -> [*]",
789+
"THROW -> [*]",
790+
"2: BLOCK -> [*]",
791+
"STMT -> [*]",
792+
"READWRITE(j, null) -> [*, *, *]",
793+
"END"
794+
);
795+
}
796+
797+
798+
public void testCatchThrowUncaughtException() throws Exception {
762799
assertCfg("void",
763800
"try {",
764801
" if (b) throw uncheckedException2;",

0 commit comments

Comments
 (0)