(release builds) some locals are not displayed in clang builds |
|||||||
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.
,
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?
,
Sep 8 2017
I'm happy to call this bug fixed and no longer a blocker for clang, based on my analysis of the crash dumps.
,
Sep 8 2017
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?
,
Sep 8 2017
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.
,
Sep 8 2017
,
Sep 11 2017
,
Sep 13 2017
,
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
,
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
,
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
,
Oct 5 2017
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).
,
Oct 5 2017
Do you want me to verify this fix, if so, can you list two official versions I can run script to compare?
,
Oct 5 2017
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.
,
Oct 5 2017
sure, I'll crank the cogwheel and have some data later today.
,
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 |
|||||||
Comment 1 by wfh@chromium.org
, Sep 8 2017This 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