upload clang pdbs to chrome's symbol server

upload clang pdbs to chrome's symbol server

BUG=682500
NOTRY=true

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

[Nico, 2017-02-21 23:05:10]

Description was changed from

==========
upload clang pdbs to somewhere

.

BUG=
==========

to

==========
upload clang pdbs to somewhere

BUG=682500
==========

[Nico, 2017-02-21 23:10:11]

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

[Nico, 2017-02-21 23:10:11]

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
File tools/clang/scripts/package.py (right):

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
tools/clang/scripts/package.py:305: pdbdir = 'clangpdb-' + stamp
I have many questions about this part.

1. The bug says https://chromium-browser-symsrv.commondatastorage.googleapis.com
. This currently puts it in gs://chromium-browser-clang/win/clangpdb-1234.tgz Is
chromium-browser-symsrv better? If so, which exact file name should I use there?

2. As far as I know, the upstream cmake files basically build bin/clang.exe and
then copy it to bin/clang-cl.exe. We only package up clang-cl.exe. However, the
pdb gets created at bin/clang.pdb (due to the upstream setup).
a) Should we copy clang.pdb to clang-cl.pdb before zipping it up?
b) Is the name of a pdb embedded into the pdb?
c) Does it matter that .exe and its .pdb have the same basename (ignoring
extension)?

3. This currently uses tgz -- is that good, or is zip better, or something else
altogether? (All the other archives package.py currently creates are tgz, but it
doesn't matter too much what this one uses as long as python can write it)

clang.pdb is a bit over 400 MB before zipping.

[scottmg, 2017-02-21 23:38:49]

You're gonna love this!

+sebmarchand +brucedawson since they have probably wrestled with this more
recently and can likely correct some of the details on paths.

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
File tools/clang/scripts/package.py (right):

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
tools/clang/scripts/package.py:305: pdbdir = 'clangpdb-' + stamp
On 2017/02/21 23:10:11, Nico wrote:
> I have many questions about this part.
> 
> 1. The bug says
https://chromium-browser-symsrv.commondatastorage.googleapis.com
> . This currently puts it in gs://chromium-browser-clang/win/clangpdb-1234.tgz
Is
> chromium-browser-symsrv better? If so, which exact file name should I use
there?
> 
> 2. As far as I know, the upstream cmake files basically build bin/clang.exe
and
> then copy it to bin/clang-cl.exe. We only package up clang-cl.exe. However,
the
> pdb gets created at bin/clang.pdb (due to the upstream setup).
> a) Should we copy clang.pdb to clang-cl.pdb before zipping it up?
> b) Is the name of a pdb embedded into the pdb?
> c) Does it matter that .exe and its .pdb have the same basename (ignoring
> extension)?
> 
> 3. This currently uses tgz -- is that good, or is zip better, or something
else
> altogether? (All the other archives package.py currently creates are tgz, but
it
> doesn't matter too much what this one uses as long as python can write it)
> 
> clang.pdb is a bit over 400 MB before zipping.

Yes, chromium-browser-symsrv is better because Chrome devs will have that in
their _NT_SYMBOL_PATH, so tools will just work (msvs, windbg, etw, etc.)

If you look at the the official.desktop builder on uberchrome in the "official
symbols" step, that's what you want to do.

That step sneakily does two things, the first is to use Breakpad dump_syms and
upload a text version for the crash server.

The second down near the bottom half of the logs (I just learned this now) is to
push the raw pdbs and the binary into the gs bucket.

The second part is what we'd want to do here for clang.

So, you'd want to not tar it up, and also upload the binary, which allows
debugging crash dumps.

The target paths are finicky. Taking chrome_child.dll as an example from a
recent log https://goto.google.com/drevv

gs://chromium-browser-symsrv/chrome_child.dll/58ACAFB932d5000/chrome_child.dll 

That hex string is a concatenation of two things

dumpbin /headers chrome_child.dll | grep "date stamp"
and
dumpbin /headers chrome_child.dll | grep "size of image"

both of those numbers formatted as hex. There's something fiddly about this that
I don't quite remember, I think the first ones is %08X and the second is %x
(maybe? +Seb and +Bruce who might remember).

Secondarily, the PDB. It goes into something like

gs://chromium-browser-symsrv/chrome_child.dll.pdb/F71B007136FC43EFB957AB08B2E150441/chrome_child.dll.pd_

*That* hex string is the RSDS GUID from the binary (dumpbin /all mybinary.exe |
grep "Format: RSDS") concatenated with the "age" of the RSDS record (which is
the number after the comma following the GUID), so it'll generally be 32+1
characters long, but occasionally longer.

Whew, did you get all that?

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/upd...
File tools/clang/scripts/update.py (right):

https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/upd...
tools/clang/scripts/update.py:622: # Build PDBs for archieval on Windows.  Don't
use RelWithDebInfo since it
No 'e' in archival.

[scottmg, 2017-02-21 23:44:58]

On 2017/02/21 23:38:49, scottmg wrote:
> You're gonna love this!
> 
> +sebmarchand +brucedawson since they have probably wrestled with this more
> recently and can likely correct some of the details on paths.
> 
>
https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
> File tools/clang/scripts/package.py (right):
> 
>
https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/pac...
> tools/clang/scripts/package.py:305: pdbdir = 'clangpdb-' + stamp
> On 2017/02/21 23:10:11, Nico wrote:
> > I have many questions about this part.
> > 
> > 1. The bug says
> https://chromium-browser-symsrv.commondatastorage.googleapis.com
> > . This currently puts it in
gs://chromium-browser-clang/win/clangpdb-1234.tgz
> Is
> > chromium-browser-symsrv better? If so, which exact file name should I use
> there?
> > 
> > 2. As far as I know, the upstream cmake files basically build bin/clang.exe
> and
> > then copy it to bin/clang-cl.exe. We only package up clang-cl.exe. However,
> the
> > pdb gets created at bin/clang.pdb (due to the upstream setup).
> > a) Should we copy clang.pdb to clang-cl.pdb before zipping it up?
> > b) Is the name of a pdb embedded into the pdb?
> > c) Does it matter that .exe and its .pdb have the same basename (ignoring
> > extension)?
> > 
> > 3. This currently uses tgz -- is that good, or is zip better, or something
> else
> > altogether? (All the other archives package.py currently creates are tgz,
but
> it
> > doesn't matter too much what this one uses as long as python can write it)
> > 
> > clang.pdb is a bit over 400 MB before zipping.
> 
> Yes, chromium-browser-symsrv is better because Chrome devs will have that in
> their _NT_SYMBOL_PATH, so tools will just work (msvs, windbg, etw, etc.)
> 
> If you look at the the official.desktop builder on uberchrome in the "official
> symbols" step, that's what you want to do.
> 
> That step sneakily does two things, the first is to use Breakpad dump_syms and
> upload a text version for the crash server.
> 
> The second down near the bottom half of the logs (I just learned this now) is
to
> push the raw pdbs and the binary into the gs bucket.
> 
> The second part is what we'd want to do here for clang.
> 
> So, you'd want to not tar it up, and also upload the binary, which allows
> debugging crash dumps.
> 
> The target paths are finicky. Taking chrome_child.dll as an example from a
> recent log https://goto.google.com/drevv
> 
> gs://chromium-browser-symsrv/chrome_child.dll/58ACAFB932d5000/chrome_child.dll

> 
> That hex string is a concatenation of two things
> 
> dumpbin /headers chrome_child.dll | grep "date stamp"
> and
> dumpbin /headers chrome_child.dll | grep "size of image"
> 
> both of those numbers formatted as hex. There's something fiddly about this
that
> I don't quite remember, I think the first ones is %08X and the second is %x
> (maybe? +Seb and +Bruce who might remember).
> 
> Secondarily, the PDB. It goes into something like
> 
>
gs://chromium-browser-symsrv/chrome_child.dll.pdb/F71B007136FC43EFB957AB08B2E150441/chrome_child.dll.pd_
> 
> *That* hex string is the RSDS GUID from the binary (dumpbin /all mybinary.exe
|
> grep "Format: RSDS") concatenated with the "age" of the RSDS record (which is
> the number after the comma following the GUID), so it'll generally be 32+1
> characters long, but occasionally longer.

Oh, and this "age" is %d, not %x. Because why? Because because. (which ISTR
Bruce fixing around when he started. :)

> 
> Whew, did you get all that?
> 
>
https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/upd...
> File tools/clang/scripts/update.py (right):
> 
>
https://codereview.chromium.org/2709613004/diff/20001/tools/clang/scripts/upd...
> tools/clang/scripts/update.py:622: # Build PDBs for archieval on Windows. 
Don't
> use RelWithDebInfo since it
> No 'e' in archival.

[brucedawson, 2017-02-22 00:20:45]

Thanks for the brain dump Scott. I think that's *mostly* accurate.

I blogged about this from a different perspective four years ago, here:

https://randomascii.wordpress.com/2013/03/09/symbols-the-microsoft-way/

The one mistake in Scott's information is that the age needs to be %x. dumpbin
prints it as %d but symbol servers expect %x. It's mostly irrelevant since for a
clean build it will always be '1', but there you go. From my post:

"This means that if you parse the dumpbin output you need to convert the age to
an integer and the print it as hexadecimal. If you get this wrong then the bug
won’t show up until you encounter a PDB with an age of ten or greater.
Wonderful."

For the PE paths my article was assuming you were grabbing strings from dumpbin
so I used %s, but if printing numbers you are correct that the second one is %x.
The first one is either %8X or %X (weird that it seems to be capital X - I'd
never noticed that before) with the '8' being irrelevant because the dates are
all well into eight digits.

From my article (updated a year or two ago apparently):

"One extra quirk is that Microsoft’s linker/debugger toolchains lower-case the
names of PE files. This means that if you use a case-sensitive file system for
your symbol server (as Chrome does) then you have to upload your symbols using
lower-case file names. The Chrome team discovered this the hard way. The PDB
names are extracted from the PE files, with the case intact, so they just have
to match."

Also, you can compress the files. The last letter of the extension gets changed
to an underscore to indicate that they are compressed. I can't remember what
tool has to be used to compress them, but Chrome's symbols (and PE files) are
compressed so we can copy from that.

I use RetrieveSymbols.exe
(https://github.com/google/UIforETW/tree/master/RetrieveSymbols) when testing
symbol servers - it lets you pass the time/size/GUID/age/filename and then
retrieves the files. I can certainly help with testing because I seem to fix
symbol server problems at every job I have.

[scottmg, 2017-02-22 19:43:12]

On 2017/02/22 00:20:45, brucedawson wrote:
> Thanks for the brain dump Scott. I think that's *mostly* accurate.
> 
> I blogged about this from a different perspective four years ago, here:
> 
> https://randomascii.wordpress.com/2013/03/09/symbols-the-microsoft-way/
> 
> The one mistake in Scott's information is that the age needs to be %x. dumpbin
> prints it as %d but symbol servers expect %x. It's mostly irrelevant since for
a
> clean build it will always be '1', but there you go. From my post:

Doh, I knew we'd had it wrong at some point but apparently I remembered the
exact opposite lesson.

> 
> "This means that if you parse the dumpbin output you need to convert the age
to
> an integer and the print it as hexadecimal. If you get this wrong then the bug
> won’t show up until you encounter a PDB with an age of ten or greater.
> Wonderful."
> 
> For the PE paths my article was assuming you were grabbing strings from
dumpbin
> so I used %s, but if printing numbers you are correct that the second one is
%x.
> The first one is either %8X or %X (weird that it seems to be capital X - I'd
> never noticed that before) with the '8' being irrelevant because the dates are
> all well into eight digits.
> 
> From my article (updated a year or two ago apparently):
> 
> "One extra quirk is that Microsoft’s linker/debugger toolchains lower-case the
> names of PE files. This means that if you use a case-sensitive file system for
> your symbol server (as Chrome does) then you have to upload your symbols using
> lower-case file names. The Chrome team discovered this the hard way. The PDB
> names are extracted from the PE files, with the case intact, so they just have
> to match."
> 
> Also, you can compress the files. The last letter of the extension gets
changed
> to an underscore to indicate that they are compressed. I can't remember what
> tool has to be used to compress them, but Chrome's symbols (and PE files) are
> compressed so we can copy from that.

It looks like we use

compress_cmd = ['makecab', '/D', 'CompressionType=LZX', '/D',
                  'CompressionMemory=21']

see build-internal in official_utils.py. It's all pretty super.

> 
> I use RetrieveSymbols.exe
> (https://github.com/google/UIforETW/tree/master/RetrieveSymbols) when testing
> symbol servers - it lets you pass the time/size/GUID/age/filename and then
> retrieves the files. I can certainly help with testing because I seem to fix
> symbol server problems at every job I have.

[Nico, 2017-02-22 20:28:00]

Thanks for the excellent code-by-numbers instructions! It looks like all the
clang upload bots are currently broken due to some infra change, so I'll need to
unbreak them first before coming back to this.

[Nico, 2017-03-08 22:41:12]

I uploaded an untested WIP thing (waiting for my local build), but one question:

https://codereview.chromium.org/2709613004/diff/80001/tools/clang/scripts/pac...
File tools/clang/scripts/package.py (right):

https://codereview.chromium.org/2709613004/diff/80001/tools/clang/scripts/pac...
tools/clang/scripts/package.py:156:
'gs://chromium-browser-symsrv/clang.pdb/%s/clang.pdb' %
Should this be clang.pdb/%s/clang.pdb, or clang-cl.pdb/%s/clang.pdb? How does
dbghelp do this, does it do "exe basename with .pdb instead of .exe" or does it
do "get pdb name from dumpbin /all | find Format:"?

[Nico, 2017-03-08 23:19:42]

Ok, please take a look, now I even tested it.

I ran this locally, but uploading gave me 403s, so I commented out the upload
bits. I probably didn't set up a .boto on my Windows bots. vadimsh tells me that
the trybots have write permissions, so they should have more luck than me.

[Nico, 2017-03-08 23:19:55]

(the trybots that build clang packages when we do rolls, that is)

[Nico, 2017-03-08 23:20:10]

Description was changed from

==========
upload clang pdbs to somewhere

BUG=682500
==========

to

==========
upload clang pdbs to chrome's symbol server

BUG=682500
==========

[Nico, 2017-03-08 23:21:07]

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

[commit-bot: I haz the power, 2017-03-08 23:21:43]

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-03-09 00:15:59]

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

[commit-bot: I haz the power, 2017-03-09 00:16:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Nico, 2017-03-09 00:37:40]

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

[commit-bot: I haz the power, 2017-03-09 00:38: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-03-09 01:44:34]

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

[commit-bot: I haz the power, 2017-03-09 01:44:35]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-03-09 17:09:48]

scottmg: ping

[scottmg, 2017-03-09 17:53:23]

sorry, bonus stability sheriff shift and perf and yaddayadda

lgtm

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
File tools/clang/scripts/package.py (right):

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
tools/clang/scripts/package.py:120: # 
chromium-browser-symsrv/clang-cl.exe/ABCDEFAB01234/clang-cl.exe
Since you're cab compressing the binary too, make this filename match the one
below.

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
tools/clang/scripts/package.py:154: 'gs://chromium-browser-symsrv/' + dest]
indent

[Nico, 2017-03-09 17:56:03]

Thanks!

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
File tools/clang/scripts/package.py (right):

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
tools/clang/scripts/package.py:120: # 
chromium-browser-symsrv/clang-cl.exe/ABCDEFAB01234/clang-cl.exe
On 2017/03/09 17:53:23, scottmg wrote:
> Since you're cab compressing the binary too, make this filename match the one
> below.

Done.

https://codereview.chromium.org/2709613004/diff/120001/tools/clang/scripts/pa...
tools/clang/scripts/package.py:154: 'gs://chromium-browser-symsrv/' + dest]
On 2017/03/09 17:53:23, scottmg wrote:
> indent

Done.

[Nico, 2017-03-09 17:56:08]

The CQ bit was checked by thakis@chromium.org

[Nico, 2017-03-09 17:56:09]

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

[commit-bot: I haz the power, 2017-03-09 17:56:36]

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

[Nico, 2017-03-09 18:49:39]

Description was changed from

==========
upload clang pdbs to chrome's symbol server

BUG=682500
==========

to

==========
upload clang pdbs to chrome's symbol server

BUG=682500
NOTRY=true
==========

[Nico, 2017-03-09 18:49:49]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2017-03-09 18:49:54]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2017-03-09 18:50:08]

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-03-09 18:55:10]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489085394776450,
"parent_rev": "f2d4ff52c748b24a2a57a6f3d3fe7573744442ba", "commit_rev":
"d8adfa3476b2aaba341cf01f3b6a8365dcf250c7"}

[commit-bot: I haz the power, 2017-03-09 18:55:51]

Description was changed from

==========
upload clang pdbs to chrome's symbol server

BUG=682500
NOTRY=true
==========

to

==========
upload clang pdbs to chrome's symbol server

BUG=682500
NOTRY=true

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

[commit-bot: I haz the power, 2017-03-09 18:55:53]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d8adfa3476b2aaba341cf01f3b6a...