diff --git a/External/CheckerGccPlugins/CMakeLists.txt b/External/CheckerGccPlugins/CMakeLists.txt index 49afe0d20ad2e0d5c5c9ac84012707ed6dec54fd..abd27d41b6bfc9051402992691ebd6f853830498 100644 --- a/External/CheckerGccPlugins/CMakeLists.txt +++ b/External/CheckerGccPlugins/CMakeLists.txt @@ -1,4 +1,4 @@ -# $Id: CMakeLists.txt 781116 2016-10-28 23:16:43Z ssnyder $ +# $Id: CMakeLists.txt 796948 2017-02-13 22:53:01Z ssnyder $ # The name of the package: atlas_subdir( CheckerGccPlugins ) @@ -66,6 +66,7 @@ CheckerGccPlugins_test( thread5 ) CheckerGccPlugins_test( thread6 ) CheckerGccPlugins_test( thread7 ) CheckerGccPlugins_test( thread8 ) +CheckerGccPlugins_test( thread9 ) #CheckerGccPlugins_test( usingns1 ) #CheckerGccPlugins_test( usingns2 ) #CheckerGccPlugins_test( usingns3 ) diff --git a/External/CheckerGccPlugins/THREAD_CHECKS b/External/CheckerGccPlugins/THREAD_CHECKS index 9303f101248581702f8bbd4937fdc14ae5ee8daa..d5307d478294bdb629338aa016fa5319b8a748cc 100644 --- a/External/CheckerGccPlugins/THREAD_CHECKS +++ b/External/CheckerGccPlugins/THREAD_CHECKS @@ -138,3 +138,93 @@ void f2(const int* y) - Adding a not_const_thread_safe attribute to the function, when it discards const from something that is not an argument. + + +========================================================================= +check_mutable (thread6_test) + +Check for writing to a mutable class member, or for taking +a non-const reference to a mutable class member, within a const function. + +Examples: + +void foo(int&); +struct S +{ + mutable int x; + void f1(int y) const { x = y; } + void f2() const { foo(x); } +}; + + + Can suppress by: + + - Making the function non-const. + + - Adding a thread_safe attribute to the member declaration. + + - Adding a not_thread_safe attribute to the function. + + +========================================================================= +mutable/static checks (thread7_test) + +TBD + +========================================================================= +check_discarded_const_from_return (thread8_test) + + +Check for discarding const from a pointer/reference in a function return. + +Example: + +const int* xx(); +int* f1() +{ + return const_cast<int*>(xx()); +} + + Can suppress by: + + - Adding a not_thread_safe attribute to the function. + + - Adding a not_const_thread_safe attribute to the function. + + +========================================================================= +check_returns (thread9_test) + +Check for returning a non-const data member from a const function. + +Example: + +struct S +{ + int* x; + int* f1() const; +}; + + +int* S::f1() const +{ + return x; +} + + +NB: Doesn't find all the ways in which such a member can leak. + We don't diagnose, for example: + +int* foo(int* x) { return x; } +int* S::f1() const +{ + return foo(x); +} + + + Can suppress by: + + - Adding a not_const_thread_safe attribute to the function. + + - Making the function non-const. + diff --git a/External/CheckerGccPlugins/share/thread6_test.ref b/External/CheckerGccPlugins/share/thread6_test.ref index b050335fb26e1892cf7b4029dd1a8ceeff0f7736..aa4b3ac7b82d3866045d477be0829987b248e4e6 100644 --- a/External/CheckerGccPlugins/share/thread6_test.ref +++ b/External/CheckerGccPlugins/share/thread6_test.ref @@ -1,40 +1,40 @@ /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In member function 'void S::f1(int) const': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:27:8: warning: Setting mutable field 'S::x' within thread-safe function 'void S::f1(int) const'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:28:5: warning: Setting mutable field 'S::x' within thread-safe function 'void S::f1(int) const'; may not be thread-safe. x = y; - ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:27:8: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + ~~^~~ +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:28:5: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In function 'void f2(const S*)': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:33:12: warning: Setting mutable field 'S::x' within thread-safe function 'void f2(const S*)'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:34:8: warning: Setting mutable field 'S::x' within thread-safe function 'void f2(const S*)'; may not be thread-safe. s->x = 10; - ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:33:12: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + ~~~~~^~~~ +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:34:8: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In function 'void f4(const S*)': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:40:5: warning: Taking non-const reference to mutable field 'S::x' within thread-safe function 'void f4(const S*)'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:41:5: warning: Taking non-const reference to mutable field 'S::x' within thread-safe function 'void f4(const S*)'; may not be thread-safe. f3(&s->x); - ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:40:5: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + ~~^~~~~~~ +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:41:5: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In function 'int* f5(const S*)': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:45:14: warning: Taking non-const reference to mutable field 'S::x' within thread-safe function 'int* f5(const S*)'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:46:14: warning: Taking non-const reference to mutable field 'S::x' within thread-safe function 'int* f5(const S*)'; may not be thread-safe. return &s->x; ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:45:14: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:46:14: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In member function 'void S::f6(int) const': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:51:10: warning: Setting mutable field 'S::b' within thread-safe function 'void S::f6(int) const'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:52:7: warning: Setting mutable field 'S::b' within thread-safe function 'void S::f6(int) const'; may not be thread-safe. b.z = y; - ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:51:10: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + ~~~~^~~ +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:52:7: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In function 'int* f7(const S*)': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:56:16: warning: Taking non-const reference to mutable field 'S::b' within thread-safe function 'int* f7(const S*)'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:57:16: warning: Taking non-const reference to mutable field 'S::b' within thread-safe function 'int* f7(const S*)'; may not be thread-safe. return &s->b.z; ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:56:16: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:57:16: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In function 'int* f8(const S*)': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:61:20: warning: Taking non-const reference to mutable field 'S::bb' within thread-safe function 'int* f8(const S*)'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:62:20: warning: Taking non-const reference to mutable field 'S::bb' within thread-safe function 'int* f8(const S*)'; may not be thread-safe. return &s->bb[2].z; ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:61:20: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:62:20: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. /home/sss/atlas/rootaccess/../test/thread6_test.cxx: In member function 'void S::f9(int) const': -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:67:14: warning: Setting mutable field 'S::bb' within thread-safe function 'void S::f9(int) const'; may not be thread-safe. +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:68:11: warning: Setting mutable field 'S::bb' within thread-safe function 'void S::f9(int) const'; may not be thread-safe. bb[0].z = y; - ^ -/home/sss/atlas/rootaccess/../test/thread6_test.cxx:67:14: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + ~~~~~~~~^~~ +/home/sss/atlas/rootaccess/../test/thread6_test.cxx:68:11: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. diff --git a/External/CheckerGccPlugins/share/thread9_test.ref b/External/CheckerGccPlugins/share/thread9_test.ref new file mode 100644 index 0000000000000000000000000000000000000000..2c8ec47fd0d7576fa190a9ba05930b3bd736e59b --- /dev/null +++ b/External/CheckerGccPlugins/share/thread9_test.ref @@ -0,0 +1,10 @@ +/home/sss/atlas/rootaccess/../test/thread9_test.cxx: In member function 'int* S::f1() const': +/home/sss/atlas/rootaccess/../test/thread9_test.cxx:19:10: warning: Returning non-const pointer or reference member 'S::x' from structure 'const struct S' within const member function 'int* S::f1() const'; may not be thread-safe. + return y; + ^ +/home/sss/atlas/rootaccess/../test/thread9_test.cxx:8:8: note: Declared here: + int* x; + ^ +/home/sss/atlas/rootaccess/../test/thread9_test.cxx:19:10: note: See <https://twiki.cern.ch/twiki/bin/view/AtlasComputing/CheckerGccPlugins#thread_plugin>. + return y; + ^ diff --git a/External/CheckerGccPlugins/src/thread_plugin.cxx b/External/CheckerGccPlugins/src/thread_plugin.cxx index bd9512f3d895a664c013f03db5733552f3f7ada0..e283125faf3cbee31ed0fde1d89781275b3f5516 100644 --- a/External/CheckerGccPlugins/src/thread_plugin.cxx +++ b/External/CheckerGccPlugins/src/thread_plugin.cxx @@ -635,11 +635,81 @@ void check_calls (gimplePtr stmt, function* fun) } +// Test to see if a pointer value comes directly or indirectly from +// a structure. Return the structure object if so, and set FIELD +// to thee referenced member. +tree pointer_from_struct (tree val, tree& field) +{ + if (TREE_CODE (val) == COMPONENT_REF) { + field = TREE_OPERAND (val, 1); + return get_inner (val); + } + + tree valtest = get_inner (val); + tree valtype = TREE_TYPE (valtest); + if (!POINTER_TYPE_P(valtype)) + return NULL_TREE; + + if (TREE_CODE (val) == ADDR_EXPR) + val = TREE_OPERAND (val, 0); + if (TREE_CODE (val) == MEM_REF) + val = TREE_OPERAND (val, 0); + + if (TREE_CODE (val) != SSA_NAME) return NULL_TREE; + + gimplePtr stmt = SSA_NAME_DEF_STMT (val); + if (!stmt) return NULL_TREE; + //debug_gimple_stmt (stmt); + //fprintf (stderr, "code %s\n", get_tree_code_name(gimple_expr_code(stmt))); + + if (is_gimple_assign (stmt) && (gimple_expr_code(stmt) == VAR_DECL || + gimple_expr_code(stmt) == PARM_DECL || + gimple_expr_code(stmt) == POINTER_PLUS_EXPR || + gimple_expr_code(stmt) == ADDR_EXPR || + gimple_expr_code(stmt) == COMPONENT_REF)) + { + //fprintf (stderr, "recurse\n"); + return pointer_from_struct (gimple_op(stmt, 1), field); + } + else if (gimple_code (stmt) == GIMPLE_PHI) { + size_t nop = gimple_num_ops (stmt); + for (size_t i = 0; i < nop; i++) { + tree op = gimple_op (stmt, i); + tree ret = pointer_from_struct (op, field); + if (ret) return ret; + } + } + return NULL_TREE; +} + + +void check_returns (gimplePtr stmt, function* fun) +{ + if (gimple_code (stmt) != GIMPLE_RETURN) return; + //debug_gimple_stmt (stmt); + tree retval = gimple_op (stmt, 0); + tree field = NULL_TREE; + tree s = pointer_from_struct (retval, field); + if (s && TREE_READONLY (TREE_TYPE (s))) { + warning_at (gimple_location (stmt), 0, + "Returning non-const pointer or reference member %<%E%> from structure %<%D%> within const member function %<%D%>; may not be thread-safe.", + field, TREE_TYPE (s), fun->decl); + inform (DECL_SOURCE_LOCATION (field), "Declared here:"); + CheckerGccPlugins::inform_url (gimple_location (stmt), url); + } +} + + unsigned int thread_pass::thread_execute (function* fun) { if (!check_thread_safety_p (fun->decl)) return 0; + const bool static_memfunc_p = DECL_CONST_MEMFUNC_P (fun->decl); + tree rettype = TREE_TYPE (DECL_RESULT (fun->decl)); + const bool nonconst_pointer_return_p = POINTER_TYPE_P (rettype) && !TYPE_READONLY (TREE_TYPE (rettype)); + const bool not_const_thread_safe = has_attrib (fun, "not_const_thread_safe"); + basic_block bb; FOR_EACH_BB_FN(bb, fun) { for (gimple_stmt_iterator si = gsi_start_bb (bb); @@ -652,6 +722,10 @@ unsigned int thread_pass::thread_execute (function* fun) check_direct_static_use (stmt, fun); check_assignments (stmt, fun); check_calls (stmt, fun); + + if (static_memfunc_p && + nonconst_pointer_return_p && !not_const_thread_safe) + check_returns (stmt, fun); } } diff --git a/External/CheckerGccPlugins/test/thread6_test.cxx b/External/CheckerGccPlugins/test/thread6_test.cxx index 31f0bbf8846bb630d758194245b42c6d0176f6fd..9af27810950b782c4792d24a1b6f22523424f53c 100644 --- a/External/CheckerGccPlugins/test/thread6_test.cxx +++ b/External/CheckerGccPlugins/test/thread6_test.cxx @@ -1,4 +1,4 @@ -// testing mutable checks +// testing check_mutable #pragma ATLAS check_thread_safety @@ -19,6 +19,7 @@ struct S void f6(int y) const; void f9(int y) const; void f12(int y); + void f15 [[gnu::not_thread_safe]] (int y) const; }; @@ -92,3 +93,7 @@ void f14(S* s) } +void S::f15(int y) const +{ + x = y; +} diff --git a/External/CheckerGccPlugins/test/thread9_test.cxx b/External/CheckerGccPlugins/test/thread9_test.cxx new file mode 100644 index 0000000000000000000000000000000000000000..9f34d85803e2523eb5b03d019bbe1d65d5c59c58 --- /dev/null +++ b/External/CheckerGccPlugins/test/thread9_test.cxx @@ -0,0 +1,41 @@ +// testing check_returns + +#pragma ATLAS check_thread_safety + + +struct S +{ + int* x; + int* f1() const; + int* f2 [[gnu::not_const_thread_safe]] () const; + const int* f3() const; + int* f4(); +}; + + +int* S::f1() const +{ + int* y = x; + return y; +} + + +int* S::f2() const +{ + int* y = x; + return y; +} + + +const int* S::f3() const +{ + return x; +} + + +int* S::f4() +{ + return x; +} + +