[llvm] r315991 - Fix implicit null check with negative offset

Tue Oct 17 04:47:36 PDT 2017

Author: yuyichao
Date: Tue Oct 17 04:47:36 2017
New Revision: 315991

URL: http://llvm.org/viewvc/llvm-project?rev=315991&view=rev
Log:
Fix implicit null check with negative offset

Summary:
It seems that negative offset was accidentally allowed in D17967.
AFAICT small negative offset should be valid (always raise segfault) on all archs that I'm aware of (especially x86, which is the only one with this optimization enabled) and such case can be useful when loading hiden metadata from an object.

However, like the positive side, it should only be done within a certain limit.
For now, use the same limit on the positive side for the negative side.
A separate option can be added if needs appear.

Reviewers: mcrosier, skatkov

Reviewed By: skatkov

Subscribers: sanjoy, llvm-commits

Differential Revision: https://reviews.llvm.org/D38925

Modified:
    llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp
    llvm/trunk/test/CodeGen/X86/implicit-null-check-negative.ll
    llvm/trunk/test/CodeGen/X86/implicit-null-check.ll

Modified: llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp?rev=315991&r1=315990&r2=315991&view=diff
==============================================================================

--- llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp (original)
+++ llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp Tue Oct 17 04:47:36 2017
@@ -369,7 +369,7 @@ ImplicitNullChecks::isSuitableMemoryOp(M
   // We want the mem access to be issued at a sane offset from PointerReg,
   // so that if PointerReg is null then the access reliably page faults.
   if (!((MI.mayLoad() || MI.mayStore()) && !MI.isPredicable() &&
-        Offset < PageSize))
+        -PageSize < Offset && Offset < PageSize))
     return SR_Unsuitable;
 
   // Finally, check whether the current memory access aliases with previous one.

Modified: llvm/trunk/test/CodeGen/X86/implicit-null-check-negative.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/implicit-null-check-negative.ll?rev=315991&r1=315990&r2=315991&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/implicit-null-check-negative.ll (original)
+++ llvm/trunk/test/CodeGen/X86/implicit-null-check-negative.ll Tue Oct 17 04:47:36 2017
@@ -37,6 +37,22 @@ define i32 @imp_null_check_gep_load(i32*
   ret i32 %t
 }
 
+define i32 @imp_null_check_neg_gep_load(i32* %x) {
+ entry:
+  %c = icmp eq i32* %x, null
+  br i1 %c, label %is_null, label %not_null, !make.implicit !0
+
+ is_null:
+  ret i32 42
+
+ not_null:
+; null - 5000 * sizeof(i32) lies outside the null page and hence the
+; load to %t cannot be assumed to be reliably faulting.
+  %x.gep = getelementptr i32, i32* %x, i32 -5000
+  %t = load i32, i32* %x.gep
+  ret i32 %t
+}
+
 define i32 @imp_null_check_load_no_md(i32* %x) {
 ; This is fine, except it is missing the !make.implicit metadata.
  entry:

Modified: llvm/trunk/test/CodeGen/X86/implicit-null-check.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/implicit-null-check.ll?rev=315991&r1=315990&r2=315991&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/implicit-null-check.ll (original)
+++ llvm/trunk/test/CodeGen/X86/implicit-null-check.ll Tue Oct 17 04:47:36 2017
@@ -182,6 +182,28 @@ define void @imp_null_check_store(i32* %
   ret void
 }
 
+define i32 @imp_null_check_neg_gep_load(i32* %x) {
+; CHECK-LABEL: _imp_null_check_neg_gep_load:
+; CHECK: [[BB0_imp_null_check_neg_gep_load:L[^:]+]]:
+; CHECK: movl -128(%rdi), %eax
+; CHECK: retq
+; CHECK: [[BB1_imp_null_check_neg_gep_load:LBB7_[0-9]+]]:
+; CHECK: movl $42, %eax
+; CHECK: retq
+
+ entry:
+  %c = icmp eq i32* %x, null
+  br i1 %c, label %is_null, label %not_null, !make.implicit !0
+
+ is_null:
+  ret i32 42
+
+ not_null:
+  %x.gep = getelementptr i32, i32* %x, i32 -32
+  %t = load i32, i32* %x.gep
+  ret i32 %t
+}
+
 !0 = !{}
 
 ; CHECK-LABEL: __LLVM_FaultMaps:
@@ -194,7 +216,7 @@ define void @imp_null_check_store(i32* %
 ; CHECK-NEXT: .short 0
 
 ; # functions:
-; CHECK-NEXT: .long 7
+; CHECK-NEXT: .long 8
 
 ; FunctionAddr:
 ; CHECK-NEXT: .quad _imp_null_check_add_result
@@ -262,6 +284,19 @@ define void @imp_null_check_store(i32* %
 ; CHECK-NEXT: .long [[BB1_imp_null_check_load]]-_imp_null_check_load
 
 ; FunctionAddr:
+; CHECK-NEXT: .quad _imp_null_check_neg_gep_load
+; NumFaultingPCs
+; CHECK-NEXT: .long 1
+; Reserved:
+; CHECK-NEXT: .long 0
+; Fault[0].Type:
+; CHECK-NEXT: .long 1
+; Fault[0].FaultOffset:
+; CHECK-NEXT: .long [[BB0_imp_null_check_neg_gep_load]]-_imp_null_check_neg_gep_load
+; Fault[0].HandlerOffset:
+; CHECK-NEXT: .long [[BB1_imp_null_check_neg_gep_load]]-_imp_null_check_neg_gep_load
+
+; FunctionAddr:
 ; CHECK-NEXT: .quad _imp_null_check_store
 ; NumFaultingPCs
 ; CHECK-NEXT: .long 1
@@ -289,7 +324,7 @@ define void @imp_null_check_store(i32* %
 
 ; OBJDUMP: FaultMap table:
 ; OBJDUMP-NEXT: Version: 0x1
-; OBJDUMP-NEXT: NumFunctions: 7
+; OBJDUMP-NEXT: NumFunctions: 8
 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 5
 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
@@ -301,6 +336,8 @@ define void @imp_null_check_store(i32* %
 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 3
 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
+; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 4
+; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
 ; OBJDUMP-NEXT: Fault kind: FaultingStore, faulting PC offset: 0, handling PC offset: 7
 ; OBJDUMP-NEXT: FunctionAddress: 0x000000, NumFaultingPCs: 1
 ; OBJDUMP-NEXT: Fault kind: FaultingLoad, faulting PC offset: 0, handling PC offset: 11


