New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 861636 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Wrong Magic Signature "FX_OutOfMemoryTerminate" is given to crash reports because of ThinLTO

Project Member Reported by hashimoto@chromium.org, Jul 9

Issue description

(From https://bugs.chromium.org/p/chromium/issues/detail?id=508489#c33)

In M67, http://crrev.com/c/949089 enabled ThinLTO[1] on all AMD64 platforms.
This resulted in folding base::debug::BreakDebugger() and FX_OutOfMemoryTerminate() (and 2 other functions ->  issue 860850 ) together.
Because of this, the crash server cannot label stack traces which contain base::debug::BreakDebugger() correctly, and many crash reports were labelled with wrong Magic Signature "FX_OutOfMemoryTerminate".

[1]: https://clang.llvm.org/docs/ThinLTO.html
 
3 years ago, I landed http://crrev.com/345663 to make base::debug::BreakDebugger() unfoldable, but it stopped working after ThinLTO was enabled.

The most straight forward solution is that making it unique in a way even ThinLTO cannot deduplicate (e.g. outputting a log line which contains the name of the function).
Owner: hashimoto@chromium.org
Status: Started (was: Available)
Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/1128693 to disable optimization for base::debug::Alias().
This will make base::debug::BreakDebugger() unique again.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a31f26075580e73b05da1bb1d899b5edec38cea8

commit a31f26075580e73b05da1bb1d899b5edec38cea8
Author: Ryo Hashimoto <hashimoto@chromium.org>
Date: Mon Jul 09 18:08:14 2018

Do not optimize base::debug::Alias().

Optimization is already disabled for MSVC, do the same thing for Clang
too.

BUG= 861636 
TEST=Run dump_syms against chrome (w/ symbol_level=3, use_thin_lto=true,
use_lld=true) and manually checked the output.

Change-Id: Ia275e2310ccb8e70a2392d95ca4acadf2f218b01
Reviewed-on: https://chromium-review.googlesource.com/1128693
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573365}
[modify] https://crrev.com/a31f26075580e73b05da1bb1d899b5edec38cea8/base/debug/alias.cc

Labels: Merge-Request-67 Merge-Request-68
Requesting to merge to 68 and 67 (if it's not too late).
https://crrev.com/573365 is needed to fix the crash server's dashboard.
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 10

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
It's too late for M67.  Has the change been tested? Assume it's limited to the dump output?
https://crrev.com/573365 makes crash reports include more info, and fixes the wrong Magic Signatures displayed on the crash server.

I did manual testing locally, but unfortunately the change is not pushed to any canary/dev release because of issue 860584 :(
Labels: -Merge-Request-67 -Merge-Review-68 Merge-Rejected-67 Merge-Approved-68
Seems critical for the crash dumps.
Approving merge to M68 Chrome OS.

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 12

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35360985c2b93b7aa49667a26626df3644c20052

commit 35360985c2b93b7aa49667a26626df3644c20052
Author: Ryo Hashimoto <hashimoto@chromium.org>
Date: Thu Jul 12 06:05:05 2018

Do not optimize base::debug::Alias().

Optimization is already disabled for MSVC, do the same thing for Clang
too.

BUG= 861636 
TEST=Run dump_syms against chrome (w/ symbol_level=3, use_thin_lto=true,
use_lld=true) and manually checked the output.

Change-Id: Ia275e2310ccb8e70a2392d95ca4acadf2f218b01
Reviewed-on: https://chromium-review.googlesource.com/1128693
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#573365}(cherry picked from commit a31f26075580e73b05da1bb1d899b5edec38cea8)
Reviewed-on: https://chromium-review.googlesource.com/1134607
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#652}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/35360985c2b93b7aa49667a26626df3644c20052/base/debug/alias.cc

Status: Fixed (was: Started)

Sign in to add a comment