New issue
Advanced search Search tips

Issue 801563 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 101600

Blocking:
issue 800760



Sign in to add a comment

Enable -Wglobal-constructors warning for non-test targets

Project Member Reported by gab@chromium.org, Jan 12 2018

Issue description

If we can enable -Wglobal-constructors on non-test targets (test isn't possible because of the way gtest works; nor desirable IMO as it might be too restrictive for tricky tests), then we can drop LazyInstance in favor of constexpr default initialization in a bunch of places and be confident we don't incur static initializers :)

Ultimately helps mitigate priority inversion in initialization of globals (issue 800760).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 19 2018

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

commit a9698a9ad30bff2bfc150fddd4b380b05d6c6eff
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Jan 19 09:57:35 2018

Enable -Wglobal-constructors in //base.

Fix an unintentional dynamic allocation in base/android.

Explicitly allow dynamic allocation in two other places
where compile-time isn't possible and I *think* are benign.

Bug: 801563, 800760
Change-Id: I183e18d21af31625a8729aff44b89be85dcd1220
Reviewed-on: https://chromium-review.googlesource.com/866738
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530465}
[modify] https://crrev.com/a9698a9ad30bff2bfc150fddd4b380b05d6c6eff/base/BUILD.gn
[modify] https://crrev.com/a9698a9ad30bff2bfc150fddd4b380b05d6c6eff/base/android/library_loader/library_prefetcher.cc
[modify] https://crrev.com/a9698a9ad30bff2bfc150fddd4b380b05d6c6eff/base/message_loop/message_pump_mac.mm
[modify] https://crrev.com/a9698a9ad30bff2bfc150fddd4b380b05d6c6eff/base/process/memory_win.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2018

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

commit c78d4f5766ee9d828176da85673319cf1235f9e6
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Jan 19 12:11:22 2018

Add -Wexit-time-destructors to //base

This was not obvious to me but -Wglobal-constructors implies
-Wexit-time-destructors (because an exit time destructor implies a
registration at initialization time), full discussion @
http://clang-developers.42468.n3.nabble.com/Wglobal-constructors-warns-on-constexpr-default-constructor-evaluated-at-compile-time-td4059304.html

Adding the flag to be explicit about it.

R=dcheng@chromium.org

Bug: 801563
Change-Id: I0f57a4073eb8afcb2bac65ba7a8826f1b8d14cb4
Reviewed-on: https://chromium-review.googlesource.com/870551
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530490}
[modify] https://crrev.com/c78d4f5766ee9d828176da85673319cf1235f9e6/base/BUILD.gn

Comment 3 by gab@chromium.org, Jan 22 2018

Cc: dcheng@chromium.org
Status: Assigned (was: Started)
I'm still strongly interested in this but will yield this task for now, at least made it in //base.

Comment 4 by gab@chromium.org, May 3 2018

Cc: gab@chromium.org
Components: -Internals>TaskScheduler
Labels: Hotlist-GoodFirstBug
Owner: ----
Status: Available (was: Assigned)
Blockedon: 101600
-Wexit-time-destructors is issue 101600

Sign in to add a comment