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?
(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?
> 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.
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.
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)
> Second, it adds alloc_dealloc_mismatch=0 to the options. Maybe only do this when our custom allocator is enabled?
(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+
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.