[llvm] r290397 - Don't consider allocsize functions to be allocation functions.
George Burgess IV via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 22 17:18:09 PST 2016
Author: gbiv
Date: Thu Dec 22 19:18:09 2016
New Revision: 290397
URL: http://llvm.org/viewvc/llvm-project?rev=290397&view=rev
Log:
Don't consider allocsize functions to be allocation functions.
This patch fixes some ASAN unittest failures on FreeBSD. See the
cfe-commits email thread for r290169 for more on those.
According to the LangRef, the allocsize attribute only tells us about
the number of bytes that exist at the memory location pointed to by the
return value of a function. It does not necessarily mean that the
function will only ever allocate. So, we need to be very careful about
treating functions with allocsize as general allocation functions. This
patch makes us fully conservative in this regard, though I suspect that
we have room to be a bit more aggressive if we want.
This has a FIXME that can be fixed by a relatively straightforward
refactor; I just wanted to keep this patch minimal. If this sticks, I'll
come back and fix it in a few days.
Added:
llvm/trunk/unittests/Analysis/MemoryBuiltinsTest.cpp
Modified:
llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
llvm/trunk/unittests/Analysis/CMakeLists.txt
Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=290397&r1=290396&r2=290397&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
+++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Thu Dec 22 19:18:09 2016
@@ -109,25 +109,6 @@ static Optional<AllocFnsTy> getAllocatio
if (!Callee)
return None;
- // If it has allocsize, we can skip checking if it's a known function.
- //
- // MallocLike is chosen here because allocsize makes no guarantees about the
- // nullness of the result of the function, nor does it deal with strings, nor
- // does it require that the memory returned is zeroed out.
- const AllocType AllocSizeAllocTy = MallocLike;
- if ((AllocTy & AllocSizeAllocTy) == AllocSizeAllocTy &&
- Callee->hasFnAttribute(Attribute::AllocSize)) {
- Attribute Attr = Callee->getFnAttribute(Attribute::AllocSize);
- std::pair<unsigned, Optional<unsigned>> Args = Attr.getAllocSizeArgs();
-
- AllocFnsTy Result;
- Result.AllocTy = AllocSizeAllocTy;
- Result.NumParams = Callee->getNumOperands();
- Result.FstParam = Args.first;
- Result.SndParam = Args.second.getValueOr(-1);
- return Result;
- }
-
// Make sure that the function is available.
StringRef FnName = Callee->getName();
LibFunc::Func TLIFn;
@@ -163,6 +144,32 @@ static Optional<AllocFnsTy> getAllocatio
return None;
}
+static Optional<AllocFnsTy> getAllocationSize(const Value *V,
+ const TargetLibraryInfo *TLI) {
+ // Prefer to use existing information over allocsize. This will give us an
+ // accurate AllocTy.
+ if (Optional<AllocFnsTy> Data =
+ getAllocationData(V, AnyAlloc, TLI, /*LookThroughBitCast=*/false))
+ return Data;
+
+ // FIXME: Not calling getCalledFunction twice would be nice.
+ const Function *Callee = getCalledFunction(V, /*LookThroughBitCast=*/false);
+ if (!Callee || !Callee->hasFnAttribute(Attribute::AllocSize))
+ return None;
+
+ Attribute Attr = Callee->getFnAttribute(Attribute::AllocSize);
+ std::pair<unsigned, Optional<unsigned>> Args = Attr.getAllocSizeArgs();
+
+ AllocFnsTy Result;
+ // Because allocsize only tells us how many bytes are allocated, we're not
+ // really allowed to assume anything, so we use MallocLike.
+ Result.AllocTy = MallocLike;
+ Result.NumParams = Callee->getNumOperands();
+ Result.FstParam = Args.first;
+ Result.SndParam = Args.second.getValueOr(-1);
+ return Result;
+}
+
static bool hasNoAliasAttr(const Value *V, bool LookThroughBitCast) {
ImmutableCallSite CS(LookThroughBitCast ? V->stripPointerCasts() : V);
return CS && CS.paramHasAttr(AttributeSet::ReturnIndex, Attribute::NoAlias);
@@ -505,8 +512,7 @@ SizeOffsetType ObjectSizeOffsetVisitor::
}
SizeOffsetType ObjectSizeOffsetVisitor::visitCallSite(CallSite CS) {
- Optional<AllocFnsTy> FnData =
- getAllocationData(CS.getInstruction(), AnyAlloc, TLI);
+ Optional<AllocFnsTy> FnData = getAllocationSize(CS.getInstruction(), TLI);
if (!FnData)
return unknown();
@@ -765,8 +771,7 @@ SizeOffsetEvalType ObjectSizeOffsetEvalu
}
SizeOffsetEvalType ObjectSizeOffsetEvaluator::visitCallSite(CallSite CS) {
- Optional<AllocFnsTy> FnData =
- getAllocationData(CS.getInstruction(), AnyAlloc, TLI);
+ Optional<AllocFnsTy> FnData = getAllocationSize(CS.getInstruction(), TLI);
if (!FnData)
return unknown();
Modified: llvm/trunk/unittests/Analysis/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CMakeLists.txt?rev=290397&r1=290396&r2=290397&view=diff
==============================================================================
--- llvm/trunk/unittests/Analysis/CMakeLists.txt (original)
+++ llvm/trunk/unittests/Analysis/CMakeLists.txt Thu Dec 22 19:18:09 2016
@@ -14,6 +14,7 @@ add_llvm_unittest(AnalysisTests
CGSCCPassManagerTest.cpp
LazyCallGraphTest.cpp
LoopPassManagerTest.cpp
+ MemoryBuiltinsTest.cpp
ScalarEvolutionTest.cpp
TBAATest.cpp
ValueTrackingTest.cpp
Added: llvm/trunk/unittests/Analysis/MemoryBuiltinsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/MemoryBuiltinsTest.cpp?rev=290397&view=auto
==============================================================================
--- llvm/trunk/unittests/Analysis/MemoryBuiltinsTest.cpp (added)
+++ llvm/trunk/unittests/Analysis/MemoryBuiltinsTest.cpp Thu Dec 22 19:18:09 2016
@@ -0,0 +1,50 @@
+//===- MemoryBuiltinsTest.cpp - Tests for utilities in MemoryBuiltins.h ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/MemoryBuiltins.h"
+#include "llvm/IR/Attributes.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+// allocsize should not imply that a function is a traditional allocation
+// function (e.g. that can be optimized out/...); it just tells us how many
+// bytes exist at the pointer handed back by the function.
+TEST(AllocSize, AllocationBuiltinsTest) {
+ LLVMContext Context;
+ Module M("", Context);
+ IntegerType *ArgTy = Type::getInt32Ty(Context);
+
+ Function *AllocSizeFn = Function::Create(
+ FunctionType::get(Type::getInt8PtrTy(Context), {ArgTy}, false),
+ GlobalValue::ExternalLinkage, "F", &M);
+
+ AllocSizeFn->addFnAttr(Attribute::getWithAllocSizeArgs(Context, 1, None));
+
+ // 100 is arbitrary.
+ std::unique_ptr<CallInst> Caller(
+ CallInst::Create(AllocSizeFn, {ConstantInt::get(ArgTy, 100)}));
+
+ const TargetLibraryInfo *TLI = nullptr;
+ EXPECT_FALSE(isNoAliasFn(Caller.get(), TLI));
+ EXPECT_FALSE(isMallocLikeFn(Caller.get(), TLI));
+ EXPECT_FALSE(isCallocLikeFn(Caller.get(), TLI));
+ EXPECT_FALSE(isAllocLikeFn(Caller.get(), TLI));
+
+ // FIXME: We might be able to treat allocsize functions as general allocation
+ // functions. For the moment, being conservative seems better (and we'd have
+ // to plumb stuff around `isNoAliasFn`).
+ EXPECT_FALSE(isAllocationFn(Caller.get(), TLI));
+}
+}
More information about the llvm-commits
mailing list