[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
ConcreteCactus wrote: > To keep everything organized, I'd suggest creating a new branch for CSA > checker and close this PR (you can always reopen it later). > > For the CSA part, you can start by reading [this developer > guide](https://clang-analyzer.llvm.org/checker_dev_manual.html) (probably a > bit outdated?). This [original white > paper](https://lcs.ios.ac.cn/~xzx/memmodel.pdf) is very helpful in > understanding the core principles of the CSA and how it all started. > [haoNoQ's guide](https://github.com/haoNoQ/clang-analyzer-guide) gives > excellent practical description of how to write checkers and what functions > the CSA has. Thank you, I'll open a new PR when I'm ready with the CSA checker. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
https://github.com/ConcreteCactus closed https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
vbvictor wrote: > Should I close this PR, and open a new one when I'm ready with the CSA > checker? To keep everything organized, I'd suggest creating a new branch for CSA checker and close this PR (you can always reopen it later). For the CSA part, you can start by reading [this developer guide](https://clang-analyzer.llvm.org/checker_dev_manual.html) (probably a bit outdated?). This [original white paper](https://lcs.ios.ac.cn/~xzx/memmodel.pdf) is very helpful in understanding the core principles of the CSA and how it all started. [haoNoQ's guide](https://github.com/haoNoQ/clang-analyzer-guide) gives excellent practical description of how to write checkers and what functions the CSA has. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
ConcreteCactus wrote: Hi Everyone, Thank you for the detailed suggestions. Now that I have a better idea of what CSA is, I also think that this check could be better implemented in clang-static-analyzer. Being able to reduce the number of false positives using path sensitive analysis is in my opinion the most important advantage of using CSA. What do you think? Should I close this PR, and open a new one when I'm ready with the CSA checker? By the way, I don't have any users. This check is more like a University project I picked. The reason I started with clang-tidy is simply because that was the suggestion my teacher gave me. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
vbvictor wrote: Thank you for your long and detailed answer! Indeed, the check can benefit a lot from path-sensitive analysis: - Easier traversing with less boilerplate code. - Easier handling of edge-cases like struct fields and unions, support of local variables. - Fewer false-positives with more positives. So from my perspective, this check can live as a clang-tidy check. However, looking at all benefits of path-sensitive analysis of CSA, the check would look a _lot better_ as a CSA check. I understand that CSA checks might be harder to write since CSA itself has higher entry threshold than clang-tidy, but it's hard to give a thoughtful and careful review of such big clang-tidy check, especially that has non-trivial manual AST-visiting. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
haoNoQ wrote: > `// To represent fields in structs or unions we use numbered FieldIndices.` This piece of complexity is also trivialized by path sensitive analysis. You'll simply ask whether it's an access to the same memory address or not. No need to describe what the address is or who owns it. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
haoNoQ wrote:
Ok I kinda forgot about the elephant in the room.
With path sensitivity you'll be able to handle more kinds of variables. Not
just global variables but also stuff in heap memory. As well as someone else's
stack variables passed by reference down the stack. Or it could be, like, just
*some* pointer or reference passed to us from god knows where. We don't have to
know where and by whom exactly the memory it points to has been allocated. We
just need to know whether it's the same memory address at two different points
of the program.
The static analyzer will model such aliasing for you and give you that
information for free, so that at any point of time you know which pointer
points to which storage, even if you can't (or don't want to) describe that
storage. It'd magically work correctly even if that information depends on the
exact execution path up to that point.
This could make your check significantly more powerful. And that's very hard
without path sensitivity.
FWIW one case you can probably handle without path sensitivity is: member
variables inside a method. Eg., in
```c++
class MyClass {
int memberVar;
public:
int foo() {
memberVar++;
return memberVar;
}
void bar() {
memberVar + foo(); // should warn
}
};
```
you don't really need to know what `this` points to, as long as you confirm
that it points to the same thing inside `foo()` and `bar()`. Which is trivial
because the `foo()` invocation inside `bar()` is invoked directly on `this`.
This is so easy because the `this` pointer cannot be overwritten at runtime, so
you don't have to worry about code like `this = &other`.
You can probably handle references similarly, since they also cannot be
reassigned at runtime. It doesn't matter that you don't know what the reference
refers to:
```c++
int foo(int &refVar) {
refVar++;
return refVar;
}
void bar(int &refVar) {
refVar + foo(refVar); // should warn
}
```
But when it comes to pointers and heap storage, you'll probably have a very
hard time with path sensitivity.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
haoNoQ wrote:
> Looking at the check code in general, I have some concerns whether this check
> should be implemented as a Clang Static Analyzer (CSA) check instead of a
> clang-tidy check. My main reason is that we now have a lot of manual node
> traversing, even an `ExecutionVisitor`. This whole _traversing part_ must be
> already implemented in the core engine of the CSA. Even in [Getting
> Involved](https://clang.llvm.org/extra/clang-tidy/Contributing.html#choosing-the-right-place-for-your-check)
> guide it mentions that CSA should be used for control flow analysis, and we
> clearly have it here.
>
> I kindly ask for an opinion of clang-tidy maintainers on this matter,
> @HerrCai0907 @carlosgalvezp @PiotrZSL.
>
> Also, kindly ask CSA maintainers to take a brief look at this checker. From
> your point of view, will it be easier and less code consuming to implement
> such checker as a part of CSA? @steakhal @NagyDonat @haoNoQ.
Oh good question. Generally it's a bit more nuanced than that. The static
analyzer is doing a lot more than AST traversal; it's doing so-called "path
sensitive" analysis which tries to enumerate different *execution paths* in the
program. Which is very good at finding realistic execution paths on which
something "bad" *may* happen, and not suitable for all other kinds of analysis.
Yes, I think this checker could be implemented as a path-sensitive checker in
the static analyzer. It does indeed search for "bad" things that *may* happen,
which is exactly what the path-sensitive engine is good at. Path-sensitive
analysis is more precise, it may help you naturally eliminate certain classes
of false positives that would be extremely difficult to eliminate manually.
However, it's also significantly more expensive, so it'll run slower for the
users who aren't already using path-other sensitive checks (the performance
cost will be negligible for users who are already enabling at least one other
path-sensitive checker). Additionally, because of being more expensive and
explosive in complexity, it may suffer from more false negatives, simply
because it's running out of the analysis "budget".
This won't necessarily eliminate a lot of code from the checker. Path-sensitive
checkers often end up more simple and elegant. You'll still need to identify
the places where the execution may be unsequenced, we won't do that for you.
But then you'll put markers in the path-sensitive state when you're inside
those statements, and focus on responding to global variable accesses while
those markers are present. We'll do the rest of the traversal for you.
Path-sensitive analysis may also save you a lot of follow-up polish work (I
haven't looked at how much of that you've already done).
Now, in practice, the best thing about path-sensitive analysis is that it
naturally helps you avoid false positives of the following nature:
```c++
int globalVar;
int foo(bool shouldInc) {
if (shouldInc)
globalVar++;
return globalVar;
}
void bar() {
globalVar + foo(false); // should not warn
}
```
In this case the problematic mutation cannot happen in practice because the
branch on which the increment happens is entirely unreachable in the context of
`bar()`. The function `foo()` may increment the variable in other situations
but it will never do that when called from inside `bar()` specifically.
These kinds of false positives may happen in a variety of different ways and
you'll have a very hard time pattern-matching them all down on the AST level
without a powerful principled solution that the path-sensitive engine
implements. If you're using path-sensitive analysis, the false positive is
eliminated automatically, by construction, without any effort on your part. You
will simply never notice the increment of the global variable when you're
analyzing `bar()` so you won't have to worry about reacting to it incorrectly.
The analyzer simply knows that this execution path does not exist, so it'll
never show it to you.
Whether you actually worry about these false positives, is entirely up to you
though. It's a matter of your relationship with your users, a matter of
understanding their demands, and also a matter of observing the behavior of the
checker on real-world code. I cannot answer that question for you, you'll need
to experiment.
We've had a similar situation in our "don't call virtual methods on a partially
constructed object" checker
(https://clang.llvm.org/docs/analyzer/checkers.html#cplusplus-purevirtualcall-c,
https://clang.llvm.org/docs/analyzer/checkers.html#optin-cplusplus-virtualcall).
It turned out that in many cases the code was deliberately written in such a
way that the constructor may call a function in turn may eventually call a
virtual method but in reality the constructor passes a flag that makes sure the
virtual method is never called from inside that function. So our initial
attempt to avoid path-sensitive analysis has fai
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
+that are inside the same translation unit. Global unions and structs are also
+handled. For example:
+
+.. code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+In the above example, the struct fields ``A`` and ``B`` are treated as
+separate global variables, while an access (i.e. read or write) to the struct
+itself is treated as an access to both ``A`` and ``B``.
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+ When ``true``, treat function calls with mutable reference or pointer
+ parameters as writes to the parameter.
+
+ The default value is ``false``.
EugeneZelenko wrote:
```suggestion
Default is `false`.
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
+that are inside the same translation unit. Global unions and structs are also
+handled. For example:
+
+.. code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+In the above example, the struct fields ``A`` and ``B`` are treated as
+separate global variables, while an access (i.e. read or write) to the struct
+itself is treated as an access to both ``A`` and ``B``.
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+ When ``true``, treat function calls with mutable reference or pointer
+ parameters as writes to the parameter.
+
+ The default value is ``false``.
+
+ For example, the following code block will get flagged if
+ ``HandleMutableFunctionParametersAsWrites`` is ``true``:
+
+ .. code-block:: c++
+
+ void func(int& a);
+ int globalVar;
+
+ int main() {
+ // func could write to globalVar here
+ int a = globalVar + func(globalVar);
+ }
+
+ When ``HandleMutableFunctionParametersAsWrites`` is set to `true`, the
EugeneZelenko wrote:
```suggestion
When :option:`HandleMutableFunctionParametersAsWrites` is set to `true`, the
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
+that are inside the same translation unit. Global unions and structs are also
+handled. For example:
+
+.. code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+In the above example, the struct fields ``A`` and ``B`` are treated as
+separate global variables, while an access (i.e. read or write) to the struct
+itself is treated as an access to both ``A`` and ``B``.
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+ When ``true``, treat function calls with mutable reference or pointer
+ parameters as writes to the parameter.
+
+ The default value is ``false``.
+
+ For example, the following code block will get flagged if
+ ``HandleMutableFunctionParametersAsWrites`` is ``true``:
EugeneZelenko wrote:
```suggestion
:option:`HandleMutableFunctionParametersAsWrites` is `true`:
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
+that are inside the same translation unit. Global unions and structs are also
+handled. For example:
+
+.. code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+In the above example, the struct fields ``A`` and ``B`` are treated as
+separate global variables, while an access (i.e. read or write) to the struct
+itself is treated as an access to both ``A`` and ``B``.
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+ When ``true``, treat function calls with mutable reference or pointer
+ parameters as writes to the parameter.
+
+ The default value is ``false``.
+
+ For example, the following code block will get flagged if
+ ``HandleMutableFunctionParametersAsWrites`` is ``true``:
+
+ .. code-block:: c++
+
+ void func(int& a);
+ int globalVar;
+
+ int main() {
+ // func could write to globalVar here
+ int a = globalVar + func(globalVar);
+ }
+
+ When ``HandleMutableFunctionParametersAsWrites`` is set to `true`, the
+ ``func(globalVar)`` call is treated as a write to ``globalVar``. Because no
+ sequencing is defined for the ``+`` operator, a write to ``globalVar``
+ inside ``c`` would be undefined behavior.
+
+ When ``HandleMutableFunctionParametersAsWrites`` is set to ``false``, the
EugeneZelenko wrote:
```suggestion
When :option:`HandleMutableFunctionParametersAsWrites` is set to `false`, the
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
+that are inside the same translation unit. Global unions and structs are also
+handled. For example:
+
+.. code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+In the above example, the struct fields ``A`` and ``B`` are treated as
+separate global variables, while an access (i.e. read or write) to the struct
+itself is treated as an access to both ``A`` and ``B``.
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+ When ``true``, treat function calls with mutable reference or pointer
EugeneZelenko wrote:
```suggestion
When `true`, treat function calls with mutable reference or pointer
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,100 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the ``-Wunsequenced`` Clang
warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well. For example:
+
+.. code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+Because there is no sequencing defined for the ``-`` operator, ``a`` and
``a++``
+could be evaluated in any order. The compiler can even interleave the
evaluation
+of the sides as this is undefined behavior. The above code would generate a
+warning when ``-Wunsequenced`` (or ``-Wsequence-point`` in GCC) is enabled.
+
+However, global variables allow for more complex scenarios that
+``-Wunsequenced`` doesn't detect. E.g.
+
+.. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This clang-tidy check attempts to detect such cases. It recurses into functions
EugeneZelenko wrote:
```suggestion
This check attempts to detect such cases. It recurses into functions
```
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
steakhal wrote: CC @michael-jabbour-sonarsource https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
https://github.com/ConcreteCactus updated
https://github.com/llvm/llvm-project/pull/130421
From 0e9da921d39573f5a7c920b2bdd572b83f194e7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81ron=20H=C3=A1rn=C3=A1si?=
Date: Fri, 22 Nov 2024 21:43:04 +0100
Subject: [PATCH] [clang-tidy] Added bugprone-unsequenced-global-accesses check
This check attempts to detect unsequenced accesses to global
variables. It recurses into function calls in the same translation unit,
and can handle fields on global structs/unions.
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt| 1 +
.../UnsequencedGlobalAccessesCheck.cpp| 741 ++
.../bugprone/UnsequencedGlobalAccessesCheck.h | 35 +
.../clang-tidy/cert/CERTTidyModule.cpp| 6 +
.../clang-tidy/utils/ExecutionVisitor.h | 195 +
clang-tools-extra/docs/ReleaseNotes.rst | 14 +
.../bugprone/unsequenced-global-accesses.rst | 100 +++
.../docs/clang-tidy/checks/cert/exp30-c.rst | 10 +
.../docs/clang-tidy/checks/cert/exp50-cpp.rst | 10 +
.../docs/clang-tidy/checks/list.rst | 3 +
.../bugprone/unsequenced-global-accesses.cpp | 601 ++
12 files changed, 1719 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.h
create mode 100644 clang-tools-extra/clang-tidy/utils/ExecutionVisitor.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsequenced-global-accesses.rst
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cert/exp30-c.rst
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cert/exp50-cpp.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsequenced-global-accesses.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index ed1fd138d8f1b..86e51dfd18f54 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -95,6 +95,7 @@
#include "UnintendedCharOstreamOutputCheck.h"
#include "UniquePtrArrayMismatchCheck.h"
#include "UnsafeFunctionsCheck.h"
+#include "UnsequencedGlobalAccessesCheck.h"
#include "UnusedLocalNonTrivialVariableCheck.h"
#include "UnusedRaiiCheck.h"
#include "UnusedReturnValueCheck.h"
@@ -128,6 +129,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-chained-comparison");
CheckFactories.registerCheck(
"bugprone-compare-pointer-to-member-virtual-function");
+CheckFactories.registerCheck(
+"bugprone-unsequenced-global-accesses");
CheckFactories.registerCheck(
"bugprone-copy-constructor-init");
CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index d862794cde323..8f361ba9eb5f3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -96,6 +96,7 @@ add_clang_library(clangTidyBugproneModule STATIC
UnhandledSelfAssignmentCheck.cpp
UniquePtrArrayMismatchCheck.cpp
UnsafeFunctionsCheck.cpp
+ UnsequencedGlobalAccessesCheck.cpp
UnusedLocalNonTrivialVariableCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
new file mode 100644
index 0..28bb43d5c4c7d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
@@ -0,0 +1,741 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UnsequencedGlobalAccessesCheck.h"
+
+#include "../utils/ExecutionVisitor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+// An AccesKind represents one access to a global variable.
+//
+// The unchecked versions represent reads/writes that are not handled by
+// -Wunsequenced. (e.g. accesses inside functions).
+enum AccessKind : uint8_t { AkRead = 0, AkWrite, AkLast };
+
+static constexpr uint8_t AkCount = AkLast;
+
+// The TraversalResultKind represents a set of accesses.
+// Bits are corresponding to the AccessKind enum values.
+using TraversalResultKind = uint8_t;
+static constexpr TraversalResultKind TrInvalid = 0;
+static constexpr TraversalResultKind TrRead = 1 << AkRead;
+static constexpr TraversalResultKind TrWrite =
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
+
+void func(int& a);
ConcreteCactus wrote:
Thank you I'll test and fix the documentation and the formatting issues. I'll
run clang-tidy as well.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
+
+void func(int& a);
vbvictor wrote:
You can read full guide at
https://clang.llvm.org/extra/clang-tidy/Contributing.html, it says how and what
you should install and do.
Also, Fix clang-format issues and clang-tidy issues (if present), Currently
there isn't a CI job for clang-tidy run so please do it locally.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h --
clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.h
clang-tools-extra/clang-tidy/utils/ExecutionVisitor.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsequenced-global-accesses.cpp
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
``
View the diff from clang-format here.
``diff
diff --git
a/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
index e275df1c4..28bb43d5c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsequencedGlobalAccessesCheck.cpp
@@ -19,11 +19,7 @@ namespace {
//
// The unchecked versions represent reads/writes that are not handled by
// -Wunsequenced. (e.g. accesses inside functions).
-enum AccessKind : uint8_t {
- AkRead = 0,
- AkWrite,
- AkLast
-};
+enum AccessKind : uint8_t { AkRead = 0, AkWrite, AkLast };
static constexpr uint8_t AkCount = AkLast;
@@ -307,7 +303,7 @@ void UnsequencedGlobalAccessesCheck::check(
const llvm::SmallVector &Globals =
Visitor.getGlobalsFound();
- for (const TraversalAggregation& Global : Globals)
+ for (const TraversalAggregation &Global : Globals)
if (Global.shouldBeReported())
diag(E->getBeginLoc(), "read/write conflict on global variable " +
Global.getDeclName().getAsString());
@@ -315,7 +311,7 @@ void UnsequencedGlobalAccessesCheck::check(
const llvm::SmallVector &ObjectGlobals =
Visitor.getObjectGlobalsFound();
- for (const ObjectTraversalAggregation& ObjectGlobal : ObjectGlobals)
+ for (const ObjectTraversalAggregation &ObjectGlobal : ObjectGlobals)
if (ObjectGlobal.shouldBeReported())
diag(E->getBeginLoc(), "read/write conflict on the field of the global "
"object " +
@@ -541,7 +537,7 @@ void GlobalRWVisitor::addGlobal(DeclarationName Name,
SourceLocation Loc,
bool IsWrite) {
AccessKind Access = IsWrite ? AkWrite : AkRead;
- for (TraversalAggregation& Global : GlobalsFound) {
+ for (TraversalAggregation &Global : GlobalsFound) {
if (Global.getDeclName() == Name) {
Global.addGlobalRW(Loc, Access, TraversalIndex);
return;
@@ -556,7 +552,7 @@ void GlobalRWVisitor::addField(DeclarationName Name,
bool IsWrite) {
AccessKind Access = IsWrite ? AkWrite : AkRead;
- for (ObjectTraversalAggregation& ObjectGlobal : ObjectGlobalsFound) {
+ for (ObjectTraversalAggregation &ObjectGlobal : ObjectGlobalsFound) {
if (ObjectGlobal.getDeclName() == Name) {
ObjectGlobal.addFieldRW(Loc, FieldIndices, Access, TraversalIndex);
return;
@@ -607,8 +603,7 @@ void TraversalAggregation::addGlobalRW(SourceLocation Loc,
AccessKind Access,
break;
}
case AkRead: {
-if (!(MainPart.Kind & TrWrite) &&
-(OtherPart.Kind & TrWrite))
+if (!(MainPart.Kind & TrWrite) && (OtherPart.Kind & TrWrite))
MainPart = OtherPart;
OtherPart = TraversalResult(Index, Loc, Access);
break;
@@ -630,8 +625,7 @@ bool TraversalAggregation::hasTwoAccesses() const {
}
bool TraversalAggregation::hasConflictingOperations() const {
- return hasTwoAccesses() &&
- ((MainPart.Kind | OtherPart.Kind) & TrWrite);
+ return hasTwoAccesses() && ((MainPart.Kind | OtherPart.Kind) & TrWrite);
}
bool TraversalAggregation::shouldBeReported() const {
``
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
+
+void func(int& a);
vbvictor wrote:
You need Sphinx installed in your system.
Please test your docs that they look correct.
https://clang.llvm.org/extra/clang-tidy/Contributing.html#documenting-your-check
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
+
+void func(int& a);
ConcreteCactus wrote:
No, my ninja can't find target `docs-clang-tools-html`. Maybe I didn't enable
its project when I was generating with cmake.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,86 @@
+.. title:: clang-tidy - bugprone-unsequenced-global-accesses
+
+bugprone-unsequenced-global-accesses
+
+
+Finds unsequenced actions (i.e. unsequenced write and read/write)
+on global variables nested in functions in the same translation unit.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This check is similar to the `-Wunsequenced` Clang warning,
+however it only looks at global variables and therefore can find unsequenced
+actions recursively inside function calls as well.
+
+For example code-block:: c++
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+`-Wunsequenced` doesn't detect. E.g. code-block:: c++
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This check attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. The flagged cases can overlap with
+`-Wunsequenced`. Global unions and structs are also handled.
+For example code-block:: c++
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+~~~
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+When `true`, treat function calls with mutable reference or pointer parameters
vbvictor wrote:
Whole description of the option should be with 2 spaces ident
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@ +.. title:: clang-tidy - bugprone-conflicting-global-accesses + +bugprone-conflicting-global-accesses + + +Finds conflicting accesses on global variables. + +Modifying twice or reading and modifying a memory location without a +defined sequence of the operations is either undefined behavior or has +unspecified order. This checker is similar to the -Wunsequenced clang warning, +however it only looks at global variables and therefore can find conflicting +actions recursively inside functions as well. ConcreteCactus wrote: Right, I wasn't sure whether to include/exclude `-Wunsequenced`. I removed the code parts that excluded flagging those cases, and noted later in the documentation that there could be some overlap. https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
ConcreteCactus wrote:
I clarified the description and added a more descriptive code example. Hope
this helps.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - bugprone-conflicting-global-accesses
+
+bugprone-conflicting-global-accesses
+
+
+Finds conflicting accesses on global variables.
+
+Modifying twice or reading and modifying a memory location without a
+defined sequence of the operations is either undefined behavior or has
+unspecified order. This checker is similar to the -Wunsequenced clang warning,
+however it only looks at global variables and therefore can find conflicting
+actions recursively inside functions as well.
+
+For example::
+
+int a = 0;
+int b = (a++) - a; // This is flagged by -Wunsequenced.
+
+However global variables allow for more complex scenarios that
+-Wunsequenced doesn't detect. E.g. ::
+
+int globalVar = 0;
+
+int incFun() {
+ globalVar++;
+ return globalVar;
+}
+
+int main() {
+ return globalVar + incFun(); // This is not detected by -Wunsequenced.
+}
+
+This checker attempts to detect such cases. It recurses into functions that are
+inside the same translation unit. It also attempts not to flag cases that are
+already covered by -Wunsequenced. Global unions and structs are also handled.
+For example::
+
+typedef struct {
+int A;
+float B;
+} IntAndFloat;
+
+IntAndFloat GlobalIF;
+
+int globalIFGetSum() {
+int sum = GlobalIF.A + (int)GlobalIF.B;
+GlobalIF = (IntAndFloat){};
+return sum;
+}
+
+int main() {
+// The following printf could give different results on different
+// compilers.
+printf("sum: %i, int: %i", globalIFGetSum(), GlobalIF.A);
+}
+
+Options
+---
+
+.. option:: HandleMutableFunctionParametersAsWrites
+
+It's possible to enable handling mutable reference and pointer function
+parameters as writes using the HandleMutableFunctionParametersAsWrites
+option. For example:
+
+void func(int& a);
ConcreteCactus wrote:
I added the code block directive as requested, but I wasn't sure how it works
with the option directive, so I added the code block outside of it.
https://github.com/llvm/llvm-project/pull/130421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added bugprone-unsequenced-global-accesses check (PR #130421)
https://github.com/ConcreteCactus edited https://github.com/llvm/llvm-project/pull/130421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
