New issue
Advanced search Search tips

Issue 869503 link

Starred by 2 users
Status: Fixed
Owner: ----
Closed: Aug 6
Cc:
brucedaw...@chromium.org
robliao@chromium.org
wfh@chromium.org
h...@chromium.org
r...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Turn down link.exe bots

Project Member Reported by thakis@chromium.org, Jul 31 
We currently have 4 bots using link.exe on the clangtot waterfall.

Now that we're on lld on stable, we can probably remove them.

Unless someone shouts, I'll do this soon.


(They're currently red and have been for a while; with:

FAILED: test_process.exe test_process.exe.pdb 
cmd /c C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py delete-file ./test_process.exe.pdb && C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /OUT:./test_process.exe /PDB:./test_process.exe.pdb @./test_process.exe.rsp
libcmt.lib(exe_main.obj) : error LNK2019: unresolved external symbol _main referenced in function "int __cdecl __scrt_common_main_seh(void)" (?__scrt_common_main_seh@@YAHXZ)

https://cs.chromium.org/chromium/src/chrome/chrome_cleaner/test/test_process_main.cc?type=cs&sq=package:chromium&g=0&l=30 -- I think this is missing a /subsystem:windows flag and doesn't have wWinMain()s parameters exactly right to get the autodetection of the flag working in link.exe -- this specific thing is probably an lld bug, but if we turn down these bots anyways we probably don't need to worry about it.)

Comment 1 by brucedaw...@chromium.org, Aug 1 

Given that we're not looking at the bots we might as well turn them down.

I looked into the link.exe failure and it is, as you suspected, an lld bug. The command-line we send to the linker for test_process.exe contains /SUBSYSTEM:CONSOLE,5.02. link.exe honors that and lld-link.exe ignores it. I filed https://bugs.llvm.org/show_bug.cgi?id=38397 and I'll land a change to fix the command-line arguments for test_process.exe (crrev.com/c/1157656).
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 3 

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

commit 32a236abb4fa125e97fd3e157bc2d6c76bce976a
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Aug 03 19:01:43 2018

Specify correct /SUBSYSTEM for test_process.exe

test_process.exe was being linked with /SUBSYSTEM:CONSOLE, despite being
a windowed app with a wWinMain entry point. With lld-link.exe this seems
to work fine, suggesting that lld ignores the/SUBSYSTEM flag. lld-link
may change this behavior (https://bugs.llvm.org/show_bug.cgi?id=38397)
so it seems worthwhile to fix the build flags for this executable. Doing
so will also be less confusing.

Bug:  869503 
Change-Id: I52a61aab678fa135f3da8c10bc82a4cdf649b5ae
Reviewed-on: https://chromium-review.googlesource.com/1157656
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Chris Sharp <csharp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580616}
[modify] https://crrev.com/32a236abb4fa125e97fd3e157bc2d6c76bce976a/chrome/chrome_cleaner/test/BUILD.gn

Comment 3 by viswa.karala@chromium.org, Aug 6

Labels: Needs-Milestone

Comment 4 by ajha@chromium.org, Aug 6

Labels: -Needs-Milestone

Comment 5 by thakis@chromium.org, Aug 6 

(After https://reviews.llvm.org/D50316 lld-link should fail like link.exe for this particular example. Once it's landed and rolled in, I'll send try jobs for a revert of your cl to verify. Won't land the revert of course.)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 6 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/master-manager/+/9424fdecea84cf8b44c029936832330d06d3c7d6

commit 9424fdecea84cf8b44c029936832330d06d3c7d6
Author: Nico Weber <thakis@google.com>
Date: Mon Aug 06 13:11:20 2018

Comment 9 by thakis@chromium.org, Aug 6

Status: Fixed (was: Unconfirmed)

Comment 10 by thakis@chromium.org, Aug 28 

I sent a try job with a patch that undoes bruce's change in comment 8 here: https://chromium-review.googlesource.com/c/chromium/src/+/1194471

lld now fails too, but the error message is less good (I'll make a patch):

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

FAILED: test_process.exe test_process.exe.pdb 
ninja -t msvc -e environment.x86 -- ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /OUT:./test_process.exe /PDB:./test_process.exe.pdb @./test_process.exe.rsp
lld-link: error: entry point must be defined

Comment 11 by thakis@chromium.org, Sep 16 

Filed https://bugs.llvm.org/show_bug.cgi?id=38972 for the better diag on missing main.

Comment 12 by thakis@chromium.org, Oct 3 

Finally fixed that one in upstream r343698, so we should now have parity with link.exe for that particular issue.
►

Sign in to add a comment