751195 - Turn ASan function instrumentation blacklist into code annotations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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)]

Attachment: [Part 1] Annotate MarkRangeConservatively function

First part, this blacklists the MarkRangeConservatively function.

Comment 2

[Christian Holler (:decoder)]

Manually sent this to try since autoland seems to be broken.

Comment 3

[Christian Holler (:decoder)]

Attachment: [Part 2] Remove asan-blacklist.txt file from tree

Comment 4

[Christian Holler (:decoder)]

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 [inactive unless it's an emergency] (:billm)]

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)]

Attachment: [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] (NOT official MoCo now)]

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]]

https://hg.mozilla.org/mozilla-central/rev/da3bc1a73045

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/844211849448

Comment 10

[Mozilla RelEng Bot]

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.