[PATCH] D66330: Fix use-after-free in CodeGenPrepare

[Brent Royal-Gordon via Phabricator via llvm-commits]

brentdax created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

If OptimizeExtractBits() encountered a shift instruction with no operands at all, it would erase the instruction, but still return false. This previously didn’t matter because its caller would always return after processing the instruction, but https://reviews.llvm.org/D63233 changed the function’s caller to fall through if it returned false, which would then cause a use-after-free detectable by ASAN.

This change makes OptimizeExtractBits return true if it removes a shift instruction with no users, terminating processing of the instruction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66330

Files:
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/test/Transforms/CodeGenPrepare/sink-shift-and-trunc.ll


Index: llvm/test/Transforms/CodeGenPrepare/sink-shift-and-trunc.ll
===================================================================
--- llvm/test/Transforms/CodeGenPrepare/sink-shift-and-trunc.ll
+++ llvm/test/Transforms/CodeGenPrepare/sink-shift-and-trunc.ll
@@ -1,5 +1,5 @@
 ; REQUIRES: aarch64-registered-target
-; RUN: opt -codegenprepare -mtriple=arm64-apple=ios -S -o - %s | FileCheck %s
+; RUN: opt -codegenprepare -mtriple=arm64-apple-ios -S -o - %s | FileCheck %s
 
 @first_ones = external global [65536 x i8]
 
@@ -58,6 +58,23 @@
   ret i32 %retval.0, !dbg !63
 }
 
+define i32 @shift_unused(i32 %a) {
+; CHECK-LABEL: @shift_unused
+
+  %as = lshr i32 %a, 3
+  br label %BB2
+
+; CHECK-NEXT: BB2:
+BB2:
+  ; CodeGenPrepare was erasing the unused lshr instruction, but then further
+  ; processing the instruction after it was freed. If this bug is still present,
+  ; this test will always crash in an LLVM built with ASAN enabled, and may
+  ; crash even if ASAN is not enabled.
+
+; CHECK-NEXT: ret i32
+  ret i32 %a
+}
+
 ; CHECK: [[shift1_loc]] = !DILocation(line: 1
 ; CHECK: [[trunc1_loc]] = !DILocation(line: 2
 ; CHECK: [[shift2_loc]] = !DILocation(line: 3
Index: llvm/lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -1682,10 +1682,11 @@
     TheUse = InsertedShift;
   }
 
-  // If we removed all uses, nuke the shift.
+  // If we removed all uses, or there are none, nuke the shift.
   if (ShiftI->use_empty()) {
     salvageDebugInfo(*ShiftI);
     ShiftI->eraseFromParent();
+    MadeChange = true;
   }
 
   return MadeChange;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66330.215530.patch
Type: text/x-patch
Size: 1687 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190816/2709f697/attachment.bin>