Skip to content

Commit

Permalink
Revert "[jitlink/rtdydl][checker] Add TargetFlag dependent disassembl…
Browse files Browse the repository at this point in the history 
…er switching support"

This reverts commit 4b17c81.

Caused buildbot failures:
https://lab.llvm.org/buildbot/#/builders/230/builds/18341
https://lab.llvm.org/buildbot/#/builders/109/builds/73169
https://lab.llvm.org/buildbot/#/builders/67/builds/12597
  • Loading branch information
Tom Weaver authored and Tom Weaver committed Sep 8, 2023
1 parent feb7bea commit 4871a9c
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 253 deletions.
6 changes: 4 additions & 2 deletions llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,10 @@ class Symbol {
this->S = static_cast<uint8_t>(S);
}

/// Get the target flags of this Symbol.
TargetFlagsType getTargetFlags() const { return TargetFlags; }
/// Check whether the given target flags are set for this Symbol.
bool hasTargetFlags(TargetFlagsType Flags) const {
return static_cast<TargetFlagsType>(TargetFlags) & Flags;
}

/// Set the target flags for this Symbol.
void setTargetFlags(TargetFlagsType Flags) {
Expand Down
3 changes: 0 additions & 3 deletions llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ namespace llvm {
namespace jitlink {
namespace aarch32 {

/// Check whether the given target flags are set for this Symbol.
bool hasTargetFlags(Symbol &Sym, TargetFlagsType Flags);

/// JITLink-internal AArch32 fixup kinds
enum EdgeKind_aarch32 : Edge::Kind {

Expand Down
27 changes: 6 additions & 21 deletions llvm/include/llvm/ExecutionEngine/RuntimeDyldChecker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ExecutionEngine/JITSymbol.h"
#include "llvm/Support/Endian.h"
#include "llvm/TargetParser/SubtargetFeature.h"
#include "llvm/TargetParser/Triple.h"
#include <optional>

#include <cstdint>
Expand All @@ -31,9 +29,6 @@ class RuntimeDyld;
class RuntimeDyldCheckerImpl;
class raw_ostream;

/// Holds target-specific properties for a symbol.
using TargetFlagsType = uint8_t;

/// RuntimeDyld invariant checker for verifying that RuntimeDyld has
/// correctly applied relocations.
///
Expand Down Expand Up @@ -83,11 +78,10 @@ class RuntimeDyldChecker {
public:
MemoryRegionInfo() = default;

/// Constructor for symbols/sections with content and TargetFlag.
MemoryRegionInfo(ArrayRef<char> Content, JITTargetAddress TargetAddress,
TargetFlagsType TargetFlags)
/// Constructor for symbols/sections with content.
MemoryRegionInfo(ArrayRef<char> Content, JITTargetAddress TargetAddress)
: ContentPtr(Content.data()), Size(Content.size()),
TargetAddress(TargetAddress), TargetFlags(TargetFlags) {}
TargetAddress(TargetAddress) {}

/// Constructor for zero-fill symbols/sections.
MemoryRegionInfo(uint64_t Size, JITTargetAddress TargetAddress)
Expand Down Expand Up @@ -133,20 +127,10 @@ class RuntimeDyldChecker {
/// Return the target address for this region.
JITTargetAddress getTargetAddress() const { return TargetAddress; }

/// Get the target flags for this Symbol.
TargetFlagsType getTargetFlags() const { return TargetFlags; }

/// Set the target flags for this Symbol.
void setTargetFlags(TargetFlagsType Flags) {
assert(Flags <= 1 && "Add more bits to store more than one flag");
TargetFlags = Flags;
}

private:
const char *ContentPtr = nullptr;
uint64_t Size = 0;
JITTargetAddress TargetAddress = 0;
TargetFlagsType TargetFlags = 0;
};

using IsSymbolValidFunction = std::function<bool(StringRef Symbol)>;
Expand All @@ -164,8 +148,9 @@ class RuntimeDyldChecker {
GetSectionInfoFunction GetSectionInfo,
GetStubInfoFunction GetStubInfo,
GetGOTInfoFunction GetGOTInfo,
support::endianness Endianness, Triple TT,
SubtargetFeatures TF, raw_ostream &ErrStream);
support::endianness Endianness,
MCDisassembler *Disassembler, MCInstPrinter *InstPrinter,
raw_ostream &ErrStream);
~RuntimeDyldChecker();

/// Check a single expression against the attached RuntimeDyld
Expand Down
13 changes: 4 additions & 9 deletions llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ namespace llvm {
namespace jitlink {
namespace aarch32 {

/// Check whether the given target flags are set for this Symbol.
bool hasTargetFlags(Symbol &Sym, TargetFlagsType Flags) {
return static_cast<TargetFlagsType>(Sym.getTargetFlags()) & Flags;
}

/// Encode 22-bit immediate value for branch instructions without J1J2 range
/// extension (formats B T4, BL T1 and BLX T2).
///
Expand Down Expand Up @@ -292,7 +287,7 @@ Error applyFixupData(LinkGraph &G, Block &B, const Edge &E) {
int64_t Addend = E.getAddend();
Symbol &TargetSymbol = E.getTarget();
uint64_t TargetAddress = TargetSymbol.getAddress().getValue();
assert(!hasTargetFlags(TargetSymbol, ThumbSymbol));
assert(!TargetSymbol.hasTargetFlags(ThumbSymbol));

// Regular data relocations have size 4, alignment 1 and write the full 32-bit
// result to the place; no need for overflow checking. There are three
Expand Down Expand Up @@ -346,14 +341,14 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
int64_t Addend = E.getAddend();
Symbol &TargetSymbol = E.getTarget();
uint64_t TargetAddress = TargetSymbol.getAddress().getValue();
if (hasTargetFlags(TargetSymbol, ThumbSymbol))
if (TargetSymbol.hasTargetFlags(ThumbSymbol))
TargetAddress |= 0x01;

switch (Kind) {
case Thumb_Jump24: {
if (!checkOpcode<Thumb_Jump24>(R))
return makeUnexpectedOpcodeError(G, R, Kind);
if (!hasTargetFlags(TargetSymbol, ThumbSymbol))
if (!(TargetSymbol.hasTargetFlags(ThumbSymbol)))
return make_error<JITLinkError>("Branch relocation needs interworking "
"stub when bridging to ARM: " +
StringRef(G.getEdgeKindName(Kind)));
Expand All @@ -380,7 +375,7 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,

// The call instruction itself is Thumb. The call destination can either be
// Thumb or Arm. We use BL to stay in Thumb and BLX to change to Arm.
bool TargetIsArm = !hasTargetFlags(TargetSymbol, ThumbSymbol);
bool TargetIsArm = !TargetSymbol.hasTargetFlags(ThumbSymbol);
bool InstrIsBlx = (R.Lo & FixupInfo<Thumb_Call>::LoBitNoBlx) == 0;
if (TargetIsArm != InstrIsBlx) {
if (LLVM_LIKELY(TargetIsArm)) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ExecutorAddr getJITSymbolPtrForSymbol(Symbol &Sym, const Triple &TT) {
case Triple::armeb:
case Triple::thumb:
case Triple::thumbeb:
if (hasTargetFlags(Sym, aarch32::ThumbSymbol)) {
if (Sym.hasTargetFlags(aarch32::ThumbSymbol)) {
// Set LSB to indicate thumb target
assert(Sym.isCallable() && "Only callable symbols can have thumb flag");
assert((Sym.getAddress().getValue() & 0x01) == 0 && "LSB is clear");
Expand Down
162 changes: 15 additions & 147 deletions llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,9 @@
#include "RuntimeDyldCheckerImpl.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCDisassembler/MCDisassembler.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstPrinter.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCRegisterInfo.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/MC/MCTargetOptions.h"
#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/MSVCErrorWorkarounds.h"
#include "llvm/Support/MemoryBuffer.h"
Expand All @@ -32,19 +25,6 @@

using namespace llvm;

namespace {
struct TargetInfo {
const Target *TheTarget;
std::unique_ptr<MCSubtargetInfo> STI;
std::unique_ptr<MCRegisterInfo> MRI;
std::unique_ptr<MCAsmInfo> MAI;
std::unique_ptr<MCContext> Ctx;
std::unique_ptr<MCDisassembler> Disassembler;
std::unique_ptr<MCInstrInfo> MII;
std::unique_ptr<MCInstPrinter> InstPrinter;
};
} // anonymous namespace

namespace llvm {

// Helper class that implements the language evaluated by RuntimeDyldChecker.
Expand Down Expand Up @@ -296,20 +276,6 @@ class RuntimeDyldCheckerExprEval {
"");

unsigned OpIdx = OpIdxExpr.getValue();

auto printInst = [this](StringRef Symbol, MCInst Inst,
raw_string_ostream &ErrMsgStream) {
auto TI = getTargetInfo(
Checker.getTripleFromTargetFlag(Checker.getTargetFlag(Symbol)));
if (auto E = TI.takeError()) {
errs() << "Error obtaining instruction printer: "
<< toString(std::move(E)) << "\n";
return std::make_pair(EvalResult(ErrMsgStream.str()), "");
}
Inst.dump_pretty(ErrMsgStream, TI->InstPrinter.get());
return std::make_pair(EvalResult(ErrMsgStream.str()), "");
};

if (OpIdx >= Inst.getNumOperands()) {
std::string ErrMsg;
raw_string_ostream ErrMsgStream(ErrMsg);
Expand All @@ -318,8 +284,8 @@ class RuntimeDyldCheckerExprEval {
<< "'. Instruction has only "
<< format("%i", Inst.getNumOperands())
<< " operands.\nInstruction is:\n ";

return printInst(Symbol, Inst, ErrMsgStream);
Inst.dump_pretty(ErrMsgStream, Checker.InstPrinter);
return std::make_pair(EvalResult(ErrMsgStream.str()), "");
}

const MCOperand &Op = Inst.getOperand(OpIdx);
Expand All @@ -328,8 +294,9 @@ class RuntimeDyldCheckerExprEval {
raw_string_ostream ErrMsgStream(ErrMsg);
ErrMsgStream << "Operand '" << format("%i", OpIdx) << "' of instruction '"
<< Symbol << "' is not an immediate.\nInstruction is:\n ";
Inst.dump_pretty(ErrMsgStream, Checker.InstPrinter);

return printInst(Symbol, Inst, ErrMsgStream);
return std::make_pair(EvalResult(ErrMsgStream.str()), "");
}

return std::make_pair(EvalResult(Op.getImm()), RemainingExpr);
Expand Down Expand Up @@ -720,101 +687,31 @@ class RuntimeDyldCheckerExprEval {

bool decodeInst(StringRef Symbol, MCInst &Inst, uint64_t &Size,
int64_t Offset) const {
auto TI = getTargetInfo(
Checker.getTripleFromTargetFlag(Checker.getTargetFlag(Symbol)));

if (auto E = TI.takeError()) {
errs() << "Error obtaining disassembler: " << toString(std::move(E))
<< "\n";
return false;
}

MCDisassembler *Dis = Checker.Disassembler;
StringRef SymbolMem = Checker.getSymbolContent(Symbol);
ArrayRef<uint8_t> SymbolBytes(SymbolMem.bytes_begin() + Offset,
SymbolMem.size() - Offset);

MCDisassembler::DecodeStatus S =
TI->Disassembler->getInstruction(Inst, Size, SymbolBytes, 0, nulls());
Dis->getInstruction(Inst, Size, SymbolBytes, 0, nulls());

return (S == MCDisassembler::Success);
}

Expected<TargetInfo>
getTargetInfo(const Triple &TT,
const SubtargetFeatures &TF = SubtargetFeatures()) const {

auto TripleName = TT.str();
std::string ErrorStr;
const Target *TheTarget =
TargetRegistry::lookupTarget(TripleName, ErrorStr);
if (!TheTarget)
return make_error<StringError>("Error accessing target '" + TripleName +
"': " + ErrorStr,
inconvertibleErrorCode());

std::unique_ptr<MCSubtargetInfo> STI(
TheTarget->createMCSubtargetInfo(TripleName, "", TF.getString()));
if (!STI)
return make_error<StringError>("Unable to create subtarget for " +
TripleName,
inconvertibleErrorCode());

std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
if (!MRI)
return make_error<StringError>("Unable to create target register info "
"for " +
TripleName,
inconvertibleErrorCode());

MCTargetOptions MCOptions;
std::unique_ptr<MCAsmInfo> MAI(
TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
if (!MAI)
return make_error<StringError>("Unable to create target asm info " +
TripleName,
inconvertibleErrorCode());

auto Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(),
MRI.get(), STI.get());

std::unique_ptr<MCDisassembler> Disassembler(
TheTarget->createMCDisassembler(*STI, *Ctx));
if (!Disassembler)
return make_error<StringError>("Unable to create disassembler for " +
TripleName,
inconvertibleErrorCode());

std::unique_ptr<MCInstrInfo> MII(TheTarget->createMCInstrInfo());
if (!MII)
return make_error<StringError>("Unable to create instruction info for" +
TripleName,
inconvertibleErrorCode());

std::unique_ptr<MCInstPrinter> InstPrinter(TheTarget->createMCInstPrinter(
Triple(TripleName), 0, *MAI, *MII, *MRI));
if (!InstPrinter)
return make_error<StringError>(
"Unable to create instruction printer for" + TripleName,
inconvertibleErrorCode());

return TargetInfo({TheTarget, std::move(STI), std::move(MRI),
std::move(MAI), std::move(Ctx), std::move(Disassembler),
std::move(MII), std::move(InstPrinter)});
}
};
} // namespace llvm

RuntimeDyldCheckerImpl::RuntimeDyldCheckerImpl(
IsSymbolValidFunction IsSymbolValid, GetSymbolInfoFunction GetSymbolInfo,
GetSectionInfoFunction GetSectionInfo, GetStubInfoFunction GetStubInfo,
GetGOTInfoFunction GetGOTInfo, support::endianness Endianness, Triple TT,
SubtargetFeatures TF, raw_ostream &ErrStream)
GetGOTInfoFunction GetGOTInfo, support::endianness Endianness,
MCDisassembler *Disassembler, MCInstPrinter *InstPrinter,
raw_ostream &ErrStream)
: IsSymbolValid(std::move(IsSymbolValid)),
GetSymbolInfo(std::move(GetSymbolInfo)),
GetSectionInfo(std::move(GetSectionInfo)),
GetStubInfo(std::move(GetStubInfo)), GetGOTInfo(std::move(GetGOTInfo)),
Endianness(Endianness), TT(std::move(TT)), TF(std::move(TF)),
ErrStream(ErrStream) {}
Endianness(Endianness), Disassembler(Disassembler),
InstPrinter(InstPrinter), ErrStream(ErrStream) {}

bool RuntimeDyldCheckerImpl::check(StringRef CheckExpr) const {
CheckExpr = CheckExpr.trim();
Expand Down Expand Up @@ -925,36 +822,6 @@ StringRef RuntimeDyldCheckerImpl::getSymbolContent(StringRef Symbol) const {
return {SymInfo->getContent().data(), SymInfo->getContent().size()};
}

TargetFlagsType RuntimeDyldCheckerImpl::getTargetFlag(StringRef Symbol) const {
auto SymInfo = GetSymbolInfo(Symbol);
if (!SymInfo) {
logAllUnhandledErrors(SymInfo.takeError(), errs(), "RTDyldChecker: ");
return TargetFlagsType{};
}
return SymInfo->getTargetFlags();
}

Triple
RuntimeDyldCheckerImpl::getTripleFromTargetFlag(TargetFlagsType Flag) const {
Triple TheTriple = TT;

switch (TT.getArch()) {
case Triple::ArchType::arm:
if (~Flag & 0x1)
return TT;
TheTriple.setArchName((Twine("thumb") + TT.getArchName().substr(3)).str());
return TheTriple;
case Triple::ArchType::thumb:
if (Flag & 0x1)
return TT;
TheTriple.setArchName((Twine("arm") + TT.getArchName().substr(5)).str());
return TheTriple;

default:
return TT;
}
}

std::pair<uint64_t, std::string> RuntimeDyldCheckerImpl::getSectionAddr(
StringRef FileName, StringRef SectionName, bool IsInsideLoad) const {

Expand Down Expand Up @@ -1017,12 +884,13 @@ std::pair<uint64_t, std::string> RuntimeDyldCheckerImpl::getStubOrGOTAddrFor(
RuntimeDyldChecker::RuntimeDyldChecker(
IsSymbolValidFunction IsSymbolValid, GetSymbolInfoFunction GetSymbolInfo,
GetSectionInfoFunction GetSectionInfo, GetStubInfoFunction GetStubInfo,
GetGOTInfoFunction GetGOTInfo, support::endianness Endianness, Triple TT,
SubtargetFeatures TF, raw_ostream &ErrStream)
GetGOTInfoFunction GetGOTInfo, support::endianness Endianness,
MCDisassembler *Disassembler, MCInstPrinter *InstPrinter,
raw_ostream &ErrStream)
: Impl(::std::make_unique<RuntimeDyldCheckerImpl>(
std::move(IsSymbolValid), std::move(GetSymbolInfo),
std::move(GetSectionInfo), std::move(GetStubInfo),
std::move(GetGOTInfo), Endianness, std::move(TT), std::move(TF),
std::move(GetGOTInfo), Endianness, Disassembler, InstPrinter,
ErrStream)) {}

RuntimeDyldChecker::~RuntimeDyldChecker() = default;
Expand Down

0 comments on commit 4871a9c

Please sign in to comment.