Last Comment Bug 751195 - Turn ASan function instrumentation blacklist into code annotations
: Turn ASan function instrumentation blacklist into code annotations
Status: RESOLVED FIXED
[sg:want][asan-build-blocker]
: sec-want
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: mozilla15
Assigned To: Christian Holler (:decoder)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-02 08:45 PDT by Christian Holler (:decoder)
Modified: 2012-05-11 15:20 PDT (History)
8 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
[Part 1] Annotate MarkRangeConservatively function (1.54 KB, patch)
2012-05-03 04:57 PDT, Christian Holler (:decoder)
wmccloskey: review+
Details | Diff | Splinter Review
[Part 2] Remove asan-blacklist.txt file from tree (547 bytes, patch)
2012-05-03 18:31 PDT, Christian Holler (:decoder)
khuey: review+
gary: checkin+
Details | Diff | Splinter Review
[Part 1] Annotate MarkRangeConservatively function (1.21 KB, patch)
2012-05-04 17:02 PDT, Christian Holler (:decoder)
choller: review+
gary: checkin+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-05-02 08:45:28 PDT
I recently got advised by the Google developers of AddressSanitizer that the way of blacklisting (preventing instrumentation) of functions we use (-mllvm -asan-blacklist file.txt) is deprecated. Instead we should use __attribute__((no_address_safety_analysis)) on every blacklisted function (guarded with an #ifdef MOZ_ASAN so gcc does not warn about the unknown attribute).

At the same time, we will be able to remove the build/asan/asan-blacklist.txt file that got recently added.

I'm filing this in the JS engine component because all blacklisted functions are located here.
Comment 1 Christian Holler (:decoder) 2012-05-03 04:57:48 PDT
Created attachment 620652 [details] [diff] [review]
[Part 1] Annotate MarkRangeConservatively function

First part, this blacklists the MarkRangeConservatively function.
Comment 2 Christian Holler (:decoder) 2012-05-03 17:03:00 PDT
Manually sent this to try since autoland seems to be broken.
Comment 3 Christian Holler (:decoder) 2012-05-03 18:31:25 PDT
Created attachment 620920 [details] [diff] [review]
[Part 2] Remove asan-blacklist.txt file from tree
Comment 4 Christian Holler (:decoder) 2012-05-03 18:32:57 PDT
We won't need any further blacklisting for now as per discussion with espindola, we will be building debug+opt builds to avoid stack space exhaustion. If this becomes an issue later, we can still prevent certain functions from being instrumented or inlined.
Comment 5 Bill McCloskey (:billm) 2012-05-04 16:18:01 PDT
Comment on attachment 620652 [details] [diff] [review]
[Part 1] Annotate MarkRangeConservatively function

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

::: js/public/Utility.h
@@ +98,5 @@
>  #define JS_STATIC_ASSERT_IF(cond, expr)  MOZ_STATIC_ASSERT_IF(cond, expr, "JS_STATIC_ASSERT_IF")
>  
> +
> +#ifdef MOZ_ASAN
> +#define MOZ_ASAN_BLACKLIST JS_NEVER_INLINE __attribute__((no_address_safety_analysis))

Please move this to mfbt/Attributes.h and use MOZ_NEVER_INLINE instead.
Comment 6 Christian Holler (:decoder) 2012-05-04 17:02:23 PDT
Created attachment 621223 [details] [diff] [review]
[Part 1] Annotate MarkRangeConservatively function

Address review comments, looks much better like this. Thanks billm! :)

Carrying r+ and requesting checkin.
Comment 7 Gary Kwong [:gkw] [:nth10sd] 2012-05-04 23:12:02 PDT
Part 1 landed as:

http://hg.mozilla.org/integration/mozilla-inbound/rev/da3bc1a73045

and part 2 landed as:

http://hg.mozilla.org/integration/mozilla-inbound/rev/844211849448
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-05-05 20:26:24 PDT
https://hg.mozilla.org/mozilla-central/rev/da3bc1a73045
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-05-05 20:26:33 PDT
https://hg.mozilla.org/mozilla-central/rev/844211849448
Comment 10 Mozilla RelEng Bot 2012-05-11 15:20:35 PDT
Autoland Patchset:
	Patches: 620652
	Branch: mozilla-central => try
Patch 620652 could not be applied to mozilla-central.
patching file js/src/jsgc.cpp
Hunk #1 FAILED at 1079
1 out of 1 hunks FAILED -- saving rejects to file js/src/jsgc.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patch 620652 could not be applied to mozilla-central.
patching file js/public/Utility.h
Hunk #1 FAILED at 91
1 out of 1 hunks FAILED -- saving rejects to file js/public/Utility.h.rej
patching file js/src/jsgc.cpp
Hunk #1 FAILED at 1079
1 out of 1 hunks FAILED -- saving rejects to file js/src/jsgc.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patchset could not be applied and pushed.

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