PNaCl: Fix ReplacePtrsWithInts to handle some corner cases correctly

Running the LLVM test suite with the ReplacePtrsWithInts pass enabled
produced a single failure (in MultiSource/Applications/SPASS),
revealing a corner case in which a mixture of forward and backward
references plus a bitcast causes the pass to fail (see
@forwards_reference() in the test).

The problem was that we were doing replaceAllUsesWith() on a
placeholder value too early.  RewriteMap was mapping a bitcast to a
placeholder P, but RewriteMap's reference to P didn't get updated by
P->replaceAllUsesWith() and P became a dangling pointer.

The fix is:

 * Change convert() to strip off casts first, so that RewriteMap isn't
   used for mapping casts to converted values.

 * Defer the replaceAllUsesWith() calls until after creating all the
   replacement instructions.  This makes the pass more robust against
   instruction ordering in the input.

   This requires debug instrinsics to be updated in a separate pass,
   because replaceAllUsesWith() doesn't work for references by
   metadata nodes.

This also fixes some pathological corner cases of cyclic references in
unreachable blocks.

Fix indentation in one place.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3343
TEST=replace-ptrs-with-ints.ll + LLVM test suite

Review URL: https://codereview.chromium.org/15761003

1 files changed, 84 insertions, 4 deletions

diff --git a/test/Transforms/NaCl/replace-ptrs-with-ints.ll b/test/Transforms/NaCl/replace-ptrs-with-ints.ll
index c45ca322cd..bab6101848 100644
--- a/test/Transforms/NaCl/replace-ptrs-with-ints.ll
+++ b/test/Transforms/NaCl/replace-ptrs-with-ints.ll
@@ -26,16 +26,79 @@ declare i8* @declared_func(i8*, i64)
 ; CHECK: declare i32 @declared_func(i32, i64)
 
 
-define void @self_reference(i8* %ptr) {
+define void @self_reference_phi(i8* %ptr) {
 entry:
   br label %loop
 loop:
   %x = phi i8* [ %x, %loop ], [ %ptr, %entry ]
   br label %loop
 }
-; CHECK: define void @self_reference(i32 %ptr) {
+; CHECK: define void @self_reference_phi(i32 %ptr) {
 ; CHECK: %x = phi i32 [ %x, %loop ], [ %ptr, %entry ]
 
+; Self-referencing bitcasts are possible in unreachable basic blocks.
+; It is not very likely that we will encounter this, but we handle it
+; for completeness.
+define void @self_reference_bitcast(i8** %dest) {
+  ret void
+unreachable_loop:
+  store i8* %self_ref, i8** %dest
+  %self_ref = bitcast i8* %self_ref to i8*
+  store i8* %self_ref, i8** %dest
+  br label %unreachable_loop
+}
+; CHECK: define void @self_reference_bitcast(i32 %dest) {
+; CHECK: store i32 undef, i32* %dest.asptr
+; CHECK: store i32 undef, i32* %dest.asptr
+
+define void @circular_reference_bitcasts(i8** %dest) {
+  ret void
+unreachable_loop:
+  store i8* %cycle1, i8** %dest
+  %cycle1 = bitcast i8* %cycle2 to i8*
+  %cycle2 = bitcast i8* %cycle1 to i8*
+  br label %unreachable_loop
+}
+; CHECK: define void @circular_reference_bitcasts(i32 %dest) {
+; CHECK: store i32 undef, i32* %dest.asptr
+
+define void @circular_reference_inttoptr(i8** %dest) {
+  ret void
+unreachable_loop:
+  %ptr = inttoptr i32 %int to i8*
+  %int = ptrtoint i8* %ptr to i32
+  store i8* %ptr, i8** %dest
+  br label %unreachable_loop
+}
+; CHECK: define void @circular_reference_inttoptr(i32 %dest) {
+; CHECK: store i32 undef, i32* %dest.asptr
+
+define i8* @forwards_reference(%struct** %ptr) {
+  br label %block1
+block2:
+  ; Forwards reference to %val.
+  %cast = bitcast %struct* %val to i8*
+  br label %block3
+block1:
+  %val = load %struct** %ptr
+  br label %block2
+block3:
+  ; Backwards reference to a forwards reference that has already been
+  ; resolved.
+  ret i8* %cast
+}
+; CHECK: define i32 @forwards_reference(i32 %ptr) {
+; CHECK-NEXT: br label %block1
+; CHECK: block2:
+; CHECK-NEXT: br label %block3
+; CHECK: block1:
+; CHECK-NEXT: %ptr.asptr = inttoptr i32 %ptr to i32*
+; CHECK-NEXT: %val = load i32* %ptr.asptr
+; CHECK-NEXT: br label %block2
+; CHECK: block3:
+; CHECK-NEXT: ret i32 %val
+
+
 define i8* @phi_multiple_entry(i1 %arg, i8* %ptr) {
 entry:
   br i1 %arg, label %done, label %done
@@ -198,8 +261,8 @@ define i8* @indirect_call(i8* (i8*)* %func, i8* %arg) {
   ret i8* %result
 }
 ; CHECK: define i32 @indirect_call(i32 %func, i32 %arg) {
-; CHECK-NEXT: %func.asfuncptr = inttoptr i32 %func to i32 (i32)*
-; CHECK-NEXT: %result = call i32 %func.asfuncptr(i32 %arg)
+; CHECK-NEXT: %func.asptr = inttoptr i32 %func to i32 (i32)*
+; CHECK-NEXT: %result = call i32 %func.asptr(i32 %arg)
 ; CHECK-NEXT: ret i32 %result
 
 
@@ -302,6 +365,23 @@ define void @alloca_lower_alignment() {
 ; CHECK-NEXT: alloca [4 x i8], align 1
 
 
+; This tests for a bug in which, when processing the store's %buf2
+; operand, ReplacePtrsWithInts accidentally strips off the ptrtoint
+; cast that it previously introduced for the 'alloca', causing an
+; internal sanity check to fail.
+define void @alloca_cast_stripping() {
+  %buf = alloca i32
+  %buf1 = ptrtoint i32* %buf to i32
+  %buf2 = inttoptr i32 %buf1 to i32*
+  store i32 0, i32* %buf2
+  ret void
+}
+; CHECK: define void @alloca_cast_stripping() {
+; CHECK-NEXT: %buf = alloca [4 x i8]
+; CHECK-NEXT: %buf.bc = bitcast [4 x i8]* %buf to i32*
+; CHECK-NEXT: store i32 0, i32* %buf.bc
+
+
 define i1 @compare(i8* %ptr1, i8* %ptr2) {
   %cmp = icmp ult i8* %ptr1, %ptr2
   ret i1 %cmp