From b554343e1d72104f0b26e749c1877d0566667a85 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Mon, 2 May 2016 18:51:34 -0700 Subject: [PATCH] Fix vtable corruption. Due to failing to keep track of superclass implementations of interface methods we could end up in situations where methods were placed onto a class's vtable multiple times. This could cause virtual and interface dispatches on subclasses to fail by causing corruption of the subclass's vtable and iftable. Bug: 28333278 (cherry picked from commit d6c2bfaff8850a9a02ee9b75cf8c96eadd8d5c69) Change-Id: I37d9740ca912daf37cdf9ff82697bbc5db46177a --- runtime/class_linker.cc | 9 ++++++++ test/960-default-smali/expected.txt | 16 ++++++++++++++ test/960-default-smali/src/Foo.java | 20 ++++++++++++++++++ test/960-default-smali/src/Fooer.java | 19 +++++++++++++++++ test/960-default-smali/src/K.java | 17 +++++++++++++++ test/960-default-smali/src/L.java | 17 +++++++++++++++ test/960-default-smali/src/M.java | 21 +++++++++++++++++++ test/960-default-smali/src/classes.xml | 38 ++++++++++++++++++++++++++++++++++ 8 files changed, 157 insertions(+) create mode 100644 test/960-default-smali/src/Foo.java create mode 100644 test/960-default-smali/src/Fooer.java create mode 100644 test/960-default-smali/src/K.java create mode 100644 test/960-default-smali/src/L.java create mode 100644 test/960-default-smali/src/M.java diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index c92304f64..e7a560b39 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -6635,6 +6635,15 @@ bool ClassLinker::LinkInterfaceMethods( // The method is not overridable by a default method (i.e. it is directly implemented // in some class). Therefore move onto the next interface method. continue; + } else { + // If the super-classes method is override-able by a default method we need to keep + // track of it since though it is override-able it is not guaranteed to be 'overridden'. + // If it turns out not to be overridden and we did not keep track of it we might add it + // to the vtable twice, causing corruption in this class and possibly any subclasses. + DCHECK(vtable_impl == nullptr || vtable_impl == supers_method) + << "vtable_impl was " << PrettyMethod(vtable_impl) << " and not 'nullptr' or " + << PrettyMethod(supers_method) << " as expected. IFTable appears to be corrupt!"; + vtable_impl = supers_method; } } // If we haven't found it yet we should search through the interfaces for default methods. diff --git a/test/960-default-smali/expected.txt b/test/960-default-smali/expected.txt index 7671eed5d..f3db93f87 100644 --- a/test/960-default-smali/expected.txt +++ b/test/960-default-smali/expected.txt @@ -82,3 +82,19 @@ J-virtual A.SayHiTwice()='Hi Hi ' J-interface Greeter.SayHiTwice()='Hi Hi ' J-virtual J.SayHiTwice()='Hi Hi ' End testing for type J +Testing for type K +K-interface Foo.bar()='foobar' +K-virtual K.bar()='foobar' +End testing for type K +Testing for type L +L-interface Foo.bar()='foobar' +L-virtual K.bar()='foobar' +L-virtual L.bar()='foobar' +End testing for type L +Testing for type M +M-interface Foo.bar()='BAZ!' +M-interface Fooer.bar()='BAZ!' +M-virtual K.bar()='BAZ!' +M-virtual L.bar()='BAZ!' +M-virtual M.bar()='BAZ!' +End testing for type M diff --git a/test/960-default-smali/src/Foo.java b/test/960-default-smali/src/Foo.java new file mode 100644 index 000000000..ed5b35f47 --- /dev/null +++ b/test/960-default-smali/src/Foo.java @@ -0,0 +1,20 @@ +/* + * Copyright 2016 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. + */ +interface Foo { + public default String bar() { + return "foobar"; + } +} diff --git a/test/960-default-smali/src/Fooer.java b/test/960-default-smali/src/Fooer.java new file mode 100644 index 000000000..d8a5f6163 --- /dev/null +++ b/test/960-default-smali/src/Fooer.java @@ -0,0 +1,19 @@ +/* + * Copyright 2016 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. + */ + +interface Fooer extends Foo { + public String bar(); +} diff --git a/test/960-default-smali/src/K.java b/test/960-default-smali/src/K.java new file mode 100644 index 000000000..4426be719 --- /dev/null +++ b/test/960-default-smali/src/K.java @@ -0,0 +1,17 @@ +/* + * Copyright 2016 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 K implements Foo { } diff --git a/test/960-default-smali/src/L.java b/test/960-default-smali/src/L.java new file mode 100644 index 000000000..c08ab72a9 --- /dev/null +++ b/test/960-default-smali/src/L.java @@ -0,0 +1,17 @@ +/* + * Copyright 2016 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 L extends K { } diff --git a/test/960-default-smali/src/M.java b/test/960-default-smali/src/M.java new file mode 100644 index 000000000..affe7e9c9 --- /dev/null +++ b/test/960-default-smali/src/M.java @@ -0,0 +1,21 @@ +/* + * Copyright 2016 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 M extends L implements Fooer { + public String bar() { + return "BAZ!"; + } +} diff --git a/test/960-default-smali/src/classes.xml b/test/960-default-smali/src/classes.xml index 0aa41f7fb..f3e50c570 100644 --- a/test/960-default-smali/src/classes.xml +++ b/test/960-default-smali/src/classes.xml @@ -81,6 +81,27 @@ + + + + Foo + + + + + + + + + + + + Fooer + + + bar + + @@ -123,5 +144,22 @@ GetPlace + + + + + + bar + + + + + + Foo + + + bar + + -- 2.11.0