[webkit-reviews] review granted: [Bug 124610] [ASAN] WebKitLauncher: Include libasancrashreporter.dylib in DYLD_INSERT_LIBRARIES if it exists : [Attachment 217365] Patch

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted David Farler
<dfarler at apple.com>'s request for review:
Bug 124610: [ASAN] WebKitLauncher: Include libasancrashreporter.dylib in
DYLD_INSERT_LIBRARIES if it exists
https://bugs.webkit.org/show_bug.cgi?id=124610

Attachment 217365: Patch
https://bugs.webkit.org/attachment.cgi?id=217365&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=217365&action=review


r=me assuming that the scenarios I mentioned work correctly.

> Tools/WebKitLauncher/WebKitNightlyEnabler.m:195
> +    if (pathToASanCrashReporterLib)
> +	   setenv("DYLD_INSERT_LIBRARIES", [pathToASanCrashReporterLib
UTF8String], 1);

This will result in the library being injected in to any subprocess launched by
Safari. Back in the day that would include applications launched via
LaunchServices calls that Safari happened to make (e.g., double-clicking on a
file in the Downloads popover). It'd be worth confirming that this doesn't
cause any problems.

Have you confirmed that this is sufficient to have the dylib be injected in to
the XPC services that WebKit2 launches (e.g., both the web and network
processes)?

> Tools/WebKitLauncher/main.m:245
> +	   dyldInsertLibraries = [@[pathToASanCrashReporterLib,
pathToEnablerLib] componentsJoinedByString:@":"];

Our style calls for spaces inside array literal: @[ foo, bar ].