Closed Bug 1251936 Opened 9 years ago Closed 8 years ago

Stand up Windows x86 debug static analysis builds

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(firefox53 fixed)

RESOLVED FIXED
Tracking Status
firefox53 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

Attachments

(2 files, 8 obsolete files)

3.25 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
2.94 KB, patch
froydnj
: review+
Details | Diff | Splinter Review
I did this in bug 1015018 for Mac. Time to do it for Windows!
Jordan, please note that these are mostly untested stuff I copied based on the existing configs, since as far as I remember there is no way to test them on try without committing the buildbot-configs and ash-mozharness changes. Once we do that and the builds turn green on try, I can move them to the default set for m-c.
Comment on attachment 8724548 [details] [diff] [review] Part 3: Add buildbot configs for Windows x86 static analysis buillds Review of attachment 8724548 [details] [diff] [review]: ----------------------------------------------------------------- so while this patch will do exactly what you need and it lgtm, we can add an intermediate step in to test this only on try without risk closing inbound/central. to add this to try and have it run only when you explicitly call it in try commit msg, could you take this example and apply it your win32-st-an-debug needs before you land this patch: https://hg.mozilla.org/build/buildbot-configs/diff/34925337358e/mozilla/config.py I throw in tips on how to do that in in-line comments below ::: mozilla/config.py @@ +71,5 @@ > 'linux64-av': {}, > 'macosx64-debug': {}, > 'macosx64-st-an-debug': {}, > 'win32-debug': {}, > + 'win32-st-an-debug': {}, remove this line (as it adds it across all branches by default) @@ +1513,5 @@ > 'dep_signing_servers': 'dep-signing', > 'tooltool_manifest_src': 'browser/config/tooltool-manifests/win32/releng.manifest', > 'tooltool_script': ['python', '/c/mozilla-build/tooltool.py'], > }, > + 'win32-st-an-debug': { keep this @@ +2889,5 @@ > BRANCHES['try']['platforms']['linux64-sh-haz']['slaves'] = TRY_SLAVES['mock'] > BRANCHES['try']['platforms']['linux64-br-haz']['slaves'] = TRY_SLAVES['mock'] > BRANCHES['try']['platforms']['linux64-cc']['slaves'] = TRY_SLAVES['mock'] > BRANCHES['try']['platforms']['win32-debug']['slaves'] = TRY_SLAVES['win64-rev2'] > +BRANCHES['try']['platforms']['win32-st-an-debug']['slaves'] = TRY_SLAVES['win64-rev2'] keep this @@ +3013,5 @@ > del branch['platforms']['linux64-st-an-debug'] > if 'macosx64-st-an-debug' in branch['platforms']: > del branch['platforms']['macosx64-st-an-debug'] > + if 'win32-st-an-debug' in branch['platforms']: > + del branch['platforms']['win32-st-an-debug'] remove this for now. add: https://hg.mozilla.org/build/buildbot-configs/diff/34925337358e/mozilla/config.py#l1.129
Attachment #8724548 - Flags: review?(jlund)
Comment on attachment 8724546 [details] [diff] [review] Part 1: Add a mozconfig for Windows x86 static analysis builds Review of attachment 8724546 [details] [diff] [review]: ----------------------------------------------------------------- I'm probably not the best to review the mozconfig. if it works for you, it works for me but if you want reassurance, I'd r? someone from #build
Attachment #8724546 - Flags: review?(jlund) → review+
Comment on attachment 8724547 [details] [diff] [review] Part 2: Add a mozharness config script for 32-bit Windows static analysis builds Review of attachment 8724547 [details] [diff] [review]: ----------------------------------------------------------------- patch looks good but mozharness lives in tree now: http://hg.mozilla.org/mozilla-central/file/9da51cb4974e/testing/mozharness/README.txt this repo isn't used anywhere
Attachment #8724547 - Flags: review?(jlund)
(In reply to Jordan Lund (:jlund) from comment #7) > Comment on attachment 8724547 [details] [diff] [review] > Part 2: Add a mozharness config script for 32-bit Windows static analysis > builds > > Review of attachment 8724547 [details] [diff] [review]: > ----------------------------------------------------------------- > > patch looks good but mozharness lives in tree now: > http://hg.mozilla.org/mozilla-central/file/9da51cb4974e/testing/mozharness/ > README.txt > > this repo isn't used anywhere Oh fun! Thanks, I'll rework this patch in-tree and will address your comments on the other patch.
Attachment #8724546 - Flags: review?(ted)
Attachment #8724546 - Flags: review?(ted) → review+
Attachment #8724548 - Attachment is obsolete: true
Attachment #8724547 - Attachment is obsolete: true
Comment on attachment 8726794 [details] [diff] [review] Part 3: Add buildbot configs for Windows x86 static analysis buillds Review of attachment 8726794 [details] [diff] [review]: ----------------------------------------------------------------- lgtm with one line addition. see comment below ::: mozilla/config.py @@ +1512,5 @@ > 'dep_signing_servers': 'dep-signing', > 'tooltool_manifest_src': 'browser/config/tooltool-manifests/win32/releng.manifest', > 'tooltool_script': ['python', '/c/mozilla-build/tooltool.py'], > }, > + 'win32-st-an-debug': { I forgot, can you add the following to this dict: 'try_by_default': False,
Attachment #8726794 - Flags: review?(jlund) → review+
Comment on attachment 8726796 [details] [diff] [review] Part 2: Add a mozharness config script for 32-bit Windows static analysis builds Review of attachment 8726796 [details] [diff] [review]: ----------------------------------------------------------------- looks sane to me :)
Attachment #8726796 - Flags: review?(jlund) → review+
Thanks for the reviews! Should I wait for a reconfig after landing part 3?
Flags: needinfo?(jlund)
Keywords: leave-open
(In reply to :Ehsan Akhgari from comment #13) > Thanks for the reviews! > > Should I wait for a reconfig after landing part 3? yep. you'll need to wait till this is on production (usually happens once every two days): > https://hg.mozilla.org/build/buildbot-configs/rev/ae056e1b3ad7 it will take at most 1.5h for buildbot masters to reconfig after it merges to prod. after that, you are good to try on try
Flags: needinfo?(jlund)
(In reply to Jordan Lund (:jlund) from comment #16) > (In reply to :Ehsan Akhgari from comment #13) > > Thanks for the reviews! > > > > Should I wait for a reconfig after landing part 3? > I triggered a reconfig and thought about this. you can try this on try from now on wards: http://hg.mozilla.org/build/buildbot-configs/rev/82435f4d7281
Thanks, Jordan! I tried pushing this to try: <https://treeherder.mozilla.org/#/jobs?repo=try&revision=1de8fd0f726f>. The build fails with: 13:29:08 INFO - Using in-tree mozconfig 13:29:08 INFO - Reading from file c:\builds\moz2_slave\try-w32-st-an-d-00000000000000\build\src\browser/config/mozconfigs/win32/debug-static-analysis 13:29:08 FATAL - unable to open c:\builds\moz2_slave\try-w32-st-an-d-00000000000000\build\src\browser/config/mozconfigs/win32/debug-static-analysis: No such file or directory It looks like this error is coming from <https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py#1107>, but I'm not sure why. This is from a normal successful Windows x86 debug build: 13:33:55 INFO - Using in-tree mozconfig 13:33:55 INFO - Reading from file c:\builds\moz2_slave\try-w32-d-00000000000000000000\build\src\browser/config/mozconfigs/win32/debug 13:33:56 INFO - Contents: ... There is no difference that I can think of which might be causing this. Can you think of what's happening here?
Flags: needinfo?(jlund)
Note that I pushed another experiment <https://treeherder.mozilla.org/#/jobs?repo=try&revision=914b8ecbe7a2> where I changed "debug-static-analysis" to "debug" in the name of the mozconfig, and the same error occurred. It seems like c:\builds\moz2_slave\try-w32-st-an-d-00000000000000\build\src either doesn't exist or doesn't point to a valid source directory. This might be caused by the "checkout-sources" step missing from testing/mozharness/configs/builds/releng_sub_windows_configs/32_stat_and_debug.py. Pushing another try job to see if that's the problem: <https://treeherder.mozilla.org/#/jobs?repo=try&revision=11c78ab80409>
Looks like that was the issue!
Flags: needinfo?(jlund)
yeah sorry, 'checkout-sources' clones gecko. I missed that in the review
Depends on: 1254807
Comment on attachment 8728162 [details] [diff] [review] Part 4: Include checkout-sources in the mozharness config for Win32 static analysis builds Review of attachment 8728162 [details] [diff] [review]: ----------------------------------------------------------------- sorry for the delay. was dealing with bustage all day yesterday.
Attachment #8728162 - Flags: review?(jlund) → review+
Comment on attachment 8728163 [details] [diff] [review] Part 5: Include the rustc toolchain in the tooltool manifest for Win32 static analysis builds Review of attachment 8728163 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #8728163 - Flags: review?(jlund) → review+
No worries, thanks for the reviews so far!
I don't see this on treeherder, is it disabled/hidden or I missed anything?
(In reply to Ting-Yu Chou [:ting] from comment #31) > I don't see this on treeherder, is it disabled/hidden or I missed anything? I believe this is only available on try at the moment.
I used "try: -b do -p win32-st-an -u none -t none" to run the task [1], and the results are all (3/3) busted: 01:00:55 INFO - Calling ['c:\\mozilla-build\\python27\\python.exe', 'mach', '--log-no-times', 'build', '-v'] with output_timeout 4800 01:00:56 INFO - Error loading mozconfig: c:/builds/moz2_slave/try-w32-st-an-d-00000000000000/build/src/.mozconfig 01:00:56 INFO - Evaluation of your mozconfig exited with an error. This could be triggered 01:00:56 INFO - by a command inside your mozconfig failing. Please change your mozconfig 01:00:56 INFO - to not error and/or to catch errors in executed commands. 01:00:56 INFO - mozconfig output: 01:00:56 INFO - ------BEGIN_MK_OPTION 01:00:56 INFO - AUTOCLOBBER=1 ... 01:00:56 INFO - --enable-clang-plugin 01:00:56 INFO - ------END_AC_OPTION 01:00:56 INFO - c:/builds/moz2_slave/try-w32-st-an-d-00000000000000/build/src/build/win32/mozconfig.vs2015-win64: line 6: cd: /c/builds/moz2_slave/try-w32-st-an-d-00000000000000/build/src/vs2015u3: No such file or directory 01:00:56 ERROR - Return code: 1 01:00:56 WARNING - setting return code to 2 01:00:56 FATAL - 'mach build' did not run successfully. Please check log for errors. 01:00:56 FATAL - Running post_fatal callback... 01:00:56 FATAL - Exiting -1 [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddb75313fff8
It seems that browser/config/tooltool-manifests/win32/clang.manifest needs to include vs2015u3. Try again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bfe72e4a66f
(In reply to Ting-Yu Chou [:ting] from comment #34) > It seems that browser/config/tooltool-manifests/win32/clang.manifest needs > to include vs2015u3. Try again: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bfe72e4a66f Added cargo to the manifest as well, but fails building clang-plugin.dll with unresolved external symbols, I guess it's because of this: 18:26:22 INFO - clangASTMatchers.lib(ASTMatchersInternal.cpp.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in clang-plugin.obj 18:26:22 INFO - clangASTMatchers.lib(ASTMatchFinder.cpp.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in clang-plugin.obj [1] https://treeherder.mozilla.org/logviewer.html#?job_id=30048095&repo=try#L5242-L5487
We need a newer clang build on x86 for vs2015u3, specifically r270458 needs to be included. I assume bug 1306650 patch part 13 [1] would get us a new build on https://api.pub.build.mozilla.org/tooltool/, which later I can update browser/config/tooltool-manifests/win32/clang.manifest for the newer version. [1] https://hg.mozilla.org/mozilla-central/rev/402494fd3465
(In reply to Ting-Yu Chou [:ting] from comment #36) > to be included. I assume bug 1306650 patch part 13 [1] would get us a new > build on https://api.pub.build.mozilla.org/tooltool/, which later I can Unfortunately this does not happen and I don't have the token tooltool.upload.public in https://api.pub.build.mozilla.org for uploading.
How could I get the permission of tooltool.upload.public in https://api.pub.build.mozilla.org? If I can't, should I follow bug 1042132 comment 1 for uploading?
Flags: needinfo?(catlee)
Ehsan / Nathan - can you vouch that Ting-Yu should have tooltool upload permissions?
Flags: needinfo?(nfroyd)
Flags: needinfo?(ehsan)
Flags: needinfo?(catlee)
I vouch for Ting-Yu. (Ehsan is on PTO for a while.)
Flags: needinfo?(nfroyd)
Flags: needinfo?(ehsan)
I was attempting to wait to do this until m-c was buildable by default with clang-cl for 32-bit; https://reviews.llvm.org/D26335 is the last blocker that I know of, although I think bug 1313280 might break things is easily fixable ways. 64-bit requires a few additional changes to m-c. Once that's done, I was going stand up ordinary clang-cl (no static analysis) builds in taskcluster to ensure that m-c remains buildable with clang-cl; the buildbot stuff might work, but I think releng would be much happier if the buildbot code went away sooner rather than later, and it's much, much easier to work with the taskcluster infrastructure than buildbot. Then that build can be converted to a static analysis build...there are probably a number of things to fix in our Windows-specific code with respect to our static analysis tools.
(In reply to Ting-Yu Chou [:ting] from comment #38) > How could I get the permission of tooltool.upload.public in > https://api.pub.build.mozilla.org? If I can't, should I follow bug 1042132 > comment 1 for uploading? Per nathan's vouch, I've added `tchou@m.c` to the permissions group. If instead this is your @gmail or some other ldap, feel free to n-i me and I'll adjust. You'll need to logout and log in again on relengapi in order to be able to get a token that has the upload permission.
I failed uploading the files with this message, retried 5 times but not working and I found there're now 5 the same batches in https://api.pub.build.mozilla.org/tooltool/... $ python tooltool.py upload --authentication-file=.tooltool-token --message "Bug 1251936 - New Windows clang for x86 /x64, r283955" INFO - clang64.tar.bz2: starting upload INFO - clang32.tar.bz2: starting upload ERROR - clang32.tar.bz2: failed Traceback (most recent call last): File "tooltool.py", line 791, in _s3_upload (resp.status, resp.reason, resp_body)) RuntimeError: Non-200 return from AWS: 400 Bad Request <?xml version="1.0" encoding="UTF-8"?><Error><Code>RequestTimeout</Code><Message>Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed.</Message><RequestId>20E806D572EAA723</RequestId><HostId>tIFXayLqhUbP7PZ17XSaKW+PyUMNFBtCbGiUb3QFT37phf7xQmSjo3gvDosD7FvYo1i7PR+jDB8yxOEfus9fuI51i/5OM3oV</HostId></Error> ERROR - clang64.tar.bz2: failed Traceback (most recent call last): File "tooltool.py", line 791, in _s3_upload (resp.status, resp.reason, resp_body)) RuntimeError: Non-200 return from AWS: 400 Bad Request <?xml version="1.0" encoding="UTF-8"?><Error><Code>RequestTimeout</Code><Message>Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed.</Message><RequestId>CF7F00947BB490F2</RequestId><HostId>SpjvbaWXoeMW/vJTcNgMJ9p7gA9LAwkWvxq9P4sBzphpEHlehyytIg3ciDO8J7OhrHm8rijubqEDhVyMe9n1Y1QUCCSKktBh</HostId></Error>
Flags: needinfo?(bugspam.Callek)
That sounds like a suspicious error message. Let me flag our current maintainer of this code for an idea.
Flags: needinfo?(bugspam.Callek) → needinfo?(rgarbas)
That error is from Amazon S3. I wonder if you have some kind of network issue between you and the Amazon S3 servers? Perhaps a tcpdump of the transaction could help to determine that. I don't know what S3's request timeout is, but I'd guess it's not less than 30 seconds..
(In reply to Ting-Yu Chou [:ting] from comment #43) > $ python tooltool.py upload --authentication-file=.tooltool-token --message > "Bug 1251936 - New Windows clang for x86 > /x64, r283955" Did you build that on your local machine or through the taskcluster stuff? It would be better to do the latter.
Flags: needinfo?(janus926)
OK, I'll try to build it by taskcluster. But will there be any differences from taskcluster one? I built it locally with command: ./mach python build/build-clang/build-clang.py -c build/build-clang/clang-static-analysis-win32.json
Flags: needinfo?(janus926)
(In reply to Ting-Yu Chou [:ting] from comment #47) > OK, I'll try to build it by taskcluster. But will there be any differences > from taskcluster one? I built it locally with command: > > ./mach python build/build-clang/build-clang.py -c > build/build-clang/clang-static-analysis-win32.json You can push patches to try with: -b o -p none -j win32-clang-cl -t none -u none
The build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffbd5f48c967a0cf774decaea4cb6c4d8c258262. However I still failed uploading to tooltool, filed bug 1316249 for that so not spamming here.
i guess i'm slowly taking over ( https://api.pub.build.mozilla.org ) but i'm yet to dig into tooltool. i think :dustin: might have better understanding what is happening, but i'll keep my eye on bug 1316249
Flags: needinfo?(rgarbas)
Depends on: 1316545
Depends on: 1319330
Depends on: 1321453
Once bug 1321453 is fixed, we should be able to stand up Windows 32/64 bit opt/debug static analysis checking builds. I'll just add new patches here as this is not closed, let me know if I should file a new one.
(In reply to Ting-Yu Chou [:ting] from comment #51) > Once bug 1321453 is fixed, we should be able to stand up Windows 32/64 bit > opt/debug static analysis checking builds. Nathan, are you working on this?
Flags: needinfo?(nfroyd)
(In reply to Ting-Yu Chou [:ting] from comment #52) > (In reply to Ting-Yu Chou [:ting] from comment #51) > > Once bug 1321453 is fixed, we should be able to stand up Windows 32/64 bit > > opt/debug static analysis checking builds. > > Nathan, are you working on this? Sure, I can do this. Our clang-cl builds are now building without -fallback, so once we fix all the static analysis errors, I think we can just add static analysis into those builds.
Flags: needinfo?(nfroyd)
(In reply to Nathan Froyd [:froydnj] from comment #53) > (In reply to Ting-Yu Chou [:ting] from comment #52) > > (In reply to Ting-Yu Chou [:ting] from comment #51) > > > Once bug 1321453 is fixed, we should be able to stand up Windows 32/64 bit > > > opt/debug static analysis checking builds. > > > > Nathan, are you working on this? > > Sure, I can do this. Our clang-cl builds are now building without > -fallback, so once we fix all the static analysis errors, I think we can > just add static analysis into those builds. Agreed. I also think we should rename them to show up as "S" jobs. Thankfully with TaskCluster that's a simple patch to taskcluster/ci/build/windows.yml :-)
Bug 1321453 is now fixed. As the holidays is around the corner, let me know if you want me to take it, we have only two days off on 12/26 and 1/2 in Taipei.
I am ambivalent about renaming them to 'S' on taskcluster; let me know if you feel strongly about that (and perhaps updating the taskcluster description).
Attachment #8821591 - Flags: review?(ehsan)
Attachment #8724546 - Attachment is obsolete: true
Attachment #8726794 - Attachment is obsolete: true
Attachment #8726796 - Attachment is obsolete: true
Attachment #8728162 - Attachment is obsolete: true
Attachment #8728163 - Attachment is obsolete: true
Attachment #8728671 - Attachment is obsolete: true
What I like about Ehsan's "S" suggestion is that "you broke the static analysis build" seems more likely to be fixed than "you broke this alternative compiler thing".
Comment on attachment 8821591 [details] [diff] [review] enable static analysis on clang-cl builds Review of attachment 8821591 [details] [diff] [review]: ----------------------------------------------------------------- I do think we should rename the jobs mostly for the reason dmajor mentioned. I'll submit a patch for that in a minute. :-)
Attachment #8821591 - Flags: review?(ehsan) → review+
Also I think we should write to dev-platform about this.
Comment on attachment 8821602 [details] [diff] [review] Rename the Windows clang-cl builds to static analysis builds Review of attachment 8821602 [details] [diff] [review]: ----------------------------------------------------------------- I was going to write to dev-platform about these builds! Great minds think alike.
Attachment #8821602 - Flags: review?(nfroyd) → review+
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5ff2c66f4020 enable static analysis on clang-cl builds; r=ehsan https://hg.mozilla.org/integration/mozilla-inbound/rev/1f3cc2963f09 Rename the Windows clang-cl builds to static analysis builds; r=froydnj
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/806029a2c31d followup - add explicit to some Windows-only IPC classes on a CLOSED TREE; r=bustage
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2fc498f1d5bc followup - make nsTArray<mozilla::gfx::FilterPrimitiveDescription> use copies on a CLOSED TREE; r=bustage
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c4efe5014971 turn static analysis off on windows debug clang-cl builds; r=bustage
(In reply to Pulsebot from comment #65) > Pushed by nfroyd@mozilla.com: > https://hg.mozilla.org/integration/mozilla-inbound/rev/c4efe5014971 > turn static analysis off on windows debug clang-cl builds; r=bustage We ran into some issues with debug builds, and it also looks like there's a bug in the plugin that causes it to not consider std::atomic memmovable on Windows, leading to lots of problems. Ehsan's looking into the plugin problem, and then we can reland things.
Depends on: 1325694
Blocks: 1325719
Blocks: 1325723
Blocks: 1325726
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e6fdd363cb76 Mark AutoSetXPCOMLoadOnMainThread's constructor as explicit https://hg.mozilla.org/integration/mozilla-inbound/rev/a197222090fb Re-enable Windows static analysis builds
Keywords: leave-open
Ehsan & Nathan, sorry I made the mistake that didn't check the debug build, I'll be more careful and make sure I don't miss anything before I say it is fixed.
No worries, it's rare for an error to only be detected in one configuration!
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: