From: David Brazdil Date: Wed, 24 Jun 2015 13:23:56 +0000 (+0100) Subject: ART: Stop creating a fallthrough block for Goto X-Git-Tag: android-x86-7.1-r1~889^2~898^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=fe659462e7d58bb2585b1bd029f9e08fd9dd32ae;p=android-x86%2Fart.git ART: Stop creating a fallthrough block for Goto Optimizing's Builder used to create a basic block after a Goto under the assumption that control flow can fall through. Bug: 19084197 Change-Id: Id85f31df98a4177466750d3cd0bc8bb74782ca2d --- diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc index c49752642..48ec497f3 100644 --- a/compiler/optimizing/builder.cc +++ b/compiler/optimizing/builder.cc @@ -378,15 +378,15 @@ bool HGraphBuilder::ComputeBranchTargets(const uint16_t* code_ptr, dex_pc += instruction.SizeInCodeUnits(); code_ptr += instruction.SizeInCodeUnits(); - if (code_ptr >= code_end) { - if (instruction.CanFlowThrough()) { + if (instruction.CanFlowThrough()) { + if (code_ptr >= code_end) { // In the normal case we should never hit this but someone can artificially forge a dex // file to fall-through out the method code. In this case we bail out compilation. return false; + } else if (FindBlockStartingAt(dex_pc) == nullptr) { + block = new (arena_) HBasicBlock(graph_, dex_pc); + branch_targets_.Put(dex_pc, block); } - } else if (FindBlockStartingAt(dex_pc) == nullptr) { - block = new (arena_) HBasicBlock(graph_, dex_pc); - branch_targets_.Put(dex_pc, block); } } else if (instruction.IsSwitch()) { SwitchTable table(instruction, dex_pc, instruction.Opcode() == Instruction::SPARSE_SWITCH); diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 01eb2d7f8..feb9afe99 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -1035,9 +1035,8 @@ HBasicBlock* HBasicBlock::SplitAfter(HInstruction* cursor) { bool HBasicBlock::IsSingleGoto() const { HLoopInformation* loop_info = GetLoopInformation(); - // TODO: Remove the null check b/19084197. - return GetFirstInstruction() != nullptr - && GetPhis().IsEmpty() + DCHECK(EndsWithControlFlowInstruction()); + return GetPhis().IsEmpty() && GetFirstInstruction() == GetLastInstruction() && GetLastInstruction()->IsGoto() // Back edges generate the suspend check. diff --git a/test/517-checker-builder-fallthrough/expected.txt b/test/517-checker-builder-fallthrough/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/517-checker-builder-fallthrough/info.txt b/test/517-checker-builder-fallthrough/info.txt new file mode 100644 index 000000000..5789b5657 --- /dev/null +++ b/test/517-checker-builder-fallthrough/info.txt @@ -0,0 +1,2 @@ +Regression test for optimizing's builder which created a block under +the assumption that Goto can fall through. diff --git a/test/517-checker-builder-fallthrough/smali/TestCase.smali b/test/517-checker-builder-fallthrough/smali/TestCase.smali new file mode 100644 index 000000000..bc9502b31 --- /dev/null +++ b/test/517-checker-builder-fallthrough/smali/TestCase.smali @@ -0,0 +1,67 @@ +# Copyright (C) 2015 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +.class public LTestCase; + +.super Ljava/lang/Object; + +.field public static value:Z + +## CHECK-START: int TestCase.testCase(int) builder (after) + +## CHECK: name "B0" +## CHECK: <> IntConstant 0 + +## CHECK: name "B1" +## CHECK: successors "B5" "B2" +## CHECK: StoreLocal [v0,<>] +## CHECK: If + +## CHECK: name "B2" +## CHECK: successors "B4" +## CHECK: Goto + +## CHECK: name "B3" +## CHECK: Return + +## CHECK: name "B4" +## CHECK: successors "B3" +## CHECK: Goto + +## CHECK: name "B5" +## CHECK: successors "B3" +## CHECK: Goto + +.method public static testCase(I)I + .registers 2 + + const/4 v0, 0 + packed-switch v0, :switch_data + goto :default + + :switch_data + .packed-switch 0x0 + :case + .end packed-switch + + :return + return v1 + + :default + goto :return + + :case + goto :return + +.end method diff --git a/test/517-checker-builder-fallthrough/src/Main.java b/test/517-checker-builder-fallthrough/src/Main.java new file mode 100644 index 000000000..23d94e697 --- /dev/null +++ b/test/517-checker-builder-fallthrough/src/Main.java @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.lang.reflect.*; + +public class Main { + + public static int runTest(int input) throws Exception { + Class c = Class.forName("TestCase"); + Method m = c.getMethod("testCase", new Class[] { int.class }); + return (Integer) m.invoke(null, input); + } + + public static void main(String[] args) throws Exception { + if (runTest(42) != 42) { + throw new Error("Expected 42"); + } + } +}