From e8ecae31715af69ec9add3e57dd27959646992a3 Mon Sep 17 00:00:00 2001 From: Titus Winters Date: Thu, 3 Nov 2016 12:09:26 -0400 Subject: [PATCH 1/7] Added some example tooling discussion. --- CppCoreGuidelines.md | 60 +++++++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 6b6ee80..db9d232 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7324,9 +7324,9 @@ But heed the warning: [Avoid "naked" `union`s](#Ru-naked) ##### Example // Short string optimization - + constexpr size_t buffer_size = 16; // Slightly larger than the size of a pointer - + class Immutable_string { public: Immutable_string(const char* str) : @@ -7339,18 +7339,18 @@ But heed the warning: [Avoid "naked" `union`s](#Ru-naked) strcpy_s(string_ptr, size + 1, str); } } - + ~Immutable_string() { if (size >= buffer_size) delete string_ptr; } - + const char* get_str() const { return (size < buffer_size) ? string_buffer : string_ptr; } - + private: // If the string is short enough, we store the string itself // instead of a pointer to the string. @@ -7358,7 +7358,7 @@ But heed the warning: [Avoid "naked" `union`s](#Ru-naked) char* string_ptr; char string_buffer[buffer_size]; }; - + const size_t size; }; @@ -11635,16 +11635,40 @@ this can be a security risk. ##### Enforcement -Some is possible, do at least something. -There are commercial and open-source tools that try to address this problem, but static tools often have many false positives and run-time tools often have a significant cost. -We hope for better tools. +When possible, rely on tooling enforcement, but be aware that any tooling +solution has costs and blind spots. Defense in depth (multiple tools, multiple +approaches) is particularly valuable here. -Help the tools: +In the realm of static enforcement, +both [clang](http://clang.llvm.org/docs/ThreadSafetyAnalysis.html) and some +older verisons of [gcc](https://gcc.gnu.org/wiki/ThreadSafetyAnnotation) have +some support for static annotation of thread safety properties. Consistent use +of this technique turns many classes of thread-safety errors into compile-time +errors. The annotations are generally local (marking a particular member +variable as guarded by a particular mutex), and are usually easy to +learn. However, as with many static tools, it can often present false +negatives - cases that should have been caught but were allowed. -* less global data -* fewer `static` variables -* more use of stack memory (and don't pass pointers around too much) -* more immutable data (literals, `constexpr`, and `const`) +Clang's [Thread Sanitizer](http://clang.llvm.org/docs/ThreadSanitizer.html) (aka +tsan) is a powerful example of dynamic tools: it changes the build and execution +of your program to add bookkeeping on memory access, absolutely identifying data +races in a given execution of your binary. The cost for this is both memory +(5-10x in most cases) and CPU slowdown (2-20x). Dynamic tools like this are best +when applied to integration tests, canary pushes, or unittests that operate on +multiple threads. Workload matters: When tsan identifies a problem, it is +effectively always an actual data race, but it can only identify races seen in a +given execution. + +There are many other tools, both commercial and open-source. Thread safety is +challenging, often getting the better of experienced programmers: tooling is an +important strategy to mitigate those risks. + +There are other ways you can mitigate the chance of data races: + +* Avoid global data +* Avoid `static` variables +* More use of value types on the stack (and don't pass pointers around too much) +* More use of immutable data (literals, `constexpr`, and `const`) ### CP.3: Minimize explicit sharing of writable data @@ -12647,7 +12671,7 @@ Example with thread-safe static local variables of C++11. static My_class my_object; // Constructor called only once // ... } - + class My_class { public: @@ -12670,7 +12694,7 @@ Double-checked locking is easy to mess up. If you really need to write your own ##### Example, bad -Even if the following example works correctly on most hardware platforms, it is not guaranteed to work by the C++ standard. The x_init.load(memory_order_relaxed) call may see a value from outside of the lock guard. +Even if the following example works correctly on most hardware platforms, it is not guaranteed to work by the C++ standard. The x_init.load(memory_order_relaxed) call may see a value from outside of the lock guard. atomic x_init; @@ -12687,12 +12711,12 @@ Even if the following example works correctly on most hardware platforms, it is One of the conventional patterns is below. std::atomic state; - + // If state == SOME_ACTION_NEEDED maybe an action is needed, maybe not, we need to // check again in a lock. However, if state != SOME_ACTION_NEEDED, then we can be // sure that an action is not needed. This is the basic assumption of double-checked // locking. - + if (state == SOME_ACTION_NEEDED) { std::lock_guard lock(mutex); From 3768e82fc3b987e89a0cfacbbd77cbcaff4fdf30 Mon Sep 17 00:00:00 2001 From: Titus Winters Date: Thu, 10 Nov 2016 13:35:22 -0500 Subject: [PATCH 2/7] Fix spelling / expand dictionary. --- CppCoreGuidelines.md | 6 +++--- scripts/hunspell/isocpp.dic | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index db9d232..bafed29 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -11641,7 +11641,7 @@ approaches) is particularly valuable here. In the realm of static enforcement, both [clang](http://clang.llvm.org/docs/ThreadSafetyAnalysis.html) and some -older verisons of [gcc](https://gcc.gnu.org/wiki/ThreadSafetyAnnotation) have +older versions of [GCC](https://gcc.gnu.org/wiki/ThreadSafetyAnnotation) have some support for static annotation of thread safety properties. Consistent use of this technique turns many classes of thread-safety errors into compile-time errors. The annotations are generally local (marking a particular member @@ -11650,12 +11650,12 @@ learn. However, as with many static tools, it can often present false negatives - cases that should have been caught but were allowed. Clang's [Thread Sanitizer](http://clang.llvm.org/docs/ThreadSanitizer.html) (aka -tsan) is a powerful example of dynamic tools: it changes the build and execution +TSAN) is a powerful example of dynamic tools: it changes the build and execution of your program to add bookkeeping on memory access, absolutely identifying data races in a given execution of your binary. The cost for this is both memory (5-10x in most cases) and CPU slowdown (2-20x). Dynamic tools like this are best when applied to integration tests, canary pushes, or unittests that operate on -multiple threads. Workload matters: When tsan identifies a problem, it is +multiple threads. Workload matters: When TSAN identifies a problem, it is effectively always an actual data race, but it can only identify races seen in a given execution. diff --git a/scripts/hunspell/isocpp.dic b/scripts/hunspell/isocpp.dic index 1d67fbb..adc12cd 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -1,7 +1,9 @@ ' 0xFF0000 0b0101'0101 +10x '14 +20x 2D 2K 2ndEdition @@ -69,6 +71,7 @@ CComPtr cerr chrono cin +Clang's class' clib Cline99 @@ -492,6 +495,7 @@ toolchains TotallyOrdered TP tradeoff +TSAN TSs tt typeid From fda2d8879d495c3f1ea8643994870a79b791d9f9 Mon Sep 17 00:00:00 2001 From: Titus Winters Date: Thu, 10 Nov 2016 18:07:13 -0500 Subject: [PATCH 3/7] unittest is a word --- scripts/hunspell/isocpp.dic | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/hunspell/isocpp.dic b/scripts/hunspell/isocpp.dic index adc12cd..d4ae653 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -508,6 +508,7 @@ undetached unenforceable uninit uniqueptrparam +unittest unnamed2 use1 users' From a968af59d0be0047a2fcf7f6697e0cad8e8b9b8e Mon Sep 17 00:00:00 2001 From: Sergey Zubkov Date: Sun, 18 Dec 2016 14:46:11 -0500 Subject: [PATCH 4/7] C.138 initial content --- CppCoreGuidelines.md | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 8df991e..f861b15 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -6561,11 +6561,49 @@ This a relatively rare use because implementation can often be organized into a ##### Reason -??? +Without a using declaration, member functions in the derived class hide the entire inherited overload sets. -##### Example +##### Example, bad - ??? + #include + class B { + public: + virtual int f(int i) { std::cout << "f(int): "; return i; } + virtual double f(double d) { std::cout << "f(double): "; return d; } + }; + class D: public B { + public: + int f(int i) override { std::cout << "f(int): "; return i+1; } + }; + int main() + { + D d; + std::cout << d.f(2) << '\n'; // prints "f(int): 3" + std::cout << d.f(2.3) << '\n'; // prints "f(int): 3" + } + +##### Example, good + + class D: public B { + public: + int f(int i) override { std::cout << "f(int): "; return i+1; } + using B::f; // exposes f(double) + }; + +##### Note + +This issue affects both virtual and non-virtual member functions + +For variadic bases, C++17 introduced a variadic form of the using-declaration, + + template + struct Overloader : Ts... { + using Ts::operator()...; // exposes operator() from every base + }; + +##### Enforcement + +Diagnose name hiding ### C.139: Use `final` sparingly From f7aedf3d75514253fc8f12823600cc683af0d6ee Mon Sep 17 00:00:00 2001 From: Andrew Pardoe Date: Mon, 30 Jan 2017 11:55:59 -0800 Subject: [PATCH 5/7] Fixing typos in last commit. Thank you, Sergey & Jonathan. --- CppCoreGuidelines.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 770c235..3bf4366 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -3774,17 +3774,17 @@ Concrete types are also often referred to as value types to distinguish them fro Concrete type rule summary: -* [C.10: Prefer a concrete types over class hierarchies](#Rc-concrete) +* [C.10: Prefer concrete types over class hierarchies](#Rc-concrete) * [C.11: Make concrete types regular](#Rc-regular) -### C.10 Prefer a concrete types over class hierarchies +### C.10 Prefer concrete types over class hierarchies ##### Reason A concrete type is fundamentally simpler than a hierarchy: easier to design, easier to implement, easier to use, easier to reason about, smaller, and faster. You need a reason (use cases) for using a hierarchy. -n + ##### Example class Point1 { From 9ddb9e16489136c9b749bddbef64c908229cd40e Mon Sep 17 00:00:00 2001 From: Andrew Pardoe Date: Mon, 6 Feb 2017 11:51:03 -0800 Subject: [PATCH 6/7] Merge parts of PR #787 --- CppCoreGuidelines.md | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 5b06449..fe282cc 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -11701,30 +11701,6 @@ When possible, rely on tooling enforcement, but be aware that any tooling solution has costs and blind spots. Defense in depth (multiple tools, multiple approaches) is particularly valuable here. -In the realm of static enforcement, -both [clang](http://clang.llvm.org/docs/ThreadSafetyAnalysis.html) and some -older versions of [GCC](https://gcc.gnu.org/wiki/ThreadSafetyAnnotation) have -some support for static annotation of thread safety properties. Consistent use -of this technique turns many classes of thread-safety errors into compile-time -errors. The annotations are generally local (marking a particular member -variable as guarded by a particular mutex), and are usually easy to -learn. However, as with many static tools, it can often present false -negatives - cases that should have been caught but were allowed. - -Clang's [Thread Sanitizer](http://clang.llvm.org/docs/ThreadSanitizer.html) (aka -TSAN) is a powerful example of dynamic tools: it changes the build and execution -of your program to add bookkeeping on memory access, absolutely identifying data -races in a given execution of your binary. The cost for this is both memory -(5-10x in most cases) and CPU slowdown (2-20x). Dynamic tools like this are best -when applied to integration tests, canary pushes, or unittests that operate on -multiple threads. Workload matters: When TSAN identifies a problem, it is -effectively always an actual data race, but it can only identify races seen in a -given execution. - -There are many other tools, both commercial and open-source. Thread safety is -challenging, often getting the better of experienced programmers: tooling is an -important strategy to mitigate those risks. - There are other ways you can mitigate the chance of data races: * Avoid global data From 47e5764e6ef69c3477186f75c1dbd5c763bfd825 Mon Sep 17 00:00:00 2001 From: Andrew Pardoe Date: Mon, 6 Feb 2017 11:55:23 -0800 Subject: [PATCH 7/7] Update date --- CppCoreGuidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index fe282cc..7625979 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -1,6 +1,6 @@ # C++ Core Guidelines -February 1, 2017 +February 6, 2017 Editors: