From 4667b35dbf14877ced91b77375e1acd922b02bbf Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 9 Sep 2021 17:06:46 +0100 Subject: vdex: add checks in the event of file corruption. It's unclear yet why the vdex files are being corrupted. But system server, which is reading these vdex files should be robust to any vdex corruption. Bug: 199309980 Bug: 199395272 Test: test.py (cherry picked from commit a74a7071490e47e1b5590cc19726f1620fd0ee43) (cherry picked from commit a35586522fd398fb2845a9ae8427aa4853f155be) Merged-In: Ia85ab8b23a0be4069cfa058a86fdf561f1ceb432 Change-Id: I558238fd7cc0d7bc2f89f989ad53db8eb7a2eb24 --- libdexfile/dex/dex_file.h | 2 +- runtime/oat_file.cc | 110 +++++++++++++++++++++++++++++++++++++++++----- runtime/vdex_file.cc | 3 +- runtime/vdex_file.h | 5 ++- 4 files changed, 104 insertions(+), 16 deletions(-) diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h index 5363b00c24..e3027fc95f 100644 --- a/libdexfile/dex/dex_file.h +++ b/libdexfile/dex/dex_file.h @@ -738,7 +738,7 @@ class DexFile { } // Used by oat writer. - void SetOatDexFile(OatDexFile* oat_dex_file) const { + void SetOatDexFile(const OatDexFile* oat_dex_file) const { oat_dex_file_ = oat_dex_file; } diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 8f653c2282..14e7a1b60e 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -168,7 +168,7 @@ class OatFileBase : public OatFile { bool Setup(int zip_fd, ArrayRef dex_filenames, std::string* error_msg); - void Setup(const std::vector& dex_files); + bool Setup(const std::vector& dex_files, std::string* error_msg); // Setters exposed for ElfOatFile. @@ -476,18 +476,71 @@ static bool ReadIndexBssMapping(OatFile* oat_file, return true; } -void OatFileBase::Setup(const std::vector& dex_files) { +static bool ComputeAndCheckTypeLookupTableData(const DexFile::Header& header, + const uint8_t* type_lookup_table_start, + const VdexFile* vdex_file, + const uint8_t** type_lookup_table_data, + std::string* error_msg) { + if (type_lookup_table_start == nullptr || + reinterpret_cast(type_lookup_table_start)[0] == 0) { + *type_lookup_table_data = nullptr; + return true; + } + + *type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + size_t expected_table_size = TypeLookupTable::RawDataLength(header.class_defs_size_); + size_t found_size = reinterpret_cast(type_lookup_table_start)[0]; + if (UNLIKELY(found_size != expected_table_size)) { + *error_msg = + StringPrintf("In vdex file '%s' unexpected type lookup table size: found %zu, expected %zu", + vdex_file->GetName().c_str(), + found_size, + expected_table_size); + return false; + } + if (UNLIKELY(!vdex_file->Contains(*type_lookup_table_data))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid type lookup table pointer %p not in [%p, %p]", + vdex_file->GetName().c_str(), + type_lookup_table_data, + vdex_file->Begin(), + vdex_file->End()); + return false; + } + if (UNLIKELY(!vdex_file->Contains(*type_lookup_table_data + expected_table_size - 1))) { + *error_msg = + StringPrintf("In vdex file '%s' found overflowing type lookup table %p not in [%p, %p]", + vdex_file->GetName().c_str(), + type_lookup_table_data + expected_table_size, + vdex_file->Begin(), + vdex_file->End()); + return false; + } + if (UNLIKELY(!IsAligned<4>(type_lookup_table_start))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid type lookup table alignment %p", + vdex_file->GetName().c_str(), + type_lookup_table_start); + return false; + } + return true; +} + +bool OatFileBase::Setup(const std::vector& dex_files, std::string* error_msg) { uint32_t i = 0; const uint8_t* type_lookup_table_start = nullptr; for (const DexFile* dex_file : dex_files) { - type_lookup_table_start = vdex_->GetNextTypeLookupTableData(type_lookup_table_start, i++); std::string dex_location = dex_file->GetLocation(); std::string canonical_location = DexFileLoader::GetDexCanonicalLocation(dex_location.c_str()); + type_lookup_table_start = vdex_->GetNextTypeLookupTableData(type_lookup_table_start, i++); const uint8_t* type_lookup_table_data = nullptr; - if (type_lookup_table_start != nullptr && - (reinterpret_cast(type_lookup_table_start[0]) != 0)) { - type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + if (!ComputeAndCheckTypeLookupTableData(dex_file->GetHeader(), + type_lookup_table_start, + vdex_.get(), + &type_lookup_table_data, + error_msg)) { + return false; } // Create an OatDexFile and add it to the owning container. OatDexFile* oat_dex_file = new OatDexFile( @@ -497,7 +550,6 @@ void OatFileBase::Setup(const std::vector& dex_files) { dex_location, canonical_location, type_lookup_table_data); - dex_file->SetOatDexFile(oat_dex_file); oat_dex_files_storage_.push_back(oat_dex_file); // Add the location and canonical location (if different) to the oat_dex_files_ table. @@ -508,6 +560,11 @@ void OatFileBase::Setup(const std::vector& dex_files) { oat_dex_files_.Put(canonical_key, oat_dex_file); } } + // Now that we've created all the OatDexFile, update the dex files. + for (i = 0; i < dex_files.size(); ++i) { + dex_files[i]->SetOatDexFile(oat_dex_files_storage_[i]); + } + return true; } bool OatFileBase::Setup(int zip_fd, @@ -1583,7 +1640,11 @@ class OatFileBackedByVdex final : public OatFileBase { oat_file->SetVdex(vdex_file.release()); oat_file->SetupHeader(dex_files.size()); // Initialize OatDexFiles. - oat_file->Setup(dex_files); + std::string error_msg; + if (!oat_file->Setup(dex_files, &error_msg)) { + LOG(WARNING) << "Could not create in-memory vdex file: " << error_msg; + return nullptr; + } return oat_file.release(); } @@ -1601,6 +1662,25 @@ class OatFileBackedByVdex final : public OatFileBase { for (const uint8_t* dex_file_start = vdex_file->GetNextDexFileData(nullptr, i); dex_file_start != nullptr; dex_file_start = vdex_file->GetNextDexFileData(dex_file_start, ++i)) { + const DexFile::Header* header = reinterpret_cast(dex_file_start); + if (UNLIKELY(!vdex_file->Contains(dex_file_start))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid dex file pointer %p not in [%p, %p]", + dex_location.c_str(), + dex_file_start, + vdex_file->Begin(), + vdex_file->End()); + return nullptr; + } + if (UNLIKELY(!vdex_file->Contains(dex_file_start + header->file_size_ - 1))) { + *error_msg = + StringPrintf("In vdex file '%s' found overflowing dex file %p not in [%p, %p]", + dex_location.c_str(), + dex_file_start + header->file_size_, + vdex_file->Begin(), + vdex_file->End()); + return nullptr; + } if (UNLIKELY(!DexFileLoader::IsVersionAndMagicValid(dex_file_start))) { *error_msg = StringPrintf("In vdex file '%s' found dex file with invalid dex file version", @@ -1612,10 +1692,14 @@ class OatFileBackedByVdex final : public OatFileBase { std::string canonical_location = DexFileLoader::GetDexCanonicalLocation(location.c_str()); type_lookup_table_start = vdex_file->GetNextTypeLookupTableData(type_lookup_table_start, i); const uint8_t* type_lookup_table_data = nullptr; - if (type_lookup_table_start != nullptr && - (reinterpret_cast(type_lookup_table_start[0]) != 0)) { - type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + if (!ComputeAndCheckTypeLookupTableData(*header, + type_lookup_table_start, + vdex_file, + &type_lookup_table_data, + error_msg)) { + return nullptr; } + OatDexFile* oat_dex_file = new OatDexFile(oat_file.get(), dex_file_start, vdex_file->GetLocationChecksum(i), @@ -1656,7 +1740,9 @@ class OatFileBackedByVdex final : public OatFileBase { return nullptr; } oat_file->SetupHeader(oat_file->external_dex_files_.size()); - oat_file->Setup(MakeNonOwningPointerVector(oat_file->external_dex_files_)); + if (!oat_file->Setup(MakeNonOwningPointerVector(oat_file->external_dex_files_), error_msg)) { + return nullptr; + } } return oat_file.release(); diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc index 29efd4016f..967167961c 100644 --- a/runtime/vdex_file.cc +++ b/runtime/vdex_file.cc @@ -192,7 +192,8 @@ const uint8_t* VdexFile::GetNextTypeLookupTableData(const uint8_t* cursor, } else { const uint8_t* data = cursor + sizeof(uint32_t) + reinterpret_cast(cursor)[0]; // TypeLookupTables are required to be 4 byte aligned. the OatWriter makes sure they are. - CHECK_ALIGNED(data, 4); + // We don't check this here to be defensive against corrupted vdex files. + // Callers should check the returned value matches their expectations. return data; } } diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h index eb8b81742b..a66ff88fb2 100644 --- a/runtime/vdex_file.h +++ b/runtime/vdex_file.h @@ -157,8 +157,6 @@ class VdexFile { return size; } - bool IsDexSectionValid() const; - bool HasDexSection() const { return GetSectionHeader(VdexSection::kDexFileSection).section_size != 0u; } @@ -251,6 +249,9 @@ class VdexFile { const uint8_t* Begin() const { return mmap_.Begin(); } const uint8_t* End() const { return mmap_.End(); } size_t Size() const { return mmap_.Size(); } + bool Contains(const uint8_t* pointer) const { + return pointer >= Begin() && pointer < End(); + } const VdexFileHeader& GetVdexFileHeader() const { return *reinterpret_cast(Begin()); -- cgit v1.2.3 From 9c70cdb384578e5c37b828cf4f629c929d743d1f Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Sun, 29 Aug 2021 16:51:55 +0100 Subject: Partial LSE: handle all kinds of infinite loops. A subgraph could also have an infinite loop. Test: 826-inifinite-loop Bug: 196246395 Merged-In: Ifd1e1ae0f42dfe2cc156386fc166101c20748fc9 (cherry picked from commit 96dadefd24331f6808cb287048269ba772423c33) Change-Id: I410ec26aefbc42629ba7dbe5a19a87399cefe396 (cherry picked from commit 1c7eeae26429bf76c47a1eaf6eddf514c000ba71) --- compiler/optimizing/execution_subgraph.cc | 13 +++++-------- test/826-infinite-loop/expected-stderr.txt | 0 test/826-infinite-loop/expected-stdout.txt | 0 test/826-infinite-loop/info.txt | 2 ++ test/826-infinite-loop/src/Main.java | 26 ++++++++++++++++++++++++++ 5 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 test/826-infinite-loop/expected-stderr.txt create mode 100644 test/826-infinite-loop/expected-stdout.txt create mode 100644 test/826-infinite-loop/info.txt create mode 100644 test/826-infinite-loop/src/Main.java diff --git a/compiler/optimizing/execution_subgraph.cc b/compiler/optimizing/execution_subgraph.cc index 6d105668c0..66fdfcda5b 100644 --- a/compiler/optimizing/execution_subgraph.cc +++ b/compiler/optimizing/execution_subgraph.cc @@ -86,12 +86,6 @@ void ExecutionSubgraph::Prune() { ScopedArenaVector> results( graph_->GetBlocks().size(), temporaries.Adapter(kArenaAllocLSA)); unreachable_blocks_.ClearAllBits(); - // TODO We should support infinite loops as well. - if (UNLIKELY(graph_->GetExitBlock() == nullptr)) { - // Infinite loop - valid_ = false; - return; - } // Fills up the 'results' map with what we need to add to update // allowed_successors in order to prune sink nodes. bool start_reaches_end = false; @@ -170,8 +164,11 @@ void ExecutionSubgraph::Prune() { << "current path size: " << current_path.size() << " cur_block id: " << cur_block->GetBlockId() << " entry id " << graph_->GetEntryBlock()->GetBlockId(); - DCHECK(!visiting.IsBitSet(id)) - << "Somehow ended up in a loop! This should have been caught before now! " << id; + if (visiting.IsBitSet(id)) { + // TODO We should support infinite loops as well. + start_reaches_end = false; + break; + } std::bitset& result = results[id]; if (cur_block == graph_->GetExitBlock()) { start_reaches_end = true; diff --git a/test/826-infinite-loop/expected-stderr.txt b/test/826-infinite-loop/expected-stderr.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/826-infinite-loop/expected-stdout.txt b/test/826-infinite-loop/expected-stdout.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/826-infinite-loop/info.txt b/test/826-infinite-loop/info.txt new file mode 100644 index 0000000000..13a89d8ca2 --- /dev/null +++ b/test/826-infinite-loop/info.txt @@ -0,0 +1,2 @@ +Regression test for partial escape elimination, which used to crash when +visiting an infinite loop. diff --git a/test/826-infinite-loop/src/Main.java b/test/826-infinite-loop/src/Main.java new file mode 100644 index 0000000000..85bcb28baf --- /dev/null +++ b/test/826-infinite-loop/src/Main.java @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2021 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. + */ + +final class Main { + public static void main(String[] args) { + Object o = new Object(); + if (args.length == 0) { + while (true) { + System.out.println(new Object()); + } + } + } +} -- cgit v1.2.3 From a5f978ceb39619b96633d95fd4430f26d8e402b7 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Mon, 13 Sep 2021 14:07:10 +0100 Subject: Temporarily disable partial LSE. Due to a bug in it. Add a regression test. Bug: 197818595 Test: 828-partial-lse (cherry picked from commit 18074d2b59ae56dcfccea770ceb515215c8eb53f) (cherry picked from commit a38b7355a603772424a8212840849094ce0ddeb3) Merged-In: I65da4f7ef09cd2a1f6c4b21799ecd7a42c1adac2 Change-Id: I3eb52dc091264e617bb711260fb4950d15c7f50b --- compiler/optimizing/load_store_elimination.h | 2 +- test/530-checker-lse/src/Main.java | 257 --------------------------- test/639-checker-code-sinking/src/Main.java | 15 +- test/828-partial-lse/expected-stderr.txt | 0 test/828-partial-lse/expected-stdout.txt | 0 test/828-partial-lse/info.txt | 2 + test/828-partial-lse/src/Main.java | 53 ++++++ 7 files changed, 60 insertions(+), 269 deletions(-) create mode 100644 test/828-partial-lse/expected-stderr.txt create mode 100644 test/828-partial-lse/expected-stdout.txt create mode 100644 test/828-partial-lse/info.txt create mode 100644 test/828-partial-lse/src/Main.java diff --git a/compiler/optimizing/load_store_elimination.h b/compiler/optimizing/load_store_elimination.h index e73ef5ef34..6ad2eb2c51 100644 --- a/compiler/optimizing/load_store_elimination.h +++ b/compiler/optimizing/load_store_elimination.h @@ -27,7 +27,7 @@ class LoadStoreElimination : public HOptimization { public: // Whether or not we should attempt partial Load-store-elimination which // requires additional blocks and predicated instructions. - static constexpr bool kEnablePartialLSE = true; + static constexpr bool kEnablePartialLSE = false; // Controls whether to enable VLOG(compiler) logs explaining the transforms taking place. static constexpr bool kVerboseLoggingMode = false; diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java index 35f1dc2ee4..a707a8ae5c 100644 --- a/test/530-checker-lse/src/Main.java +++ b/test/530-checker-lse/src/Main.java @@ -3964,249 +3964,6 @@ public class Main { return res; } - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (before) - /// CHECK-DAG: ParameterValue - /// CHECK-DAG: NewInstance - /// CHECK-DAG: InvokeStaticOrDirect - /// CHECK-DAG: InvokeStaticOrDirect - /// CHECK-DAG: InvokeStaticOrDirect - /// CHECK-DAG: InstanceFieldSet - /// CHECK-DAG: InstanceFieldSet - /// CHECK-DAG: InstanceFieldSet - /// CHECK-DAG: InstanceFieldGet - /// CHECK-DAG: InstanceFieldGet - // - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK-DAG: ParameterValue - /// CHECK-DAG: NewInstance - /// CHECK-DAG: Phi - // - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK: InvokeStaticOrDirect - /// CHECK: InvokeStaticOrDirect - /// CHECK: InvokeStaticOrDirect - // - /// CHECK-NOT: InvokeStaticOrDirect - - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:false - /// CHECK-NOT: InstanceFieldSet predicated:false - // - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK: InstanceFieldGet - // - /// CHECK-NOT: InstanceFieldGet - // - /// CHECK-START: int Main.$noinline$testPartialEscape2(TestClass, boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - // - /// CHECK-NOT: PredicatedInstanceFieldGet - private static int $noinline$testPartialEscape2(TestClass obj, boolean escape) { - TestClass i = new SubTestClass(); - if ($noinline$getBoolean(escape)) { - i.next = obj; - $noinline$Escape(i); - } else { - i.next = obj; - } - $noinline$clobberObservables(); - // Predicated-get - TestClass res = i.next; - // Predicated-set - i.next = null; - return res.i; - } - - /// CHECK-START: float Main.$noinline$testPartialEscape3_float(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: float Main.$noinline$testPartialEscape3_float(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: float Main.$noinline$testPartialEscape3_float(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: float Main.$noinline$testPartialEscape3_float(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static float $noinline$testPartialEscape3_float(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.floatField -= 1f; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.floatField *= 10; - // Predicated get - return tc.floatField; - } - - /// CHECK-START: double Main.$noinline$testPartialEscape3_double(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: double Main.$noinline$testPartialEscape3_double(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: double Main.$noinline$testPartialEscape3_double(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: double Main.$noinline$testPartialEscape3_double(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static double $noinline$testPartialEscape3_double(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.doubleField -= 1d; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.doubleField *= 10; - // Predicated get - return tc.doubleField; - } - - /// CHECK-START: short Main.$noinline$testPartialEscape3_short(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: short Main.$noinline$testPartialEscape3_short(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: short Main.$noinline$testPartialEscape3_short(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: short Main.$noinline$testPartialEscape3_short(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static short $noinline$testPartialEscape3_short(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.shortField -= 1; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.shortField *= 10; - // Predicated get - return tc.shortField; - } - - /// CHECK-START: byte Main.$noinline$testPartialEscape3_byte(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: byte Main.$noinline$testPartialEscape3_byte(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: byte Main.$noinline$testPartialEscape3_byte(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: byte Main.$noinline$testPartialEscape3_byte(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static byte $noinline$testPartialEscape3_byte(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.byteField -= 1; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.byteField *= 10; - // Predicated get - return tc.byteField; - } - - /// CHECK-START: int Main.$noinline$testPartialEscape3_int(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: int Main.$noinline$testPartialEscape3_int(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: int Main.$noinline$testPartialEscape3_int(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: int Main.$noinline$testPartialEscape3_int(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static int $noinline$testPartialEscape3_int(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.intField -= 1; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.intField *= 10; - // Predicated get - return tc.intField; - } - - /// CHECK-START: long Main.$noinline$testPartialEscape3_long(boolean) load_store_elimination (before) - /// CHECK-NOT: Phi - /// CHECK-NOT: PredicatedInstanceFieldGet - // - /// CHECK-START: long Main.$noinline$testPartialEscape3_long(boolean) load_store_elimination (after) - /// CHECK: Phi - /// CHECK: Phi - /// CHECK-NOT: Phi - // - /// CHECK-START: long Main.$noinline$testPartialEscape3_long(boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet predicated:true - /// CHECK-NOT: InstanceFieldSet predicated:true - // - /// CHECK-START: long Main.$noinline$testPartialEscape3_long(boolean) load_store_elimination (after) - /// CHECK: PredicatedInstanceFieldGet - /// CHECK-NOT: PredicatedInstanceFieldGet - private static long $noinline$testPartialEscape3_long(boolean escape) { - TestClass4 tc = new TestClass4(); - if ($noinline$getBoolean(escape)) { - $noinline$Escape4(tc); - } else { - tc.longField -= 1; - } - // Partial escape - $noinline$clobberObservables(); - // Predicated set - tc.longField *= 10; - // Predicated get - return tc.longField; - } - private static void $noinline$clobberObservables() {} static void assertLongEquals(long result, long expected) { @@ -4613,19 +4370,5 @@ public class Main { assertLongEquals(testOverlapLoop(50), 7778742049l); assertIntEquals($noinline$testPartialEscape1(new TestClass(), true), 1); assertIntEquals($noinline$testPartialEscape1(new TestClass(), false), 0); - assertIntEquals($noinline$testPartialEscape2(new TestClass(), true), 1); - assertIntEquals($noinline$testPartialEscape2(new TestClass(), false), 0); - assertDoubleEquals($noinline$testPartialEscape3_double(true), -20d); - assertDoubleEquals($noinline$testPartialEscape3_double(false), -40d); - assertFloatEquals($noinline$testPartialEscape3_float(true), -20f); - assertFloatEquals($noinline$testPartialEscape3_float(false), -40f); - assertIntEquals($noinline$testPartialEscape3_int(true), -20); - assertIntEquals($noinline$testPartialEscape3_int(false), -40); - assertIntEquals($noinline$testPartialEscape3_byte(true), -20); - assertIntEquals($noinline$testPartialEscape3_byte(false), -40); - assertIntEquals($noinline$testPartialEscape3_short(true), -20); - assertIntEquals($noinline$testPartialEscape3_short(false), -40); - assertLongEquals($noinline$testPartialEscape3_long(true), -20); - assertLongEquals($noinline$testPartialEscape3_long(false), -40); } } diff --git a/test/639-checker-code-sinking/src/Main.java b/test/639-checker-code-sinking/src/Main.java index 28fa57cfbf..91c3ec48ab 100644 --- a/test/639-checker-code-sinking/src/Main.java +++ b/test/639-checker-code-sinking/src/Main.java @@ -110,8 +110,6 @@ public class Main { /// CHECK: <> IntConstant 42 /// CHECK: begin_block /// CHECK: <> LoadClass class_name:Main - /// CHECK: If - /// CHECK: begin_block /// CHECK: <> NewInstance [<>] /// CHECK: InstanceFieldSet [<>,<>] /// CHECK: Throw @@ -121,14 +119,14 @@ public class Main { /// CHECK-NOT: NewInstance /// CHECK: If /// CHECK: begin_block + /// CHECK: <> LoadClass class_name:java.lang.Error + /// CHECK-NOT: begin_block /// CHECK: <> LoadClass class_name:Main /// CHECK-NOT: begin_block /// CHECK: <> NewInstance [<>] /// CHECK-NOT: begin_block /// CHECK: InstanceFieldSet [<>,<>] /// CHECK-NOT: begin_block - /// CHECK: <> LoadClass class_name:java.lang.Error - /// CHECK-NOT: begin_block /// CHECK: <> NewInstance [<>] /// CHECK-NOT: begin_block /// CHECK: Throw [<>] @@ -325,12 +323,7 @@ public class Main { /// CHECK: <> IntConstant 42 /// CHECK: <> IntConstant 43 /// CHECK: <> LoadClass class_name:Main - /// CHECK: If - /// CHECK: begin_block - // Moved to throw block by partial-LSE and DCE. /// CHECK: <> NewInstance [<>] - // These were moved by partial LSE and order of sets is not observable and are - // in an arbitrary order. /// CHECK-DAG: InstanceFieldSet [<>,<>] /// CHECK-DAG: InstanceFieldSet [<>,<>] /// CHECK: Throw @@ -342,14 +335,14 @@ public class Main { /// CHECK-NOT: NewInstance /// CHECK: If /// CHECK: begin_block + /// CHECK: <> LoadClass class_name:java.lang.Error + /// CHECK-NOT: begin_block /// CHECK: <> LoadClass class_name:Main /// CHECK: <> NewInstance [<>] /// CHECK-NOT: begin_block /// CHECK-DAG: InstanceFieldSet [<>,<>] /// CHECK-DAG: InstanceFieldSet [<>,<>] /// CHECK-NOT: begin_block - /// CHECK: <> LoadClass class_name:java.lang.Error - /// CHECK-NOT: begin_block /// CHECK: NewInstance [<>] /// CHECK: Throw /// CHECK-NOT: InstanceFieldSet diff --git a/test/828-partial-lse/expected-stderr.txt b/test/828-partial-lse/expected-stderr.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/828-partial-lse/expected-stdout.txt b/test/828-partial-lse/expected-stdout.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/828-partial-lse/info.txt b/test/828-partial-lse/info.txt new file mode 100644 index 0000000000..8d28cbdbfa --- /dev/null +++ b/test/828-partial-lse/info.txt @@ -0,0 +1,2 @@ +Regression test for the partial LSE pass, see +https://issuetracker.google.com/197818595. diff --git a/test/828-partial-lse/src/Main.java b/test/828-partial-lse/src/Main.java new file mode 100644 index 0000000000..2dde0eff95 --- /dev/null +++ b/test/828-partial-lse/src/Main.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2021 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 Main { + + public static void $noinline$testMain(Main m) { + expectEquals(0, m.myField); + } + + public static void main(String[] args) { + $noinline$doTest(true); + $noinline$doTest(false); + } + + public static void $noinline$doTest(boolean testValue) { + Main m = new Main(); + // LSE will find that this store can be removed, as both branches override the value with a new + // one. + m.myField = 42; + if (testValue) { + // LSE will remove this store as well, as it's the value after the store of 42 is removed. + m.myField = 0; + // This makes sure `m` gets materialized. At this point, the bug is that the partial LSE + // optimization thinks the value incoming this block for `m.myField` is 42, however that + // store, as well as the store to 0, have been removed. + $noinline$testMain(m); + } else { + m.myField = 3; + expectEquals(3, m.myField); + } + } + + public static void expectEquals(int expected, int actual) { + if (expected != actual) { + throw new Error("Expected " + expected + ", got " + actual); + } + } + + int myField = 0; +} -- cgit v1.2.3 From 020c79e5f2b61924d6e21e8162f52e992d7b30fe Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Mon, 13 Sep 2021 17:28:00 +0100 Subject: Update .oat version after disabling partial LSE. This will ensure we don't take any .oat file that has the bogus generated code. (cherry picked from commit 776a1c1447be1e504c2013c5d170d08ef4907d7f) (cherry picked from commit 8246b0cbfd4c5e19596e2dac0c1377c56f444d5f) Test: test.py Bug: 197981962 Merged-In: Ic14d18d310bdcd408c1f6e2777ef53a041fb2f12 Change-Id: Ic66ee1998f490c15b95279f3b4881f9581772190 --- runtime/oat.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/oat.h b/runtime/oat.h index ab45b84888..31a328d979 100644 --- a/runtime/oat.h +++ b/runtime/oat.h @@ -32,8 +32,8 @@ class InstructionSetFeatures; class PACKED(4) OatHeader { public: static constexpr std::array kOatMagic { { 'o', 'a', 't', '\n' } }; - // Last oat version changed reason: Apex versions in key/value store. - static constexpr std::array kOatVersion { { '1', '9', '5', '\0' } }; + // Last oat version changed reason: Disable partial LSE b/197818595. + static constexpr std::array kOatVersion { { '1', '9', '9', '\0' } }; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; static constexpr const char* kDebuggableKey = "debuggable"; -- cgit v1.2.3