[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

Charles Zhang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 2 11:12:27 PDT 2019


czhang marked 2 inline comments as done.
czhang added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:33-35
+  // This may work fine when optimization is enabled because bar() can
+  // be turned into a constant 7.  But without optimization, it can
+  // cause problems. Therefore, we must err on the side of conservatism.
----------------
aaron.ballman wrote:
> czhang wrote:
> > aaron.ballman wrote:
> > > czhang wrote:
> > > > aaron.ballman wrote:
> > > > > What problems can be caused here? Typically, dynamic init is only problematic when it happens before main() is executed (because of initialization order dependencies), but that doesn't apply to local statics.
> > > > Consider the case when synchronization is disabled for static initialization, and two threads call `foo2` for the first time. It may be the case that they both try and initialize the static variable at the same time with different values (since the dynamic initializer may not be pure), creating a race condition.
> > > > Consider the case when synchronization is disabled for static initialization
> > > 
> > > This is a compiler bug, though: http://eel.is/c++draft/stmt.dcl#4.sentence-3
> > Sorry, I guess I didn't make it clear enough in the rst documentation file, but this check is for those who explicitly enable the -fno-threadsafe-statics flag because they provide their own synchronization. Then they would like to check if the headers they didn't write may possibly run into this issue when compiling with this flag.
> Ah! Thank you for the explanation. In that case, this behavior makes more sense, but I think you should only warn if the user has enabled that feature flag rather than always warning.
I haven't been able to find much documentation on how to actually make a tidy check run against a feature flag. Is it possible to do this? I would think that said users would manually run this check on their header files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62829/new/

https://reviews.llvm.org/D62829





More information about the cfe-commits mailing list