New issue
Advanced search Search tips

Issue 871418 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Roll clang again

Project Member Reported by p...@chromium.org, Aug 6

Issue description

Tracking bug for the next Clang roll.

Previous roll:  issue 866225  (clang r338452).
 
Blocking: 846464
once r339165 rolls in, i can send a try job with a revert for https://chromium-review.googlesource.com/1157656 and verify that lld-link now fails the same way that link.exe used to.
Blocking: 872043
Blocking: 872611
Blocking: 866651
r339345 for parts of issue 866651
Blockedon: 872926
clang r339420 for other parts of issue 866651

The previous roll was reverted due to  issue 872926 , so we need to figure that out before rolling again.
The previous roll re-landed in crrev.com/582951

Upstream, libc++ changed back to the always_inline behaviour in r339874, so when we roll past that we should drop the _LIBCPP_HIDE_FROM_ABI define.


The roll will also include the version change to 8, so should bump the version number/remove the work-around from crrev.com/579800
Blockedon: 874997
r340025 fixes a regression in clang-cl /P
r340079 fixes the debug info verification failure we see on the thinlto bots.
Blockedon: 875620
I think we can roll. If I understand correctly, we should leave the _LIBCPP_HIDE_FROM_ABI defines alone for this roll, so we still get the always_inline behavior. Let me know if that's not the case.

I started making packages for r340207 here:
https://chromium-review.googlesource.com/c/chromium/src/+/1182196

I made sure to pick up Peter's package.py changes for llvm-undname and llvm-pdbutil.
The Windows package is failing because 'tail' is missing, that's 

The Mac package is failing because some aligned allocation libc++ test is XPASSing:

-- Testing: 48025 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80
XPASS: libc++ :: libcxx/memory/aligned_allocation_macro.pass.cpp (40513 of 48025)
******************** TEST 'libc++ :: libcxx/memory/aligned_allocation_macro.pass.cpp' FAILED ********************
********************
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 987.12s
********************
Unexpected Passing Tests (1):
    libc++ :: libcxx/memory/aligned_allocation_macro.pass.cpp
  Expected Passes    : 40250
  Expected Failures  : 104
  Unsupported Tests  : 7670
  Unexpected Passes  : 1
FAILED: CMakeFiles/check-all

I'm not sure what to do next with that info.
Blocking: 876139
Looks like ldionne touched these a bunch, maybe ping them on llvm irc and see what they say?

(tail is  issue 876139 )
Blockedon: 876139
Blocking: -876139
I went ahead and marked the test UNSUPPORTED instead of XFAIL in r340427 and tried to roll at that rev, but it looks like there are goma issues with the package. We'll see what's up.
Blockedon: 877235
Owner: r...@chromium.org
Status: Assigned (was: Untriaged)
We had some goma troubles, so I'm making new packages at r340740. The waterfall looks exceptionally green!
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 28

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

commit 6c2527114e7363fe0656c4aaca8e580ae26b1a5b
Author: Reid Kleckner <rnk@google.com>
Date: Tue Aug 28 19:43:10 2018

Roll clang 338452:340740

Update clang_version from 7.0.0 to 8.0.0 and remove the code to remove
the old library directory.

Remove the _LIBCPP_HIDE_FROM_ABI define, it is no longer needed.

Bug:  871418 ,  872926 
Change-Id: Ic02dcfbf17c4190fc4359271d880b7e65bf7ba27
Reviewed-on: https://chromium-review.googlesource.com/1182196
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586816}
[modify] https://crrev.com/6c2527114e7363fe0656c4aaca8e580ae26b1a5b/build/config/compiler/BUILD.gn
[modify] https://crrev.com/6c2527114e7363fe0656c4aaca8e580ae26b1a5b/build/toolchain/toolchain.gni
[modify] https://crrev.com/6c2527114e7363fe0656c4aaca8e580ae26b1a5b/tools/clang/scripts/update.py

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 28

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

commit 7cb8c3d3ef8f1e0d24e7986de28f885d1e394242
Author: Reid Kleckner <rnk@chromium.org>
Date: Tue Aug 28 20:45:18 2018

Revert "Roll clang 338452:340740"

This reverts commit 6c2527114e7363fe0656c4aaca8e580ae26b1a5b.

Reason for revert: Breaks Google Chrome Linux x64 bot somehow:
https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Linux%20x64/35385

LINK ./chrome step fails with null pointer assertion in isa<ReturnInst>.

Original change's description:
> Roll clang 338452:340740
> 
> Update clang_version from 7.0.0 to 8.0.0 and remove the code to remove
> the old library directory.
> 
> Remove the _LIBCPP_HIDE_FROM_ABI define, it is no longer needed.
> 
> Bug:  871418 ,  872926 
> Change-Id: Ic02dcfbf17c4190fc4359271d880b7e65bf7ba27
> Reviewed-on: https://chromium-review.googlesource.com/1182196
> Commit-Queue: Reid Kleckner <rnk@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Peter Collingbourne <pcc@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#586816}

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

Change-Id: Ib32fe653d23cd7d4ce27d4cfc20faf2eec2a6ce0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  871418 ,  872926 
Reviewed-on: https://chromium-review.googlesource.com/1194830
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586839}
[modify] https://crrev.com/7cb8c3d3ef8f1e0d24e7986de28f885d1e394242/build/config/compiler/BUILD.gn
[modify] https://crrev.com/7cb8c3d3ef8f1e0d24e7986de28f885d1e394242/build/toolchain/toolchain.gni
[modify] https://crrev.com/7cb8c3d3ef8f1e0d24e7986de28f885d1e394242/tools/clang/scripts/update.py

I tested r340874 locally in the ThinLTO official linux build configuration and confirmed it doesn't have the issue that the last roll had.

New packages coming here:
https://chromium-review.googlesource.com/c/chromium/src/+/1194698
Finding a working upstream revision is tricky. I've updated rnk's CL to try r340924, hopefully that works.

I noticed that gvnhoist was enabled upstream in r340922, which might something to watch out for.
> I noticed that gvnhoist was enabled upstream in r340922, which might something to watch out for.
Never mind, it was reverted in 340925
Project Member

Comment 27 by bugdroid1@chromium.org, Aug 30

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

commit aa63cf49d5bf75d3f856e7c791c7cc29946c783b
Author: Hans Wennborg <hans@chromium.org>
Date: Thu Aug 30 14:17:07 2018

ash: fix order of initialization bug in ShelfWidget

Initializing ShelfWidget::delegate_view_ causes
DelegateView::UpdateOpaqueBackground() to run, which refers back to
ShelfWidget::background_animator_ which hasn't been initialized yet.
This caused MSan failures with a new Clang version, see
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_chromeos_msan_rel_ng/894

The fix is to initialize background_animator_ first.

Bug:  871418 
Change-Id: I1cc4888e9ba5b3f2c5a9166f3e99125c84b3d796
Reviewed-on: https://chromium-review.googlesource.com/1196426
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587560}
[modify] https://crrev.com/aa63cf49d5bf75d3f856e7c791c7cc29946c783b/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/aa63cf49d5bf75d3f856e7c791c7cc29946c783b/ash/shelf/shelf_widget.h

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 30

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

commit d54494cbad8d5ff0a8cd74024902c8bd86258ae4
Author: Hans Wennborg <hans@chromium.org>
Date: Thu Aug 30 15:59:01 2018

Roll clang 338452:340925

Update clang_version from 7.0.0 to 8.0.0 and remove the code to remove
the old library directory.

Remove the _LIBCPP_HIDE_FROM_ABI define, it is no longer needed.

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

Bug:  871418 ,  872926 
Change-Id: Ibf6734500aa75a349fa772b225b8b15fddfe3661
Reviewed-on: https://chromium-review.googlesource.com/1194698
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587602}
[modify] https://crrev.com/d54494cbad8d5ff0a8cd74024902c8bd86258ae4/build/config/compiler/BUILD.gn
[modify] https://crrev.com/d54494cbad8d5ff0a8cd74024902c8bd86258ae4/build/toolchain/toolchain.gni
[modify] https://crrev.com/d54494cbad8d5ff0a8cd74024902c8bd86258ae4/tools/clang/scripts/update.py

So far so good.

The only thing that came up is a new CF issue 879410
Status: Fixed (was: Assigned)
Another thing:
Findit (https://goo.gl/kROfz5) identified this CL at revision 587602 as the culprit
for introducing flakiness in the tests as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZDU0NDk0Y2JhZDhkNWZmMGE4Y2Q3NDAyNDkwMmM4YmQ4NjI1OGFlNAw

But overall this seems to be sticking, so closing.

Next roll:  issue 880827 
Blocking: 875627

Sign in to add a comment