797900 - Disable certain crashtests under AddressSanitizer

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Christian Holler (:decoder)]

Currently, two crashtests are failing under ASan, one is bug 787715, the other one is an intermittent failure from one of the image tests, which does not look like a browser bug though, but rather a problem in ASan itself:

REFTEST TEST-START | file:///home/cltbld/talos-slave/test/build/reftest/tests/image/test/crashtests/invalid-size-second-frame.gif | 2017 / 2117 (95%)
==2227== ERROR: Failed to allocate 0x100001000 (4294971392) bytes of MmapNewPagesAndPoisonShadow
==2227== AddressSanitizer CHECK failed: /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc:50 "(("unable to mmap" && 0)) != (0)" (0x0, 0x0)


I would like to disable both of the tests when running with ASan because they prevent the rest of the tests from running through on try. But in order to do so, a variable must be defined in the reftest sandbox that allows to detect if we're running an ASan build.

I'll attach a patch that adds this, and disables the two tests. If the test disabling should be separated from this patch, let me know.

Comment 1

[Christian Holler (:decoder)]

Attachment: Patch

Patch as described in comment 0. dbaron, ted mentioned you should review this rather than him, so r? you :)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 668020 [details] [diff] [review]
Patch

If ASAN is a build-time switch only (i.e., not both a build-time and run-time switch), then this looks ok.

(But if it has a run-time switch too, then these should only get disabled when it's actually running; otherwise we risk losing coverage if we start building it more but not running it.)

Comment 4

[Christian Holler (:decoder)]

(In reply to David Baron [:dbaron] from comment #2)
> Comment on attachment 668020 [details] [diff] [review]
> Patch
> 
> If ASAN is a build-time switch only (i.e., not both a build-time and
> run-time switch), then this looks ok.

Yes, this is a build-time thing only :)

There are a few more tests in the same area that trigger the same failure randomly (I think 4-5 other tests, mainly .xul). Shall I include them also in this patch?

Comment 7

[JP Rosevear [:jpr]]

Is there any reason not to land this?

Comment 8

[Christian Holler (:decoder)]

After creating the patch I noticed that the crash in bug 787715 is triggered by more than just the one xul file mentioned in the patch here. We could disable all of them, but I was still hoping that someone could just fix the bug. 

For the gif test, we should land it in any case, and I can do that.

Comment 9

[Christian Holler (:decoder)]

Landed minus the xul part, we'll take care of that separately.

https://hg.mozilla.org/integration/mozilla-inbound/rev/109d927c0696

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/109d927c0696