This is an archive of the discontinued LLVM Phabricator instance.

Differential D87074

[StackProtector] Fix crash with vararg due to not checking LocationSize validity.
ClosedPublic

Authored by aemerson on Sep 2 2020, 11:21 PM.

Details

Reviewers
john.brawn
eli.friedman
efriedma
Commits
rG2878ecc90f1f: [StackProtector] Fix crash with vararg due to not checking LocationSize…
Summary

This seems to have been introduced in D75695

Diff Detail

Event Timeline

aemerson created this revision.Sep 2 2020, 11:21 PM
Herald added a subscriber: hiraditya. · View Herald TranscriptSep 2 2020, 11:21 PM
aemerson requested review of this revision.Sep 2 2020, 11:21 PM
efriedma accepted this revision.Sep 2 2020, 11:44 PM
efriedma added a subscriber: efriedma.

LGTM.

clang basically never generates va_arg, but I guess it's relevant for other frontends.

This revision is now accepted and ready to land.Sep 2 2020, 11:44 PM
Harbormaster completed remote builds in B70515: Diff 289645.Sep 3 2020, 12:02 AM
Closed by commit rG2878ecc90f1f: [StackProtector] Fix crash with vararg due to not checking LocationSize… (authored by aemerson). · Explain WhySep 3 2020, 12:09 AM
This revision was automatically updated to reflect the committed changes.
aemerson added a commit: rG2878ecc90f1f: [StackProtector] Fix crash with vararg due to not checking LocationSize….

Revision Contents

PathSize
llvm/
lib/
CodeGen/
3 lines
test/
CodeGen/
X86/
17 lines

Diff 289647

llvm/lib/CodeGen/StackProtector.cpp

Show First 20 Lines • Show All 164 Lines • ▼ Show 20 Lines
bool StackProtector::HasAddressTaken(const Instruction *AI, bool StackProtector::HasAddressTaken(const Instruction *AI,
uint64_t AllocSize) { uint64_t AllocSize) {
const DataLayout &DL = M->getDataLayout(); const DataLayout &DL = M->getDataLayout();
for (const User *U : AI->users()) { for (const User *U : AI->users()) {
const auto *I = cast<Instruction>(U); const auto *I = cast<Instruction>(U);
// If this instruction accesses memory make sure it doesn't access beyond // If this instruction accesses memory make sure it doesn't access beyond
// the bounds of the allocated object. // the bounds of the allocated object.
Optional<MemoryLocation> MemLoc = MemoryLocation::getOrNone(I); Optional<MemoryLocation> MemLoc = MemoryLocation::getOrNone(I);
if (MemLoc.hasValue() && MemLoc->Size.getValue() > AllocSize) if (MemLoc.hasValue() && MemLoc->Size.hasValue() &&
MemLoc->Size.getValue() > AllocSize)
return true; return true;
switch (I->getOpcode()) { switch (I->getOpcode()) {
case Instruction::Store: case Instruction::Store:
if (AI == cast<StoreInst>(I)->getValueOperand()) if (AI == cast<StoreInst>(I)->getValueOperand())
return true; return true;
break; break;
case Instruction::AtomicCmpXchg: case Instruction::AtomicCmpXchg:
// cmpxchg conceptually includes both a load and store from the same // cmpxchg conceptually includes both a load and store from the same
▲ Show 20 Lines • Show All 416 Lines • Show Last 20 Lines

llvm/test/CodeGen/X86/stack-guard-memloc-vararg.ll

  • This file was added.
; RUN: llc -mtriple=x86_64 -O0 < %s | FileCheck %s
; Check that we don't crash on this input.
; CHECK-LABEL: @foo
; CHECK: __stack_chk_guard
; CHECK: retq
define hidden void @foo(i8** %ptr) #0 {
entry:
%args.addr = alloca i8*, align 8
%0 = va_arg i8** %args.addr, i8*
store i8* %0, i8** %ptr
ret void
}
attributes #0 = { sspstrong }
attributes #1 = { optsize }