[llvm] r206828 - Use unique_ptr to handle ownership of Value*s in Cloning unit tests.

David Blaikie dblaikie at gmail.com
Mon Apr 21 22:38:45 PDT 2014


Unreverting your revert and running under valgrind didn't produce any
failures - any ideas how else I might reproduce this? (it looks like
it's a checking-malloc failure on MacOS? Presumably some kind of
checked runtime?) Maybe I'll pester Eric to reproduce on a mac (I have
a Macbook Air, but don't generally have code/devtools/etc on it)

On Mon, Apr 21, 2014 at 7:24 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Thanks David.
>
> I’ve reverted it in the meantime: r206839.
>
> -Quentin
>
> On Apr 21, 2014, at 6:39 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Will be able to have a look in a few hours. Feel free to revert in the
> interim.
>
> On Apr 21, 2014 6:03 PM, "Quentin Colombet" <qcolombet at apple.com> wrote:
>>
>> Hi David,
>>
>> Looks like your commit broke some bots:
>>
>> http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14682
>>
>> ******************** TEST 'LLVM-Unit ::
>> Transforms/Utils/Release+Asserts/UtilsTests/CloneInstruction.Exact' FAILED
>> ********************
>> Note: Google Test filter = CloneInstruction.Exact
>> [==========] Running 1 test from 1 test case.
>> [----------] Global test environment set-up.
>> [----------] 1 test from CloneInstruction
>> [ RUN      ] CloneInstruction.Exact
>> UtilsTests(60096) malloc: *** error for object 0x104f036c0: pointer being
>> freed was not allocated
>> *** set a breakpoint in malloc_error_break to debug
>> 0  UtilsTests        0x0000000104d9ee48
>> llvm::sys::PrintStackTrace(__sFILE*) + 40
>> 1  UtilsTests        0x0000000104d9f324 _ZL13SignalHandleri + 564
>> 2  libsystem_c.dylib 0x00007fff94461cfa _sigtramp + 26
>> 3  libsystem_c.dylib 0x0000000000000190 _sigtramp + 1807344816
>> 4  libsystem_c.dylib 0x00007fff9445f84c free + 389
>> 5  UtilsTests        0x0000000104cbc2c0
>> std::__1::__tree<std::__1::unique_ptr<llvm::Value,
>> std::__1::default_delete<llvm::Value> >,
>> std::__1::less<std::__1::unique_ptr<llvm::Value,
>> std::__1::default_delete<llvm::Value> > >,
>> std::__1::allocator<std::__1::unique_ptr<llvm::Value,
>> std::__1::default_delete<llvm::Value> > >
>> >::destroy(std::__1::__tree_node<std::__1::unique_ptr<llvm::Value,
>> std::__1::default_delete<llvm::Value> >, void*>*) + 64
>> 6  UtilsTests        0x0000000104cb46e0 (anonymous
>> namespace)::CloneInstruction::TearDown() + 64
>> 7  UtilsTests        0x0000000104ccfed0 testing::TestInfo::Run() + 832
>> 8  UtilsTests        0x0000000104cd0430 testing::TestCase::Run() + 448
>> 9  UtilsTests        0x0000000104cd75d5
>> testing::internal::UnitTestImpl::RunAllTests() + 1317
>> 10 UtilsTests        0x0000000104cd709a testing::UnitTest::Run() + 106
>> 11 UtilsTests        0x0000000104ce5061 main + 65
>> 12 UtilsTests        0x0000000104cacd54 start + 52
>> 13 UtilsTests        0x0000000000000002 start + 4214567650
>>
>> ********************
>>
>>
>>
>> Could you take a look please?
>>
>> Thanks,
>> -Quentin
>>
>> On Apr 21, 2014, at 4:47 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Mon Apr 21 18:47:47 2014
>> New Revision: 206828
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206828&view=rev
>> Log:
>> Use unique_ptr to handle ownership of Value*s in Cloning unit tests.
>>
>> Modified:
>>    llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
>>
>> Modified: llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=206828&r1=206827&r2=206828&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Transforms/Utils/Cloning.cpp (original)
>> +++ llvm/trunk/unittests/Transforms/Utils/Cloning.cpp Mon Apr 21 18:47:47
>> 2014
>> @@ -10,7 +10,6 @@
>> #include "llvm/Transforms/Utils/Cloning.h"
>> #include "llvm/ADT/ArrayRef.h"
>> #include "llvm/ADT/STLExtras.h"
>> -#include "llvm/ADT/SmallPtrSet.h"
>> #include "llvm/IR/Argument.h"
>> #include "llvm/IR/Constant.h"
>> #include "llvm/IR/DebugInfo.h"
>> @@ -25,6 +24,8 @@
>> #include "llvm/IR/LLVMContext.h"
>> #include "gtest/gtest.h"
>>
>> +#include <set>
>> +
>> using namespace llvm;
>>
>> namespace {
>> @@ -38,34 +39,38 @@ protected:
>>   template <typename T>
>>   T *clone(T *V1) {
>>     Value *V2 = V1->clone();
>> -    Orig.insert(V1);
>> -    Clones.insert(V2);
>> +    std::unique_ptr<Value> V(V1);
>> +    if (!Orig.insert(std::move(V)).second)
>> +      V.release(); // this wasn't the first time we added the element, so
>> the
>> +                   // set already had ownership
>> +    Clones.insert(std::unique_ptr<Value>(V2));
>>     return cast<T>(V2);
>>   }
>>
>> -  void eraseClones() {
>> -    DeleteContainerPointers(Clones);
>> -  }
>> +  void eraseClones() { Clones.clear(); }
>>
>>   virtual void TearDown() {
>>     eraseClones();
>> -    DeleteContainerPointers(Orig);
>> -    delete V;
>> +    Orig.clear();
>> +    V.reset();
>>   }
>>
>> -  SmallPtrSet<Value *, 4> Orig;   // Erase on exit
>> -  SmallPtrSet<Value *, 4> Clones; // Erase in eraseClones
>> +  std::set<std::unique_ptr<Value>> Orig;   // Erase on exit
>> +  std::set<std::unique_ptr<Value>> Clones; // Erase in eraseClones
>>
>>   LLVMContext context;
>> -  Value *V;
>> +  std::unique_ptr<Value> V;
>> };
>>
>> TEST_F(CloneInstruction, OverflowBits) {
>> -  V = new Argument(Type::getInt32Ty(context));
>> +  V = make_unique<Argument>(Type::getInt32Ty(context));
>>
>> -  BinaryOperator *Add = BinaryOperator::Create(Instruction::Add, V, V);
>> -  BinaryOperator *Sub = BinaryOperator::Create(Instruction::Sub, V, V);
>> -  BinaryOperator *Mul = BinaryOperator::Create(Instruction::Mul, V, V);
>> +  BinaryOperator *Add =
>> +      BinaryOperator::Create(Instruction::Add, V.get(), V.get());
>> +  BinaryOperator *Sub =
>> +      BinaryOperator::Create(Instruction::Sub, V.get(), V.get());
>> +  BinaryOperator *Mul =
>> +      BinaryOperator::Create(Instruction::Mul, V.get(), V.get());
>>
>>   BinaryOperator *AddClone = this->clone(Add);
>>   BinaryOperator *SubClone = this->clone(Sub);
>> @@ -131,12 +136,12 @@ TEST_F(CloneInstruction, OverflowBits) {
>> }
>>
>> TEST_F(CloneInstruction, Inbounds) {
>> -  V = new Argument(Type::getInt32PtrTy(context));
>> +  V = make_unique<Argument>(Type::getInt32PtrTy(context));
>>
>>   Constant *Z = Constant::getNullValue(Type::getInt32Ty(context));
>>   std::vector<Value *> ops;
>>   ops.push_back(Z);
>> -  GetElementPtrInst *GEP = GetElementPtrInst::Create(V, ops);
>> +  GetElementPtrInst *GEP = GetElementPtrInst::Create(V.get(), ops);
>>   EXPECT_FALSE(this->clone(GEP)->isInBounds());
>>
>>   GEP->setIsInBounds();
>> @@ -144,9 +149,10 @@ TEST_F(CloneInstruction, Inbounds) {
>> }
>>
>> TEST_F(CloneInstruction, Exact) {
>> -  V = new Argument(Type::getInt32Ty(context));
>> +  V = make_unique<Argument>(Type::getInt32Ty(context));
>>
>> -  BinaryOperator *SDiv = BinaryOperator::Create(Instruction::SDiv, V, V);
>> +  BinaryOperator *SDiv =
>> +      BinaryOperator::Create(Instruction::SDiv, V.get(), V.get());
>>   EXPECT_FALSE(this->clone(SDiv)->isExact());
>>
>>   SDiv->setIsExact(true);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>




More information about the llvm-commits mailing list