Re: [PATCH] tests: make SANITIZE=address imply TEST_NO_MALLOC_CHECK

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 12, 2022, 07:44 UTC
Message-ID: <220412.867d7uivuh.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqk0bvfcd6.fsf@gitster.g>

On Mon, Apr 11 2022, Junio C Hamano wrote:

Show 16 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I wonder why we have to justify that we'll only turn on
>> TEST_NO_MALLOC_CHECK if it's SANITIZE=address.
>>
>> I.e. we also have SANITIZE=undefined, wouldn't it be more future-proof
>> to just say that these analysis options are mutually exclusive by
>> default?
>
> Given that the SANITIZE mechanism itself allows more than one to be
> requested at the same time, it is unclear to me why other checks
> like undefined needs to exclude checks done by other mechanisms like
> MALLOC_CHECK_ by default.  If I correctly read under-the-three-dash
> commentary Phillip wrote, it's not like that use of MALLOC_CHECK_
> inherently interferes with the way SANITIZE=undefined wants to work,
> no?

Because:

 * It makes it slower, and part of the utility of these checks is that
   they run in a timely fashion.

 * We add these glibc checks because we'd like to catch malloc()/free()
   issues, and run the test suite with them by default.

   Someone using the SANITIZE=* feature is almost certain to be also
   doing a "normal" test run, so I don't think we're getting anything
   extra by combining the two, except needlessly slowing it down.

 * Even though SANITIZE=leak,address & valgrind are strictly speaking
   incompatible with the glibc check, having inject itself into other
   sanitize modes is surely going to make debugging harder until you
   discover that we're also injecting the custom malloc.