Skip to content

Commit

Permalink
[CodeGenPrepare] Fix use-after-free
Browse files Browse the repository at this point in the history
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.

Patch by: @brentdax (Brent Royal-Gordon)

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

llvm-svn: 369168
  • Loading branch information
rotateright committed Aug 16, 2019
1 parent d0797ec commit acceedb
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/CodeGenPrepare.cpp
Expand Up @@ -1682,10 +1682,11 @@ static bool OptimizeExtractBits(BinaryOperator *ShiftI, ConstantInt *CI,
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;
Expand Down
17 changes: 17 additions & 0 deletions llvm/test/Transforms/CodeGenPrepare/sink-shift-and-trunc.ll
Expand Up @@ -58,6 +58,23 @@ return: ; preds = %if.then17, %if.end1
ret i32 %retval.0, !dbg !63
}

; 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.

define i32 @shift_unused(i32 %a) {
; CHECK-LABEL: @shift_unused(
; CHECK-NEXT: BB2:
; CHECK-NEXT: ret i32 [[A:%.*]]
;
%as = lshr i32 %a, 3
br label %BB2

BB2:
ret i32 %a
}

; CHECK: [[shift1_loc]] = !DILocation(line: 1
; CHECK: [[trunc1_loc]] = !DILocation(line: 2
; CHECK: [[shift2_loc]] = !DILocation(line: 3
Expand Down

0 comments on commit acceedb

Please sign in to comment.