Add the ability to defer the initialization of the SyzyAsan crash reporter.

Add the ability to defer the initialization of the SyzyAsan crash reporter.

This CL adds a new flag to the SYZYASAN_RTL_OPTIONS (which can also be set at
instrumentation time) to delay the initialization of the crash reporter. By
default the runtime try to initialize its crash reporter (e.g. connecting to the
Crashpad one) during its initialization (i.e. when the SyzyAsan runtime get
loaded), but in some cases this can fail because the crash reporter that we try
to connect to isn't ready yet. This function adds a new function that the
instrumented process can call once it has initialized his crash reporter.

We don't need this for Chrome because by the time we load the instrumented
chrome.dll (which loads syzyasan_rtl.dll) we know that the Crashpad reporter has
been initialized so we can connect to it directly, but this doesn't work when
instrumenting a .exe file, so if it want to use the advanced crash reporting it'll
need to use this new flag at compile time and to manually call the
asan_InitializeCrashHandler function once its crash reporter is ready.

* By 'crash reporter' I mean Crashpad.



BUG=chromium:557759
Review-Url: https://codereview.chromium.org/2576003002
Committed: https://github.com/google/syzygy/commit/666a753011a7f9f2f10adadf6ae5e65d8b6af7b4

[Sébastien Marchand, 2016-12-14 22:56:25]

sebmarchand@chromium.org changed reviewers:
+ chrisha@chromium.org

[Sébastien Marchand, 2016-12-14 22:56:26]

Here's my attempt at making SyzyAsan work with setup.exe (we need a deferred
initialization of the crash reporter).

The unittest doesn't work yet, but does this approach makes sense?

https://codereview.chromium.org/2576003002/diff/20001/syzygy/agent/asan/rtl_i...
File syzygy/agent/asan/rtl_impl_unittest.cc (right):

https://codereview.chromium.org/2576003002/diff/20001/syzygy/agent/asan/rtl_i...
syzygy/agent/asan/rtl_impl_unittest.cc:157: agent::asan::ReporterInterface*
reporter = asan_runtime_.crash_reporter();
This test doesn't work as this test fixture can't initialize a Crashpad
reporter, I'll try to find how to fix this.

[chrisha, 2016-12-15 18:48:49]

I'd prefer this in conjunction with a flag. The presence of the flag in the
instrumented image would mean that the reporter doesn't even attempt to
initialize it to begin with... I think being explicit is easier to understand
than "magic".

https://codereview.chromium.org/2576003002/diff/20001/syzygy/agent/asan/gen/s...
File syzygy/agent/asan/gen/system_interceptors.def (right):

https://codereview.chromium.org/2576003002/diff/20001/syzygy/agent/asan/gen/s...
syzygy/agent/asan/gen/system_interceptors.def:142: ; Reinitialize the SyzyAsan
crash reporter.
This is initializing the crash reporter, not reinitializing it, no?

[Sébastien Marchand, 2016-12-15 22:20:21]

Wdyt of this approach? unittests still don't work

[Sébastien Marchand, 2017-01-10 21:49:20]

PTAL.

[chrisha, 2017-01-11 18:57:29]

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/rtl_...
File syzygy/agent/asan/rtl_impl.h (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:117: 
More comments here please. Exactly what this does, how it is supposed to work,
that this does nothing if the resulting flag isn't used, etc.

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
File syzygy/agent/asan/runtime.cc (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
syzygy/agent/asan/runtime.cc:1207: DCHECK(!crash_reporter_initialized_);
Document this contract in the function description.

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
File syzygy/agent/asan/runtime.h (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
syzygy/agent/asan/runtime.h:200: // Initialize the crash reporter used by the
runtime.
More comments about how this is related to the deferred initialization flag.
What happens if I call this after it's already initialized? If the flag isn't
specified, etc? Detail the "contract" associated with use of this function.

[Sébastien Marchand, 2017-01-11 19:52:42]

Description was changed from

==========
Add an entry point to reinitialize the crash reporter.

BUG=chromium:557759
==========

to

==========
Add the ability to defer the initialization of the SyzyAsan crash reporter.

This CL adds a new flag to the SYZYASAN_RTL_OPTIONS (which can also be set at
instrumentation time) to delay the initialization of the crash reporter. By
default the runtime try to initialize its crash reporter (e.g. connecting to the
Crashpad one) during its initialization (i.e. when the SyzyAsan runtime get
loaded), but in some cases this can fail because the crash reporter that we try
to connect to isn't ready yet. This function adds a new function that the
instrumented process can call once it has initialized his crash reporter.

We don't need this for Chrome because by the time we load the instrumented
chrome.dll (which loads syzyasan_rtl.dll) we know that the Crashpad reporter has
been initialized so we can connect to it directly, but this doesn't work when
instrumenting a .exe file, so if it want to use the advanced crash reporting
it'll
need to use this new flag at compile time and to manually call the
asan_InitializeCrashHandler function once its crash reporter is ready.

* By 'crash reporter' I mean Crashpad.



BUG=chromium:557759
==========

[Sébastien Marchand, 2017-01-11 21:38:08]

Added more comments and updated the CL description.

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/rtl_...
File syzygy/agent/asan/rtl_impl.h (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:117: 
On 2017/01/11 18:57:28, chrisha (slow) wrote:
> More comments here please. Exactly what this does, how it is supposed to work,
> that this does nothing if the resulting flag isn't used, etc.

Done.

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
File syzygy/agent/asan/runtime.cc (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
syzygy/agent/asan/runtime.cc:1207: DCHECK(!crash_reporter_initialized_);
On 2017/01/11 18:57:28, chrisha (slow) wrote:
> Document this contract in the function description.

Done. Turned this into a CHECK.

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
File syzygy/agent/asan/runtime.h (right):

https://codereview.chromium.org/2576003002/diff/160001/syzygy/agent/asan/runt...
syzygy/agent/asan/runtime.h:200: // Initialize the crash reporter used by the
runtime.
On 2017/01/11 18:57:28, chrisha (slow) wrote:
> More comments about how this is related to the deferred initialization flag.
> What happens if I call this after it's already initialized? If the flag isn't
> specified, etc? Detail the "contract" associated with use of this function.

Done.

[Sébastien Marchand, 2017-01-13 18:04:58]

ping?

[chrisha, 2017-01-13 18:16:26]

lgtm!

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
File syzygy/agent/asan/rtl_impl.h (right):

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:116: // This functions allows to manually
initialize the crash reporter used by the
allows manually initializing* and* is* used

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:121: // instrumentation time) and should only be
called once, trying to initialize
once. Trying to initialize the crash handler a second time will result in
undefined behaviour.

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:128: // initialization flag):
In addition to instrumenting with the deferred initialization flag, the
instrumented image should do something like the following:

...

[Sébastien Marchand, 2017-01-13 20:56:02]

Thanks, committing, I've also replaced handler by reporter in the comments to be
more consistent.

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
File syzygy/agent/asan/rtl_impl.h (right):

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:116: // This functions allows to manually
initialize the crash reporter used by the
On 2017/01/13 18:16:26, chrisha (slow) wrote:
> allows manually initializing* and* is* used

Done.

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:121: // instrumentation time) and should only be
called once, trying to initialize
On 2017/01/13 18:16:26, chrisha (slow) wrote:
> once. Trying to initialize the crash handler a second time will result in
> undefined behaviour.

Done.

https://codereview.chromium.org/2576003002/diff/180001/syzygy/agent/asan/rtl_...
syzygy/agent/asan/rtl_impl.h:128: // initialization flag):
On 2017/01/13 18:16:26, chrisha (slow) wrote:
> In addition to instrumenting with the deferred initialization flag, the
> instrumented image should do something like the following:
> 
> ...

Done.

[Sébastien Marchand, 2017-01-13 20:56:10]

The CQ bit was checked by sebmarchand@chromium.org

[Sébastien Marchand, 2017-01-13 20:56:11]

The patchset sent to the CQ was uploaded after l-g-t-m from chrisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2576003002/#ps200001
(title: "Fix comments.")

[commit-bot: I haz the power, 2017-01-13 20:56:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 21:39:14]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1484340970925140,
"parent_rev": "37f2efe2518802e568d2b620309c0c4a939e52f1", "commit_rev":
"666a753011a7f9f2f10adadf6ae5e65d8b6af7b4"}

[commit-bot: I haz the power, 2017-01-13 21:39:17]

Description was changed from

==========
Add the ability to defer the initialization of the SyzyAsan crash reporter.

This CL adds a new flag to the SYZYASAN_RTL_OPTIONS (which can also be set at
instrumentation time) to delay the initialization of the crash reporter. By
default the runtime try to initialize its crash reporter (e.g. connecting to the
Crashpad one) during its initialization (i.e. when the SyzyAsan runtime get
loaded), but in some cases this can fail because the crash reporter that we try
to connect to isn't ready yet. This function adds a new function that the
instrumented process can call once it has initialized his crash reporter.

We don't need this for Chrome because by the time we load the instrumented
chrome.dll (which loads syzyasan_rtl.dll) we know that the Crashpad reporter has
been initialized so we can connect to it directly, but this doesn't work when
instrumenting a .exe file, so if it want to use the advanced crash reporting
it'll
need to use this new flag at compile time and to manually call the
asan_InitializeCrashHandler function once its crash reporter is ready.

* By 'crash reporter' I mean Crashpad.



BUG=chromium:557759
==========

to

==========
Add the ability to defer the initialization of the SyzyAsan crash reporter.

This CL adds a new flag to the SYZYASAN_RTL_OPTIONS (which can also be set at
instrumentation time) to delay the initialization of the crash reporter. By
default the runtime try to initialize its crash reporter (e.g. connecting to the
Crashpad one) during its initialization (i.e. when the SyzyAsan runtime get
loaded), but in some cases this can fail because the crash reporter that we try
to connect to isn't ready yet. This function adds a new function that the
instrumented process can call once it has initialized his crash reporter.

We don't need this for Chrome because by the time we load the instrumented
chrome.dll (which loads syzyasan_rtl.dll) we know that the Crashpad reporter has
been initialized so we can connect to it directly, but this doesn't work when
instrumenting a .exe file, so if it want to use the advanced crash reporting
it'll
need to use this new flag at compile time and to manually call the
asan_InitializeCrashHandler function once its crash reporter is ready.

* By 'crash reporter' I mean Crashpad.



BUG=chromium:557759

Review-Url: https://codereview.chromium.org/2576003002
Committed:
https://github.com/google/syzygy/commit/666a753011a7f9f2f10adadf6ae5e65d8b6af7b4
==========

[commit-bot: I haz the power, 2017-01-13 21:39:18]

Committed patchset #11 (id:200001) as
https://github.com/google/syzygy/commit/666a753011a7f9f2f10adadf6ae5e65d8b6af7b4