New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocked on:
issue 764744

Blocking:
issue 636111
issue 709690



Sign in to add a comment

(release builds) some locals are not displayed in clang builds

Project Member Reported by wfh@chromium.org, Sep 2 2017 Back to list

Issue description

e.g. pending_task variable in the chrome_child!base::debug::TaskAnnotator::RunTask stack frame of a running task.

MSVC win-pgo.

STACK FRAME - chrome_child!base::debug::TaskAnnotator::RunTask+0xff

> dv /t
class base::debug::TaskAnnotator * this = 0x00000000`01164fb8
char * queue_function = 0x000007fe`e30452e8 "MessageLoop::PostTask"
struct base::PendingTask * pending_task = 0x00000000`0039f1b0
unsigned char * trace_event_unique_category_group_enabled44 = 0x000007fe`e3baa7a0 "--- memory read error at address 0x000007fe`e3baa7a0 ---"
class trace_event_internal::ScopedTracer trace_event_unique_tracer44 = class trace_event_internal::ScopedTracer
class base::debug::ScopedTaskRunActivity task_activity = class base::debug::ScopedTaskRunActivity
class tracked_objects::Duration queue_duration = <value unavailable>
class std::array<void const *,5> task_backtrace = class std::array<void const *,5>
class tracked_objects::TaskStopwatch stopwatch = class tracked_objects::TaskStopwatch
int64 trace_event_unique_atomic44 = <Memory access error>

this (class base::debug::TaskAnnotator *) = 0x00000000`01164fb8
> dt -r this
Local var @ r13 Type base::debug::TaskAnnotator*

queue_function (char *) = 0x000007fe`e30452e8 "MessageLoop::PostTask"
pending_task (struct base::PendingTask *) = 0x00000000`0039f1b0
> dt -r pending_task
Local var @ r14 Type base::PendingTask*
   +0x000 birth_tally      : 0x00000000`03ad36a0 tracked_objects::Births
      +0x000 location_        : tracked_objects::Location
         +0x000 function_name_   : ???? 
         +0x008 file_name_       : ???? 
         +0x010 line_number_     : ??
         +0x018 program_counter_ : ???? 
      +0x020 birth_thread_    : ???? 
      +0x028 birth_count_     : ??
   +0x008 time_posted      : tracked_objects::TrackedTime
      +0x000 ms_              : 0xd9e40
   +0x010 delayed_run_time : base::TimeTicks
      +0x000 us_              : 0n0
   +0x018 task             : base::Callback<void __cdecl(void),0,0>
      +0x000 bind_state_      : scoped_refptr<base::internal::BindStateBase>
         +0x000 ptr_             : (null) 
   +0x020 posted_from      : tracked_objects::Location
      +0x000 function_name_   : 0x000007fe`e2f9f1c0  "PostDoWorkContinuationLocked"
      +0x008 file_name_       : 0x000007fe`e2f9f160  "c:\b\c\b\win64_pgo\src\third_party\webkit\source\platform\scheduler\base\task_queue_manager.cc"
      +0x010 line_number_     : 0n405
      +0x018 program_counter_ : 0x000007fe`e063428a Void
   +0x040 task_backtrace   : std::array<void const *,4>
      +0x000 _Elems           : [4] 0x000007fe`e063428a Void
   +0x060 sequence_num     : 0n601
   +0x064 nestable         : 1
   +0x065 is_high_res      : 0

Clang (62.0.3187.0)

STACK FRAME - chrome_child!base::debug::TaskAnnotator::RunTask+0x1c6
> dv /t
class base::debug::TaskAnnotator * this = <value unavailable>
char * queue_function = <value unavailable>
struct base::PendingTask * pending_task = <value unavailable>
class base::debug::ScopedTaskRunActivity task_activity = <value unavailable>
class tracked_objects::TaskStopwatch stopwatch = <value unavailable>
unsigned char * trace_event_unique_category_group_enabled44 = 0x00000000 ""
unsigned int trace_event_flags = <value unavailable>
class base::TimeDelta queue_duration = <value unavailable>
class std::array<const void *,5> task_backtrace = <value unavailable>

I do need to re-run this against latest clang so bear with me, but tracking it here as requested by bruce.
 

Comment 1 by wfh@chromium.org, Sep 8 2017

This appears fixed - looking at official builds from branch 3209, and comparing 63.0.3209.0 (msvc) and 63.0.3209.2 (clang) using crash signature chrome_child!blink::FontCache::CrashWithFontInfo.

Both showed the members of the pending_task structure which previously clang did not (note, this is being removed soon, but that's orthogonal to this test).

MSVC win-pgo: crash/8059b0c0996625a6

FRAME 94
chrome_child!base::debug::TaskAnnotator::RunTask+0x124
> dv /t
class base::debug::TaskAnnotator * this = 0x00000084`31fb22d8
char * queue_function = 0x00007ff8`d0ad8e80 "TaskQueueManager::PostTask"
struct base::PendingTask * pending_task = 0x00000084`3168e630
unsigned char * trace_event_unique_category_group_enabled44 = 0x00007ff8`d16b3620 "--- memory read error at address 0x00007ff8`d16b3620 ---"
class trace_event_internal::ScopedTracer trace_event_unique_tracer44 = class trace_event_internal::ScopedTracer
class base::debug::ScopedTaskRunActivity task_activity = class base::debug::ScopedTaskRunActivity
class base::TimeDelta queue_duration = class base::TimeDelta
class std::array<void const *,7> task_backtrace = class std::array<void const *,7>
class tracked_objects::TaskStopwatch stopwatch = class tracked_objects::TaskStopwatch
int64 trace_event_unique_atomic44 = <Memory access error>

this (class base::debug::TaskAnnotator *) = 0x00000084`31fb22d8
> dt -r this
Local var @ r13 Type base::debug::TaskAnnotator*

queue_function (char *) = 0x00007ff8`d0ad8e80 "TaskQueueManager::PostTask"
pending_task (struct base::PendingTask *) = 0x00000084`3168e630
> dt -r pending_task
Local var @ rsi Type base::PendingTask*
   +0x000 birth_tally      : 0x00000084`361e86a0 tracked_objects::Births
      +0x000 location_        : tracked_objects::Location
         +0x000 function_name_   : ???? 
         +0x008 file_name_       : ???? 
         +0x010 line_number_     : ??
         +0x018 program_counter_ : ???? 
      +0x020 birth_thread_    : ???? 
      +0x028 birth_count_     : ??
   +0x008 time_posted      : base::TimeTicks
      +0x000 us_              : 0n454602700188
   +0x010 delayed_run_time : base::TimeTicks
      +0x000 us_              : 0n0
   +0x018 task             : base::OnceCallback<void __cdecl(void)>
      +0x000 bind_state_      : scoped_refptr<base::internal::BindStateBase>
         +0x000 ptr_             : (null) 
   +0x020 posted_from      : tracked_objects::Location
      +0x000 function_name_   : 0x00007ff8`d0ae5e50  "ScheduleForResume"
      +0x008 file_name_       : 0x00007ff8`d0ae5e00  "../../third_party/WebKit/Source/core/html/parser/HTMLParserScheduler.cpp"
      +0x010 line_number_     : 0n81
      +0x018 program_counter_ : 0x00007ff8`ce12a68d Void
   +0x040 task_backtrace   : std::array<void const *,4>
      +0x000 _Elems           : [4] 0x00007ff8`ce08344d Void
   +0x060 sequence_num     : 0n348
   +0x064 nestable         : 1
   +0x065 is_high_res      : 0

clang win 64-bit crash/4e285c258a3f7745

FRAME 72
chrome_child!base::debug::TaskAnnotator::RunTask+0x235
> dv /t
class base::debug::TaskAnnotator * this = <value unavailable>
char * queue_function = 0x000007fe`d3aa1cb7 "TaskQueueManager::PostTask"
struct base::PendingTask * pending_task = 0x00000000`0024e1e0
class base::debug::ScopedTaskRunActivity task_activity = class base::debug::ScopedTaskRunActivity
class tracked_objects::TaskStopwatch stopwatch = class tracked_objects::TaskStopwatch
class base::TimeDelta queue_duration = class base::TimeDelta
class trace_event_internal::ScopedTracer trace_event_unique_tracer44 = class trace_event_internal::ScopedTracer
class std::array<const void *,7> task_backtrace = class std::array<const void *,7>
unsigned char * trace_event_unique_category_group_enabled44 = 0x000007fe`d4530990 "--- memory read error at address 0x000007fe`d4530990 ---"
unsigned int trace_event_flags = <value unavailable>
class trace_event_internal::TraceID trace_event_bind_id = <value unavailable>
struct base::trace_event::TraceEventHandle h = <value unavailable>

this (class base::debug::TaskAnnotator *) = <value unavailable>
queue_function (char *) = 0x000007fe`d3aa1cb7 "TaskQueueManager::PostTask"
pending_task (struct base::PendingTask *) = 0x00000000`0024e1e0
> dt -r pending_task
Local var @ rsi Type base::PendingTask*
   +0x000 birth_tally      : 0x00000000`0580ac60 tracked_objects::Births
      +0x000 location_        : tracked_objects::Location
         +0x000 function_name_   : ???? 
         +0x008 file_name_       : ???? 
         +0x010 line_number_     : ??
         +0x018 program_counter_ : ???? 
      +0x020 birth_thread_    : ???? 
      +0x028 birth_count_     : ??
   +0x008 time_posted      : base::TimeTicks
      =000007fe`d0480000 kHoursPerDay     : 0n12894362189
      =000007fe`d0480000 kMillisecondsPerSecond : 0n12894362189
      =000007fe`d0480000 kMillisecondsPerDay : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerMillisecond : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerSecond : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerMinute : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerHour : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerDay : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerWeek : 0n12894362189
      =000007fe`d0480000 kNanosecondsPerMicrosecond : 0n12894362189
      =000007fe`d0480000 kNanosecondsPerSecond : 0n12894362189
      +0x000 us_              : 0n4585444000
   +0x010 delayed_run_time : base::TimeTicks
      =000007fe`d0480000 kHoursPerDay     : 0n12894362189
      =000007fe`d0480000 kMillisecondsPerSecond : 0n12894362189
      =000007fe`d0480000 kMillisecondsPerDay : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerMillisecond : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerSecond : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerMinute : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerHour : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerDay : 0n12894362189
      =000007fe`d0480000 kMicrosecondsPerWeek : 0n12894362189
      =000007fe`d0480000 kNanosecondsPerMicrosecond : 0n12894362189
      =000007fe`d0480000 kNanosecondsPerSecond : 0n12894362189
      +0x000 us_              : 0n0
   +0x018 task             : base::OnceCallback<void ()>
      +0x000 bind_state_      : scoped_refptr<base::internal::BindStateBase>
         +0x000 ptr_             : (null) 
   +0x020 posted_from      : tracked_objects::Location
      +0x000 function_name_   : 0x000007fe`d4381a55  "DidLoadAllScriptBlockingResources"
      +0x008 file_name_       : 0x000007fe`d43815b3  "../../third_party/WebKit/Source/core/dom/Document.cpp"
      +0x010 line_number_     : 0n3788
      +0x018 program_counter_ : 0x000007fe`d081dac2 Void
   +0x040 task_backtrace   : std::array<const void *,4>
      +0x000 _Elems           : [4] 0x000007fe`d0516bfb Void
   +0x060 sequence_num     : 0n777
   +0x064 nestable         : 1
   +0x065 is_high_res      : 0

original files (google access only) https://drive.google.com/drive/folders/0BzKr4yVrnrZNM3BZODhReDR4d3c?usp=sharing

Comment 2 by wfh@chromium.org, Sep 8 2017

I eyeballed the rest of the dumps for differences, and one other inconsequential thing of interest that distinguished these debug outputs
was that char variables like ChromeContentClient::kNotPresent were only showing first character in clang builds, while they show all characters in msvs...?

MSVS:

      =00007ff8`d10e7bc0 kNotPresent      : [0]  "internal-not-yet-present"
      =00007ff8`d10e7ba8 kPDFExtensionPluginName : [0]  "Chrome PDF Viewer"
      =00007ff8`d0bd6330 kPDFInternalPluginName : [0]  "Chrome PDF Plugin"
      =00007ff8`d0bd60d8 kPDFPluginPath   : [0]  "internal-pdf-viewer"
      =00007ff8`cdec0000 kRemotingViewerPluginPath : [0]  "MZ???"

clang:

      =000007fe`d3de7540 kNotPresent      : [1]  "i"
      =000007fe`d3de7560 kPDFExtensionPluginName : [1]  "C"
      =000007fe`d3de7580 kPDFInternalPluginName : [1]  "C"
      =000007fe`d3de75a0 kPDFPluginPath   : [1]  "i"
      =000007fe`d0480000 kRemotingViewerPluginPath : [1]  "M"

maybe clang is typing these wrong for windbg?


Comment 3 by wfh@chromium.org, Sep 8 2017

Cc: wfh@chromium.org
Owner: ----
Status: Fixed
I'm happy to call this bug fixed and no longer a blocker for clang, based on my analysis of the crash dumps.
Thanks, but:

a) current clang has a local hotfix we put in trying to save m62. We've reverted that hotfix in the clang roll that landed today, so this will be worse again until we're done implementing the Right Fix upstream. It probably makes sense to keep this open until then.

b) can you file a separate bug about the string issue please?

Comment 5 by wfh@chromium.org, Sep 8 2017

Status: Available
un-fixing this bug. also created  issue 763580  for the string issue, but it's pretty low priority, so didn't mark it a blocker on anything.
Blocking: 636111

Comment 7 by dxf@google.com, Sep 11 2017

Owner: r...@chromium.org

Comment 8 by h...@chromium.org, Sep 13 2017

Blockedon: 764744
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 15 2017

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

commit 32750c1251012624a87fb97098bd0f6a60ffa8ed
Author: Hans Wennborg <hans@chromium.org>
Date: Fri Sep 15 23:10:14 2017

Clang: enable -instcombine-lower-dbg-declare=1

This enables better optimized debug info for address-taken values
at the cost of potentially inaccurate debug info in some situations.

We believe turning this on is the right trade-off for developers
debugging Chromium on all platforms, but especially on Windows where
values that are available in MSVC's debug info would otherwise show
as unavailable when using Clang.

The goal (crbug.com/765793) is to make Clang's optimized
debug info good enough in all cases that this compromise is not
necessary.

Note: This requires Clang r313108 or later.

Bug:  761633 ,  753736 , 765793
Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
Reviewed-on: https://chromium-review.googlesource.com/669314
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502428}
[modify] https://crrev.com/32750c1251012624a87fb97098bd0f6a60ffa8ed/build/config/compiler/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 16 2017

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

commit 5487d51f9175a69613e7b715be437ece3c1d88e5
Author: Xiaoqian Dai <xdai@chromium.org>
Date: Sat Sep 16 03:54:06 2017

Revert "Clang: enable -instcombine-lower-dbg-declare=1"

This reverts commit 32750c1251012624a87fb97098bd0f6a60ffa8ed.

Reason for revert: This CL breaks most of the informational builds https://uberchromegw.corp.google.com/i/chromeos.chrome/waterfall?reload=300. See an example here: https://uberchromegw.corp.google.com/i/chromeos.chrome/builders/veyron_minnie-tot-chrome-pfq-informational/builds/6039

Original change's description:
> Clang: enable -instcombine-lower-dbg-declare=1
> 
> This enables better optimized debug info for address-taken values
> at the cost of potentially inaccurate debug info in some situations.
> 
> We believe turning this on is the right trade-off for developers
> debugging Chromium on all platforms, but especially on Windows where
> values that are available in MSVC's debug info would otherwise show
> as unavailable when using Clang.
> 
> The goal (crbug.com/765793) is to make Clang's optimized
> debug info good enough in all cases that this compromise is not
> necessary.
> 
> Note: This requires Clang r313108 or later.
> 
> Bug:  761633 ,  753736 , 765793
> Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
> Reviewed-on: https://chromium-review.googlesource.com/669314
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502428}

TBR=thakis@chromium.org,hans@chromium.org,rnk@chromium.org

Change-Id: I4fea2ab8ec9571d4afb66cd3912c5975e0b9395c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  761633 ,  753736 , 765793
Reviewed-on: https://chromium-review.googlesource.com/669825
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502484}
[modify] https://crrev.com/5487d51f9175a69613e7b715be437ece3c1d88e5/build/config/compiler/BUILD.gn

Project Member

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

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

commit 074fa51f85b15ad2e2d6c804a03825d48dbeb063
Author: Hans Wennborg <hans@chromium.org>
Date: Mon Sep 18 19:43:09 2017

Reland "Clang: enable -instcombine-lower-dbg-declare=1"

Trying harder to exclude CrOS build configs that don't use
Chromium's Clang.

Note to CrOS folks: If this breaks build configs that are not
tested upstream, please ping me and/or llozano before reverting.

This is a reland of 32750c1251012624a87fb97098bd0f6a60ffa8ed
Original change's description:
> Clang: enable -instcombine-lower-dbg-declare=1
>
> This enables better optimized debug info for address-taken values
> at the cost of potentially inaccurate debug info in some situations.
>
> We believe turning this on is the right trade-off for developers
> debugging Chromium on all platforms, but especially on Windows where
> values that are available in MSVC's debug info would otherwise show
> as unavailable when using Clang.
>
> The goal (crbug.com/765793) is to make Clang's optimized
> debug info good enough in all cases that this compromise is not
> necessary.
>
> Note: This requires Clang r313108 or later.
>
> Bug:  761633 ,  753736 , 765793
> Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
> Reviewed-on: https://chromium-review.googlesource.com/669314
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502428}

TBR=thakis

Bug:  761633 ,  753736 , 765793
Change-Id: I2c3670873f1bd1d75cf1c41191b170983f41e0d0
Reviewed-on: https://chromium-review.googlesource.com/671523
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502646}
[modify] https://crrev.com/074fa51f85b15ad2e2d6c804a03825d48dbeb063/build/config/compiler/BUILD.gn

Status: Fixed
From #4:
> a) current clang has a local hotfix we put in trying to save m62. We've reverted that hotfix in the clang roll that landed today, so this will be worse again until we're done implementing the Right Fix upstream. It probably makes sense to keep this open until then.

I believe the flag enabled in #11 addresses this (Bug 765793 tracks getting rid of that eventually).
Do you want me to verify this fix, if so, can you list two official versions I can run script to compare?
Yesterday's canary was 63.0.3232.0 (msvc) and 63.0.3232.2 (clang). If you can give that a go, that would be great.
sure, I'll crank the cogwheel and have some data later today.
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 18

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