Commit ea5d856a authored by Bruce Dawson's avatar Bruce Dawson Committed by Commit Bot

Prevent tail-call optimizations in thread watcher

thread_watcher_report_hang.cc has various unique functions which are
designed to be on the call stack when hangs are reported. Guaranteeing
this requires making all of the functions unique and inhibiting tail
calls. The tail call inhibiting was only applied to two of the functions
so this change applies it to the other two (commented out in one of
them) and adds the NOT_TAIL_CALLED attribute to the function they all
call. This attribute makes the ordering in the functions irrelevant but
there is no cost to having belt and suspenders, so why not?

This completes the work of crrev.com/c/1373878.

Bug: 905288, 1032679
Change-Id: I383cc3453ab2f506017de1143bc8f3666ed38a47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443891Reviewed-by: default avatarJesse Doherty <jwd@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813393}
parent 7b359c7c
...@@ -13,8 +13,9 @@ namespace metrics { ...@@ -13,8 +13,9 @@ namespace metrics {
// The following are unique function names for forcing the crash when a thread // The following are unique function names for forcing the crash when a thread
// is unresponsive. This makes it possible to tell from the callstack alone what // is unresponsive. This makes it possible to tell from the callstack alone what
// thread was unresponsive. // thread was unresponsive. Inhibiting tail calls to this function ensures that
NOINLINE void ReportThreadHang() { // the caller will appear on the call stack.
NOINLINE NOT_TAIL_CALLED void ReportThreadHang() {
volatile const char* inhibit_comdat = __func__; volatile const char* inhibit_comdat = __func__;
ALLOW_UNUSED_LOCAL(inhibit_comdat); ALLOW_UNUSED_LOCAL(inhibit_comdat);
...@@ -37,35 +38,29 @@ NOINLINE void ReportThreadHang() { ...@@ -37,35 +38,29 @@ NOINLINE void ReportThreadHang() {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
NOINLINE void StartupHang() { NOINLINE void StartupHang() {
volatile int inhibit_comdat = __LINE__;
ALLOW_UNUSED_LOCAL(inhibit_comdat);
// TODO(rtenneti): http://crbug.com/440885 enable crashing after fixing false // TODO(rtenneti): http://crbug.com/440885 enable crashing after fixing false
// positive startup hang data. // positive startup hang data.
// ReportThreadHang(); // ReportThreadHang();
volatile int inhibit_comdat = __LINE__;
ALLOW_UNUSED_LOCAL(inhibit_comdat);
} }
NOINLINE void ShutdownHang() { NOINLINE void ShutdownHang() {
ReportThreadHang();
volatile int inhibit_comdat = __LINE__; volatile int inhibit_comdat = __LINE__;
ALLOW_UNUSED_LOCAL(inhibit_comdat); ALLOW_UNUSED_LOCAL(inhibit_comdat);
ReportThreadHang();
} }
#endif // !defined(OS_ANDROID) #endif // !defined(OS_ANDROID)
NOINLINE void ThreadUnresponsive_UI() { NOINLINE void ThreadUnresponsive_UI() {
ReportThreadHang(); ReportThreadHang();
// Defining |inhibit_comdat| *after* calling ReportThreadHang() prevents tail
// call optimization from not putting this function's address on the stack.
// https://crbug.com/905288#c10
volatile int inhibit_comdat = __LINE__; volatile int inhibit_comdat = __LINE__;
ALLOW_UNUSED_LOCAL(inhibit_comdat); ALLOW_UNUSED_LOCAL(inhibit_comdat);
} }
NOINLINE void ThreadUnresponsive_IO() { NOINLINE void ThreadUnresponsive_IO() {
ReportThreadHang(); ReportThreadHang();
// Defining |inhibit_comdat| *after* calling ReportThreadHang() prevents tail
// call optimization from not putting this function's address on the stack.
// https://crbug.com/905288#c10
volatile int inhibit_comdat = __LINE__; volatile int inhibit_comdat = __LINE__;
ALLOW_UNUSED_LOCAL(inhibit_comdat); ALLOW_UNUSED_LOCAL(inhibit_comdat);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment