<p dir="ltr">Sure, I'll give it a shot. </p>
<div class="gmail_quote">On Apr 21, 2014 10:43 PM, "David Blaikie" <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unreverting your revert and running under valgrind didn't produce any<br>
failures - any ideas how else I might reproduce this? (it looks like<br>
it's a checking-malloc failure on MacOS? Presumably some kind of<br>
checked runtime?) Maybe I'll pester Eric to reproduce on a mac (I have<br>
a Macbook Air, but don't generally have code/devtools/etc on it)<br>
<br>
On Mon, Apr 21, 2014 at 7:24 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br>
> Thanks David.<br>
><br>
> I’ve reverted it in the meantime: r206839.<br>
><br>
> -Quentin<br>
><br>
> On Apr 21, 2014, at 6:39 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> Will be able to have a look in a few hours. Feel free to revert in the<br>
> interim.<br>
><br>
> On Apr 21, 2014 6:03 PM, "Quentin Colombet" <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br>
>><br>
>> Hi David,<br>
>><br>
>> Looks like your commit broke some bots:<br>
>><br>
>> <a href="http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14682" target="_blank">http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14682</a><br>

>><br>
>> ******************** TEST 'LLVM-Unit ::<br>
>> Transforms/Utils/Release+Asserts/UtilsTests/CloneInstruction.Exact' FAILED<br>
>> ********************<br>
>> Note: Google Test filter = CloneInstruction.Exact<br>
>> [==========] Running 1 test from 1 test case.<br>
>> [----------] Global test environment set-up.<br>
>> [----------] 1 test from CloneInstruction<br>
>> [ RUN      ] CloneInstruction.Exact<br>
>> UtilsTests(60096) malloc: *** error for object 0x104f036c0: pointer being<br>
>> freed was not allocated<br>
>> *** set a breakpoint in malloc_error_break to debug<br>
>> 0  UtilsTests        0x0000000104d9ee48<br>
>> llvm::sys::PrintStackTrace(__sFILE*) + 40<br>
>> 1  UtilsTests        0x0000000104d9f324 _ZL13SignalHandleri + 564<br>
>> 2  libsystem_c.dylib 0x00007fff94461cfa _sigtramp + 26<br>
>> 3  libsystem_c.dylib 0x0000000000000190 _sigtramp + 1807344816<br>
>> 4  libsystem_c.dylib 0x00007fff9445f84c free + 389<br>
>> 5  UtilsTests        0x0000000104cbc2c0<br>
>> std::__1::__tree<std::__1::unique_ptr<llvm::Value,<br>
>> std::__1::default_delete<llvm::Value> >,<br>
>> std::__1::less<std::__1::unique_ptr<llvm::Value,<br>
>> std::__1::default_delete<llvm::Value> > >,<br>
>> std::__1::allocator<std::__1::unique_ptr<llvm::Value,<br>
>> std::__1::default_delete<llvm::Value> > ><br>
>> >::destroy(std::__1::__tree_node<std::__1::unique_ptr<llvm::Value,<br>
>> std::__1::default_delete<llvm::Value> >, void*>*) + 64<br>
>> 6  UtilsTests        0x0000000104cb46e0 (anonymous<br>
>> namespace)::CloneInstruction::TearDown() + 64<br>
>> 7  UtilsTests        0x0000000104ccfed0 testing::TestInfo::Run() + 832<br>
>> 8  UtilsTests        0x0000000104cd0430 testing::TestCase::Run() + 448<br>
>> 9  UtilsTests        0x0000000104cd75d5<br>
>> testing::internal::UnitTestImpl::RunAllTests() + 1317<br>
>> 10 UtilsTests        0x0000000104cd709a testing::UnitTest::Run() + 106<br>
>> 11 UtilsTests        0x0000000104ce5061 main + 65<br>
>> 12 UtilsTests        0x0000000104cacd54 start + 52<br>
>> 13 UtilsTests        0x0000000000000002 start + 4214567650<br>
>><br>
>> ********************<br>
>><br>
>><br>
>><br>
>> Could you take a look please?<br>
>><br>
>> Thanks,<br>
>> -Quentin<br>
>><br>
>> On Apr 21, 2014, at 4:47 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> Author: dblaikie<br>
>> Date: Mon Apr 21 18:47:47 2014<br>
>> New Revision: 206828<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206828&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206828&view=rev</a><br>
>> Log:<br>
>> Use unique_ptr to handle ownership of Value*s in Cloning unit tests.<br>
>><br>
>> Modified:<br>
>>    llvm/trunk/unittests/Transforms/Utils/Cloning.cpp<br>
>><br>
>> Modified: llvm/trunk/unittests/Transforms/Utils/Cloning.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=206828&r1=206827&r2=206828&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=206828&r1=206827&r2=206828&view=diff</a><br>

>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/unittests/Transforms/Utils/Cloning.cpp (original)<br>
>> +++ llvm/trunk/unittests/Transforms/Utils/Cloning.cpp Mon Apr 21 18:47:47<br>
>> 2014<br>
>> @@ -10,7 +10,6 @@<br>
>> #include "llvm/Transforms/Utils/Cloning.h"<br>
>> #include "llvm/ADT/ArrayRef.h"<br>
>> #include "llvm/ADT/STLExtras.h"<br>
>> -#include "llvm/ADT/SmallPtrSet.h"<br>
>> #include "llvm/IR/Argument.h"<br>
>> #include "llvm/IR/Constant.h"<br>
>> #include "llvm/IR/DebugInfo.h"<br>
>> @@ -25,6 +24,8 @@<br>
>> #include "llvm/IR/LLVMContext.h"<br>
>> #include "gtest/gtest.h"<br>
>><br>
>> +#include <set><br>
>> +<br>
>> using namespace llvm;<br>
>><br>
>> namespace {<br>
>> @@ -38,34 +39,38 @@ protected:<br>
>>   template <typename T><br>
>>   T *clone(T *V1) {<br>
>>     Value *V2 = V1->clone();<br>
>> -    Orig.insert(V1);<br>
>> -    Clones.insert(V2);<br>
>> +    std::unique_ptr<Value> V(V1);<br>
>> +    if (!Orig.insert(std::move(V)).second)<br>
>> +      V.release(); // this wasn't the first time we added the element, so<br>
>> the<br>
>> +                   // set already had ownership<br>
>> +    Clones.insert(std::unique_ptr<Value>(V2));<br>
>>     return cast<T>(V2);<br>
>>   }<br>
>><br>
>> -  void eraseClones() {<br>
>> -    DeleteContainerPointers(Clones);<br>
>> -  }<br>
>> +  void eraseClones() { Clones.clear(); }<br>
>><br>
>>   virtual void TearDown() {<br>
>>     eraseClones();<br>
>> -    DeleteContainerPointers(Orig);<br>
>> -    delete V;<br>
>> +    Orig.clear();<br>
>> +    V.reset();<br>
>>   }<br>
>><br>
>> -  SmallPtrSet<Value *, 4> Orig;   // Erase on exit<br>
>> -  SmallPtrSet<Value *, 4> Clones; // Erase in eraseClones<br>
>> +  std::set<std::unique_ptr<Value>> Orig;   // Erase on exit<br>
>> +  std::set<std::unique_ptr<Value>> Clones; // Erase in eraseClones<br>
>><br>
>>   LLVMContext context;<br>
>> -  Value *V;<br>
>> +  std::unique_ptr<Value> V;<br>
>> };<br>
>><br>
>> TEST_F(CloneInstruction, OverflowBits) {<br>
>> -  V = new Argument(Type::getInt32Ty(context));<br>
>> +  V = make_unique<Argument>(Type::getInt32Ty(context));<br>
>><br>
>> -  BinaryOperator *Add = BinaryOperator::Create(Instruction::Add, V, V);<br>
>> -  BinaryOperator *Sub = BinaryOperator::Create(Instruction::Sub, V, V);<br>
>> -  BinaryOperator *Mul = BinaryOperator::Create(Instruction::Mul, V, V);<br>
>> +  BinaryOperator *Add =<br>
>> +      BinaryOperator::Create(Instruction::Add, V.get(), V.get());<br>
>> +  BinaryOperator *Sub =<br>
>> +      BinaryOperator::Create(Instruction::Sub, V.get(), V.get());<br>
>> +  BinaryOperator *Mul =<br>
>> +      BinaryOperator::Create(Instruction::Mul, V.get(), V.get());<br>
>><br>
>>   BinaryOperator *AddClone = this->clone(Add);<br>
>>   BinaryOperator *SubClone = this->clone(Sub);<br>
>> @@ -131,12 +136,12 @@ TEST_F(CloneInstruction, OverflowBits) {<br>
>> }<br>
>><br>
>> TEST_F(CloneInstruction, Inbounds) {<br>
>> -  V = new Argument(Type::getInt32PtrTy(context));<br>
>> +  V = make_unique<Argument>(Type::getInt32PtrTy(context));<br>
>><br>
>>   Constant *Z = Constant::getNullValue(Type::getInt32Ty(context));<br>
>>   std::vector<Value *> ops;<br>
>>   ops.push_back(Z);<br>
>> -  GetElementPtrInst *GEP = GetElementPtrInst::Create(V, ops);<br>
>> +  GetElementPtrInst *GEP = GetElementPtrInst::Create(V.get(), ops);<br>
>>   EXPECT_FALSE(this->clone(GEP)->isInBounds());<br>
>><br>
>>   GEP->setIsInBounds();<br>
>> @@ -144,9 +149,10 @@ TEST_F(CloneInstruction, Inbounds) {<br>
>> }<br>
>><br>
>> TEST_F(CloneInstruction, Exact) {<br>
>> -  V = new Argument(Type::getInt32Ty(context));<br>
>> +  V = make_unique<Argument>(Type::getInt32Ty(context));<br>
>><br>
>> -  BinaryOperator *SDiv = BinaryOperator::Create(Instruction::SDiv, V, V);<br>
>> +  BinaryOperator *SDiv =<br>
>> +      BinaryOperator::Create(Instruction::SDiv, V.get(), V.get());<br>
>>   EXPECT_FALSE(this->clone(SDiv)->isExact());<br>
>><br>
>>   SDiv->setIsExact(true);<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
><br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>