New issue
Advanced search Search tips

Issue 860850 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 9
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Various abort() functions are folded together

Project Member Reported by thestig@chromium.org, Jul 7

Issue description

Crash report 4721a31f7749b106 is on Windows with 67.0.3396.99. The stack trace is:

(chrome_child.dll -win_util.cc:87 )	base::win::`anonymous namespace'::ForceCrashOnSigAbort
(chrome_child.dll -signal.cpp:516 )	raise
(chrome_child.dll -abort.cpp:64 )	abort
(chrome_child.dll -fx_memory.cpp:48 )	FX_OutOfMemoryTerminate()
(chrome_child.dll -SkSurface_Raster.cpp:169 )	SkSurface_Raster::onCopyOnWrite(SkSurface::ContentChangeMode)

with FX_OutOfMemoryTerminate() being out of place since that is PDFium code and the caller is Skia. This is because:

FX_OutOfMemoryTerminate()
sk_abort_no_print()
google::AssertFail()

all look the same, so the linker performed identical code folding to combine them. This makes crash reports a bit confusing to read.

This definitely happens on Linux and ChromeOS as well.
 
Related issue:  issue 861636  is for base::debug::BreakDebugger().

For base::debug::BreakDebugger, I landed http://crrev.com/345663 to make it unfoldable, but it stopped working after ThinLTO was enabled.

The most straight forward solution is that making these abort functions unique in a way even ThinLTO cannot deduplicate (e.g. outputting a log line which contains the name of the function).
I've been assigning unique numbers to the static int variables. The ThinLTO linking takes too long, so I didn't bother verifying if that is sufficient.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/43f6bc80a29fdf326729903e9f323850e9553c69

commit 43f6bc80a29fdf326729903e9f323850e9553c69
Author: Lei Zhang <thestig@chromium.org>
Date: Mon Jul 09 18:58:37 2018

Prevent FX_OutOfMemoryTerminate() from being folded by the linker.

Copy base::debug::Alias() from Chromium. Use it to prevent ICF from
combining FX_OutOfMemoryTerminate() with similar functions.

BUG= chromium:860850 

Change-Id: Ifccb05c0218f86e44b9bb235847e01383ec36b3f
Reviewed-on: https://pdfium-review.googlesource.com/37290
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

[add] https://crrev.com/43f6bc80a29fdf326729903e9f323850e9553c69/third_party/base/debug/alias.h
[modify] https://crrev.com/43f6bc80a29fdf326729903e9f323850e9553c69/third_party/BUILD.gn
[add] https://crrev.com/43f6bc80a29fdf326729903e9f323850e9553c69/third_party/base/debug/alias.cc
[modify] https://crrev.com/43f6bc80a29fdf326729903e9f323850e9553c69/core/fxcrt/fx_memory.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 9

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

commit 026cfc6cd1e24808dd1baf57992cee34dfe20799
Author: Lei Zhang <thestig@chromium.org>
Date: Mon Jul 09 20:53:49 2018

Prevent sk_abort_no_print() from being folded by the linker.

Use base::debug::Alias() to prevent ICF from combining Chromium's port
of sk_abort_no_print() with similar functions.

BUG= 860850 

Change-Id: I3c45d7aa567c2bdd2098a355cbf2e56a8e82bb34
Reviewed-on: https://chromium-review.googlesource.com/1128459
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573434}
[modify] https://crrev.com/026cfc6cd1e24808dd1baf57992cee34dfe20799/skia/ext/SkMemory_new_handler.cpp

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/c9297e896c697b5bb9c3b9e66791a834f5552170

commit c9297e896c697b5bb9c3b9e66791a834f5552170
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Jul 10 01:30:44 2018

Fix duplicate symbols in Chromium builds.

Commit 43f6bc80 copied over Chromium's base/debug/alias.cc, but that
confuses Windows builds and Linux jumbo builds in Chromium.

To fix this, wrap PDFium's copy in the pdfium namespace.

BUG= chromium:860850 
TBR=dsinclair@chromium.org

Change-Id: I1ceec28b9ce6c2893bf030a5b6564dce6d6ec376
Reviewed-on: https://pdfium-review.googlesource.com/37430
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/c9297e896c697b5bb9c3b9e66791a834f5552170/third_party/base/debug/alias.h
[modify] https://crrev.com/c9297e896c697b5bb9c3b9e66791a834f5552170/third_party/base/debug/alias.cc
[modify] https://crrev.com/c9297e896c697b5bb9c3b9e66791a834f5552170/core/fxcrt/fx_memory.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 10

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

commit 38997833f7238d644148764a095b66f6a5ac31d4
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 10 03:09:52 2018

Roll src/third_party/pdfium 245c7310a74e..c9297e896c69 (2 commits)

https://pdfium.googlesource.com/pdfium.git/+log/245c7310a74e..c9297e896c69


git log 245c7310a74e..c9297e896c69 --date=short --no-merges --format='%ad %ae %s'
2018-07-10 thestig@chromium.org Fix duplicate symbols in Chromium builds.
2018-07-09 thestig@chromium.org Prevent FX_OutOfMemoryTerminate() from being folded by the linker.


Created with:
  gclient setdep -r src/third_party/pdfium@c9297e896c69

The AutoRoll server is located here: https://pdfium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:860850 , chromium:860850 
TBR=dsinclair@chromium.org

Change-Id: Ic432e6ac152f0d9e242ae9c66930fb23e438f1d7
Reviewed-on: https://chromium-review.googlesource.com/1130679
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#573604}
[modify] https://crrev.com/38997833f7238d644148764a095b66f6a5ac31d4/DEPS

Sign in to add a comment