Closed Bug 758010 Opened 9 years ago Closed 9 years ago

C++ new/delete aren't wrapped with jemalloc on Android

Categories

(Firefox Build System :: General, defect)

ARM
Android
defect
Not set
blocker

Tracking

(firefox14 fixed, blocking-fennec1.0 betaN+)

RESOLVED FIXED
mozilla15
Tracking Status
firefox14 --- fixed
blocking-fennec1.0 --- betaN+

People

(Reporter: bjacob, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

That has caused countless crashes in C++ libraries that we import into libxul/libgkmedias: see bug 746794 for ANGLE and bug 740194 for Skia. We need a good definitive solution so we don't keep running into allocator mismatch crashes over and over again.
Blocks: 746794, 740194
Other than ANGLE and Skia, there's also other-licenses/skia-npapi and ... js/src :-/
Severity: normal → blocker
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → betaN+
Depends on: 758773
Blocks: 758858
Doing this properly is going to be tough in the given timeframe, so for now, only address the immediate Android problem. Bug 758858 filed for the long-term solution.

Try builds with this patch applied will be there once built:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-b397cfa64854/
Attachment #627451 - Flags: review?(blassey.bugs)
Attachment #627451 - Flags: review?(khuey)
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

Review of attachment 627451 [details] [diff] [review]:
-----------------------------------------------------------------

I'm just going to assume that your mangled names are correct.
Attachment #627451 - Flags: review?(khuey) → review+
https://hg.mozilla.org/mozilla-central/rev/ef0180b53eae
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Attachment #627451 - Flags: review?(blassey.bugs)
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

[Approval Request Comment]
User impact if declined: random crashes due to memory allocator mismatch
Testing completed (on m-c, etc.): Landed on m-c today.
Risk to taking this patch (and alternatives if risky): It shouldn't break anything. In the worst case scenario it might unveil other allocator mismatches, but certainly much less severe than the ones being fixed (and it's a very hypothetical scenario)
String or UUID changes made by this patch: No string change.
Attachment #627451 - Flags: approval-mozilla-aurora?
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

Please land ASAP for the Fennec beta build.
Attachment #627451 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 695068
Brown paper bag: while operator new/delete were correctly wrapped, they were wrapped to system malloc/free...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Also make operator new distinguishible from operator new[] for valgrind. (likewise for delete/delete[]).
<none>
Attachment #628888 - Flags: review?(justin.lebar+bug)
Forgot to remove a (moved) prototype.
<none>
Attachment #628904 - Flags: review?(justin.lebar+bug)
Attachment #628888 - Attachment is obsolete: true
Attachment #628888 - Flags: review?(justin.lebar+bug)
Attachment #628904 - Flags: review?(justin.lebar+bug) → review+
https://hg.mozilla.org/mozilla-central/rev/306365557d64
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to Mike Hommey [:glandium] from comment #8)
> Brown paper bag: while operator new/delete were correctly wrapped, they were

Ah .. V's cross-DSO checking failed to pick this up because it intercepts
new/new[]/delete/delete[] as well as malloc/free, and so it would not see
that new etc were routing to the wrong place.  Ah well.
(In reply to Mike Hommey [:glandium] from comment #12)
> http://hg.mozilla.org/releases/mozilla-aurora/rev/540694f5df42

Can we also land on MOBILE140_2012053010_RELBRANCH on mozilla-aurora?
(In reply to Alex Keybl [:akeybl] from comment #14)
> (In reply to Mike Hommey [:glandium] from comment #12)
> > http://hg.mozilla.org/releases/mozilla-aurora/rev/540694f5df42
> 
> Can we also land on MOBILE140_2012053010_RELBRANCH on mozilla-aurora?

done https://hg.mozilla.org/releases/mozilla-aurora/rev/3dbfe52b2649
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.