Disable alloc-dealloc checking when running tests in AddressSanitizer

RESOLVED FIXED in mozilla25

Status

Firefox Build System
General
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

(Blocks: 1 bug)

Trunk
mozilla25
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Right now, we have several alloc-dealloc mismatches being reported by ASan.

The alloc-dealloc-mismatch feature is supposed to detect mixing of C and C++ allocators, e.g. allocating memory with malloc and then deleting it, or allocating with new and then calling free. However, since we redefine new/delete using our own implementation (that is backed by malloc/free), this is not a very effective check right now.

We also have situations right now, where the function produces false positives, e.g. when a call to "new" is inlined, but the call to "delete" is not. Overall, several tests (gtests, mochitests, reftests) are failing (and aborted) because these mismatches are detected.

I suggest we disable the check until we find a better solution to use it properly. It is more important that we can run all tests (and finally get them all green).

I'll prepare patches that set ASAN_OPTIONS=alloc_dealloc_mismatch=0 when running all of the affected tests.
Didn't you successfully run tests with the mozalloc.h fixup we discussed on irc?
(Assignee)

Comment 2

5 years ago
(In reply to Mike Hommey [:glandium] from comment #1)
> Didn't you successfully run tests with the mozalloc.h fixup we discussed on
> irc?

No, unfortunately not. The TestStartupCache compiled test was giving me so much trouble that I decided it's unlikely we're going to fix all of these issues in a short time (which doesn't mean we shouldn't try it anyway).

If you want, I can provide you with instructions how to reproduce, or provide you access to one of these build environments, so you can take a look.

However, if we usually map new/delete to malloc/free anyway, is there any benefit in having this alloc-dealloc mismatch check on our codebase?

Comment 3

5 years ago
> We also have situations right now, where the function produces false positives, 
> e.g. when a call to "new" is inlined, but the call to "delete" is not.

Might be worth filing a bug against ASan, especially if you have a reduced C++ testcase.

Comment 4

5 years ago
This check also notices "delete" vs "delete[]", which matters for us more than "delete" vs "free".

That said, we should probably disable the check for now.
(Assignee)

Comment 5

5 years ago
Created attachment 781795 [details] [diff] [review]
asan-options.patch

The attached patch does two things:

First, it moves __asan_default_options from the JS engine into mozglue, because that's where it actually belongs (I've initially added it to the JS engine because we only required that one option due to ASM.js).

Second, it adds alloc_dealloc_mismatch=0 to the options.


Tested this with a local build and works like a charm. Mike, is this the right way with mozglue?
Assignee: nobody → choller
Status: NEW → ASSIGNED
Attachment #781795 - Flags: review?(mh+mozilla)

Comment 6

5 years ago
> Second, it adds alloc_dealloc_mismatch=0 to the options.

Maybe only do this when our custom allocator is enabled?
(Assignee)

Comment 7

5 years ago
(In reply to Jesse Ruderman from comment #6)
> > Second, it adds alloc_dealloc_mismatch=0 to the options.
> 
> Maybe only do this when our custom allocator is enabled?

We're always using our own new/delete as far as I can see, so there is currently no switch to disable it.
Attachment #781795 - Flags: review?(mh+mozilla) → review+

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/4f899e59e268
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

4 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.