r230160 - Relax the requirement on sized deallocation a bit: Default on unsized delete if sized delete is not provided in global scope, and -fdefine-sized-deallocation option is disabled.

[Alexey Samsonov]

On Mon, Feb 23, 2015 at 11:24 AM, Larisse Voufo <lvoufo at google.com> wrote:

>
>
> On Mon, Feb 23, 2015 at 11:16 AM, Kostya Serebryany <kcc at google.com>
> wrote:
>
>>
>>
>> On Mon, Feb 23, 2015 at 3:07 AM, Timur Iskhodzhanov <timurrrr at google.com>
>> wrote:
>>
>>> I'd like Kostya to comment.
>>>
>>> Green bots are no doubt better than red bots or no bots, but I'm not
>>> sure what level of ARM support our team has committed to.
>>>
>>
>> I'f we've broke something on a public bot we better fix it or roll it
>> back (and then fix it)
>>
>
> Could "-fdefine-sized-deallocation" help here? This enforces
> sized-deallocation with weak definitions.
>

Right, I believe this is the correct fix. I've submitted r230242 which adds
-fdefine-sized-deallocation to ASan test case.


>
>
>>
>>
>>>
>>> On Mon Feb 23 2015 at 1:43:10 PM Renato Golin <renato.golin at linaro.org>
>>> wrote:
>>>
>>>> On 23 February 2015 at 10:05, Timur Iskhodzhanov <timurrrr at google.com>
>>>> wrote:
>>>> > Kostya, what's our policy re: ARM sanitizer bots failures?
>>>>
>>>> Timur,
>>>>
>>>> As far as ARM buildbots are concerned, any breakage is critical. ARM,
>>>> like Intel, is a first-class architecture and we have to support it
>>>> fully. Whatever the buildbots pass today, they should pass tomorrow.
>>>> If we break that contract for one buildbot, we break for all of them,
>>>> and that is not acceptable.
>>>>
>>>> Your commit broke our bots by exposing a flaw in the sanitizer on the
>>>> ARM architecture. The correct way to deal with this is to revert the
>>>> patch and contact the bot owner, in this case, me, to fix the issue. I
>>>> can help you debug and even allow you into an ARM box at my house so
>>>> that you can do your tests, but as soon as we start marking
>>>> previously-passing tests as XFAIL or ignore broken bots, there will be
>>>> no stopping, and the quality of the whole toolchain will diminish.
>>>>
>>>> At Linaro, we have people working on both the address and the thread
>>>> sanitizers, and they can also work with you to fix the issue.
>>>>
>>>>
>>>> > This is a second revert in a row.
>>>>
>>>> I haven't reverted yet, just contacted the author of the patch to fix
>>>> it. If it's not possible to fix, or if other bugs start creeping in
>>>> (like was the case with your patch), I will revert them to help fix
>>>> the buildbot back to green. Another reason for reverting a patch that
>>>> is breaking a bot, is time. If the author doesn't respond in a day
>>>> after the initial breakage, we will revert the patch. That's standard
>>>> practice across all LLVM components / architectures.
>>>>
>>>> This may sound harsh, but a lot can happen in a day. This particular
>>>> failure is a clear demonstration of that, as it got introduced and
>>>> Larisse couldn't know, since the bot was already red.
>>>>
>>>>
>>>> > (see also r230019 where the failure happened after a trivial change)
>>>>
>>>> That trivial change has triggered a real bug in the sanitizer, and we
>>>> need to get at the bottom of that.
>>>>
>>>> According to the LLVM Developer Policy, patches submitted must not
>>>> regress on the make check or the test-suite on all supported
>>>> platforms. ARM is a supported platform for both Compiler-RT and ASAN,
>>>> so we should not regress.
>>>>
>>>> More importantly, you probably found a real bug in ASAN, and we should
>>>> be discussing how to fix *that*, instead of what's the policy on
>>>> reverting sanitizer changes because it fails on a platform that you're
>>>> not familiar with.
>>>>
>>>> cheers,
>>>> --renato
>>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150223/e6228c33/attachment.html>