Adding PE TimeStamp to chrome trace to ease retrieving debug information

Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded addresses.
example:
  {"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2869073002
Cr-Commit-Position: refs/heads/master@{#475930}
Committed: https://chromium.googlesource.com/chromium/src/+/a81e171d89732c62f3e8d1846ea685baa87ecd6f

[etienneb, 2017-05-09 15:55:44]

PTAL

[Primiano Tucci (use gerrit), 2017-05-09 16:34:01]

I am sympathetic with the goal, if that is the best way to fetch symbols. just
the current approach as-is has two disadvantages:
1) It grabs the PE metadata even if heap profiler is not enabled, just because
we do detailed dumps
2) It keeps grabbing PE metadata on each DETAILED dump, over and over

Can you:
- capture the module timestamp only when the heap profiler is enabled (unless
that thing has a trivial cost)?
- if that get-timestamp call isn't trivial in terms of overhead, consider using
a map to cache that.

Just asking about perf because I always hear horror stories from brucedawson
where apparently simple apis end up doing DCOM calls

https://codereview.chromium.org/2869073002/diff/1/base/trace_event/process_me...
File base/trace_event/process_memory_maps.h (right):

https://codereview.chromium.org/2869073002/diff/1/base/trace_event/process_me...
base/trace_event/process_memory_maps.h:35: uint64_t timestamp;
since this is windows only can we call it win_module_timestamp or something like
that?
Otherwise this sounds like a "timestamp of the mmap" that "ourselves from the
future" will find quite hard to understand.

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
File components/tracing/common/process_metrics_memory_dump_provider.cc (right):

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
components/tracing/common/process_metrics_memory_dump_provider.cc:25: #include
"base/win/pe_image.h"
this needs to be under #if defined(OS_WIN) right?

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
components/tracing/common/process_metrics_memory_dump_provider.cc:280:
base::win::PEImage pe_image(module_info.lpBaseOfDll);
how long does this take? is it fine to keep doing this over and over? or should
we cache this?

[etienneb, 2017-05-09 17:39:31]

PTAnL

This line:
  pe_image.GetNTHeaders()->FileHeader.TimeDateStamp;

Should result in offset computation only:

typedef struct _IMAGE_NT_HEADERS {
  DWORD                 Signature;
  IMAGE_FILE_HEADER     FileHeader;
  IMAGE_OPTIONAL_HEADER OptionalHeader;
} IMAGE_NT_HEADERS, *PIMAGE_NT_HEADERS;


typedef struct _IMAGE_FILE_HEADER {
  WORD  Machine;
  WORD  NumberOfSections;
  DWORD TimeDateStamp;
  DWORD PointerToSymbolTable;
  DWORD NumberOfSymbols;
  WORD  SizeOfOptionalHeader;
  WORD  Characteristics;
} IMAGE_FILE_HEADER, *PIMAGE_FILE_HEADER;



see:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms680336(v=vs.85).aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms680313(v=vs.85).aspx

https://codereview.chromium.org/2869073002/diff/1/base/trace_event/process_me...
File base/trace_event/process_memory_maps.h (right):

https://codereview.chromium.org/2869073002/diff/1/base/trace_event/process_me...
base/trace_event/process_memory_maps.h:35: uint64_t timestamp;
On 2017/05/09 16:34:01, Primiano Tucci wrote:
> since this is windows only can we call it win_module_timestamp or something
like
> that?
> Otherwise this sounds like a "timestamp of the mmap" that "ourselves from the
> future" will find quite hard to understand.

Agree that adding module_ will help understanding that it's not the timestamp of
the mmap.

I'm not a fan of win_ prefix.

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
File components/tracing/common/process_metrics_memory_dump_provider.cc (right):

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
components/tracing/common/process_metrics_memory_dump_provider.cc:25: #include
"base/win/pe_image.h"
On 2017/05/09 16:34:01, Primiano Tucci wrote:
> this needs to be under #if defined(OS_WIN) right?

right.

https://codereview.chromium.org/2869073002/diff/1/components/tracing/common/p...
components/tracing/common/process_metrics_memory_dump_provider.cc:280:
base::win::PEImage pe_image(module_info.lpBaseOfDll);
On 2017/05/09 16:34:01, Primiano Tucci wrote:
> how long does this take? is it fine to keep doing this over and over? or
should
> we cache this?

This is a trivial cost.
Roughly, the PE image is already in memory and this is just computing the offset
of a raw dword.

   TimeStamp = HMODULE[offset]

[Primiano Tucci (use gerrit), 2017-05-09 17:46:12]

LGTM with 1 small comment addressed.

https://codereview.chromium.org/2869073002/diff/20001/components/tracing/comm...
File components/tracing/common/process_metrics_memory_dump_provider.cc (right):

https://codereview.chromium.org/2869073002/diff/20001/components/tracing/comm...
components/tracing/common/process_metrics_memory_dump_provider.cc:28: #include
"base/win/pe_image.h"
there is another OS_WIN section down on line 51, can you merge this with that?

[etienneb, 2017-05-09 18:12:54]

fixed

https://codereview.chromium.org/2869073002/diff/20001/components/tracing/comm...
File components/tracing/common/process_metrics_memory_dump_provider.cc (right):

https://codereview.chromium.org/2869073002/diff/20001/components/tracing/comm...
components/tracing/common/process_metrics_memory_dump_provider.cc:28: #include
"base/win/pe_image.h"
On 2017/05/09 17:46:12, Primiano Tucci wrote:
> there is another OS_WIN section down on line 51, can you merge this with that?

Done.

[etienneb, 2017-05-09 18:13:22]

landing...

[etienneb, 2017-05-09 18:13:27]

The CQ bit was checked by etienneb@chromium.org

[etienneb, 2017-05-09 18:13:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/2869073002/#ps40001
(title: "nit")

[commit-bot: I haz the power, 2017-05-09 18:14:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 19:04:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 19:04:44]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[etienneb, 2017-05-11 14:52:28]

The CL as-is is not working with the component build.

  is_clang = true
  is_asan = false
  is_debug = false
  is_component_build = true

ninja: Entering directory `out\clang64'
[1/1] Regenerating ninja files
[29/27131] LINK(DLL) tracing.dll tracing.dll.lib tracing.dll.pdb
FAILED: tracing.dll tracing.dll.lib tracing.dll.pdb


error LNK2019: unresolved external symbol ...
  base::win::PEImage::RVAToAddr


Unfortunately, the pe_image is only available in the static build now.

https://cs.chromium.org/chromium/src/base/BUILD.gn?q=pe_image.cc&dr=C&l=1655

# This is the subset of files from base that should not be used with a dynamic
# library. Note that this library cannot depend on base because base depends on
# base_static.
static_library("base_static") {
  sources = [
    "base_switches.cc",
    "base_switches.h",
    "task_scheduler/switches.cc",
    "task_scheduler/switches.h",
    "win/pe_image.cc",
    "win/pe_image.h",
  ]


We may need to change that.

[chrisha, 2017-05-11 15:09:46]

The actual offset calculation is very trivial and there's no explicit need
to pass through base::PEImage to do it. The types are all available
directly in windows.h, and can be navigated in a couple lines of code.
Maybe just a helper function that does the calculation directly?

On Thu, May 11, 2017 at 10:52 AM <etienneb@chromium.org> wrote:

> The CL as-is is not working with the component build.
>
> is_clang = true
> is_asan = false
> is_debug = false
> is_component_build = true
>
> ninja: Entering directory `out\clang64'
> [1/1] Regenerating ninja files
> [29/27131] LINK(DLL) tracing.dll tracing.dll.lib tracing.dll.pdb
> FAILED: tracing.dll tracing.dll.lib tracing.dll.pdb
>
>
> error LNK2019: unresolved external symbol ...
> base::win::PEImage::RVAToAddr
>
>
> Unfortunately, the pe_image is only available in the static build now.
>
>
> https://cs.chromium.org/chromium/src/base/BUILD.gn?q=pe_image.cc&dr=C&l=1655
>
> # This is the subset of files from base that should not be used with a
> dynamic
> # library. Note that this library cannot depend on base because base
> depends on
> # base_static.
> static_library("base_static") {
> sources = [
> "base_switches.cc",
> "base_switches.h",
> "task_scheduler/switches.cc",
> "task_scheduler/switches.h",
> "win/pe_image.cc",
> "win/pe_image.h",
> ]
>
>
> We may need to change that.
>
>
>
> https://codereview.chromium.org/2869073002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[etienneb, 2017-05-11 15:16:46]

An alternative would be to call:
  GetTimestampForLoadedLibrary

see
https://msdn.microsoft.com/en-us/library/windows/desktop/ms680142(v=vs.85).aspx

This implies a dependency to dbghelp and I'm afraid by the footing note:

"All DbgHelp Functions, such as this one, are single threaded. Therefore, calls
from more than one thread to this function will likely result in unexpected
behavior or memory corruption. To avoid this, you must synchronize all
concurrent calls from more than one thread to this function."

[etienneb, 2017-05-11 17:40:34]

Description was changed from

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
==========

to

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
==========

[etienneb, 2017-05-11 17:40:34]

etienneb@chromium.org changed reviewers:
+ dcheng@chromium.org, jschuh@chromium.org

[etienneb, 2017-05-11 17:41:17]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 17:42:04]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 17:55:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 17:55:14]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[dcheng, 2017-05-11 20:31:20]

https://codereview.chromium.org/2869073002/diff/60001/base/BUILD.gn
File base/BUILD.gn (right):

https://codereview.chromium.org/2869073002/diff/60001/base/BUILD.gn#newcode1666
base/BUILD.gn:1666: deps = [ "//base/win:pe_image" ]
The deps line looks a bit out of place below the sanitizer comment. Maybe put it
above?

[etienneb, 2017-05-23 18:38:43]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 18:39:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[etienneb, 2017-05-23 18:39:25]

The CQ bit was unchecked by etienneb@chromium.org

[etienneb, 2017-05-23 18:39:52]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[etienneb, 2017-05-23 18:40:00]

The CQ bit was unchecked by etienneb@chromium.org

[etienneb, 2017-05-23 18:40:41]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 18:41:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 18:50:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 18:50:36]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[etienneb, 2017-05-23 18:57:03]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 18:57:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[etienneb, 2017-05-23 18:58:41]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 18:59:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 19:08:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 19:08:18]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[etienneb, 2017-05-23 19:30:21]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 19:31:18]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 19:39:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 19:39:29]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[etienneb, 2017-05-23 20:14:18]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 20:15:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 20:24:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 20:24:26]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[etienneb, 2017-05-29 20:12:52]

Description was changed from

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
==========

to

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[etienneb, 2017-05-29 20:14:43]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-29 20:14:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[etienneb, 2017-05-29 20:25:54]

dcheng, PTAnL.

I needed to change deps to public_deps to make it work.
Is that fine?

https://codereview.chromium.org/2869073002/diff/60001/base/BUILD.gn
File base/BUILD.gn (right):

https://codereview.chromium.org/2869073002/diff/60001/base/BUILD.gn#newcode1666
base/BUILD.gn:1666: deps = [ "//base/win:pe_image" ]
On 2017/05/11 20:31:20, dcheng wrote:
> The deps line looks a bit out of place below the sanitizer comment. Maybe put
it
> above?

Done.

[etienneb, 2017-05-29 20:26:01]

The CQ bit was checked by etienneb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-29 20:26:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-29 21:41:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-29 21:41:33]

Dry run: This issue passed the CQ dry run.

[jschuh, 2017-05-30 16:53:41]

jschuh@chromium.org changed reviewers:
+ scottmg@chromium.org

[jschuh, 2017-05-30 16:53:42]

I'm still not entirely up to speed on GN. Scott, could you take a look and
approve?

[scottmg, 2017-05-30 16:59:29]

public_deps is OK. That just means that things that depend on base_static are
allowed to include pe_image.h, which is how things were before.

The other option would be to change other targets that currently depend on
base_static to depend on both base_static and if (is_win) on base/win:pe_image.

lgtm

[dcheng, 2017-05-30 17:04:48]

lgtm

[etienneb, 2017-05-30 21:08:15]

The CQ bit was checked by etienneb@chromium.org

[etienneb, 2017-05-30 21:08:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/2869073002/#ps180001
(title: "dcheng comments")

[commit-bot: I haz the power, 2017-05-30 21:08:33]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 22:45:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 22:45:34]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[etienneb, 2017-05-31 15:15:01]

The CQ bit was checked by etienneb@chromium.org

[commit-bot: I haz the power, 2017-05-31 15:15:28]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 16:01:59]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1496243701225160,
"parent_rev": "5dd8bd3e28d83ac7d744c6e00f7255cfde70d7f1", "commit_rev":
"a81e171d89732c62f3e8d1846ea685baa87ecd6f"}

[commit-bot: I haz the power, 2017-05-31 16:02:09]

Description was changed from

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Adding PE TimeStamp to chrome trace to ease retrieving debug information

On windows, symbols servers can be queried to retrieve debug information.
The code identifier is used as a key to retrieve the exact executable.

Current chrome memory dump contains the loaded module and their loaded
addresses.
example:
 
{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\system32\\powrprof.dll","pf":0,"sa":"7ffdae7a0000","sz":"4b000"},

But to uniquely identify them and being able to retrieve debug information from
symbols servers, the TimeStamp present in PE header is required.

This patch is adding the "ts" (TimeStamp) field.

{"bs":{"pc":"0","pd":"0","pss":"0","sc":"0","sd":"0","sw":"0"},"mf":"C:\\Windows\\SYSTEM32\\ntdll.dll","pf":0,"sa":"7ffdb2170000","sz":"1c1000","ts":"580ee321"},

From these information one can query MS/Google symbols servers.
  https://msdl.microsoft.com/download/symbols/ntdll.dll/580EE3211C1000/ntdll.dl_
                                              basename / code-ident   /dll

The code-identifier is built from the timestamp AND the size of the executable



CC=chrisha@chromium.org, erikchen@chromium.org
R=primiano@chromium.org

BUG=719969
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2869073002
Cr-Commit-Position: refs/heads/master@{#475930}
Committed:
https://chromium.googlesource.com/chromium/src/+/a81e171d89732c62f3e8d1846ea6...
==========

[commit-bot: I haz the power, 2017-05-31 16:02:11]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/a81e171d89732c62f3e8d1846ea6...