VIXL Dissassembler Integration Upgrade May. 2022

This patch is needed as the VIXL disassembler visitor instrumentation
interface has changed.

VIXL now has all of its instruction specific instrumentation functions
declared as private and therefore they cannot be overriden by the
ART disassembler class as it was before.

Now it is required by VIXL to override the main catch-all generic
Visit function, which now passes with it a metadata object. This
metadata is then used in the overriding function to select which
instrumentation to perform based on the instruction type detected
in the instruction sequence at runtime.

This patch is tested against ART with the public VIXL tag 6.3.0
(https://github.com/Linaro/vixl/tree/6.3.0) having been
merged into the AOSP ./external/vixl repo.

Test: test-art-target
Test: test-art-host
Test: test-art-host-vixl
Test: run-vixl-tests
Test: art_disassembler_tests
Change-Id: I9c2b936354763f0d116dfb7fe355841b9f833a34

7 files changed, 265 insertions, 13 deletions

diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index def3190978..d9a75e44a8 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -114,6 +114,7 @@ ART_TEST_MODULES_COMMON := \
     art_dexlayout_tests \
     art_dexlist_tests \
     art_dexoptanalyzer_tests \
+    art_disassembler_tests \
     art_hiddenapi_tests \
     art_imgdiag_tests \
     art_libartbase_tests \
diff --git a/build/apex/Android.bp b/build/apex/Android.bp
index de42615972..890de9dd2a 100644
--- a/build/apex/Android.bp
+++ b/build/apex/Android.bp
@@ -408,6 +408,7 @@ art_gtests = [
     "art_dexdump_tests",
     "art_dexlayout_tests",
     "art_dexlist_tests",
+    "art_disassembler_tests",
     "art_dexoptanalyzer_tests",
     "art_imgdiag_tests",
     "art_libartbase_tests",
diff --git a/build/apex/art_apex_test.py b/build/apex/art_apex_test.py
index c9eff8d4f9..1da90298bd 100755
--- a/build/apex/art_apex_test.py
+++ b/build/apex/art_apex_test.py
@@ -676,6 +676,7 @@ class TestingTargetChecker:
     self._checker.check_art_test_executable('art_dexlayout_tests')
     self._checker.check_art_test_executable('art_dexlist_tests')
     self._checker.check_art_test_executable('art_dexoptanalyzer_tests')
+    self._checker.check_art_test_executable('art_disassembler_tests')
     self._checker.check_art_test_executable('art_imgdiag_tests')
     self._checker.check_art_test_executable('art_libartbase_tests')
     self._checker.check_art_test_executable('art_libartpalette_tests')
diff --git a/disassembler/Android.bp b/disassembler/Android.bp
index 4b55673bf0..b7f758ffdc 100644
--- a/disassembler/Android.bp
+++ b/disassembler/Android.bp
@@ -104,7 +104,6 @@ art_cc_library {
             ],
         },
     },
-
     apex_available: [
         "com.android.art",
         "com.android.art.debug",
@@ -132,3 +131,38 @@ cc_library_headers {
         "com.android.art",
     ],
 }
+
+art_cc_defaults {
+    name: "art_disassembler_tests_defaults",
+    codegen: {
+        arm64: {
+            srcs: ["disassembler_arm64_test.cc"],
+        },
+    },
+}
+
+// Version of ART gtest `art_disassembler_tests` bundled with the ART APEX on target.
+// TODO(b/192274705): Remove this module when the migration to standalone ART gtests is complete.
+art_cc_test {
+    name: "art_disassembler_tests",
+    defaults: [
+        "art_gtest_defaults",
+        "art_disassembler_tests_defaults",
+    ],
+    static_libs: [
+        "libvixld",
+    ],
+}
+
+// Standalone version of ART gtest `art_disassembler_tests`,
+// not bundled with the ART APEX on target.
+art_cc_test {
+    name: "art_standalone_disassembler_tests",
+    defaults: [
+        "art_standalone_gtest_defaults",
+        "art_disassembler_tests_defaults",
+    ],
+    static_libs: [
+        "libvixl",
+    ],
+}
diff --git a/disassembler/disassembler_arm64.cc b/disassembler/disassembler_arm64.cc
index 0d51cfdc1a..23472a8dca 100644
--- a/disassembler/disassembler_arm64.cc
+++ b/disassembler/disassembler_arm64.cc
@@ -18,6 +18,8 @@
 
 #include <inttypes.h>
 
+#include <regex>
+
 #include <sstream>
 
 #include "android-base/logging.h"
@@ -58,9 +60,34 @@ void CustomDisassembler::AppendRegisterNameToOutput(const Instruction* instr,
   Disassembler::AppendRegisterNameToOutput(instr, reg);
 }
 
-void CustomDisassembler::VisitLoadLiteral(const Instruction* instr) {
-  Disassembler::VisitLoadLiteral(instr);
+void CustomDisassembler::Visit(vixl::aarch64::Metadata* metadata, const Instruction* instr) {
+  vixl::aarch64::Disassembler::Visit(metadata, instr);
+  const std::string& form = (*metadata)["form"];
+
+  // These regexs are long, but it is an attempt to match the mapping entry keys in the
+  // #define DEFAULT_FORM_TO_VISITOR_MAP(VISITORCLASS) in the file
+  // external/vixl/src/aarch64/decoder-visitor-map-aarch64.h
+  // for the ::VisitLoadLiteralInstr, ::VisitLoadStoreUnsignedOffset or ::VisitUnconditionalBranch
+  // function addresess key values.
+  // N.B. the mapping are many to one.
+  if (std::regex_match(form, std::regex("(ldrsw|ldr|prfm)_(32|64|d|b|h|q|s)_loadlit"))) {
+    VisitLoadLiteralInstr(instr);
+    return;
+  }
 
+  if (std::regex_match(form, std::regex(
+      "(ldrb|ldrh|ldrsb|ldrsh|ldrsw|ldr|prfm|strb|strh|str)_(32|64|d|b|h|q|s)_ldst_pos"))) {
+    VisitLoadStoreUnsignedOffsetInstr(instr);
+    return;
+  }
+
+  if (std::regex_match(form, std::regex("(bl|b)_only_branch_imm"))) {
+    VisitUnconditionalBranchInstr(instr);
+    return;
+  }
+}
+
+void CustomDisassembler::VisitLoadLiteralInstr(const Instruction* instr) {
   if (!read_literals_) {
     return;
   }
@@ -69,6 +96,7 @@ void CustomDisassembler::VisitLoadLiteral(const Instruction* instr) {
   // avoid trying to fetch invalid literals (we can encounter this when
   // interpreting raw data as instructions).
   void* data_address = instr->GetLiteralAddress<void*>();
+
   if (data_address < base_address_ || data_address >= end_address_) {
     AppendToOutput(" (?)");
     return;
@@ -97,17 +125,13 @@ void CustomDisassembler::VisitLoadLiteral(const Instruction* instr) {
   }
 }
 
-void CustomDisassembler::VisitLoadStoreUnsignedOffset(const Instruction* instr) {
-  Disassembler::VisitLoadStoreUnsignedOffset(instr);
-
+void CustomDisassembler::VisitLoadStoreUnsignedOffsetInstr(const Instruction* instr) {
   if (instr->GetRn() == TR) {
     AppendThreadOfsetName(instr);
   }
 }
 
-void CustomDisassembler::VisitUnconditionalBranch(const Instruction* instr) {
-  Disassembler::VisitUnconditionalBranch(instr);
-
+void CustomDisassembler::VisitUnconditionalBranchInstr(const Instruction* instr) {
   if (instr->Mask(UnconditionalBranchMask) == BL) {
     const Instruction* target = instr->GetImmPCOffsetTarget();
     if (target >= base_address_ &&
diff --git a/disassembler/disassembler_arm64.h b/disassembler/disassembler_arm64.h
index a895dfe823..d0443d2b39 100644
--- a/disassembler/disassembler_arm64.h
+++ b/disassembler/disassembler_arm64.h
@@ -47,16 +47,20 @@ class CustomDisassembler final : public vixl::aarch64::Disassembler {
   void AppendRegisterNameToOutput(const vixl::aarch64::Instruction* instr,
                                   const vixl::aarch64::CPURegister& reg) override;
 
+  // Intercepts the instruction flow captured by the parent method,
+  // to specially instrument for particular instruction types.
+  void Visit(vixl::aarch64::Metadata* metadata, const vixl::aarch64::Instruction* instr) override;
+
+ private:
   // Improve the disassembly of literal load instructions.
-  void VisitLoadLiteral(const vixl::aarch64::Instruction* instr) override;
+  void VisitLoadLiteralInstr(const vixl::aarch64::Instruction* instr);
 
   // Improve the disassembly of thread offset.
-  void VisitLoadStoreUnsignedOffset(const vixl::aarch64::Instruction* instr) override;
+  void VisitLoadStoreUnsignedOffsetInstr(const vixl::aarch64::Instruction* instr);
 
   // Improve the disassembly of branch to thunk jumping to pointer from thread entrypoint.
-  void VisitUnconditionalBranch(const vixl::aarch64::Instruction* instr) override;
+  void VisitUnconditionalBranchInstr(const vixl::aarch64::Instruction* instr);
 
- private:
   void AppendThreadOfsetName(const vixl::aarch64::Instruction* instr);
 
   // Indicate if the disassembler should read data loaded from literal pools.
diff --git a/disassembler/disassembler_arm64_test.cc b/disassembler/disassembler_arm64_test.cc
new file mode 100644
index 0000000000..8279ed8424
--- /dev/null
+++ b/disassembler/disassembler_arm64_test.cc
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+#include <inttypes.h>
+
+#include <regex>
+
+#include <sstream>
+
+#include "common_runtime_test.h"
+#include "disassembler_arm64.h"
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#include "aarch64/disasm-aarch64.h"
+#include "aarch64/macro-assembler-aarch64.h"
+#pragma GCC diagnostic pop
+
+
+using namespace vixl::aarch64;  // NOLINT(build/namespaces)
+
+namespace art {
+namespace arm64 {
+
+/**
+ * Fixture class for the ArtDisassemblerTest tests.
+ */
+class ArtDisassemblerTest : public CommonRuntimeTest {
+ public:
+  ArtDisassemblerTest() {
+  }
+
+  void SetupAssembly(uint64_t end_address) {
+    masm.GetCPUFeatures()->Combine(vixl::CPUFeatures::All());
+
+    disamOptions.reset(new DisassemblerOptions(/* absolute_addresses= */ true,
+                                               reinterpret_cast<uint8_t*>(0x0),
+                                               reinterpret_cast<uint8_t*>(end_address),
+                                               /* can_read_literals_= */ true,
+                                               &Thread::DumpThreadOffset<PointerSize::k64>));
+    disasm.reset(new CustomDisassembler(&*disamOptions));
+    decoder.AppendVisitor(disasm.get());
+    masm.SetGenerateSimulatorCode(false);
+  }
+
+  static constexpr size_t kMaxSizeGenerated = 1024;
+
+  template <typename LamdaType>
+  void ImplantInstruction(LamdaType fn) {
+    vixl::ExactAssemblyScope guard(&masm,
+                                   kMaxSizeGenerated,
+                                   vixl::ExactAssemblyScope::kMaximumSize);
+    fn();
+  }
+
+  // Appends an instruction to the existing buffer and then
+  // attempts to match the output of that instructions disassembly
+  // against a regex expression. Fails if no match is found.
+  template <typename LamdaType>
+  void CompareInstruction(LamdaType fn, const char* EXP) {
+    ImplantInstruction(fn);
+    masm.FinalizeCode();
+
+    // This gets the last instruction in the buffer.
+    // The end address of the buffer is at the end of the last instruction.
+    // sizeof(Instruction) is 1 byte as it in an empty class.
+    // Therefore we need to go back kInstructionSize * sizeof(Instruction) bytes
+    // in order to get to the start of the last instruction.
+    const Instruction* targetInstruction =
+        masm.GetBuffer()->GetEndAddress<Instruction*>()->
+            GetInstructionAtOffset(-static_cast<signed>(kInstructionSize));
+
+    decoder.Decode(targetInstruction);
+
+    const char* disassembly = disasm->GetOutput();
+
+    if (!std::regex_match(disassembly, std::regex(EXP))) {
+      const uint32_t encoding = static_cast<uint32_t>(targetInstruction->GetInstructionBits());
+
+      printf("\nEncoding: %08" PRIx32 "\nExpected: %s\nFound:    %s\n",
+             encoding,
+             EXP,
+             disassembly);
+
+      ADD_FAILURE();
+    }
+    printf("----\n%s\n", disassembly);
+  }
+
+  std::unique_ptr<CustomDisassembler> disasm;
+  std::unique_ptr<DisassemblerOptions> disamOptions;
+  Decoder decoder;
+  MacroAssembler masm;
+};
+
+#define IMPLANT(fn)                                                          \
+  do {                                                                       \
+    ImplantInstruction([&]() { this->masm.fn; });                            \
+  } while (0)
+
+#define COMPARE(fn, output)                                                  \
+  do {                                                                       \
+    CompareInstruction([&]() { this->masm.fn; }, (output));                  \
+  } while (0)
+
+// These tests map onto the named per instruction instrumentation functions in:
+// ART/art/disassembler/disassembler_arm.cc
+// Context can be found in the logic conditional on incoming instruction types and sequences in the
+// ART disassembler. As of writing the functionality we are testing for that of additional
+// diagnostic info being appended to the end of the ART disassembly output.
+TEST_F(ArtDisassemblerTest, LoadLiteralVisitBadAddress) {
+  SetupAssembly(0xffffff);
+
+  // Check we append an erroneous hint "(?)" for literal load instructions with
+  // out of scope literal pool value addresses.
+  COMPARE(ldr(x0, vixl::aarch64::Assembler::ImmLLiteral(1000)),
+      "ldr x0, pc\\+128000 \\(addr -?0x[0-9a-fA-F]+\\) \\(\\?\\)");
+}
+
+TEST_F(ArtDisassemblerTest, LoadLiteralVisit) {
+  SetupAssembly(0xffffffffffffffff);
+
+  // Test that we do not append anything for ineligible instruction.
+  COMPARE(ldr(x0, MemOperand(x18, 0)), "ldr x0, \\[x18\\]$");
+
+  // Check we do append some extra info in the right text format for valid literal load instruction.
+  COMPARE(ldr(x0, vixl::aarch64::Assembler::ImmLLiteral(0)),
+      "ldr x0, pc\\+0 \\(addr -?0x[0-9a-f]+\\) \\(0x[0-9a-fA-F]+ / -?[0-9]+\\)");
+  COMPARE(ldr(d0, vixl::aarch64::Assembler::ImmLLiteral(0)),
+      "ldr d0, pc\\+0 \\(addr -?0x[0-9a-f]+\\) \\([0-9]+.[0-9]+e(\\+|-)[0-9]+\\)");
+}
+
+TEST_F(ArtDisassemblerTest, LoadStoreUnsignedOffsetVisit) {
+  SetupAssembly(0xffffffffffffffff);
+
+  // Test that we do not append anything for ineligible instruction.
+  COMPARE(ldr(x0, MemOperand(x18, 8)), "ldr x0, \\[x18, #8\\]$");
+  // Test that we do append the function name if the instruction is a load from the address
+  // stored in the TR register.
+  COMPARE(ldr(x0, MemOperand(x19, 8)), "ldr x0, \\[tr, #8\\] ; thin_lock_thread_id");
+}
+
+TEST_F(ArtDisassemblerTest, UnconditionalBranchNoAppendVisit) {
+  SetupAssembly(0xffffffffffffffff);
+
+  vixl::aarch64::Label destination;
+  masm.Bind(&destination);
+
+  IMPLANT(ldr(x16, MemOperand(x18, 0)));
+
+  // Test that we do not append anything for ineligible instruction.
+  COMPARE(bl(&destination),
+      "bl #-0x4 \\(addr -?0x[0-9a-f]+\\)$");
+}
+
+TEST_F(ArtDisassemblerTest, UnconditionalBranchVisit) {
+  SetupAssembly(0xffffffffffffffff);
+
+  vixl::aarch64::Label destination;
+  masm.Bind(&destination);
+
+  IMPLANT(ldr(x16, MemOperand(x19, 0)));
+  IMPLANT(br(x16));
+
+  // Test that we do append the function name if the instruction is a branch
+  // to a load that reads data from the address in the TR register, into the IPO register
+  // followed by a BR branching using the IPO register.
+  COMPARE(bl(&destination),
+      "bl #-0x8 \\(addr -?0x[0-9a-f]+\\) ; state_and_flags");
+}
+
+
+}  // namespace arm64
+}  // namespace art