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.

[Renato Golin]

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