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

[David Blaikie]

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.
>
>
>>
>>
>>>
>>>>> 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/83c431af/attachment.html>