ASan: Enable memory-saving options for tests when running on test slaves

RESOLVED FIXED

Status

--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

({sec-want})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [asan][asan-test-failure][asan-test-blocker])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 704588 [details] [diff] [review]
Patch

Running our tests (especially mochitests) on ASan builds consumes a lot of memory. In the standard configuration, this memory requirement even exceeds 2 GB of RAM, which seems to be the maximum available on the test slaves.

Previously, we unconditionally enabled several memory-safety hacks when pushing to try with tests enabled. Since we're working on enabling tests for ASan builds in bug 831491, we need to get these into the tree now. However, it would be unwise to enable these options unconditionally, even if they only affect ASan builds: With the options enabled, chances are that under rare conditions, bugs don't get detected where the default options would detect them. Therefore we should always run under default options if the machine has enough memory.

In the attached patch, I solved the problem by enabling the options only when 2 GB or less of total memory are available.

Furthermore, the patch disables most ASan capabilities unconditionally for the xpcshell server that serves the mochitests. The reason is that this process consumes a lot of memory with ASan but it's not the unit being tested here so there is little benefit in running that xpcshell instance with ASan. Since it's not possible by the build system to use a second non-asanified xpcshell here, the patch tries to cut down memory consumption as far as possible.

Patch is on try now for testing, review/feedback welcome :)
Attachment #704588 - Flags: review?(jmaher)
Comment on attachment 704588 [details] [diff] [review]
Patch

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

r=me with some error checking around the totalMemory line.

::: build/automation.py.in
@@ +814,5 @@
>      env['NS_TRACE_MALLOC_DISABLE_STACKS'] = '1'
> +
> +    # ASan specific environment stuff
> +    if IS_ASAN and (IS_LINUX or IS_MAC):
> +      totalMemory = int(os.popen("free").readlines()[1].split()[1])

have you tested this on all linux and osx platforms?  I would like to see this in a try/except block to be safer.
Attachment #704588 - Flags: review?(jmaher) → review+
(Assignee)

Updated

6 years ago
Assignee: nobody → choller
(Assignee)

Comment 2

6 years ago
I wrapped the code in a double try/except to catch OSError and possibly other errors, output a meaningful message and default to the non-memory-saving options. This is on inbound now:

https://hg.mozilla.org/integration/mozilla-inbound/rev/08c14f94fbb7
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/08c14f94fbb7
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 902157
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.