1222043 - Make "mach xpcshell-test" more TSan-friendly

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Julian Seward [:jseward]]

"mach xpcshell-test" is difficult to use for TSan builds, for two reasons:

(1) a TSan-ified process returns with a non-zero exit code if any
    races are detected.  This causes the framework to incorrectly
    conclude that some processes have failed.

(2) mach xpcshell-test ignores any existing setting of the TSAN_OPTIONS
    environment variable, and sets its own value for it.  This makes
    xpcshell-tests difficult to use in practice for TSan runs, because
    it is often useful to be able to specify, for example, a custom
    suppression file using TSAN_OPTIONS.

Comment 1

[Julian Seward [:jseward]]

Attachment: WIP patch. Addresses both issues in comment #0.

Comment 2

[James Graham [:jgraham]]

Comment on attachment 8683769 [details] [diff] [review]
WIP patch.  Addresses both issues in comment #0.

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

Seems sensible to me. Move the 66 to a global and explain it there. Put the operators in binary expressions that split over multiple lines before the linebreak rather than right after.

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8683769 [details] [diff] [review]
WIP patch.  Addresses both issues in comment #0.

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

Looks good! Please fix my nit and also address jgraham's comment (move the 66 into a global TSAN_RETURN_CODE variable or similar along with the explanation).

::: testing/xpcshell/runxpcshelltests.py
@@ +936,5 @@
> +                        and self.env["TSAN_OPTIONS"] is not None):
> +                        tsanExtraOpts = " " + self.env["TSAN_OPTIONS"]
> +
> +                    self.env["TSAN_OPTIONS"] = ("external_symbolizer_path=%s" % llvmsym
> +                                                + tsanExtraOpts)

Just make this whole block (I assume the potential extra space at the end is ok):
self.env['TSAN_OPTIONS'] = "external_symbolizer_path={} {}".format(llvmsym, self.env.get('TSAN_OPTIONS', ''))

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb74d10a76f

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4cb74d10a76f