Last Comment Bug 746951 - Avoid inlining js::MarkRangeConservatively with AddressSanitizer builds
: Avoid inlining js::MarkRangeConservatively with AddressSanitizer builds
Status: RESOLVED FIXED
[asan][sg:want]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All Linux
: -- critical (vote)
: mozilla14
Assigned To: Christian Holler (:decoder)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-19 05:44 PDT by Christian Holler (:decoder)
Modified: 2012-04-21 23:53 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (500 bytes, patch)
2012-04-19 05:46 PDT, Christian Holler (:decoder)
wmccloskey: review+
akeybl: approval‑mozilla‑central+
gary: checkin+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-04-19 05:44:17 PDT
The function js::MarkRangeConservatively is currently the only function in the JS garbage collector that produces a false positive when not blacklisted. The reason is that this function scans (and thereby touches) the stack, including the redzones that AddressSanitizer adds between stack variables. Therefore, this function has been blacklisted from the beginning, avoiding it to be instrumented at all.

However, there is a bug in how this blacklist works together with inlining: As soon as a blacklisted function is inlined, it is still instrumented because the inlining step happens before AddressSanitizer instruments the code. The best solution I can see right now is to forbid inlining this function when building with AddressSanitizer.

Therefore, I'll add a patch adds JS_NEVER_INLINE to the function when MOZ_ASAN is defined.
Comment 1 Christian Holler (:decoder) 2012-04-19 05:46:25 PDT
Created attachment 616531 [details] [diff] [review]
Patch
Comment 2 Mozilla RelEng Bot 2012-04-19 05:51:14 PDT
Autoland Patchset:
	Patches: 616531
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=bceb471aa567
Try run started, revision bceb471aa567. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=bceb471aa567
Comment 3 Mozilla RelEng Bot 2012-04-19 09:00:22 PDT
Try run for bceb471aa567 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=bceb471aa567
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-bceb471aa567
Comment 4 Christian Holler (:decoder) 2012-04-20 03:00:22 PDT
Comment on attachment 616531 [details] [diff] [review]
Patch

I'd like to order one landing, please :D
Comment 5 Gary Kwong [:gkw] [:nth10sd] 2012-04-20 10:38:16 PDT
(In reply to Christian Holler (:decoder) from comment #4)
> Comment on attachment 616531 [details] [diff] [review]
> Patch
> 
> I'd like to order one landing, please :D

Approval is required even for mozilla-inbound now, till April 24. I could land on Birch without approval, but that will land only after April 24.

Please request for approval-mozilla-central? flag, and leave a comment with the reasons for your request.

ref https://wiki.mozilla.org/Tree_Rules#mozilla-central_.28Nightly_channel.29
Comment 6 Christian Holler (:decoder) 2012-04-20 10:49:25 PDT
Comment on attachment 616531 [details] [diff] [review]
Patch

This change is not part of any build (a=npotb), as they are only active when compiling with ASan (AddressSanitizer).
Comment 7 Alex Keybl [:akeybl] 2012-04-20 15:44:59 PDT
Comment on attachment 616531 [details] [diff] [review]
Patch

Sounds good, you can just land with a=npotb but I'll approve here anyway.
Comment 8 Gary Kwong [:gkw] [:nth10sd] 2012-04-20 16:08:04 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/ba03257fab28
Comment 9 Phil Ringnalda (:philor) 2012-04-21 23:53:58 PDT
https://hg.mozilla.org/mozilla-central/rev/ba03257fab28

Note You need to log in before you can comment on or make changes to this bug.