Subzero: Fix x86-64 memory sandboxing.

Commit 2e4b960b (https://codereview.chromium.org/2084793002), which made address mode inference more aggressive, exposed a long-standing bug in memory sandboxing, which now manifests in 164.gzip. The problem is in sandboxed code like this: movl %eax, %eax movb 64(%rsp,%rax), %cl If %eax starts out -1, the mov address is something close to %rsp+4GB, instead of %rsp+63. To fix this, we need to use an lea instruction in more cases - specifically when the sandboxed address has an index register and the non-symbolic portion of the offset is nonzero. BUG= none R=jpp@chromium.org Review URL: https://codereview.chromium.org/2097193003 .

Subzero: Fix x86-64 memory sandboxing.
ecbf2c4b · Jim Stichnoth · 633394ce · ecbf2c4b
Commit ecbf2c4b authored Jun 25, 2016 by Jim Stichnoth
Hide whitespace changes
Inline Side-by-side

Showing with 17 additions and 7 deletions

IceTargetLoweringX8664.cpp src/IceTargetLoweringX8664.cpp +17 -7

No files found.
--- 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;