New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 636111
issue 709690



Sign in to add a comment

Locals from FontCache::CrashWithFontInfo missing in Win/Clang minidumps

Project Member Reported by h...@chromium.org, Oct 16 2017

Issue description

While waiting to see if  Bug 753736  was fixed, I took a look at the few crash reports that had come in, and it seems we're still missing some base::DebugAlias variables.

For example for

https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20%20AND%20custom_data.ChromeCrashProto.channel%3D%27canary%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20STRPOS(product.Version%2C%20%2764.0.3241%27)%20%3E%200%20AND%20product.Version%3D%2764.0.3241.4%27%20AND%20ReportID%3D%277b4059a864d35c52%27&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D&stbtiq=&reportid=&index=0

These vars are marked DebugAlias:

  WTF::debug::Alias(&font_description_copy);

  WTF::debug::Alias(&font_cache);
  WTF::debug::Alias(&font_mgr);
  WTF::debug::Alias(&num_families);

But font_cache, font_mgt and num_familias show "Value unavailable" in WinDBG.

That build is using Clang 315613.
 

Comment 1 by h...@chromium.org, Oct 16 2017

Blocking: 636111 709690

Comment 2 by h...@chromium.org, Oct 16 2017

Screenshot from 2017-10-16 16:01:38.png
104 KB View Download

Comment 3 by h...@chromium.org, Oct 16 2017

Preprocessed source and invocation for convenience.

"C:\\src\\chromium\\src\\third_party\\llvm-build\\Release+Asserts\\bin\\clang-cl.exe" "-cc1" "-triple" "i386-pc-windows-msvc19.11.0" "-emit-obj" "-mincremental-linker-compatible" "-main-file-name" "FontCache.cpp" "-mrelocation-model" "static" "-mthread-model" "posix" "-mdisable-fp-elim" "-relaxed-aliasing" "-fmath-errno" "-masm-verbose" "-mconstructor-aliases" "-target-cpu" "pentium4" "-flto-visibility-public-std" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-fno-rtti-data" "-stack-protector" "2" "-fms-volatile" "-fdiagnostics-format" "msvc" "-dwarf-column-info" "-debugger-tuning=gdb" "-momit-leaf-frame-pointer" "-ffunction-sections" "-fdata-sections" "-Os" -w "-fdeprecated-macro" "-ferror-limit" "19" "-fmessage-length" "0" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.11" "-std=c++14" "-fdelayed-template-parsing" "-finline-functions" "-fobjc-runtime=gcc" "-fdiagnostics-show-option" "-fcolor-diagnostics" "-vectorize-loops" "-vectorize-slp" "-mllvm" "-instcombine-lower-dbg-declare=1" "-o" "FontCache.obj" "-x" "c++" \src\tmp\a.ii
a.ii
5.2 MB Download

Comment 4 by h...@chromium.org, Oct 16 2017

Oops, I copied that build config from a release bot, so it used symbol_level=1. Here's how it's invoked on the official builder:

"C:\\src\\chromium\\src\\third_party\\llvm-build\\Release+Asserts\\bin\\clang-cl.exe" "-cc1" "-triple" "i386-pc-windows-msvc19.11.0" "-emit-obj" "-mincremental-linker-compatible" "-main-file-name" "FontCache.cpp" "-mrelocation-model" "static" "-mthread-model" "posix" "-mdisable-fp-elim" "-relaxed-aliasing" "-fmath-errno" "-masm-verbose" "-mconstructor-aliases" "-target-cpu" "pentium4" "-flto-visibility-public-std" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-fno-rtti-data" "-stack-protector" "2" "-gcodeview" "-fms-volatile" "-fdiagnostics-format" "msvc" "-gcodeview" "-debug-info-kind=limited" "-debugger-tuning=gdb" "-momit-leaf-frame-pointer" "-ffunction-sections" "-fdata-sections" "-Os" "-fdeprecated-macro" "-ferror-limit" "19" "-fmessage-length" "0" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.11" "-std=c++14" "-fdelayed-template-parsing" "-finline-functions" "-fobjc-runtime=gcc" "-fdiagnostics-show-option" "-fcolor-diagnostics" "-vectorize-loops" "-vectorize-slp" "-mllvm" "-instcombine-lower-dbg-declare=1" "-o" "obj/third_party/WebKit/Source/platform/platform/FontCache.obj" -w "-x" "c++" \src\tmp\a.ii

Comment 5 by h...@chromium.org, Oct 17 2017

Perhaps working with this on Linux would make things easier.

$ git diff
diff --git a/third_party/WebKit/Source/platform/fonts/FontCacheTest.cpp b/third_party/WebKit/Source/platform/fonts/FontCacheTest.cpp
index e1d35295a0e3..753e03fc90b9 100644
--- a/third_party/WebKit/Source/platform/fonts/FontCacheTest.cpp
+++ b/third_party/WebKit/Source/platform/fonts/FontCacheTest.cpp
@@ -18,6 +18,7 @@ TEST(FontCache, getLastResortFallbackFont) {
   ASSERT_TRUE(font_cache);
 
   FontDescription font_description;
+  FontCache::CrashWithFontInfo(&font_description);
   font_description.SetGenericFamily(FontDescription::kStandardFamily);
   RefPtr<SimpleFontData> font_data =
       font_cache->GetLastResortFallbackFont(font_description, kRetain);

$ cat out/release/args.gn
is_debug = false
use_goma = true
symbol_level = 2
$ ninja -C out/release -j500 blink_platform_unittests
$ gdb --args out/release/blink_platform_unittests --gtest_filter=FontCache.getLastResortFallbackFont --single-process-tests
Program received signal SIGABRT, Aborted.
0x00007ffff4d34c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff4d34c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff4d38028 in __GI_abort () at abort.c:89
#2  0x0000000001589382 in base::debug::BreakDebugger () at ../../base/debug/debugger_posix.cc:258
#3  0x00000000015950ba in logging::LogMessage::~LogMessage (this=0x7fffffffd268) at ../../base/logging.cc:846
#4  0x00000000014d8658 in blink::FontCache::CrashWithFontInfo (font_description=<optimized out>) at ../../third_party/WebKit/Source/platform/fonts/FontCache.cpp:422
#5  0x0000000000808892 in blink::FontCache_getLastResortFallbackFont_Test::TestBody (this=<optimized out>) at ../../third_party/WebKit/Source/platform/fonts/FontCacheTest.cpp:21
#6  0x00000000012e2776 in testing::Test::Run (this=0x3f42e7cf5a30) at ../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:928
#7  0x00000000012e3150 in testing::TestInfo::Run (this=0x3f42e7d26c30) at ../../third_party/googletest/src/googletest/src/gtest.cc:2654
#8  0x00000000012e3637 in testing::TestCase::Run (this=0x3f42e7d26d20) at ../../third_party/googletest/src/googletest/src/gtest.cc:2772
#9  0x00000000012e9927 in testing::internal::UnitTestImpl::RunAllTests (this=0x3f42e7cf7200) at ../../third_party/googletest/src/googletest/src/gtest.cc:4677
#10 0x00000000012e95b3 in testing::UnitTest::Run (this=<optimized out>) at ../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:928
#11 0x0000000001c4a4a8 in RUN_ALL_TESTS () at ../../third_party/googletest/src/googletest/include/gtest/gtest.h:2237
#12 base::TestSuite::Run (this=0x7fffffffdb78) at ../../base/test/test_suite.cc:270
#13 0x0000000000b6a200 in (anonymous namespace)::runTestSuite (testSuite=0x25c35) at ../../third_party/WebKit/Source/platform/testing/RunAllTests.cpp:44
#14 0x0000000001c5176a in Run (this=<optimized out>) at ../../base/callback.h:92
#15 base::(anonymous namespace)::LaunchUnitTestsInternal(base::RepeatingCallback<int ()> const&, unsigned long, int, bool, base::RepeatingCallback<void ()> const&) (run_test_suite=..., parallel_jobs=1, default_batch_limit=10, 
    use_job_objects=true, gtest_init=...) at ../../base/test/launcher/unit_test_launcher.cc:216
#16 0x0000000001c5162a in base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) (argc=<optimized out>, argv=<optimized out>, run_test_suite=...) at ../../base/test/launcher/unit_test_launcher.cc:475
#17 0x0000000000b6a198 in main (argc=3, argv=0x7fffffffdce8) at ../../third_party/WebKit/Source/platform/testing/RunAllTests.cpp:62
(gdb) frame 4
#4  0x00000000014d8658 in blink::FontCache::CrashWithFontInfo (font_description=<optimized out>) at ../../third_party/WebKit/Source/platform/fonts/FontCache.cpp:422
422       CHECK(false);
(gdb) info locals
font_cache = <optimized out>
num_families = <optimized out>
font_mgr = <optimized out>
font_description_copy = {family_list_ = {family_ = {string_ = {impl_ = {ptr_ = 0x0}}}, next_ = {ptr_ = 0x0}}, feature_settings_ = {ptr_ = 0x0}, variation_settings_ = {ptr_ = 0x0}, locale_ = {ptr_ = 0x0}, specified_size_ = 0, 
  computed_size_ = 0, adjusted_size_ = 0, size_adjust_ = -1, letter_spacing_ = 0, word_spacing_ = 0, font_selection_request_ = {weight = {static fractionalEntropy = 4, backing_ = 1600}, width = {static fractionalEntropy = 4, 
      backing_ = 400}, slope = {static fractionalEntropy = 4, backing_ = 0}}, {fields_ = {orientation_ = 0, width_variant_ = 0, variant_caps_ = 0, is_absolute_size_ = 0, generic_family_ = 0, kerning_ = 0, common_ligatures_state_ = 0, 
      discretionary_ligatures_state_ = 0, historical_ligatures_state_ = 0, contextual_ligatures_state_ = 0, keyword_size_ = 0, font_smoothing_ = 0, text_rendering_ = 0, synthetic_bold_ = 0, synthetic_italic_ = 0, 
      subpixel_text_position_ = 0, typesetting_features_ = 0, variant_numeric_ = 0, variant_east_asian_ = 0, subpixel_ascent_descent_ = 0}, fields_as_unsigned_ = {parts = {0, 0}}}, static default_typesetting_features_ = 0, 
  static use_subpixel_text_positioning_ = false}

Comment 6 by h...@chromium.org, Oct 17 2017

Trying to understand how this is supposed by looking at a smaller example.

$ cat /tmp/a.cc
void alias(void *var);
void crash();
int* GetFontCache();

void __attribute__((noinline)) CrashWithFontInfo() {
  int *font_cache = GetFontCache();
  alias(&font_cache);
  crash();
}

int main() {
  CrashWithFontInfo();
  return 0;
}

$ cat /tmp/b.cc
void alias(void *var) {}
void crash() { *(volatile int*)0 = 0; }
int* GetFontCache() { static int x = 0; return &x; }




With GCC:

$ gcc -g -O2 /tmp/a.cc /tmp/b.cc && gdb ./a.out
(gdb) r     
Starting program: /usr/local/google/work/llvm/build.release/a.out 

Program received signal SIGSEGV, Segmentation fault.
crash () at /tmp/b.cc:2
2       void crash() { *(volatile int*)0 = 0; }
(gdb) frame 1
#1  0x00000000004004cd in CrashWithFontInfo () at /tmp/a.cc:8
8         crash();
(gdb) info locals
font_cache = 0x600964 <GetFontCache()::x>


With Clang:

$ bin/clang -g -O2 /tmp/a.cc /tmp/b.cc && gdb ./a.out
(gdb) r
Starting program: /usr/local/google/work/llvm/build.release/a.out 

Program received signal SIGSEGV, Segmentation fault.
crash () at /tmp/b.cc:2
2       void crash() { *(volatile int*)0 = 0; }
(gdb) frame 1
#1  0x00000000004004e7 in CrashWithFontInfo () at /tmp/a.cc:8
8         crash();
(gdb) info locals
font_cache = <optimized out>


Am I holding it wrong, or does this just not work?

Comment 7 by h...@chromium.org, Oct 17 2017

Maybe the problem is that our debug info refers to font_cache by its register location instead of its stack slot:

BB#0: derived from LLVM BB %entry
        PUSH64r %RAX<undef>, %RSP<imp-def>, %RSP<imp-use>; flags: FrameSetup dbg:/tmp/a.cc:6:21
        CFI_INSTRUCTION <call frame instruction>
        CALL64pcrel32 <ga:@_Z12GetFontCachev>, <regmask %BH %BL %BP %BPL %BX %EBP %EBX %RBP %RBX %R12 %R13 %R14 %R15 %R12B %R13B %R14B %R15B %R12D %R13D %R14D %R15D %R12W %R13W %R14W %R15W>, %RSP<imp-use>, %RSP<imp-def>, %RAX<imp-def>; dbg:/tmp/a.cc:6:21
        DBG_VALUE %RAX, %noreg, !"font_cache", <!DIExpression()>; line no:6
        MOV64mr %RSP, 1, %noreg, 0, %noreg, %RAX<kill>; mem:ST8[%font_cache](tbaa=!18) dbg:/tmp/a.cc:6:8
        %RDI<def> = MOV64rr %RSP
        CALL64pcrel32 <ga:@_Z5aliasPv>, <regmask %BH %BL %BP %BPL %BX %EBP %EBX %RBP %RBX %R12 %R13 %R14 %R15 %R12B %R13B %R14B %R15B %R12D %R13D %R14D %R15D %R12W %R13W %R14W %R15W>, %RSP<imp-use>, %RDI<imp-use>, %RSP<imp-def>; dbg:/tmp/a.cc:7:3
        CALL64pcrel32 <ga:@_Z5crashv>, <regmask %BH %BL %BP %BPL %BX %EBP %EBX %RBP %RBX %R12 %R13 %R14 %R15 %R12B %R13B %R14B %R15B %R12D %R13D %R14D %R15D %R12W %R13W %R14W %R15W>, %RSP<imp-use>, %RSP<imp-def>; dbg:/tmp/a.cc:8:3
        %RAX<def> = POP64r %RSP<imp-def>, %RSP<imp-use>; flags: FrameDestroy dbg:/tmp/a.cc:9:1
        RETQ; dbg:/tmp/a.cc:9:1


Optimized:

$ bin/clang -g -O2 -c /tmp/a.cc && bin/llvm-dwarfdump a.o
..
0x00000043:     DW_TAG_variable
                  DW_AT_location        (0x00000000
                     0x0000000000000006 - 0x0000000000000012: DW_OP_reg0 RAX)   <-----
                  DW_AT_name    ("font_cache")
                  DW_AT_decl_file       ("/tmp/a.cc")
                  DW_AT_decl_line       (6)
                  DW_AT_type    (cu + 0x0073 "int*")

Not optimized:

$ bin/clang -g -O0 -c /tmp/a.cc && bin/llvm-dwarfdump a.o
..
0x00000043:     DW_TAG_variable
                  DW_AT_location        (DW_OP_fbreg -8)    <------
                  DW_AT_name    ("font_cache")
                  DW_AT_decl_file       ("/tmp/a.cc")
                  DW_AT_decl_line       (6)
                  DW_AT_type    (cu + 0x0072 "int*")



I suppose this is hard to avoid for some values, but in this case base::debug::Alias is forcing font_cache onto the stack, so maybe we can make our debug info refer to that location instead somehow.

Comment 8 by h...@chromium.org, Oct 17 2017

From an IR point of view, at -O0 we use an llvm.dbg.declare, and at -O2 we use llvm.dbg.value to describe the variable.

At -O2 at one point we go from:

define void @_Z17CrashWithFontInfov() local_unnamed_addr #0 !dbg !7 {
entry:
  %font_cache = alloca i32*, align 8
  %0 = bitcast i32** %font_cache to i8*, !dbg !14
  call void @llvm.lifetime.start.p0i8(i64 8, i8* %0) #5, !dbg !14
  call void @llvm.dbg.declare(metadata i32** %font_cache, metadata !11, metadata !DIExpression()), !dbg !15
  %call = call i32* @_Z12GetFontCachev(), !dbg !16
  store i32* %call, i32** %font_cache, align 8, !dbg !15, !tbaa !17
  call void @_Z5aliasPv(i8* %0), !dbg !21
  call void @_Z5crashv(), !dbg !22
  call void @llvm.lifetime.end.p0i8(i64 8, i8* %0) #5, !dbg !23
  ret void, !dbg !23
}

To

*** IR Dump After Combine redundant instructions ***
; Function Attrs: noinline uwtable
define void @_Z17CrashWithFontInfov() local_unnamed_addr #0 !dbg !7 {
entry:
  %font_cache = alloca i32*, align 8
  %0 = bitcast i32** %font_cache to i8*, !dbg !14
  call void @llvm.lifetime.start.p0i8(i64 8, i8* %0) #5, !dbg !14
  %call = call i32* @_Z12GetFontCachev(), !dbg !15
  call void @llvm.dbg.value(metadata i32* %call, metadata !11, metadata !DIExpression()), !dbg !16
  store i32* %call, i32** %font_cache, align 8, !dbg !16, !tbaa !17
  call void @_Z5aliasPv(i8* %0), !dbg !21
  call void @_Z5crashv(), !dbg !22
  call void @llvm.lifetime.end.p0i8(i64 8, i8* %0) #5, !dbg !23
  ret void, !dbg !23
}

Comment 9 by h...@chromium.org, Oct 17 2017

Argh! And that is exactly what rnk's r313108 fixes. That introduced -instcombine-lower-dbg-declare which allows us to turn off that instcombine.

Unfortunately, we (I) "enabled" it by passing -instcombine-lower-dbg-declare=1. But the whole point was to pass =0.

Patch coming up :-)
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 18 2017

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

commit 1a6a85d8c64b56eaad45415827a34010326a78f4
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Oct 18 01:08:16 2017

Clang: disable the llvm.dbg.declare -> value instcombine for realz

It turns out the first attempt (#502428) got the flag backwards.
*headdesks*

Bug:  761633 ,765793, 775258 
Change-Id: I6ef2203822c4b0a5d04d6c8d564e0d347b2d104d
Reviewed-on: https://chromium-review.googlesource.com/724263
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509640}
[modify] https://crrev.com/1a6a85d8c64b56eaad45415827a34010326a78f4/build/config/compiler/BUILD.gn

Sign in to add a comment