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

Issue 827075 link

Starred by 1 user
Status: Fixed
Owner:
r...@chromium.org
Closed: Mar 2018
Cc:
█ ruiu@google.com
thakis@chromium.org
inglorion@chromium.org
r...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue 792131



Sign in to add a comment

chrome_elf_import_unittests fails when linked with lld

Project Member Reported by h...@chromium.org, Mar 29 2018 
Example build: https://ci.chromium.org/buildbot/tryserver.chromium.win/win10_chromium_x64_rel_ng/111639

>type out\release\args.gn
use_lld = true
dcheck_always_on = true
is_component_build = false
is_debug = false



>out\release\chrome_elf_import_unittests.exe
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 56 parallel jobs.
[ RUN      ] ELFImportsTest.ChromeElfSanityCheck
../../chrome_elf/elf_imports_unittest.cc(112): error: Value of: match
  Actual: false
Expected: true
Illegal import in chrome_elf.dll: SHELL32.dll
[  FAILED  ] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[1/3] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[ RUN      ] ELFImportsTest.ChromeElfLoadSanityTest
../../chrome_elf/elf_imports_unittest.cc(137): error: Failed
Couldn't find
OK ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
 in output
 Note: Google Test filter = ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ELFImportsTest
[ RUN      ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
../../chrome_elf/elf_imports_unittest.cc(160): error: Expected equality of these values:
  nullptr
    Which is: NULL
  ::GetModuleHandleW(L"user32.dll")
    Which is: 00007FFCEF340000
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl (10 ms)
[----------] 1 test from ELFImportsTest (10 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (10 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl

 1 FAILED TEST

[  FAILED  ] ELFImportsTest.ChromeElfLoadSanityTest (30 ms)
[2/3] ELFImportsTest.ChromeElfLoadSanityTest (30 ms)
[3/3] ELFImportsTest.ChromeExeSanityCheck (1 ms)
Retrying 2 tests (retry #1)
[ RUN      ] ELFImportsTest.ChromeElfSanityCheck
../../chrome_elf/elf_imports_unittest.cc(112): error: Value of: match
  Actual: false
Expected: true
Illegal import in chrome_elf.dll: SHELL32.dll
[  FAILED  ] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[4/5] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[ RUN      ] ELFImportsTest.ChromeElfLoadSanityTest
../../chrome_elf/elf_imports_unittest.cc(137): error: Failed
Couldn't find
OK ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
 in output
 Note: Google Test filter = ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ELFImportsTest
[ RUN      ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
../../chrome_elf/elf_imports_unittest.cc(160): error: Expected equality of these values:
  nullptr
    Which is: NULL
  ::GetModuleHandleW(L"user32.dll")
    Which is: 00007FFCEF340000
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl (7 ms)
[----------] 1 test from ELFImportsTest (7 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (8 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl

 1 FAILED TEST

[  FAILED  ] ELFImportsTest.ChromeElfLoadSanityTest (22 ms)
[5/5] ELFImportsTest.ChromeElfLoadSanityTest (22 ms)
Retrying 2 tests (retry #2)
[ RUN      ] ELFImportsTest.ChromeElfSanityCheck
../../chrome_elf/elf_imports_unittest.cc(112): error: Value of: match
  Actual: false
Expected: true
Illegal import in chrome_elf.dll: SHELL32.dll
[  FAILED  ] ELFImportsTest.ChromeElfSanityCheck (1 ms)
[6/7] ELFImportsTest.ChromeElfSanityCheck (1 ms)
[ RUN      ] ELFImportsTest.ChromeElfLoadSanityTest
../../chrome_elf/elf_imports_unittest.cc(137): error: Failed
Couldn't find
OK ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
 in output
 Note: Google Test filter = ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ELFImportsTest
[ RUN      ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
../../chrome_elf/elf_imports_unittest.cc(160): error: Expected equality of these values:
  nullptr
    Which is: NULL
  ::GetModuleHandleW(L"user32.dll")
    Which is: 00007FFCEF340000
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl (6 ms)
[----------] 1 test from ELFImportsTest (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl

 1 FAILED TEST

[  FAILED  ] ELFImportsTest.ChromeElfLoadSanityTest (18 ms)
[7/7] ELFImportsTest.ChromeElfLoadSanityTest (18 ms)
Retrying 2 tests (retry #3)
[ RUN      ] ELFImportsTest.ChromeElfSanityCheck
../../chrome_elf/elf_imports_unittest.cc(112): error: Value of: match
  Actual: false
Expected: true
Illegal import in chrome_elf.dll: SHELL32.dll
[  FAILED  ] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[8/9] ELFImportsTest.ChromeElfSanityCheck (0 ms)
[ RUN      ] ELFImportsTest.ChromeElfLoadSanityTest
../../chrome_elf/elf_imports_unittest.cc(137): error: Failed
Couldn't find
OK ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
 in output
 Note: Google Test filter = ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ELFImportsTest
[ RUN      ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl
../../chrome_elf/elf_imports_unittest.cc(160): error: Expected equality of these values:
  nullptr
    Which is: NULL
  ::GetModuleHandleW(L"user32.dll")
    Which is: 00007FFCEF340000
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl (7 ms)
[----------] 1 test from ELFImportsTest (7 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (7 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ELFImportsTest.DISABLED_ChromeElfLoadSanityTestImpl

 1 FAILED TEST

[  FAILED  ] ELFImportsTest.ChromeElfLoadSanityTest (20 ms)
[9/9] ELFImportsTest.ChromeElfLoadSanityTest (20 ms)
2 tests failed:
    ELFImportsTest.ChromeElfLoadSanityTest (../../chrome_elf/elf_imports_unittest.cc:116)
    ELFImportsTest.ChromeElfSanityCheck (../../chrome_elf/elf_imports_unittest.cc:74)
Tests took 0 seconds.

Comment 1 by r...@chromium.org, Mar 29 2018 

Looks like we don't run this test on the ToT bots. I guess chromium/src/testing/buildbot/test_suites.pyl needs to be adjusted? Why is this file 2300 lines. ;_;

Comment 2 by r...@chromium.org, Mar 29 2018

Owner: r...@chromium.org
This looks like a command line compatibility issue. chrome_elf.dll isn't linked with /OPT:REF, but these tests expect that the dependencies on SHELL32.dll and USER32.dll from base.lib will end up getting dead stripped away.

Comment 3 by r...@chromium.org, Mar 29 2018

Status: Assigned (was: Available)

Comment 4 by r...@chromium.org, Mar 29 2018 

Oh, it's because /PROFILE implies /OPT:REF, and we just ignore it:
https://msdn.microsoft.com/en-us/library/ays5x7b0.aspx

The relevant part of the link line looks like this:
... /PROFILE /WX /OPT:ICF /DEBUG ...

Comment 5 by thakis@chromium.org, Mar 29 2018 

Reminds me a bit of  issue 689666 .

Getting /OPT:REF via /PROFILE probably is just luck. See also the TODO added in https://chromium-review.googlesource.com/c/chromium/src/+/980912/24/build/config/win/BUILD.gn and that !use_lld there that hans added because fastlink.

So while we should fix (meaning /PROFILE should imply /OPT:REF there), we can also work aroudn it in the gn files if that rolls in and we don't need an lld roll for this issue. (We might for the other one still, of course.)

Comment 6 by r...@chromium.org, Mar 29 2018 

Hey, you found the issue I was thinking of. :)

I had the same thoughts, so I sent out https://chromium-review.googlesource.com/#/c/chromium/src/+/986694 to tweak the gn.

Upstream LLD should make /PROFILE imply these flags as I'm sure other users will run into this, even if we keep that gn tweak.
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 29 2018 

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

commit 2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae
Author: Nico Weber <thakis@chromium.org>
Date: Thu Mar 29 23:44:21 2018

Run more tests on the clang tot bots.

In particular, if it runs on the main waterfall, it should also run
on the tot bots.

Adds app_shell_unittests, aura_unittests, chrome_elf_import_unittests,
compositor_unittests, install_static_unittests, mini_installer_tests,
wm_unittests.

Don't run mini_installer_tests on the cross bot due to
next_version_mini_installer.exe not yet working there.

Bug:  827075 , 827082 ,799827
Change-Id: Iffe1cf0be5a95892988aab93feefecf2c5b48cbd
Reviewed-on: https://chromium-review.googlesource.com/986679
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547025}
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/2a1e0f88dbefa67ce4832b8b59c788e4b48bfaae/testing/buildbot/test_suites.pyl
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 30 2018 

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

commit bf8b9821e804283aab1ee7a40c05d1a96eee0cf3
Author: Reid Kleckner <rnk@google.com>
Date: Fri Mar 30 04:14:46 2018

Explicitly pass linker /OPT and other flags implied by /PROFILE

LLD currently ignores /PROFILE, so it does not enable /OPT:ICF and
/OPT:REF for chrome_elf.dll. chrome_elf_import_unittests checks that the
linker strips dead dependencies on user32.dll and shell32.dll, so it
currently fails when chrome_elf.dll is linked with LLD.

I will fix this bug in upstream LLD, but in the meantime this will fix
the issue. This change also seens like it is more explicit and reduces
gn complexity, so maybe we should leave it like this after the LLD fix.

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

Bug:  chromium:827075 
Change-Id: Icd2a09aab031b7ff07738892c12a5de1ec9936b9
Reviewed-on: https://chromium-review.googlesource.com/986694
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547105}
[modify] https://crrev.com/bf8b9821e804283aab1ee7a40c05d1a96eee0cf3/build/config/win/BUILD.gn
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 30 2018 

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

commit c33f8567e9909349a0cf59854c9c268851d2c018
Author: Oleh Prypin <oprypin@chromium.org>
Date: Fri Mar 30 10:00:06 2018

Remove usage of 'is_syzyasan' which doesn't seem to exist elsewhere

Bug:  chromium:827075 
Change-Id: Id34efbfc4e8fc23abe9a8319378d890f0d040363
Reviewed-on: https://chromium-review.googlesource.com/987892
Commit-Queue: Oleh Prypin <oprypin@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547135}
[modify] https://crrev.com/c33f8567e9909349a0cf59854c9c268851d2c018/build/config/win/BUILD.gn

Comment 10 by thakis@chromium.org, Mar 30 2018 

http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180326/538500.html made /PROFILE in lld-link do the right thing.

Comment 11 by thakis@chromium.org, Mar 30 2018

Status: Fixed (was: Assigned)
https://ci.chromium.org/buildbot/chromium.clang/ToTWin64/1194 has a green run of chrome_elf_import_unittests. Fixed in lld, worked around in gn, and added binary to our bots -- fixed pretty comprehensively.
►

Sign in to add a comment