Subzero: Add a flag to mock up bounds checking on unsafe references.
The idea is that, before each load or store operation, we add a couple of compares/branches against the load/store address, one for the lower bound and one for the upper bound. The conditional branches would be to an error throwing routine, and would never be taken in practice. The compares might be against an immediate or a global location. So a load of [reg] will mock-expand to this:
cmp reg, 0
je label
cmp reg, 1
je label
label:
mov xxx, [reg]
We also make address mode inference less aggressive, because for a load of e.g. [eax+4*ecx], we can't compare that address expression against anything in any instruction, so we would have to reconstruct the address and undo at least part of the address mode inference.
The bounds-check mock is added for loads, stores, and rmw operations (with an exclusion for stores to the stack for out-arg pushes). There are probably a small handful of other cases that are missing the bounds check, but if we add the transformation inside legalize(), which is the most obvious place, we may add extra bounds checks because sometimes legalize() is called twice on the same operand.
BUG= none
R=ascull@google.com
Review URL: https://codereview.chromium.org/1338633005 .
diff --git a/src/IceClFlags.cpp b/src/IceClFlags.cpp
index 653d7f9..6c2e984 100644
--- a/src/IceClFlags.cpp
+++ b/src/IceClFlags.cpp
@@ -96,6 +96,9 @@
FunctionSections("ffunction-sections",
cl::desc("Emit functions into separate sections"));
+cl::opt<bool> MockBoundsCheck("mock-bounds-check",
+ cl::desc("Mock bounds checking on loads/stores"));
+
// Number of translation threads (in addition to the parser thread and
// the emitter thread). The special case of 0 means purely
// sequential, i.e. parser, translator, and emitter all within the
@@ -370,6 +373,7 @@
OutFlags.ForceMemIntrinOpt = false;
OutFlags.FunctionSections = false;
OutFlags.GenerateUnitTestMessages = false;
+ OutFlags.MockBoundsCheck = false;
OutFlags.PhiEdgeSplit = false;
OutFlags.RandomNopInsertion = false;
OutFlags.RandomRegAlloc = false;
@@ -432,6 +436,7 @@
OutFlags.setFunctionSections(::FunctionSections);
OutFlags.setNumTranslationThreads(::NumThreads);
OutFlags.setOptLevel(::OLevel);
+ OutFlags.setMockBoundsCheck(::MockBoundsCheck);
OutFlags.setPhiEdgeSplit(::EnablePhiEdgeSplit);
OutFlags.setRandomSeed(::RandomSeed);
OutFlags.setRandomizeAndPoolImmediatesOption(
diff --git a/src/IceClFlags.h b/src/IceClFlags.h
index f71d8f4..89f5783 100644
--- a/src/IceClFlags.h
+++ b/src/IceClFlags.h
@@ -87,6 +87,9 @@
GenerateUnitTestMessages = NewValue;
}
+ bool getMockBoundsCheck() const { return MockBoundsCheck; }
+ void setMockBoundsCheck(bool NewValue) { MockBoundsCheck = NewValue; }
+
bool getPhiEdgeSplit() const { return PhiEdgeSplit; }
void setPhiEdgeSplit(bool NewValue) { PhiEdgeSplit = NewValue; }
@@ -247,6 +250,7 @@
bool ForceMemIntrinOpt;
bool FunctionSections;
bool GenerateUnitTestMessages;
+ bool MockBoundsCheck;
bool PhiEdgeSplit;
bool RandomNopInsertion;
bool RandomRegAlloc;
diff --git a/src/IceTargetLowering.h b/src/IceTargetLowering.h
index 8620ac2..7184ff0 100644
--- a/src/IceTargetLowering.h
+++ b/src/IceTargetLowering.h
@@ -282,6 +282,7 @@
virtual void doAddressOptLoad() {}
virtual void doAddressOptStore() {}
+ virtual void doMockBoundsCheck(Operand *) {}
virtual void randomlyInsertNop(float Probability,
RandomNumberGenerator &RNG) = 0;
/// This gives the target an opportunity to post-process the lowered
diff --git a/src/IceTargetLoweringX86Base.h b/src/IceTargetLoweringX86Base.h
index 4044e36..e032ce9 100644
--- a/src/IceTargetLoweringX86Base.h
+++ b/src/IceTargetLoweringX86Base.h
@@ -187,6 +187,7 @@
void prelowerPhis() override;
void doAddressOptLoad() override;
void doAddressOptStore() override;
+ void doMockBoundsCheck(Operand *Opnd) override;
void randomlyInsertNop(float Probability,
RandomNumberGenerator &RNG) override;
diff --git a/src/IceTargetLoweringX86BaseImpl.h b/src/IceTargetLoweringX86BaseImpl.h
index 99a1aeb..c8bf29f 100644
--- a/src/IceTargetLoweringX86BaseImpl.h
+++ b/src/IceTargetLoweringX86BaseImpl.h
@@ -4066,14 +4066,16 @@
if (Func->getVMetadata()->isMultiBlock(Base) /* || Base->getUseCount() > 1*/)
return;
+ const bool MockBounds = Func->getContext()->getFlags().getMockBoundsCheck();
const VariablesMetadata *VMetadata = Func->getVMetadata();
bool Continue = true;
while (Continue) {
const Inst *Reason = nullptr;
if (matchTransitiveAssign(VMetadata, Base, Reason) ||
matchTransitiveAssign(VMetadata, Index, Reason) ||
- matchCombinedBaseIndex(VMetadata, Base, Index, Shift, Reason) ||
- matchShiftedIndex(VMetadata, Index, Shift, Reason) ||
+ (!MockBounds &&
+ matchCombinedBaseIndex(VMetadata, Base, Index, Shift, Reason)) ||
+ (!MockBounds && matchShiftedIndex(VMetadata, Index, Shift, Reason)) ||
matchOffsetBase(VMetadata, Base, Offset, Reason)) {
dumpAddressOpt(Func, Base, Index, Shift, Offset, Reason);
} else {
@@ -4104,6 +4106,65 @@
}
}
+/// Add a mock bounds check on the memory address before using it as a load or
+/// store operand. The basic idea is that given a memory operand [reg], we
+/// would first add bounds-check code something like:
+///
+/// cmp reg, <lb>
+/// jl out_of_line_error
+/// cmp reg, <ub>
+/// jg out_of_line_error
+///
+/// In reality, the specific code will depend on how <lb> and <ub> are
+/// represented, e.g. an immediate, a global, or a function argument.
+///
+/// As such, we need to enforce that the memory operand does not have the form
+/// [reg1+reg2], because then there is no simple cmp instruction that would
+/// suffice. However, we consider [reg+offset] to be OK because the offset is
+/// usually small, and so <ub> could have a safety buffer built in and then we
+/// could instead branch to a custom out_of_line_error that does the precise
+/// check and jumps back if it turns out OK.
+///
+/// For the purpose of mocking the bounds check, we'll do something like this:
+///
+/// cmp reg, 0
+/// je label
+/// cmp reg, 1
+/// je label
+/// label:
+///
+/// Also note that we don't need to add a bounds check to a dereference of a
+/// simple global variable address.
+template <class Machine>
+void TargetX86Base<Machine>::doMockBoundsCheck(Operand *Opnd) {
+ if (!Ctx->getFlags().getMockBoundsCheck())
+ return;
+ if (auto *Mem = llvm::dyn_cast<typename Traits::X86OperandMem>(Opnd)) {
+ if (Mem->getIndex()) {
+ llvm::report_fatal_error("doMockBoundsCheck: Opnd contains index reg");
+ }
+ Opnd = Mem->getBase();
+ }
+ // At this point Opnd could be nullptr, or Variable, or Constant, or perhaps
+ // something else. We only care if it is Variable.
+ auto *Var = llvm::dyn_cast_or_null<Variable>(Opnd);
+ if (Var == nullptr)
+ return;
+ // We use lowerStore() to copy out-args onto the stack. This creates a memory
+ // operand with the stack pointer as the base register. Don't do bounds
+ // checks on that.
+ if (Var->getRegNum() == Traits::RegisterSet::Reg_esp)
+ return;
+
+ typename Traits::Insts::Label *Label =
+ Traits::Insts::Label::create(Func, this);
+ _cmp(Opnd, Ctx->getConstantZero(IceType_i32));
+ _br(Traits::Cond::Br_e, Label);
+ _cmp(Opnd, Ctx->getConstantInt32(1));
+ _br(Traits::Cond::Br_e, Label);
+ Context.insert(Label);
+}
+
template <class Machine>
void TargetX86Base<Machine>::lowerLoad(const InstLoad *Load) {
// A Load instruction can be treated the same as an Assign instruction, after
@@ -4114,6 +4175,7 @@
Variable *DestLoad = Load->getDest();
Type Ty = DestLoad->getType();
Operand *Src0 = formMemoryOperand(Load->getSourceAddress(), Ty);
+ doMockBoundsCheck(Src0);
InstAssign *Assign = InstAssign::create(Func, DestLoad, Src0);
lowerAssign(Assign);
}
@@ -4307,6 +4369,7 @@
Operand *Addr = Inst->getAddr();
typename Traits::X86OperandMem *NewAddr =
formMemoryOperand(Addr, Value->getType());
+ doMockBoundsCheck(NewAddr);
Type Ty = NewAddr->getType();
if (!Traits::Is64Bit && Ty == IceType_i64) {
@@ -4661,6 +4724,7 @@
Operand *Src = RMW->getData();
Type Ty = Src->getType();
typename Traits::X86OperandMem *Addr = formMemoryOperand(RMW->getAddr(), Ty);
+ doMockBoundsCheck(Addr);
if (!Traits::Is64Bit && Ty == IceType_i64) {
Src = legalizeUndef(Src);
Operand *SrcLo = legalize(loOperand(Src), Legal_Reg | Legal_Imm);