New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 330260 link

Starred by 8 users

Issue metadata

Status: Assigned
Owner:
on vacation until Nov 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Feature


Sign in to add a comment

Make Windows ninja build deterministic

Project Member Reported by mar...@chromium.org, Dec 20 2013

Issue description

1. ninja -C out/Release browser_tests
2. ren out out2
3. gclient runhooks
2. ninja -C out/Release browser_tests

Expected:
Content of out/ and out2/ is byte-for-byte identical.

Actual:
They aren't.

This is specific about Windows/ninja/VS2013
 

Comment 1 by mar...@chromium.org, Dec 20 2013

Blocking: chromium:314403

Comment 2 by mar...@chromium.org, Dec 20 2013

Labels: -Build-Toolchain

Comment 3 by mar...@chromium.org, Sep 26 2014

Cc: mar...@chromium.org
Owner: sebmarchand@chromium.org
Seb is actually going to do the real work on this, assigning accordingly.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16 2014

------------------------------------------------------------------
r292492 | sebmarchand@chromium.org | 2014-10-16T19:30:49.092077Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_IOS_deterministic_build.json?r1=292492&r2=292491&pathrev=292492
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_Windows_deterministic_build.json?r1=292492&r2=292491&pathrev=292492
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_Linux_deterministic_build.json?r1=292492&r2=292491&pathrev=292492
   D http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_Test_deterministic_build.json?r1=292492&r2=292491&pathrev=292492
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.py?r1=292492&r2=292491&pathrev=292492
   A http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/swarming/remove_build_metadata.py?r1=292492&r2=292491&pathrev=292492
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_Android_deterministic_build.json?r1=292492&r2=292491&pathrev=292492
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipes/swarming/deterministic_build.expected/full_chromium_swarm_Mac_deterministic_build.json?r1=292492&r2=292491&pathrev=292492

Swarming|deterministic: Run zap_timestamp on the Windows binaries.

BUG=330260

Review URL: https://codereview.chromium.org/657163003
-----------------------------------------------------------------
I've removed some sources of indeterminism but there's still a LOT of different bytes between 2 builds... For chrome.dll ~85.5% of the bytes are different... It seems that it's mostly because the functions/globals have a different address in the PDB (content is the same but not at the same place).

Not sure that we can really fix this in the short term.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 17 2014

------------------------------------------------------------------
r292969 | maruel@chromium.org | 2014-11-17T17:01:40.264592Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/masters/master.chromium.swarm/master.cfg?r1=292969&r2=292968&pathrev=292969

Work around an issue with NaCl build with long path name.

R=sebmarchand@chromium.org
BUG=330260

Review URL: https://codereview.chromium.org/734673003
-----------------------------------------------------------------
Did you already try toggling off all various incremental-y things? /incremental, pdbs, etc.?

I guess it wouldn't necessarily be a useful end state, but we could at least confirm that it's only due to X  or Y and then maybe we could get Microsoft to address of offer workarounds for specific flags.
We're only testing in Release. We had went quite far, browser_tests was deterministic but it regressed recently. We also had issues with path casing w.r.t. remote compilation, which is not an issue with local compilation.

The main issue we face is the linker which seem to use a hashtable based iteration of the object files, which is dependent on the path case.
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 1 2015

------------------------------------------------------------------
r294641 | sebmarchand@chromium.org | 2015-04-01T19:59:49.598673Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py?r1=294641&r2=294640&pathrev=294641

Update the whitelist and print the list of 'unexpected equal files'.

BUG=330260

Review URL: https://codereview.chromium.org/1056503003
-----------------------------------------------------------------
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 1 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/25ee47543b2bdc7f968b360b4a5ee7646f6167f2

commit 25ee47543b2bdc7f968b360b4a5ee7646f6167f2
Author: maruel@chromium.org <maruel@chromium.org>
Date: Tue Sep 01 14:51:12 2015

Add more known non-deterministic files to Windows.

Remove interline when printing the unexpected non-deterministic files.
Update links to https.

TBR=sebmarchand@chromium.org
BUG=330260

Review URL: https://codereview.chromium.org/1325823004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@296511 0039d316-1c4b-4281-b951-d872f2087c98

[modify] http://crrev.com/25ee47543b2bdc7f968b360b4a5ee7646f6167f2/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 8 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/f39916888b8cd82a5bddbfc18eac4a740b625b78

commit f39916888b8cd82a5bddbfc18eac4a740b625b78
Author: sebmarchand@chromium.org <sebmarchand@chromium.org>
Date: Tue Sep 08 15:00:38 2015

Deterministic build: Update the blacklist.


TBR=maruel@chromium.org
BUG=330260

Review URL: https://codereview.chromium.org/1332583003

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/build@296598 0039d316-1c4b-4281-b951-d872f2087c98

[modify] http://crrev.com/f39916888b8cd82a5bddbfc18eac4a740b625b78/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py

Project Member

Comment 14 by sheriffbot@chromium.org, Jul 6 2016

Labels: Hotlist-OpenBugWithCL
A change has landed for this issue, but it's been open for over 6 months. Please review and close it if applicable. If this issue should remain open, remove the "Hotlist-OpenBugWithCL" label. If no action is taken, it will be archived in 30 days.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-OpenBugWithCL
It seems not deterministic now.
https://uberchromegw.corp.google.com/i/chromium.fyi/builders/Windows%20deterministic

Is it possible to run the builds like this?  The idea just come from what we talked in the team meeting.
1. run build on one builder.
2. run the build again on the same bot with 1. and compare with the result of 1.
3. run on the other builder and compare with the result of 1.
4. run with goma and compare with 2.

If non-determinism is caused by a compiler, comparison in 2. would detect that.
If non-determinism  is caused by a change of host, 3. would detect that.
If goma cause something non-deterministic, 4. would detect that.
That need at least two buildbot but it should be helpful for us to identify the cause of non-deterministic build.

To do that, we might need to trigger 3 from 2 i.e. trigger builder from builder.  I am not sure it possible or not.

What do you think?  Can we have some extra bots and trigger 3 from 2 to do that?
Cc: phajdan.jr@chromium.org no...@chromium.org
Yes this is doable.
- A master recipe triggers two compile subrun via Swarming.
- The Swarming task puts the sha-1 of each resulting binary in its output.
- The master recipe compares the sha-1.

The option is to isolate all the content instead of the sha-1 but this may take a while (?) Needs to be verified. The main drawback of archiving the digest instead of the content is that it's impossible to look at the diff afterward.


Project Member

Comment 18 by bugdroid1@chromium.org, Aug 8 2016

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

commit c8c2305476408e04b0809d1d036d82210bb0cb9f
Author: yyanagisawa <yyanagisawa@chromium.org>
Date: Mon Aug 08 20:31:41 2016

Add Android deterministic and Windows Clang deterministic.

BUG= 630930 
BUG=330260
BUG=314403
BUG=439737

Review-Url: https://codereview.chromium.org/2221793003
Cr-Commit-Position: refs/heads/master@{#410440}

[modify] https://crrev.com/c8c2305476408e04b0809d1d036d82210bb0cb9f/tools/mb/mb_config.pyl

Not sure this issue covers Windows Clang or not but...

I am quite sure this is caused by goma.
https://chromegw.corp.google.com/i/chromium.fyi/builders/Windows%20Clang%20deterministic/builds/7/steps/compare_build_artifacts/logs/stdio
nacl_irt_x86_32.nexe 
  0xb000    : 003ee00f2e2e2f2e2e5c6e61746976655f636c69656e742f7372632f756e7472 '.>..../..\native_client/src/untr'
              003ee00f2e2e2f2e2e2f6e61746976655f636c69656e742f7372632f756e7472 '.>..../../native_client/src/untr'
                                ^^                                                       ^

I want nacl compiler always use / as path delimiter.
Since these files (.nexe) are platform agnostic, that's a good idea.
Cc: yyanagisawa@chromium.org
Windows clang...
courgette64 and NaCl seems not be deterministic build.  If something significant difference caused by building NaCl untrusted code with Linux version NaCl compiler, it could cause this, though.
https://uberchromegw.corp.google.com/i/chromium.fyi/builders/Windows%20Clang%20deterministic/builds/898/steps/compare_build_artifacts/logs/stdio
Checking courgette64.exe difference: (330 deps)
Checking mini_installer.exe difference: (25760 deps)
  irt_x86/obj/ppapi/thunk/thunk/ppb_tcp_socket_private_thunk.o                                                                                             : different size: 127772 != 127808
  irt_x86/obj/native_client/src/untrusted/irt/irt_core_lib/irt_dyncode.o                                                                                   : different size: 6288 != 6264
  irt_x86/obj/ppapi/shared_impl/shared_impl/ppapi_preferences.o                                                                                            : different size: 187056 != 187084
  irt_x86/obj/base/i18n/bidi_line_iterator.o                                                                                                               : different size: 78192 != 78164
  irt_x86/obj/third_party/icu/icui18n/tzgnames.o                                                                                                           : different size: 172336 != 172288
  irt_x86/obj/third_party/icu/icuuc/locid.o                                                                                                                : different size: 136520 != 136516
  irt_x86/obj/third_party/icu/icuuc/pluralmap.o                                                                                                            : different size: 68132 != 68128
  irt_x64/obj/third_party/icu/icui18n/gregocal.o                                                                                                           : different size: 164424 != 164376
Checking nacl_irt_x86_32.nexe difference: (1313 deps)
  irt_x86/obj/ppapi/thunk/thunk/ppb_tcp_socket_private_thunk.o                                                     : different size: 127772 != 127808
  irt_x86/obj/native_client/src/untrusted/irt/irt_core_lib/irt_dyncode.o                                           : different size: 6288 != 6264
  irt_x86/obj/ppapi/shared_impl/shared_impl/ppapi_preferences.o                                                    : different size: 187056 != 187084
  irt_x86/obj/base/i18n/bidi_line_iterator.o                                                                       : different size: 78192 != 78164
  irt_x86/obj/third_party/icu/icui18n/tzgnames.o                                                                   : different size: 172336 != 172288
  irt_x86/obj/third_party/icu/icuuc/locid.o                                                                        : different size: 136520 != 136516
  irt_x86/obj/third_party/icu/icuuc/pluralmap.o                                                                    : different size: 68132 != 68128
Checking next_version_mini_installer.exe difference: (25825 deps)
  irt_x86/obj/ppapi/thunk/thunk/ppb_tcp_socket_private_thunk.o                                                                                             : different size: 127772 != 127808
  irt_x86/obj/native_client/src/untrusted/irt/irt_core_lib/irt_dyncode.o                                                                                   : different size: 6288 != 6264
  irt_x86/obj/ppapi/shared_impl/shared_impl/ppapi_preferences.o                                                                                            : different size: 187056 != 187084
  irt_x86/obj/base/i18n/bidi_line_iterator.o                                                                                                               : different size: 78192 != 78164
  irt_x86/obj/third_party/icu/icui18n/tzgnames.o                                                                                                           : different size: 172336 != 172288
  irt_x86/obj/third_party/icu/icuuc/locid.o                                                                                                                : different size: 136520 != 136516
  irt_x86/obj/third_party/icu/icuuc/pluralmap.o                                                                                                            : different size: 68132 != 68128
  irt_x64/obj/third_party/icu/icui18n/gregocal.o                                                                                                           : different size: 164424 != 164376
Checking ppapi_nacl_tests_pnacl_newlib_x32.nexe difference: (185 deps)
Checking ppapi_nacl_tests_pnacl_newlib_x64.nexe difference: (185 deps)
For Windows cl.exe,

https://uberchromegw.corp.google.com/i/chromium.fyi/builders/Windows%20deterministic/builds/1022/steps/compare_build_artifacts/logs/stdio

  obj/third_party/openmax_dl/dl/dl/x86SP_FFT_CToC_FC32_Inv_Radix4_ms.obj                                                                                   : 7 out of 1804 bytes are different (0.39%)
  0x10c     : 645c7372635c6f75745c52656c656173655c6f626a5c74686972645f70617274 'd\src\out\Release\obj\third_part'
              645c7372635c6f75745c72656c656173655c6f626a5c74686972645f70617274 'd\src\out\release\obj\third_part'
                                  ^                                                       ^
  0x64c     : 0003012f0000000000000000000000000000000000f3032e6465627567245300 '.../....................debug$S.'
              0003012f000000000000000000000000000000000000002e6465627567245300 '.../....................debug$S.'
                                                         ^^^
  0x66c     : 0000000200000000000301c40000000000000000000000000000000000f3032e '................................'
              0000000200000000000301c4000000000000000000000000000000000000002e '................................'
                                                                         ^^^
  0x6ac     : 0001000000f30300000000040000000000000003000000200002000000000027 '....................... .......''
              0001000000000000000000040000000000000003000000200002000000000027 '....................... .......''
                         ^^^

Mmm, difference could be caused by local and remote difference?
Cc: h...@chromium.org thakis@chromium.org
+thakis@ and hans@

Do you guys want to take ownership of this bug now that we're moving to Clang?
Sure. We won't be able to work on this immediately, but I think it's an important medium-term thing.
Cc: sebmarchand@chromium.org
Owner: thakis@chromium.org
Thanks, assigning this to you for now, keeping this as a P2.

Comment 26 by no...@chromium.org, Aug 10 2017

Cc: -no...@chromium.org
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 1 2018

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

commit 87a74692c1f308697f351676fbbb2010fe80d248
Author: Nico Weber <thakis@chromium.org>
Date: Thu Mar 01 03:45:01 2018

win: Pass /Brepro to the compiler when using LLD.

With link.exe this isn't always safe (/incremental relies on the mtime
timestamp stored in the .obj file, and there's no easy way to query if
/incremental is used), but with lld it should be.

Looking at `dumpbin /headers test.obj | findstr "time date stamp"`,
cl.exe seems to write a hash of the output to the time date stamp,
while clang-cl always seems to write 0 with /Brepro.

Bug: 330260
Change-Id: I1fd64d0f0678468bc73c672cb84a5a2d878e2e82
Reviewed-on: https://chromium-review.googlesource.com/941905
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540026}
[modify] https://crrev.com/87a74692c1f308697f351676fbbb2010fe80d248/build/config/win/BUILD.gn

Project Member

Comment 28 by bugdroid1@chromium.org, Mar 1 2018

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

commit 7c231bfc077d2e23b7f1bcecdcb6fbd21fd0639f
Author: Nico Weber <thakis@chromium.org>
Date: Thu Mar 01 21:16:58 2018

Unbreak win/CFI builds after https://chromium-review.googlesource.com/c/chromium/src/+/941905

`clang-cl /Brepro -flto=thin` causes:

    warning: argument unused during compilation: '-mno-incremental-linker-compatible'

because /Brepro is a built-in assembler flag, which isn't used in LTO builds.

(We have an open bug on clang-cl somewhere to print the original spelling of
aliased flags.)

Bug: 330260
Change-Id: I72c6fb496107a7c937019293e6ad6f749a680a20
Reviewed-on: https://chromium-review.googlesource.com/944003
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540285}
[modify] https://crrev.com/7c231bfc077d2e23b7f1bcecdcb6fbd21fd0639f/build/config/win/BUILD.gn

keyword reproducible
Project Member

Comment 30 by bugdroid1@chromium.org, Jun 13

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

commit 7f644bdff216ee98e509001cb133b65789deeaa5
Author: Nico Weber <thakis@chromium.org>
Date: Wed Jun 13 21:43:45 2018

Pass -no-canonical-prefixes to clang-cl too.

clang-cl learned this flag in https://reviews.llvm.org/rL333761 (which also has
some words on why it's not the default). #566302 was a clang roll containing
that change.

Note that we also need https://reviews.llvm.org/rL334602 for this to have
an effect in practice, and that's not rolled in yet. Once it is, goma should
be able to cache non-symbol .obj files independent of the checkout path.
(See also discussion in https://chromium-review.googlesource.com/c/chromium/src/+/1075947)

Bug: 439949,330260, 712796 
Change-Id: I4a9bc38e35884be03fd9898e7000938cb4782a54
Reviewed-on: https://chromium-review.googlesource.com/1099638
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567001}
[modify] https://crrev.com/7f644bdff216ee98e509001cb133b65789deeaa5/build/config/compiler/BUILD.gn

Here's a guess at the current status of this.

- Compiles and random other build things should be in good shape

- Links have some known (small) issues:
-- lld-link currently writes the current timestamp into the COFF file, because appcompat on win7 ( https://crbug.com/843199 ) I have plans to change that ( https://bugs.chromium.org/p/chromium/issues/detail?id=804926#c8 ) but it needs doing.
-- pdbs require absolute paths in it, so lld writes the absolute path to source files in the pdb. Since the swarming slaves don't care about the concrete absolute path, we could make that prefix some fixed fake path (https://reviews.llvm.org/D48882 -- easy to tweak to prepend something else).

There might be other unknown issues, but that's probably all that's left.
Oh, comment 22 reminds me that goma converts all paths to lower case on the remote end (bug 617318, b/64544343). So relative file paths will be different in goma and local builds for TUs that have upper-case letters in their name.
Cc: -phajdan.jr@chromium.org
Note about absolute path: issue 629090 gets a bit in the way by not having *all* bots uses the same base directory, but there's only ~3 flavors, not an infinite number.
That's why I say in comment 31 we could just lie about the absolute path prefix :-)
Project Member

Comment 36 by bugdroid1@chromium.org, Aug 3

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

commit ef36dc19986a90f49d0d31520c88d310d72bd038
Author: Nico Weber <thakis@chromium.org>
Date: Fri Aug 03 17:33:15 2018

win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time

We used to set the timestamp to a hash of the binary, similar to
https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
However, that caused an appcompat warning on Windows 7 to appear, which
interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
could help with that, but my guess it won't have an effect on Windows 7,
which likely always believes that the the coff timestamp field always stores
a timestamp).

So currently we write the current time during linking in that field, but that's
bad for build determinism and that in turn is bad for swarming test result cachability.

build/write_build_date_header.py already creates a deterministic BUILD_DATE
with several tradeoffs. Cachability wants this to change infrequently, but
things like HSTS need a "real" build date and want this to change frequently.
The compromise is: The date changes once per day in official builds, and
once a month in regular builds.

(We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
define seems nice.)

So let's use that same time as timestamp in the PE/COFF header. lld-link has a
/TIMESTAMP: flag we can use to pass in an explicit timestamp.

Since tools can't have deps, we need to compute the timestamp at gn time,
so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
that just prints the timestamp we want to use, and the old write_build_date_header.py, which
now takes that timestamp and writes the header file.

Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
pass the resultl to write_build_date_header.py which keeps running as an action
during build time (so that we at least don't need to write a file at gn time).

An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
keys per binary for uploading PE binaries to the symbol server, the other being file size.
https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
But since we only upload binaries with symbols for official chrome builds to the symbol server,
a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
have the same filename, and we might rarely build canary and beta and stable all on the
same day, but them all being the same size seems highly unlikely.)

Bug:  843199 , 804926 ,330260
Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
Reviewed-on: https://chromium-review.googlesource.com/1161104
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580585}
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/base/BUILD.gn
[add] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/compute_build_timestamp.py
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/config/win/BUILD.gn
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/dotfile_settings.gni
[add] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/timestamp.gni
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/write_build_date_header.py

Cc: zturner@chromium.org
Here's an example diff:

wm_unittests.exe                                        : DIFFERENT (expected): 16 out of 12157952 bytes are different (0.00%)
  0xafce60  : 00000000020000006000000074deaf0074ceaf005253445360511be70894f8f8 '........`...t...t...RSDS`Q......'
              00000000020000006000000074deaf0074ceaf00525344534d47e4e1842ad82c '........`...t...t...RSDSMG...*.,'
                                                              ^^^^^^^   ^^^ ^^  


The RSDS means that this is probably a PDB70DebugInfo debug info header. Looks like this is written here:

http://llvm-cs.pcc.me.uk/tools/lld/COFF/Writer.cpp#1160


  // There are two important parts to the build ID.
  // 1) If building with debug info, the COFF debug directory contains a
  //    timestamp as well as a Guid and Age of the PDB.
  // 2) In all cases, the PE COFF file header also contains a timestamp.
  // For reproducibility, instead of a timestamp we want to use a hash of the
  // binary, however when building with debug info the hash needs to take into
  // account the debug info, since it's possible to add blank lines to a file
  // which causes the debug info to change but not the generated code.
  //
  // To handle this, we first set the Guid and Age in the debug directory (but
  // only if we're doing a debug build).  Then, we hash the binary (thus causing
  // the hash to change if only the debug info changes, since the Age will be
  // different).  Finally, we write that hash into the debug directory (if
  // present) as well as the COFF file header (always).
  if (Config->Debug) {
    assert(BuildId && "BuildId is not set!");
    if (PreviousBuildId.hasValue()) {
      *BuildId->BuildId = *PreviousBuildId;
      BuildId->BuildId->PDB70.Age = BuildId->BuildId->PDB70.Age + 1;
    } else {
      BuildId->BuildId->Signature.CVSignature = OMF::Signature::PDB70;
      BuildId->BuildId->PDB70.Age = 1;
      llvm::getRandomBytes(BuildId->BuildId->PDB70.Signature, 16);
    }
  }


This means that this isn't supposed to happen in debug links. The bot sets is_debug = false but symbol_level = 1, which means (on win) we tell the compiler to not generate debug info but we do pass /debug to the linker. So the age incrementing branch here will hit.

1. This probably won't happen with symbol_level = 0. We should probably set that on the bot to first get to green.

2. We need to come up with some deterministic way to set the pdb age somehow. If we just build twice, we shouldn't increment the age if nothing has changed.
Components: Build
Nice improvement!

> 1. This probably won't happen with symbol_level = 0. We should probably set that on the bot to first get to green.

AFAIK, symbol_level=0 drops information for informative stacktraces.
So I hope option 2 is taken.

Sure, but that bot exists to check determinism, not to run tests. We definitely want to get to step 2, but I think it's good to do 1 first, get the bot green to make sure things are deterministic with linker debug info disabled, and once we have that work on 2 and re-enable symbol_level=1 -- just to make sure that the RSDS thing is really the last remaining part.

Some of the executables still print 2 chunks. The one I looked at were because the RSDS uuid are 16 bytes and spanned to hexdump lines, but maybe some executables still have additional issues. It's hard to manually audit > 300 targets, so I'd like to check that the bot really turns green with symbol_level=0.
As far as I recall, symbols were disabled because we knew MSVC didn't generate deterministic PDBs. That assumption changed with llvm.
Project Member

Comment 42 by bugdroid1@chromium.org, Aug 4

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

commit 4e9c4b371d71f4b6fe73fce2272cf4e31d11e250
Author: Nico Weber <thakis@chromium.org>
Date: Sat Aug 04 22:24:48 2018

Set symbol_level=0 on Windows Deterministic.

If /debug is passed to lld, the pdb aging in PDB70DebugInfo currently
means that repeat builds get different PDB70DebugInfo headers, so for now
don't pass /debug to the linker.

Bug: 330260
Change-Id: I86e6b470ad95f3f6556c8f8f69d6d394a32661ff
Reviewed-on: https://chromium-review.googlesource.com/1162948
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580765}
[modify] https://crrev.com/4e9c4b371d71f4b6fe73fce2272cf4e31d11e250/tools/mb/mb_config.pyl

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8939019111627229280/+/steps/compare_build_artifacts/0/stdout

Equals:           534
Expected diffs:   2
Unexpected diffs: 2

That's looking pretty good. Next steps:

0. Remove all the files in "Unexpected files with no diffs" from the blacklist

1. Figure out the 2 remaining unexpected diffs:
crashpad_snapshot_test_image_reader.exe                 : DIFFERENT (unexpected): 16 out of 328192 bytes are different (0.00%)
  0x49d40   : 00000000020000007700000054ab0400549d040052534453167b7792da17edd1 '........w...T...T...RSDS.{w.....'
              00000000020000007700000054ab0400549d0400525344535f6a365e0c6806be '........w...T...T...RSDS_j6^.h..'
                                                              ^^ ^^^^^^^^^^^                            ++++ ^
  0x49d60   : 82e3cfa14c07840e01000000433a5c625c737761726d696e675c775c69725c6b '....L.......C:\b\swarming\w\ir\k'
              2cfaccb0bfae7d7401000000433a5c625c737761726d696e675c775c69725c6b ',.....}t....C:\b\swarming\w\ir\k'
                  ^^^^^^^^^^^                                                   +     ^^
crashpad_snapshot_test_image_reader_module.dll          : DIFFERENT (unexpected): 16 out of 318976 bytes are different (0.01%)
  0x48480   : 848404005253445362d433cce674a80d07e6ca536bc77b8f01000000433a5c62 '....RSDSb.3..t.....Sk.{.....C:\b'
              8484040052534453456412755d1c325d38856082411b9c6901000000433a5c62 '....RSDSEd.u].2]8.`.A..i....C:\b'
              8484040052534453456412755d1c325d38856082411b9c6901000000433a5c62 '....RSDSEd.u].2]8.`.A..i....C:\b

(...looks like RSDS stuff again? do these two binaries not honor symbol_level or something?)


2. Figure out the two binaries with expected diffs:
mini_installer.exe                                      : DIFFERENT (expected): 807 out of 180381696 bytes are different (0.00%)
previous_version_mini_installer.exe                     : DIFFERENT (expected): 646594 out of 180199936 bytes are different (0.36%)

3. Figure out what to do about pdb aging, fix that, switch symbol level back to 1
For 0, https://chromium-review.googlesource.com/c/chromium/src/+/1162948

For 1, the two targets have:

      if (symbol_level == 0) {
        # The tests that use this executable rely on at least minimal debug
        # info.
        remove_configs = [ "//build/config/compiler:default_symbols" ]
        configs = [ "//build/config/compiler:minimal_symbols" ]
      }

Since we need to get compiler:minimal_symbols working anyhow, keeping these two whitelisted until then seems fine.
Project Member

Comment 45 by bugdroid1@chromium.org, Aug 6

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

commit 2b66f649b8ebbddfacee62e78faacbd6659aafc1
Author: Nico Weber <thakis@chromium.org>
Date: Mon Aug 06 11:41:23 2018

win: Update deterministic build whitelist now that most binaries are deterministic.

(Note that they currently are only deterministic with symbol_level=0; this
will hopefully improve soon.)

Bug: 330260
Change-Id: I8b77ccc106e87057c27a198247b180601fcb447e
Reviewed-on: https://chromium-review.googlesource.com/1163173
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580847}
[modify] https://crrev.com/2b66f649b8ebbddfacee62e78faacbd6659aafc1/tools/determinism/deterministic_build_whitelist.pyl

A green build https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/9665 !

(but only with symbol_level=0 so far, and there are still 4 files suppressed as mentioned above)
Looks like symbol_level=0 reduced cycle time of the bot from 3-3.5h to 1h20m-1h40m; compare_build_artifacts now takes 4min instead of 1h40min-ish. It could probably be much more efficient for larger executables. Probably doesn't not worth it to optimize it though.
Cc: tikuta@chromium.org
Well, that didn't last long. It's red again after tikuta's https://chromium-review.googlesource.com/c/chromium/src/+/1163583 (right the next build).


Unexpected files with diffs:
  mini_installer_tests.zip
  policy_templates.zip
  remoting-me2me-host-win.zip
  remoting-webapp.v2.zip
  v8_context_snapshot.bin


v8_context_snapshot.bin                                 : DIFFERENT (unexpected): 4 out of 1527736 bytes are different (0.00%)
  0x151e00  : 06ad1706b117c0824300009ec07ca9c602f3c0000000009f9e1675020a00b111 '........C....|............u.....'
              06ad1706b117c0824300009ec07ca9ca02f3c0000000009f9e1675020a00b111 '........C....|............u.....'
                                             ^
  0x152e00  : 06211f020cf606251f06291fc0826d0000841d58e09ec0b095f502f0c0000000 '.!.....%..)...m....X............'
              06211f020cf606251f06291fc0826d0000841d58e09ec0b095f902f0c0000000 '.!.....%..)...m....X............'
                                                                 ^
  0x15cb00  : 5ac6020c2503c000000000021c16d0c10a0000000200000016e901063d22c0ce 'Z...%.......................="..'
              5aca020c2503c000000000021c16d0c10a0000000200000016e901063d22c0ce 'Z...%.......................="..'
                 ^
  0x15cb20  : 040000020c16950706fd330601349e1675029ee0c1f45ac602000000009e8402 '..........3..4..u.....Z.........'
              040000020c16950706fd330601349e1675029ee0c1f45aca02000000009e8402 '..........3..4..u.....Z.........'
                                                             ^

Is this windows-only?


I think tikuta has a CL out for remoting-webapp.v2.zip. Not sure if that covers remoting-me2me-host-win.zip too -- if not, it probably should.


mini_installer_tests.zip is probably explained by the mini_installer.exe things above and we should just whitelist it.


policy_templates.zip is probably something that needs fixing somehow.
Blockedon: 870584
Blockedon: 804926
Project Member

Comment 52 by bugdroid1@chromium.org, Aug 7

Labels: merge-merged-3515
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c234baa7e80a72e2dbd89bedd7abd047a12f68d4

commit c234baa7e80a72e2dbd89bedd7abd047a12f68d4
Author: Dirk Pranke <dpranke@chromium.org>
Date: Tue Aug 07 22:38:06 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
> 
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
> 
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
> 
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
> 
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
> 
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
> 
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
> 
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
> 
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
> 
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166005
Reviewed-by: Alex Mineer <amineer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3515@{#6}
Cr-Branched-From: 87042be6b9b11f1fe4f18fb565cadfea71f256e7-refs/heads/master@{#581084}
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/base/BUILD.gn
[delete] https://crrev.com/26c10db8bff36a8b6fc073c0f38b1e9493cabb04/build/compute_build_timestamp.py
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/config/win/BUILD.gn
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/dotfile_settings.gni
[delete] https://crrev.com/26c10db8bff36a8b6fc073c0f38b1e9493cabb04/build/timestamp.gni
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/write_build_date_header.py

Project Member

Comment 53 by bugdroid1@chromium.org, Aug 8

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

commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Aug 08 06:26:08 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
>
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
>
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
>
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
>
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
>
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
>
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
>
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
>
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
>
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
NOTRY=true

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166203
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581485}
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/base/BUILD.gn
[delete] https://crrev.com/35ec8d11347ddcdc425f63b8da279f3c3fcfaf92/build/compute_build_timestamp.py
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/config/win/BUILD.gn
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/dotfile_settings.gni
[delete] https://crrev.com/35ec8d11347ddcdc425f63b8da279f3c3fcfaf92/build/timestamp.gni
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/write_build_date_header.py

Project Member

Comment 54 by bugdroid1@chromium.org, Aug 8

Labels: merge-merged-3516
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72c03041cad06f204c6e1c09af414381e032e999

commit 72c03041cad06f204c6e1c09af414381e032e999
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Aug 08 11:56:31 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
>
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
>
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
>
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
>
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
>
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
>
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
>
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
>
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
>
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
NOTRY=true

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166203
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#581485}(cherry picked from commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80)
Reviewed-on: https://chromium-review.googlesource.com/1167162
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3516@{#3}
Cr-Branched-From: 9cc04d3302503238bd6a70ac8f0402351e1916a1-refs/heads/master@{#581409}
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/base/BUILD.gn
[delete] https://crrev.com/c10cfa8427d4c07ef786eaf2053dee663f6150c4/build/compute_build_timestamp.py
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/config/win/BUILD.gn
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/dotfile_settings.gni
[delete] https://crrev.com/c10cfa8427d4c07ef786eaf2053dee663f6150c4/build/timestamp.gni
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/write_build_date_header.py

Project Member

Comment 55 by bugdroid1@chromium.org, Aug 17

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

commit 967d1f1e6adc9b7641a5b0174c48f0883e293880
Author: Nico Weber <thakis@chromium.org>
Date: Fri Aug 17 02:42:02 2018

Reland "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80.

Reason for revert: official android should be fine with this after #583420

Original change's description:
> Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"
> 
> This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.
> 
> Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.
> 
> Original change's description:
> > win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
> >
> > We used to set the timestamp to a hash of the binary, similar to
> > https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> > However, that caused an appcompat warning on Windows 7 to appear, which
> > interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> > could help with that, but my guess it won't have an effect on Windows 7,
> > which likely always believes that the the coff timestamp field always stores
> > a timestamp).
> >
> > So currently we write the current time during linking in that field, but that's
> > bad for build determinism and that in turn is bad for swarming test result cachability.
> >
> > build/write_build_date_header.py already creates a deterministic BUILD_DATE
> > with several tradeoffs. Cachability wants this to change infrequently, but
> > things like HSTS need a "real" build date and want this to change frequently.
> > The compromise is: The date changes once per day in official builds, and
> > once a month in regular builds.
> >
> > (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> > the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> > define seems nice.)
> >
> > So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> > /TIMESTAMP: flag we can use to pass in an explicit timestamp.
> >
> > Since tools can't have deps, we need to compute the timestamp at gn time,
> > so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> > that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> > now takes that timestamp and writes the header file.
> >
> > Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> > pass the resultl to write_build_date_header.py which keeps running as an action
> > during build time (so that we at least don't need to write a file at gn time).
> >
> > An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> > keys per binary for uploading PE binaries to the symbol server, the other being file size.
> > https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> > tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> > But since we only upload binaries with symbols for official chrome builds to the symbol server,
> > a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> > have the same filename, and we might rarely build canary and beta and stable all on the
> > same day, but them all being the same size seems highly unlikely.)
> >
> > Bug:  843199 , 804926 ,330260
> > Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> > Reviewed-on: https://chromium-review.googlesource.com/1161104
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Reviewed-by: Hans Wennborg <hans@chromium.org>
> > Commit-Queue: Nico Weber <thakis@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#580585}
> 
> TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
> NOTRY=true
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  843199 ,  804926 , 330260
> Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
> Reviewed-on: https://chromium-review.googlesource.com/1166203
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581485}

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: I136e405c84eba3f61a4ac96b2017a34ade0cfba6
Reviewed-on: https://chromium-review.googlesource.com/1179281
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583945}
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/base/BUILD.gn
[add] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/compute_build_timestamp.py
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/config/win/BUILD.gn
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/dotfile_settings.gni
[add] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/timestamp.gni
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/write_build_date_header.py

The bot started printing diffs for .o files assembled by yasm, and it points out that yasm also writes a timestamp. Looks like that can be forced to a deterministic value (0) if the env var YASM_TEST_SUITE is set while yasm runs:

https://cs.chromium.org/chromium/src/third_party/yasm/source/patched-yasm/modules/objfmts/coff/coff-objfmt.c?type=cs&q=output%5C(+file:yasm&g=0&l=1266

We should probably do this.
Project Member

Comment 57 by bugdroid1@chromium.org, Aug 20

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

commit 0186fab3ff94a5c847fdab82b8eec543ccca7c6e
Author: Nico Weber <thakis@chromium.org>
Date: Mon Aug 20 15:34:47 2018

Make yasm deterministic by setting YASM_TEST_SUITE=1 while running it.

Setting YASM_TEST_SUITE makes yasm output deterministic:
- the PE/COFF timestamp field is always 0 (this breaks link.exe /incremental,
  but we no longer user link.exe)
- in debug info, yasm identifies itself as "yasm HEAD" instead of e.g.
  "yasm 1.3.0" (we don't care much about this effect)
- in debug info, file paths are no longer absolute but relative to '.'

Bug: 330260
Change-Id: Icafe7abd6a637b86af2b5b8e7f88e0bfa042da50
Reviewed-on: https://chromium-review.googlesource.com/1180718
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584456}
[modify] https://crrev.com/0186fab3ff94a5c847fdab82b8eec543ccca7c6e/third_party/yasm/run_yasm.py

Project Member

Comment 58 by bugdroid1@chromium.org, Aug 20

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

commit 5a0e78e4225cad44aaf5be3a72df2e6391d13b5c
Author: Nico Weber <thakis@chromium.org>
Date: Mon Aug 20 21:03:03 2018

win: Update list of expected differences on deterministic builder.

Bug: 330260
Change-Id: I7e2d9d6d12f28d92ef157911edab9f3a0769cae2
Reviewed-on: https://chromium-review.googlesource.com/1181871
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584557}
[modify] https://crrev.com/5a0e78e4225cad44aaf5be3a72df2e6391d13b5c/tools/determinism/deterministic_build_whitelist.pyl

Blockedon: 818241
Project Member

Comment 61 by bugdroid1@chromium.org, Sep 7

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

commit 4e2af2bd0d74732eda977df9c3c0daed169ee536
Author: Nico Weber <thakis@chromium.org>
Date: Fri Sep 07 15:17:35 2018

Update Windows Deterministic whitelist after https://chromium-review.googlesource.com/c/chromium/tools/build/+/1185911

Like https://chromium-review.googlesource.com/1194469 but for Windows.

Most of them are due to v8_context_snapshot.bin.

Bug: 330260
Change-Id: I01b2194d3a64269f6332bc36b2783fdcc5749de0
Reviewed-on: https://chromium-review.googlesource.com/1213524
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589520}
[modify] https://crrev.com/4e2af2bd0d74732eda977df9c3c0daed169ee536/tools/determinism/deterministic_build_whitelist.pyl

I've been looking at the RSDS stuff for symbol_level=1 a bit. https://reviews.llvm.org/D51887 makes it so that lld hashes the PDB contents to produce that guid -- but the PDB contains information about the environment such as cwd, so building in different build dirs will still yield different pdb guids. It's better than a random guid, but that approach basically means that executables will only be deterministic once generated pdbs are a) deterministic b) path-independent. I'm not sure how involved that will be; maybe we want an explicit /pdbguid: switch in the meantime (which could be set to the sha1 of the head git commit or something, with some tweak if there are uncommitted changes)
Blockedon: 882997
Blockedon: 880827
Blockedon: 893935
If https://chromium-review.googlesource.com/c/chromium/src/+/1273475 and https://chromium-review.googlesource.com/c/chromium/src/+/1273479 land, we can try making the Windows deterministic bot use symbol_level=1 again and see what happens.
Project Member

Comment 67 by bugdroid1@chromium.org, Oct 11

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

commit 40fd334b2612873423051c30d6d2dd91d6081d34
Author: Nico Weber <thakis@chromium.org>
Date: Thu Oct 11 21:14:46 2018

win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG.

The linker by defaults writes the absolute path to the corresponding pdb
into executables it creates (if /DEBUG is passed).

/pdbaltpath:%_PDB% tells it to instead just write the basename of the pdb
into the executable, which makes the build more reproducible:

1. Different build directories no longer cause the part of the exe that contains
   the pdb path to be different.

2. More subtly, the pdb file contains offsets into the executable, and if the
   pdb path has different lengths on different systems, the pdb file will be
   different due to the absolute pdb path in the executable.  lld-link sets the
   UUID of the pdb to the hash of the pdb file contents, and the UUID of the
   pdb is also stored in the executable.  So this is also one of the parts
   needed to make the pdb output deterministic.

(Note that while link.exe has supported /pdbaltpath:%_PDB% for a long time,
lld-link learned about %_PDB% only very recently, and this CL depends on the
clang roll https://chromium-review.googlesource.com/c/1271718.)

Bug: 330260
Change-Id: If4c505ababa46ed4f51330521ff09f12f6840a47
Reviewed-on: https://chromium-review.googlesource.com/c/1273475
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598940}
[modify] https://crrev.com/40fd334b2612873423051c30d6d2dd91d6081d34/build/config/compiler/BUILD.gn
[modify] https://crrev.com/40fd334b2612873423051c30d6d2dd91d6081d34/build/config/win/BUILD.gn

Project Member

Comment 68 by bugdroid1@chromium.org, Oct 12

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

commit f8ebcecf7a1b5a9d628ed8dd81727da65fff7ecc
Author: Nico Weber <thakis@chromium.org>
Date: Fri Oct 12 01:20:16 2018

Revert "Set symbol_level=0 on Windows Deterministic."

This reverts commit 4e9c4b371d71f4b6fe73fce2272cf4e31d11e250.

Reason for revert: This might no longer be needed; revert to check.

(I thought the bot built in two different build dirs, and for that
to work we need to land https://chromium-review.googlesource.com/c/chromium/src/+/1273479
first, but it builds in one dir, moves that elsewhere, and builds
in that dir again. We should probably make it build in two different
dirs once that other CL is in, to make sure that that keeps working.)

Original change's description:
> Set symbol_level=0 on Windows Deterministic.
> 
> If /debug is passed to lld, the pdb aging in PDB70DebugInfo currently
> means that repeat builds get different PDB70DebugInfo headers, so for now
> don't pass /debug to the linker.
> 
> Bug: 330260
> Change-Id: I86e6b470ad95f3f6556c8f8f69d6d394a32661ff
> Reviewed-on: https://chromium-review.googlesource.com/1162948
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580765}

TBR=thakis@chromium.org,dpranke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 330260
Change-Id: I8637e6c089464eddc7d9cfc35f6fa28b0cd54987
Reviewed-on: https://chromium-review.googlesource.com/c/1277486
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599056}
[modify] https://crrev.com/f8ebcecf7a1b5a9d628ed8dd81727da65fff7ecc/tools/mb/mb_config.pyl

Project Member

Comment 69 by bugdroid1@chromium.org, Oct 12

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

commit f097036d16ea6c259ef01ddd6967895ca5a28d04
Author: Nico Weber <thakis@chromium.org>
Date: Fri Oct 12 11:47:02 2018

win: Update deterministic build whitelist.

symbol_level=1 builds seem to work now and the bot builds with it, so the
entries for crashpad_snapshot_test_image_reader and
crashpad_snapshot_test_image_reader_module.dll are no longer needed.

mini_installer.exe now has a corresponding .pdb that's also non-deterministic
(since the exe itself isn't, the pdb can't be), so add an entry for that.

TBR=erikchen

Bug: 330260
Change-Id: Ic1cde1f5a52a953d74285e4bd6a02e74c70e75b5
Reviewed-on: https://chromium-review.googlesource.com/c/1278306
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599174}
[modify] https://crrev.com/f097036d16ea6c259ef01ddd6967895ca5a28d04/tools/determinism/deterministic_build_whitelist.pyl

Blockedon: 429358
The bot is now green with symbol_level=1: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic (!)

v8_context_snapshot.bin causes many test isolates to be different, so focusing on  issue 870584  is next.
Blockedon: 894725
Project Member

Comment 73 by bugdroid1@chromium.org, Oct 12

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

commit d5b9c84416e819072903816682d08bdce9889b4b
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Oct 12 23:05:51 2018

Revert "win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG."

This reverts commit 40fd334b2612873423051c30d6d2dd91d6081d34.

Reason for revert: It breaks the "official symbols" upload step, see
crbug.com/894991

Original change's description:
> win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG.
> 
> The linker by defaults writes the absolute path to the corresponding pdb
> into executables it creates (if /DEBUG is passed).
> 
> /pdbaltpath:%_PDB% tells it to instead just write the basename of the pdb
> into the executable, which makes the build more reproducible:
> 
> 1. Different build directories no longer cause the part of the exe that contains
>    the pdb path to be different.
> 
> 2. More subtly, the pdb file contains offsets into the executable, and if the
>    pdb path has different lengths on different systems, the pdb file will be
>    different due to the absolute pdb path in the executable.  lld-link sets the
>    UUID of the pdb to the hash of the pdb file contents, and the UUID of the
>    pdb is also stored in the executable.  So this is also one of the parts
>    needed to make the pdb output deterministic.
> 
> (Note that while link.exe has supported /pdbaltpath:%_PDB% for a long time,
> lld-link learned about %_PDB% only very recently, and this CL depends on the
> clang roll https://chromium-review.googlesource.com/c/1271718.)
> 
> Bug: 330260
> Change-Id: If4c505ababa46ed4f51330521ff09f12f6840a47
> Reviewed-on: https://chromium-review.googlesource.com/c/1273475
> Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598940}

TBR=thakis@chromium.org,brucedawson@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 330260
Change-Id: Iecb3de9f19f9d4e3932784ba9e68b00001bb8986
Reviewed-on: https://chromium-review.googlesource.com/c/1279225
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: S├ębastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599398}
[modify] https://crrev.com/d5b9c84416e819072903816682d08bdce9889b4b/build/config/compiler/BUILD.gn
[modify] https://crrev.com/d5b9c84416e819072903816682d08bdce9889b4b/build/config/win/BUILD.gn

Project Member

Comment 74 by bugdroid1@chromium.org, Oct 13

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

commit e4233ebfca21e9747e5a439b1be478d5a5be3a27
Author: Nico Weber <thakis@chromium.org>
Date: Sat Oct 13 00:02:45 2018

Reland "win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG."

This reverts commit d5b9c84416e819072903816682d08bdce9889b4b.

Reason for revert: The problem this was reverted for had been fixed
hours before the revert.

Original change's description:
> Revert "win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG."
> 
> This reverts commit 40fd334b2612873423051c30d6d2dd91d6081d34.
> 
> Reason for revert: It breaks the "official symbols" upload step, see
> crbug.com/894991
> 
> Original change's description:
> > win: Pass /pdbaltpath:%_PDB% to the linker every time we pass /DEBUG.
> > 
> > The linker by defaults writes the absolute path to the corresponding pdb
> > into executables it creates (if /DEBUG is passed).
> > 
> > /pdbaltpath:%_PDB% tells it to instead just write the basename of the pdb
> > into the executable, which makes the build more reproducible:
> > 
> > 1. Different build directories no longer cause the part of the exe that contains
> >    the pdb path to be different.
> > 
> > 2. More subtly, the pdb file contains offsets into the executable, and if the
> >    pdb path has different lengths on different systems, the pdb file will be
> >    different due to the absolute pdb path in the executable.  lld-link sets the
> >    UUID of the pdb to the hash of the pdb file contents, and the UUID of the
> >    pdb is also stored in the executable.  So this is also one of the parts
> >    needed to make the pdb output deterministic.
> > 
> > (Note that while link.exe has supported /pdbaltpath:%_PDB% for a long time,
> > lld-link learned about %_PDB% only very recently, and this CL depends on the
> > clang roll https://chromium-review.googlesource.com/c/1271718.)
> > 
> > Bug: 330260
> > Change-Id: If4c505ababa46ed4f51330521ff09f12f6840a47
> > Reviewed-on: https://chromium-review.googlesource.com/c/1273475
> > Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
> > Commit-Queue: Nico Weber <thakis@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#598940}
> 
> TBR=thakis@chromium.org,brucedawson@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 330260
> Change-Id: Iecb3de9f19f9d4e3932784ba9e68b00001bb8986
> Reviewed-on: https://chromium-review.googlesource.com/c/1279225
> Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: S├ębastien Marchand <sebmarchand@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599398}

TBR=thakis@chromium.org,brucedawson@chromium.org,sebmarchand@chromium.org

Change-Id: I663fad392bee488d50800eba94e17f6c2cc5e228
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 330260
Reviewed-on: https://chromium-review.googlesource.com/c/1279366
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599428}
[modify] https://crrev.com/e4233ebfca21e9747e5a439b1be478d5a5be3a27/build/config/compiler/BUILD.gn
[modify] https://crrev.com/e4233ebfca21e9747e5a439b1be478d5a5be3a27/build/config/win/BUILD.gn

Project Member

Comment 75 by bugdroid1@chromium.org, Oct 17

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

commit 94c429b15406d3fcad5b3904f4c14f2e6d38aafa
Author: Nico Weber <thakis@chromium.org>
Date: Wed Oct 17 23:38:21 2018

win: Pass a fake fixed /pdbsourcepath: for (non-sanitizer) symbol_level=1 builds.

/pdbsourcepath: before this change here was used to tell lld-link about the
build directory. The setup was that the compiler can produce debug info
with relative paths in its output (to make them build-dir independent, which
helps goma cache obj files), and then at link time those relative paths were
made absolute.

In symbol_level=1 builds, the obj files don't contain any debug info, the
linker just adds enough debug info at link time to get symbolized stacks
(without line numbers). So we don't need to pass a real build directory in that
case and can pass a fake fixed path. (sanitizer builds do pass
-gline-tables-only and hence have some debug info in the obj files even with
symbol_level=1, do don't do this there).

After http://reviews.llvm.org/rL344061, /pdbsourcepath: is also used as the
base path in all other places that contain absolute paths:

- The "cwd" stored in the env block in the pdb is /pdbsourcepath: if present
- The "exe" stored in the env block in the pdb is made absolute relative to
  /pdbsourcepath: instead of the cwd
- The "pdb" stored in the env block in the pdb is made absolute relative to
  /pdbsourcepath: instead of the cwd
- For making absolute paths to .obj files referenced from the pdb

This makes PDBs independent of the build dir in symbol_level=1 builds, and since
the hash of the PDB contents are copied as PDB UUID into the executable,
it makes executables reproducibe in symbol_level=1 builds too.

Bug: 330260
Change-Id: Iaf0bd9f8259b3c97b5c6cff497d31d6043faa807
Reviewed-on: https://chromium-review.googlesource.com/c/1273479
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600599}
[modify] https://crrev.com/94c429b15406d3fcad5b3904f4c14f2e6d38aafa/build/config/compiler/BUILD.gn

Blockedon: 897854
Blockedon: 897980
Project Member

Comment 78 by bugdroid1@chromium.org, Oct 24

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

commit 538e9529538ef3f9b73136fb34c87308aa91b76a
Author: Nico Weber <thakis@chromium.org>
Date: Wed Oct 24 01:02:54 2018

Move 'Windows deterministic' from FYI to the main waterfall.

Bug:  897980 ,330260
Change-Id: I71bb304c6398484d5af5a981fdbc09314d69dd22
Reviewed-on: https://chromium-review.googlesource.com/c/1295272
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602198}
[modify] https://crrev.com/538e9529538ef3f9b73136fb34c87308aa91b76a/infra/config/global/luci-milo.cfg
[modify] https://crrev.com/538e9529538ef3f9b73136fb34c87308aa91b76a/tools/mb/mb_config.pyl

Project Member

Comment 79 by bugdroid1@chromium.org, Oct 24

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

commit 538e9529538ef3f9b73136fb34c87308aa91b76a
Author: Nico Weber <thakis@chromium.org>
Date: Wed Oct 24 01:02:54 2018

Move 'Windows deterministic' from FYI to the main waterfall.

Bug:  897980 ,330260
Change-Id: I71bb304c6398484d5af5a981fdbc09314d69dd22
Reviewed-on: https://chromium-review.googlesource.com/c/1295272
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602198}
[modify] https://crrev.com/538e9529538ef3f9b73136fb34c87308aa91b76a/infra/config/global/luci-milo.cfg
[modify] https://crrev.com/538e9529538ef3f9b73136fb34c87308aa91b76a/tools/mb/mb_config.pyl

Project Member

Comment 80 by bugdroid1@chromium.org, Oct 24

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

commit c1917dd47a5fed7edf77bfa80ed3e3dccf17b693
Author: Marc Treib <treib@chromium.org>
Date: Wed Oct 24 11:35:53 2018

Revert "Move 'Windows deterministic' from FYI to the main waterfall."

This reverts commit 538e9529538ef3f9b73136fb34c87308aa91b76a.

Reason for revert: "Windows deterministic" has been consistently failing on the main waterfall:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic

First failing build:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Windows%20deterministic/10487

Error message from the lookup_GN_args step looks like this:
Traceback (most recent call last):
  File "C:\b\swarming\w\ir\kitchen-workdir\src\tools\mb\mb.py", line 63, in Main
    ret = self.args.func()
  File "C:\b\swarming\w\ir\kitchen-workdir\src\tools\mb\mb.py", line 332, in CmdLookup
    vals = self.Lookup()
  File "C:\b\swarming\w\ir\kitchen-workdir\src\tools\mb\mb.py", line 614, in Lookup
    config = self.ConfigFromArgs()
  File "C:\b\swarming\w\ir\kitchen-workdir\src\tools\mb\mb.py", line 696, in ConfigFromArgs
    (self.args.builder, self.args.master, self.args.config_file))
MBErr: Builder name "Windows deterministic"  not found under masters[chromium.fyi] in "C:\b\swarming\w\ir\kitchen-workdir\src\tools\mb\mb_config.pyl"


Original change's description:
> Move 'Windows deterministic' from FYI to the main waterfall.
> 
> Bug:  897980 ,330260
> Change-Id: I71bb304c6398484d5af5a981fdbc09314d69dd22
> Reviewed-on: https://chromium-review.googlesource.com/c/1295272
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602198}

TBR=thakis@chromium.org,dpranke@chromium.org,martiniss@chromium.org

Change-Id: Ib9aff7c076f1f0e3192788f10da80036b4262ed1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  897980 , 330260
Reviewed-on: https://chromium-review.googlesource.com/c/1297139
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602297}
[modify] https://crrev.com/c1917dd47a5fed7edf77bfa80ed3e3dccf17b693/infra/config/global/luci-milo.cfg
[modify] https://crrev.com/c1917dd47a5fed7edf77bfa80ed3e3dccf17b693/tools/mb/mb_config.pyl

Project Member

Comment 81 by bugdroid1@chromium.org, Oct 24

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

commit 702055f4e67df682129a726ec29291ecbd24c4c3
Author: Nico Weber <thakis@chromium.org>
Date: Wed Oct 24 11:50:22 2018

Reland "Move 'Windows deterministic' from FYI to the main waterfall."

This is a reland of 538e9529538ef3f9b73136fb34c87308aa91b76a

It was reverted at https://chromium-review.googlesource.com/c/1297139
but the problem has already been fixed in
https://chromium-review.googlesource.com/c/1297262.

Original change's description:
> Move 'Windows deterministic' from FYI to the main waterfall.
>
> Bug:  897980 ,330260
> Change-Id: I71bb304c6398484d5af5a981fdbc09314d69dd22
> Reviewed-on: https://chromium-review.googlesource.com/c/1295272
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602198}

TBR=thakis@chromium.org,dpranke@chromium.org,martiniss@chromium.org

No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Bug:  897980 , 330260
Change-Id: I03a1784ea48bcaa6567d4ec2dd482d218d0f4740
Reviewed-on: https://chromium-review.googlesource.com/c/1297426
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602303}
[modify] https://crrev.com/702055f4e67df682129a726ec29291ecbd24c4c3/infra/config/global/luci-milo.cfg
[modify] https://crrev.com/702055f4e67df682129a726ec29291ecbd24c4c3/tools/mb/mb_config.pyl

Project Member

Comment 82 by bugdroid1@chromium.org, Oct 26

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

commit e2a95639125afd08739a36d0e7f367f7b8abb952
Author: Nico Weber <thakis@chromium.org>
Date: Fri Oct 26 19:32:31 2018

win: Make chrome.7z generation more deterministic.

mini_installer.exe embeds chrome.7z, and chrome.7z contains timestamps
of all files in the archive. To make chrome.7z more deterministic, give
all files in it deterministic timestamps.

Bug: 330260
Change-Id: Ia251dd38177acbebdde288b86826986f441b2d4d
Reviewed-on: https://chromium-review.googlesource.com/c/1302393
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603171}
[modify] https://crrev.com/e2a95639125afd08739a36d0e7f367f7b8abb952/build/write_build_date_header.py
[modify] https://crrev.com/e2a95639125afd08739a36d0e7f367f7b8abb952/chrome/installer/mini_installer/BUILD.gn
[modify] https://crrev.com/e2a95639125afd08739a36d0e7f367f7b8abb952/chrome/tools/build/win/create_installer_archive.py

mini_installer.exe includes chrome.7z, which basically includes all built binaries (except tests).

With local workarounds for issue 899442 (just don't build the 3 affected files on goma), issue 899437 (manually edit the map cc file to contain something incorrect but path-independent) and https://chromium-review.googlesource.com/c/chromium/src/+/1303887 (which currently only works in 64-bit builds), almost all of the inputs are deterministic:

$ for f in $(find out/gn2/gen/chrome/installer/mini_installer/mini_installer/temp_installer_archive/ -type f); do cmp $f ${f/gn2/gn}; done                      out/gn2/gen/chrome/installer/mini_installer/mini_installer/temp_installer_archive/Chrome-bin/72.0.3593.0/nacl_irt_x86_64.nexe out/gn/gen/chrome/installer/mini_installer/mini_installer/temp_installer_archive/Chrome-bin/72.0.3593.0/nacl_irt_x86_64.nexe differ: char 417, line 1
out/gn2/gen/chrome/installer/mini_installer/mini_installer/temp_installer_archive/Chrome-bin/72.0.3593.0/v8.dll out/gn/gen/chrome/installer/mini_installer/mini_installer/temp_installer_archive/Chrome-bin/72.0.3593.0/v8.dll differ: char 17917089, line 26404

Project Member

Comment 84 by bugdroid1@chromium.org, Oct 30

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

commit de2f0bfe2c61fe2a823640d2f5ffcada755cac59
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 30 10:21:53 2018

win: Add hacky ml.exe wrapper that makes ml's output deterministic after the fact.

Without this, chrome.exe.pdb contains two absolute paths (to the two obj files
built by ml64 for crashpad).

Bug: 762167,330260,899438
Change-Id: If4e0195b51f7d9ff70d890fe3a5302f5961c94d6
Reviewed-on: https://chromium-review.googlesource.com/c/1303887
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603852}
[modify] https://crrev.com/de2f0bfe2c61fe2a823640d2f5ffcada755cac59/build/toolchain/win/BUILD.gn
[add] https://crrev.com/de2f0bfe2c61fe2a823640d2f5ffcada755cac59/build/toolchain/win/ml.py

compare_build_artifacts.py hacked up to print a report for v8.dll:

C:\src\chrome\src>python tools\determinism\compare_build_artifacts.py -f out\gn -s out\gn2 -t win
Epoch: 0e 48 d8 5b
v8.dll: DIFFERENT (unexpected): 23 out of 26576384 bytes are different (0.00%)
  0x11164a0 : 7f31e06320a6aa630000000090050000a0050000780100002007000078010000 '.1.c ..c............x... ...x...'
              3fb1dbde45a3c7bf0000000090050000a0050000780100002007000078010000 '?...E...............x... ...x...'
              +++++++++++++ +                                                   ^  ^^^^^
  0x140dfc0 : 45b04803c3488b55f04823d0660f28c1488bc2e9b1fdffff488bc14883e00348 'E.H..H.U.H#.f.(.H.......H..H...H'
              45b04803c3488b55f04823d0488bc2660f28c1e9b1fdffff488bc14883e00348 'E.H..H.U.H#.H..f.(......H..H...H'
                                      ++++++                                                +++
  0x1715ec0 : d092b95b000000000200000023000000d8687101d85e710152534453ec652843 '...[........#....hq..^q.RSDS.e(C'
              d092b95b000000000200000023000000d8687101d85e710152534453432ed310 '...[........#....hq..^q.RSDSC...'
                                                                        ++++++                              + ^^
  0x1715ee0 : 48733c574c4c44205044422e0100000076382e646c6c2e706462000000000000 'Hs<WLLD PDB.....v8.dll.pdb......'
              5d4256f34c4c44205044422e0100000076382e646c6c2e706462000000000000 ']BV.LLD PDB.....v8.dll.pdb......'
              ++ ^^^^                                                           ^^^^
Equals:           0
Expected diffs:   0
Unexpected diffs: 1
Unexpected files with diffs:

  v8.dll
Checking v8.dll difference: (1040 deps)
  obj/v8/v8_external_snapshot/embedded.obj                                    : 19 out of 5107415 bytes are different (0.00%)
  0x120     : 0000000000000000000810007f31e06320a6aa630000000090050000a0050000 '.............1.c ..c............'
              0000000000000000000810003fb1dbde45a3c7bf0000000090050000a0050000 '............?...E...............'
                                      +++++++++++++ +                                       ^  ^^^^^
  0x2f7c40  : 0000488b45a8488d5801488b45b04803c3488b55f04823d0660f28c1488bc2e9 '..H.E.H.X.H.E.H..H.U.H#.f.(.H...'
              0000488b45a8488d5801488b45b04803c3488b55f04823d0488bc2660f28c1e9 '..H.E.H.X.H.E.H..H.U.H#.H..f.(..'
                                                              ++++++                                    +++
  0x4cc800  : 0000000000fba677da0100000000002e64617461000000000000000200000003 '.......w........data............'
              0000000000838262320100000000002e64617461000000000000000200000003 '.......b2.......data............'
                        ^^^^ ^^^                                                       ^^
Blockedon: v8:8391
This work has broken Windows tests. In particular, now that multiple builds of different versions use the same timestamps and coincidentally have the same file size, caching in the Windows loader causes mini_installer tests to fail. See  issue 900121  for the gory details. Can you roll back r603171?
> Can you roll back r603171?

Nico is currently out of office. If tests are broken, just hit the revert button.
I commented on https://bugs.chromium.org/p/chromium/issues/detail?id=900121#c7 , I think your analysis is at least incomplete.
Project Member

Comment 90 by bugdroid1@chromium.org, Nov 5

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

commit 05eb6e237704dc0e3fcdaac39ac35edb71c0cb58
Author: Greg Thompson <grt@chromium.org>
Date: Mon Nov 05 12:14:55 2018

Revert "win: Make chrome.7z generation more deterministic."

This reverts commit e2a95639125afd08739a36d0e7f367f7b8abb952.

Reason for revert: This breaks tests on account of fooling the Windows loader. See https://bugs.chromium.org/p/chromium/issues/detail?id=900121#c6 for details.

Original change's description:
> win: Make chrome.7z generation more deterministic.
> 
> mini_installer.exe embeds chrome.7z, and chrome.7z contains timestamps
> of all files in the archive. To make chrome.7z more deterministic, give
> all files in it deterministic timestamps.
> 
> Bug: 330260
> Change-Id: Ia251dd38177acbebdde288b86826986f441b2d4d
> Reviewed-on: https://chromium-review.googlesource.com/c/1302393
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#603171}

TBR=gab@chromium.org,thakis@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 330260
Change-Id: I261401c27226ab9e0cf30bc07a47ecf8399f491b
Reviewed-on: https://chromium-review.googlesource.com/c/1317569
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605299}
[modify] https://crrev.com/05eb6e237704dc0e3fcdaac39ac35edb71c0cb58/build/write_build_date_header.py
[modify] https://crrev.com/05eb6e237704dc0e3fcdaac39ac35edb71c0cb58/chrome/installer/mini_installer/BUILD.gn
[modify] https://crrev.com/05eb6e237704dc0e3fcdaac39ac35edb71c0cb58/chrome/tools/build/win/create_installer_archive.py

Sign in to add a comment