Add debug code to help understand SEGV in ~ErrorScreen.

Add debug code to help understand SEGV in ~ErrorScreen.

BUG=396557
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289279

[Thiemo Nagel, 2014-08-01 15:01:19]

Hi Yuri,

what would you think about this approach?  (I'm certainly not an expert in
diagnosing memory corruption.)

In my opinion, it is better to crash than to continue running with an invalid
state (we can't rule out memory corruption on a larger scale).  Also, to keep
crashing makes it less likely that the problem is forgotten eventually.  And in
case the problem gets worse (due to changes in other parts of the code) we will
be alerted by crash reports coming in.

Thank you and best regards,
Thiemo

[Thiemo Nagel, 2014-08-07 14:28:19]

Friendly ping.

[ygorshenin1, 2014-08-12 15:13:38]

Sorry for the delay in response - I was OOO. Good idea, thanks! LGTM

[ygorshenin1, 2014-08-12 15:15:35]

nit: probably it worth to make both canaries volatile - just to prevent smart
optimizations like constant propagation.

[Thiemo Nagel, 2014-08-12 16:01:15]

volatile sounds like a good idea.  I've also changed the uint to a void* so that
the canaries are of the same size as the pointer which they surround.  What do
you think now?

[ygorshenin1, 2014-08-13 08:36:51]

https://codereview.chromium.org/424613003/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/screens/error_screen.h (right):

https://codereview.chromium.org/424613003/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/screens/error_screen.h:106: volatile void*
canary_1_;  // For debugging of https://crbug.com/396557.
As you're just reading pointer's value, not a value pointers point to, you can
simply replace "void*" by "long" - it has the same size as a pointer but less
obfuscating.

[Thiemo Nagel, 2014-08-13 09:38:48]

https://codereview.chromium.org/424613003/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/screens/error_screen.h (right):

https://codereview.chromium.org/424613003/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/screens/error_screen.h:106: volatile void*
canary_1_;  // For debugging of https://crbug.com/396557.
The sizes of pointer and long depend on the data model.  Although I'm inclined
to believe that sizeof(void*)==sizeof(long) for all architectures supported by
Chromium OS, technically there is no guarantee for that and thus I think that
it's actually less obfuscating to use void* here.  (For example Windows 64bit
uses the LLP64 data model where sizeof(void*)==8 and sizeof(long)==4.)

What do you think?

[ygorshenin1, 2014-08-13 10:22:09]

It looks like you need to replace "volatile void*" by "void* volatile" since
only in the latter case pointer value will be checked each time pointer is used.

[Thiemo Nagel, 2014-08-13 11:26:40]

Thanks.  C is hard.  Are you happy with the latest version?

[ygorshenin1, 2014-08-13 11:58:27]

Thanks Thiemo, LGTM!

[Thiemo Nagel, 2014-08-13 12:06:00]

Thank you, too!

[Thiemo Nagel, 2014-08-13 12:06:04]

The CQ bit was checked by tnagel@chromium.org

[commit-bot: I haz the power, 2014-08-13 12:06:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tnagel@chromium.org/424613003/40001

[commit-bot: I haz the power, 2014-08-13 13:20:00]

Committed patchset #3 (40001) as 289279