[llvm-commits] [llvm] r166517 - /llvm/trunk/include/llvm/ADT/StringSet.h

[Sean Silva]

>> Maybe we should have a buildbot which runs the unittests under
>> valgrind. Seems like that would be an incredibly easy way to avoid
>> most problems of this kind in the future.
>
>
> we used to have one but it seems to have disappeared.  I can host it on one
> of
> the gcc compile farm machines if you want to ressurect it.

I think that would be good; thanks for offering. It seems silly to
have these kinds of bugs when they are so easy to prevent.

-- Sean Silva

On Fri, Oct 26, 2012 at 4:32 AM, Duncan Sands <baldrick at free.fr> wrote:
> Hi Sean,
>
>
> On 26/10/12 06:49, Sean Silva wrote:
>>>
>>> We (Google) have tests which depend on Clang libraries; our heap leak
>>> checker complained, and I eventually managed to get Valgrind to
>>> complain about the same leak when running a Clang unit test binary.
>>
>>
>> Maybe we should have a buildbot which runs the unittests under
>> valgrind. Seems like that would be an incredibly easy way to avoid
>> most problems of this kind in the future.
>
>
> we used to have one but it seems to have disappeared.  I can host it on one
> of
> the gcc compile farm machines if you want to ressurect it.
>
> Ciao, Duncan.
>
>
>>
>> -- Sean Silva
>>
>> On Thu, Oct 25, 2012 at 9:28 PM, Matt Beaumont-Gay <matthewbg at google.com>
>> wrote:
>>>
>>> We (Google) have tests which depend on Clang libraries; our heap leak
>>> checker complained, and I eventually managed to get Valgrind to
>>> complain about the same leak when running a Clang unit test binary.
>>>
>>> On Thu, Oct 25, 2012 at 11:27 AM, Sean Silva <silvas at purdue.edu> wrote:
>>>>
>>>> How did you find this?
>>>>
>>>> -- Sean Silva
>>>>
>>>> On Tue, Oct 23, 2012 at 6:55 PM, Michael J. Spencer
>>>> <bigcheesegs at gmail.com> wrote:
>>>>>
>>>>> Author: mspencer
>>>>> Date: Tue Oct 23 17:55:54 2012
>>>>> New Revision: 166517
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=166517&view=rev
>>>>> Log:
>>>>> [Support/StringSet] Fix memory leak when inserted key already exists.
>>>>>
>>>>> Modified:
>>>>>      llvm/trunk/include/llvm/ADT/StringSet.h
>>>>>
>>>>> Modified: llvm/trunk/include/llvm/ADT/StringSet.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringSet.h?rev=166517&r1=166516&r2=166517&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/ADT/StringSet.h (original)
>>>>> +++ llvm/trunk/include/llvm/ADT/StringSet.h Tue Oct 23 17:55:54 2012
>>>>> @@ -29,8 +29,13 @@
>>>>>         assert(!InLang.empty());
>>>>>         const char *KeyStart = InLang.data();
>>>>>         const char *KeyEnd = KeyStart + InLang.size();
>>>>> -      return base::insert(llvm::StringMapEntry<char>::
>>>>> -                          Create(KeyStart, KeyEnd,
>>>>> base::getAllocator(), '+'));
>>>>> +      llvm::StringMapEntry<char> *Entry = llvm::StringMapEntry<char>::
>>>>> +                            Create(KeyStart, KeyEnd,
>>>>> base::getAllocator(), '+');
>>>>> +      if (!base::insert(Entry)) {
>>>>> +        Entry->Destroy(base::getAllocator());
>>>>> +        return false;
>>>>> +      }
>>>>> +      return true;
>>>>>       }
>>>>>     };
>>>>>   }
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits