https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/96295
From 0c57ad1ca36a841dff700eb98f878475e0243b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 21 Jun 2024 12:13:02 +0200 Subject: [PATCH 1/3] [clang][analyzer] Improve documentation of checker 'cplusplus.Move' (NFC) --- clang/docs/analyzer/checkers.rst | 39 +++++++++++++++++-- .../clang/StaticAnalyzer/Checkers/Checkers.td | 21 +++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index b8d5f372bdf61..445f434e1e6ce 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -420,21 +420,52 @@ around, such as ``std::string_view``. cplusplus.Move (C++) """""""""""""""""""" -Method calls on a moved-from object and copying a moved-from object will be reported. - +Find use-after-move bugs in C++. This includes method calls on moved-from +objects, assignment of a moved-from object, and repeated move of a moved-from +object. .. code-block:: cpp - struct A { + struct A { void foo() {} }; - void f() { + void f1() { A a; A b = std::move(a); // note: 'a' became 'moved-from' here a.foo(); // warn: method call on a 'moved-from' object 'a' } + void f2() { + A a; + A b = std::move(a); + A c(std::move(a)); // warn: move of an already moved-from object + } + + void f3() { + A a; + A b = std::move(a); + b = a; // warn: copy of moved-from object + } + +The checker option ``WarnOn`` controls on what objects the use-after-move is +checked. The most strict value is ``KnownsOnly``, in this mode only objects are +checked whose type is known to be move-unsafe. These include most STL objects +(but excluding move-safe ones) and smart pointers. With option value +``KnownsAndLocals`` local variables (of any type) are additionally checked. The +idea behind this is that local variables are usually not tempting to be re-used +so an use after move is more likely a bug than with member variables. With +option value ``All`` any use-after move condition is checked on all kinds of +variables, excluding global variables and known move-safe cases. Default value +is ``KnownsAndLocals``. + +Call of methods named ``empty()`` or ``isEmpty()`` are allowed on moved-from +objects because these methods are considered as move-safe. Functions called +``reset()``, ``destroy()``, ``clear()``, ``assign``, ``resize``, ``shrink`` are +treated as state-reset functions and are allowed on moved-from objects, these +make the object valid again. This applies to any type of object (not only STL +ones). + .. _cplusplus-NewDelete: cplusplus.NewDelete (C++) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 429c334a0b24b..6e224a4e098ad 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -686,22 +686,11 @@ def MoveChecker: Checker<"Move">, CheckerOptions<[ CmdLineOption<String, "WarnOn", - "In non-aggressive mode, only warn on use-after-move of " - "local variables (or local rvalue references) and of STL " - "objects. The former is possible because local variables (or " - "local rvalue references) are not tempting their user to " - "re-use the storage. The latter is possible because STL " - "objects are known to end up in a valid but unspecified " - "state after the move and their state-reset methods are also " - "known, which allows us to predict precisely when " - "use-after-move is invalid. Some STL objects are known to " - "conform to additional contracts after move, so they are not " - "tracked. However, smart pointers specifically are tracked " - "because we can perform extra checking over them. In " - "aggressive mode, warn on any use-after-move because the " - "user has intentionally asked us to completely eliminate " - "use-after-move in his code. Values: \"KnownsOnly\", " - "\"KnownsAndLocals\", \"All\".", + "With setting \"KnownsOnly\" warn only on objects with known " + "move semantics like smart pointers and other STL objects. " + "With setting \"KnownsAndLocals\" warn additionally on local " + "variables (or rvalue references). With setting \"All\" warn " + "on all variables (excluding global variables).", "KnownsAndLocals", Released> ]>, From 866655581a1e1f0779542737a3f9d427a8d067b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 21 Jun 2024 16:35:09 +0200 Subject: [PATCH 2/3] using bullet point list for option values --- clang/docs/analyzer/checkers.rst | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 445f434e1e6ce..42c097d973d53 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -449,17 +449,21 @@ object. } The checker option ``WarnOn`` controls on what objects the use-after-move is -checked. The most strict value is ``KnownsOnly``, in this mode only objects are -checked whose type is known to be move-unsafe. These include most STL objects -(but excluding move-safe ones) and smart pointers. With option value -``KnownsAndLocals`` local variables (of any type) are additionally checked. The -idea behind this is that local variables are usually not tempting to be re-used -so an use after move is more likely a bug than with member variables. With -option value ``All`` any use-after move condition is checked on all kinds of -variables, excluding global variables and known move-safe cases. Default value -is ``KnownsAndLocals``. - -Call of methods named ``empty()`` or ``isEmpty()`` are allowed on moved-from +checked: + +* The most strict value is ``KnownsOnly``, in this mode only objects are + checked whose type is known to be move-unsafe. These include most STL objects + (but excluding move-safe ones) and smart pointers. +* With option value ``KnownsAndLocals`` local variables (of any type) are + additionally checked. The idea behind this is that local variables are + usually not tempting to be re-used so an use after move is more likely a bug + than with member variables. +* With option value ``All`` any use-after move condition is checked on all + kinds of variables, excluding global variables and known move-safe cases. + +Default value is ``KnownsAndLocals``. + +Calls of methods named ``empty()`` or ``isEmpty()`` are allowed on moved-from objects because these methods are considered as move-safe. Functions called ``reset()``, ``destroy()``, ``clear()``, ``assign``, ``resize``, ``shrink`` are treated as state-reset functions and are allowed on moved-from objects, these From d1e29ae2910bdee1a2ffaaac04b80a957a3965c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 27 Jun 2024 11:37:05 +0200 Subject: [PATCH 3/3] corrected failing test --- .../Analysis/analyzer-checker-option-help.c | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/clang/test/Analysis/analyzer-checker-option-help.c b/clang/test/Analysis/analyzer-checker-option-help.c index 5f95569e58498..c95b8e7064c5f 100644 --- a/clang/test/Analysis/analyzer-checker-option-help.c +++ b/clang/test/Analysis/analyzer-checker-option-help.c @@ -34,27 +34,7 @@ // CHECK-STABLE: OPTIONS: // // CHECK-STABLE: cplusplus.Move:WarnOn -// CHECK-STABLE-SAME: (string) In non-aggressive mode, only warn -// CHECK-STABLLE: on use-after-move of local variables (or -// CHECK-STABLLE: local rvalue references) and of STL objects. -// CHECK-STABLLE: The former is possible because local variables -// CHECK-STABLLE: (or local rvalue references) are not tempting -// CHECK-STABLLE: their user to re-use the storage. The latter -// CHECK-STABLLE: is possible because STL objects are known -// CHECK-STABLLE: to end up in a valid but unspecified state -// CHECK-STABLLE: after the move and their state-reset methods -// CHECK-STABLLE: are also known, which allows us to predict -// CHECK-STABLLE: precisely when use-after-move is invalid. -// CHECK-STABLLE: Some STL objects are known to conform to -// CHECK-STABLLE: additional contracts after move, so they -// CHECK-STABLLE: are not tracked. However, smart pointers -// CHECK-STABLLE: specifically are tracked because we can -// CHECK-STABLLE: perform extra checking over them. In aggressive -// CHECK-STABLLE: mode, warn on any use-after-move because -// CHECK-STABLLE: the user has intentionally asked us to completely -// CHECK-STABLLE: eliminate use-after-move in his code. Values: -// CHECK-STABLLE: "KnownsOnly", "KnownsAndLocals", "All". -// CHECK-STABLLE: (default: KnownsAndLocals) +// CHECK-STABLE-SAME: (string) With setting "KnownsOnly" warn // CHECK-STABLE-NOT: debug.AnalysisOrder:* // CHECK-DEVELOPER: debug.AnalysisOrder:* _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits