[compiler-rt] r221769 - delete => delete[] for arrays.

[David Blaikie]

On Wed, Nov 12, 2014 at 4:43 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, Nov 12, 2014 at 4:36 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
>
>> On Wed, Nov 12, 2014 at 2:40 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Wed, Nov 12, 2014 at 2:18 PM, Richard Trieu <rtrieu at google.com>
>>> wrote:
>>>
>>>> On Wed, Nov 12, 2014 at 10:58 AM, David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Nov 11, 2014 at 8:19 PM, Richard Trieu <rtrieu at google.com>
>>>>> wrote:
>>>>>
>>>>>> Author: rtrieu
>>>>>> Date: Tue Nov 11 22:19:57 2014
>>>>>> New Revision: 221769
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=221769&view=rev
>>>>>> Log:
>>>>>> delete => delete[] for arrays.
>>>>>>
>>>>>
>>>>> We could probably just replace all these news and deletes with
>>>>> unique_ptr<T> and unique_ptr<T[]>?
>>>>>
>>>>>
>>>> Possibly, but it also seems reasonable that tests for the sanitizers
>>>> wanted to directly access memory as opposed to going through a unique_ptr.
>>>> For instance, the asan test is set up to get the line of the memory access,
>>>> which would not work if the access happened in a unique_ptr method.
>>>>
>>>
>>> Fair - though I think the memory access would actually still be in the
>>> test, rather than in unique_ptr - even if you use unique_ptr's operator*,
>>> it'll return (at the machine level) a pointer, which will be dereferenced
>>> in the caller. (if not, and that particular memory access location is
>>> desired, it could still be written using unique_ptr::get)
>>>
>>> Alexey - any ideas if these can reasonably moved to unique_ptr?
>>>
>>
>> Yeah, I agree that we'd still be able to get correct source code
>> locations with std::unique_ptr, but I don't think we should add it here.
>> Direct access to array elements (i.e. x[9]++; instead of (*x)[9]++; or
>> x.get()[9]++) is more readable,
>>
>
> unique_ptr<T[]> has an operator[] overload
>
>
>> and we don't have any ownership passing involved or early-exits due to
>> the nature of unit tests.
>>
>
Sure - but we still managed to write it incorrectly by using delete instead
of delete[]...


>
>>
>>>
>>>
>>>>
>>>>>> Modified:
>>>>>>     compiler-rt/trunk/lib/asan/tests/asan_test.cc
>>>>>>     compiler-rt/trunk/lib/msan/tests/msan_test.cc
>>>>>>
>>>>>> Modified: compiler-rt/trunk/lib/asan/tests/asan_test.cc
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?rev=221769&r1=221768&r2=221769&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- compiler-rt/trunk/lib/asan/tests/asan_test.cc (original)
>>>>>> +++ compiler-rt/trunk/lib/asan/tests/asan_test.cc Tue Nov 11 22:19:57
>>>>>> 2014
>>>>>> @@ -832,7 +832,7 @@ NOINLINE static int LargeFunction(bool d
>>>>>>    x[18]++;
>>>>>>    x[19]++;
>>>>>>
>>>>>> -  delete x;
>>>>>> +  delete[] x;
>>>>>>    return res;
>>>>>>  }
>>>>>>
>>>>>>
>>>>>> Modified: compiler-rt/trunk/lib/msan/tests/msan_test.cc
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/tests/msan_test.cc?rev=221769&r1=221768&r2=221769&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- compiler-rt/trunk/lib/msan/tests/msan_test.cc (original)
>>>>>> +++ compiler-rt/trunk/lib/msan/tests/msan_test.cc Tue Nov 11 22:19:57
>>>>>> 2014
>>>>>> @@ -570,7 +570,7 @@ TEST(MemorySanitizer, fread) {
>>>>>>    EXPECT_NOT_POISONED(x[16]);
>>>>>>    EXPECT_NOT_POISONED(x[31]);
>>>>>>    fclose(f);
>>>>>> -  delete x;
>>>>>> +  delete[] x;
>>>>>>  }
>>>>>>
>>>>>>  TEST(MemorySanitizer, read) {
>>>>>> @@ -583,7 +583,7 @@ TEST(MemorySanitizer, read) {
>>>>>>    EXPECT_NOT_POISONED(x[16]);
>>>>>>    EXPECT_NOT_POISONED(x[31]);
>>>>>>    close(fd);
>>>>>> -  delete x;
>>>>>> +  delete[] x;
>>>>>>  }
>>>>>>
>>>>>>  TEST(MemorySanitizer, pread) {
>>>>>> @@ -596,7 +596,7 @@ TEST(MemorySanitizer, pread) {
>>>>>>    EXPECT_NOT_POISONED(x[16]);
>>>>>>    EXPECT_NOT_POISONED(x[31]);
>>>>>>    close(fd);
>>>>>> -  delete x;
>>>>>> +  delete[] x;
>>>>>>  }
>>>>>>
>>>>>>  TEST(MemorySanitizer, readv) {
>>>>>> @@ -2807,7 +2807,7 @@ TEST(MemorySanitizer, scanf) {
>>>>>>    EXPECT_NOT_POISONED(s[4]);
>>>>>>    EXPECT_NOT_POISONED(s[5]);
>>>>>>    EXPECT_POISONED(s[6]);
>>>>>> -  delete s;
>>>>>> +  delete[] s;
>>>>>>    delete d;
>>>>>>  }
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141112/eb4eb3ef/attachment.html>