832989 - Disable TestPoisonArea test under ASan due to incompatibility

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Christian Holler (:decoder)]

Attachment: Patch

The mentioned test is causing orange for ASan try-builds. According to dbaron, the test expects children to segfault which makes it incompatible when running under ASan. The attached patch disables it for ASan builds.

Comment 1

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

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

Presumably asan is an option that heavily instruments the code, so it's never something we'd consider turning on by default (even for debug builds)?  If that's the case, r=dbaron

Comment 2

[Ed Morley [:emorley]]

I don't believe we need to define CPP_UNIT_TESTS outside of the conditional, CCing :gps. (Also the build peers are keen to start having their review of build changes aiui)

Comment 3

[Zack Weinberg (:zwol)]

What does the test do (instead of segfaulting) when run under ASan, and is it possible to modify the test itself so that it still works when compiled that way?

Comment 4

[Christian Holler (:decoder)]

(In reply to Zack Weinberg (:zwol) from comment #3)
> What does the test do (instead of segfaulting) when run under ASan, and is
> it possible to modify the test itself so that it still works when compiled
> that way?

The test aborts with an ASan trace because ASan catches the segfault. It would be possible to parse stderr output in the test, but I don't see any benefits from doing that. ASan builds are in general used to do crash testing, so what we're testing here isn't really relevant for the build type.

Comment 5

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

(In reply to Ed Morley (Away 18th-20th Jan) [:edmorley UTC+0] from comment #2)
> I don't believe we need to define CPP_UNIT_TESTS outside of the conditional,
> CCing :gps.

We don't need to, but I think it's probably better to leave it because:

 (a) if somebody comes along later to add a unit test, they're less likely to accidentally add it inside the conditional due to not paying attention

 (b) if somebody comes along later and sees the conditional, they're less likely come along later and clobber CPP_UNIT_TESTS after the conditional

Comment 6

[Christian Holler (:decoder)]

(In reply to David Baron [:dbaron] from comment #5)

>  (a) if somebody comes along later to add a unit test, they're less likely
> to accidentally add it inside the conditional due to not paying attention

That was exactly my intention :)

Comment 7

[Christian Holler (:decoder)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6025bc9ec1d8

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/6025bc9ec1d8