Turn ASan function instrumentation blacklist into code annotations

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

({sec-want})

Trunk
mozilla15
All
Linux
sec-want
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:want][asan-build-blocker])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 620652 [details] [diff] [review]
[Part 1] Annotate MarkRangeConservatively function

First part, this blacklists the MarkRangeConservatively function.
Assignee: general → choller
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Whiteboard: [asan][sg:want] → [asan][sg:want][autoland-try]

Updated

5 years ago
Whiteboard: [asan][sg:want][autoland-try] → [asan][sg:want][autoland-in-queue]
Keywords: sec-want
(Assignee)

Comment 2

5 years ago
Manually sent this to try since autoland seems to be broken.
Whiteboard: [asan][sg:want][autoland-in-queue] → [asan][sg:want]
(Assignee)

Updated

5 years ago
Attachment #620652 - Flags: review?(bhackett1024)
(Assignee)

Comment 3

5 years ago
Created attachment 620920 [details] [diff] [review]
[Part 2] Remove asan-blacklist.txt file from tree
Attachment #620920 - Flags: review?(khuey)
(Assignee)

Comment 4

5 years ago
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.
Attachment #620920 - Flags: review?(khuey) → review+
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.
Attachment #620652 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 6

5 years ago
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.
Attachment #620652 - Attachment is obsolete: true
Attachment #621223 - Flags: review+
Attachment #621223 - Flags: checkin?(gary)
(Assignee)

Updated

5 years ago
Attachment #620920 - Flags: checkin?(gary)
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
Target Milestone: --- → mozilla15
Attachment #620920 - Flags: checkin?(gary) → checkin+
Attachment #621223 - Flags: checkin?(gary) → checkin+
(Assignee)

Updated

5 years ago
Whiteboard: [asan][sg:want] → [sg:want][asan-build-blocker]
https://hg.mozilla.org/mozilla-central/rev/da3bc1a73045
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/844211849448

Comment 10

5 years ago
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.
You need to log in before you can comment on or make changes to this bug.