From 6d5643b2c9a4da4c9fe1d7ad968a43889c795add Mon Sep 17 00:00:00 2001 From: scott snyder <sss@karma> Date: Wed, 15 Nov 2017 16:36:42 +0100 Subject: [PATCH] CheckerGccPlugins: Passing a static mutex object is ok. Add mutexes to the exception list for the thread-safety check on passing a static object. Also add std::shared_timed_mutex as a known mutex type. --- External/CheckerGccPlugins/THREAD_CHECKS | 1 + .../CheckerGccPlugins/share/thread3_test.ref | 44 +++++++++---------- .../CheckerGccPlugins/src/thread_plugin.cxx | 4 ++ .../CheckerGccPlugins/test/thread3_test.cxx | 11 +++++ 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/External/CheckerGccPlugins/THREAD_CHECKS b/External/CheckerGccPlugins/THREAD_CHECKS index fc6e379b..da921ab4 100644 --- a/External/CheckerGccPlugins/THREAD_CHECKS +++ b/External/CheckerGccPlugins/THREAD_CHECKS @@ -128,6 +128,7 @@ void f1() Passing a std::atomic value of a fundamental type is considered ok. +Passing mutexes also considered ok. Can suppress by: - Declaring value as thread_local. diff --git a/External/CheckerGccPlugins/share/thread3_test.ref b/External/CheckerGccPlugins/share/thread3_test.ref index ddb3db83..afdceed2 100644 --- a/External/CheckerGccPlugins/share/thread3_test.ref +++ b/External/CheckerGccPlugins/share/thread3_test.ref @@ -1,38 +1,38 @@ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx: In function 'void f1()': -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:27:15: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo1' within function 'void f1()'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread3_test.cxx: In function 'void f1()': +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:30:7: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo1' within function 'void f1()'; may not be thread-safe. foo1(3, &y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:15:12: note: Declared here: + ~~~~^~~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:18:12: note: Declared here: static int y1; ^~ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:27:15: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:30:7: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. foo1(3, &y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:28:14: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo2' within function 'void f1()'; may not be thread-safe. + ~~~~^~~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:31:7: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo2' within function 'void f1()'; may not be thread-safe. foo2(3, y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:15:12: note: Declared here: + ~~~~^~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:18:12: note: Declared here: static int y1; ^~ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:28:14: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:31:7: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. foo2(3, y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:29:15: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo3' within function 'void f1()'; may not be thread-safe. + ~~~~^~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:32:7: warning: Static expression 'y1' passed to pointer or reference function argument of 'foo3' within function 'void f1()'; may not be thread-safe. foo3(3, &y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:15:12: note: Declared here: + ~~~~^~~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:18:12: note: Declared here: static int y1; ^~ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:29:15: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:32:7: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. foo3(3, &y1); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx: In function 'void f2()': -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:48:11: warning: Static expression 'c1' passed to pointer or reference function argument of 'C::foo' within function 'void f2()'; may not be thread-safe. + ~~~~^~~~~~~~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx: In function 'void f2()': +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:51:9: warning: Static expression 'c1' passed to pointer or reference function argument of 'C::foo' within function 'void f2()'; may not be thread-safe. c1.foo(); - ^ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:43:10: note: Declared here: + ~~~~~~^~ +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:46:10: note: Declared here: static C c1; ^~ -/afs/cern.ch/user/s/ssnyder/atlas-work3/External/CheckerGccPlugins/test/thread3_test.cxx:48:11: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread3_test.cxx:51:9: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. c1.foo(); - ^ + ~~~~~~^~ diff --git a/External/CheckerGccPlugins/src/thread_plugin.cxx b/External/CheckerGccPlugins/src/thread_plugin.cxx index 9e278349..0c904c84 100644 --- a/External/CheckerGccPlugins/src/thread_plugin.cxx +++ b/External/CheckerGccPlugins/src/thread_plugin.cxx @@ -379,6 +379,7 @@ bool is_mutex (tree type) TFF_SCOPE + TFF_CHASE_TYPEDEF); if (strcmp (name, "std::mutex") == 0) return true; + if (strcmp (name, "std::shared_timed_mutex") == 0) return true; if (strcmp (name, "std::recursive_mutex") == 0) return true; if (strcmp (name, "TVirtualMutex") == 0) return true; return false; @@ -1088,6 +1089,9 @@ void check_pass_static_by_call (Attributes_t attribs, if (fndecl && is_atomic (arg_test) && is_atomic (DECL_CONTEXT (fndecl))) return; + // Ok if it's a mutex. + if (is_mutex (arg_test)) return; + // FNDECL could be null in the case of a call through a function pointer. // Try to print a nice error in that case. tree fnerr = fndecl; diff --git a/External/CheckerGccPlugins/test/thread3_test.cxx b/External/CheckerGccPlugins/test/thread3_test.cxx index 514b21d8..17b942ce 100644 --- a/External/CheckerGccPlugins/test/thread3_test.cxx +++ b/External/CheckerGccPlugins/test/thread3_test.cxx @@ -9,6 +9,9 @@ template <class T> struct atomic { atomic& operator++ (); T val() const; }; + +class mutex {}; +class shared_timed_mutex {}; } @@ -77,3 +80,11 @@ void f6() { foo4 (foo5); } + + +std::shared_timed_mutex m; +void f7 (std::shared_timed_mutex&); +void f8() +{ + f7(m); +} -- GitLab