Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Unified Diff: src/IceTargetLoweringX8664.cpp

Issue 2097193003: Subzero: Fix x86-64 memory sandboxing. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/IceTargetLoweringX8664.cpp
diff --git a/src/IceTargetLoweringX8664.cpp b/src/IceTargetLoweringX8664.cpp
index 37b7b36a5441d512a7acc5e095650d4e8bd1d884..9e6bf7488f84cd121a9ed945337458057a9a935a 100644
--- a/src/IceTargetLoweringX8664.cpp
+++ b/src/IceTargetLoweringX8664.cpp
@@ -408,18 +408,28 @@ Traits::X86OperandMem *TargetX8664::_sandbox_mem_reference(X86OperandMem *Mem) {
// NeedsLea is a flag indicating whether Mem needs to be materialized to a GPR
// prior to being used. A LEA is needed if Mem.Offset is a constant
- // relocatable, or if Mem.Offset is negative. In both these cases, the LEA is
- // needed to ensure the sandboxed memory operand will only use the lower
- // 32-bits of T+Offset.
+ // relocatable with a nonzero offset, or if Mem.Offset is a nonzero immediate;
+ // but only when the address mode contains a "user" register other than the
+ // rsp/rbp/r15 base. In both these cases, the LEA is needed to ensure the
+ // sandboxed memory operand will only use the lower 32-bits of T+Offset.
bool NeedsLea = false;
- if (Offset != nullptr) {
- if (llvm::isa<ConstantRelocatable>(Offset)) {
- NeedsLea = true;
+ if (!Mem->getIsRebased()) {
+ bool IsOffsetZero = false;
+ if (Offset == nullptr) {
+ IsOffsetZero = true;
+ } else if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(Offset)) {
+ IsOffsetZero = (CR->getOffset() == 0);
} else if (const auto *Imm = llvm::dyn_cast<ConstantInteger32>(Offset)) {
- NeedsLea = Imm->getValue() < 0;
+ IsOffsetZero = (Imm->getValue() == 0);
} else {
llvm::report_fatal_error("Unexpected Offset type.");
}
+ if (!IsOffsetZero) {
+ if (Base != nullptr && Base != ZeroReg)
+ NeedsLea = true;
+ if (Index != nullptr && Index != ZeroReg)
+ NeedsLea = true;
+ }
}
RegNumT RegNum, RegNum32;
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698