Skip to content

Commit

Permalink
VIXL Dissassembler Integration Upgrade May. 2022
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Greg Cawthorne authored and Ulya Trofimovich committed Jun 9, 2022
1 parent b205efd commit bb3ef5a
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 13 deletions.
1 change: 1 addition & 0 deletions build/Android.gtest.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions build/apex/Android.bp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions build/apex/art_apex_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ def run(self):
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')
Expand Down
36 changes: 35 additions & 1 deletion disassembler/Android.bp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ art_cc_library {
],
},
},

apex_available: [
"com.android.art",
"com.android.art.debug",
Expand Down Expand Up @@ -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",
],
}
40 changes: 32 additions & 8 deletions disassembler/disassembler_arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include <inttypes.h>

#include <regex>

#include <sstream>

#include "android-base/logging.h"
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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_ &&
Expand Down
12 changes: 8 additions & 4 deletions disassembler/disassembler_arm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
187 changes: 187 additions & 0 deletions disassembler/disassembler_arm64_test.cc
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit bb3ef5a

Please sign in to comment.