[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-08 Thread Donát Nagy via cfe-commits 

https://github.com/NagyDonat closed 
https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Artem Dergachev via cfe-commits 


@@ -87,7 +90,7 @@ class MIGChecker : public Checker,
 #undef CALL
   };
 
-  CallDescription OsRefRetain{{"os_ref_retain"}, 1};
+  CallDescription OsRefRetain{CDM::SimpleFunc, {"os_ref_retain"}, 1};

haoNoQ wrote:

Yes, this is a plain C function. Definitely not builtin. May or may not come 
from system headers depending on what we're compiling.

https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Artem Dergachev via cfe-commits 

https://github.com/haoNoQ edited https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Artem Dergachev via cfe-commits 

https://github.com/haoNoQ approved this pull request.

Looks awesome thank you!

https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Donát Nagy via cfe-commits 

https://github.com/NagyDonat edited 
https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Donát Nagy via cfe-commits 


@@ -46,13 +46,13 @@ class MIGChecker : public Checker,
   // additionally an argument of a MIG routine, the checker keeps track of that
   // information and issues a warning when an error is returned from the
   // respective routine.
-  std::vector> Deallocators = {
+  CallDescriptionMap Deallocators = {
 #define CALL(required_args, deallocated_arg, ...)  
\
-  {{{__VA_ARGS__}, required_args}, deallocated_arg}
-  // E.g., if the checker sees a C function 'vm_deallocate' that is
-  // defined on class 'IOUserClient' that has exactly 3 parameters, it 
knows
-  // that argument #1 (starting from 0, i.e. the second argument) is going
-  // to be consumed in the sense of the MIG consume-on-success convention.
+  {{CDM::SimpleFunc, {__VA_ARGS__}, required_args}, deallocated_arg}

NagyDonat wrote:

`CDM::SimpleFunc` is used for the sake of consistency with the case of 
`os_ref_retain` (and because I'm not familiar with this calling convention).

https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Donát Nagy via cfe-commits 


@@ -87,7 +90,7 @@ class MIGChecker : public Checker,
 #undef CALL
   };
 
-  CallDescription OsRefRetain{{"os_ref_retain"}, 1};
+  CallDescription OsRefRetain{CDM::SimpleFunc, {"os_ref_retain"}, 1};

NagyDonat wrote:

`CDM::SimpleFunc` is used instead of `CDM::CLibrary` because there might be 
some headers that declare `os_ref_retain` as a `static` and non-`inline` 
function -- e.g. random internet search provided [this source 
file](http://www.newosxbook.com/src.jl?tree=&file=/xnu-8792.81.2/libkern/os/refcnt.h).

https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Donát Nagy (NagyDonat)


Changes

This commit explicitly specifies the matching mode (C library function, any 
non-method function, or C++ method) for the `CallDescription`s constructed in 
the checker `osx.MIG`.

The code was simplified to use a `CallDescriptionMap` instead of a raw vector 
of pairs.

This change won't cause major functional changes, but isn't NFC because it 
ensures that e.g. call descriptions for a non-method function won't 
accidentally match a method that has the same name.

Separate commits have already performed this change in other checkers:
- easy cases: e2f1cbae45f81f3cd9a4d3c2bcf69a094eb060fa, 
6d64f8e1feee014e72730a78b62d9d415df112ff
- MallocChecker: d6d84b5d1448e4f2e24b467a0abcf42fe9d543e9
- iterator checkers: 06eedffe0d2782922e63cc25cb927f4acdaf7b30
- InvalidPtr checker: 024281d4d26344f9613b9115ea1fcbdbdba23235
- apiModeling.llvm.ReturnValue: 97dd8e3c4f38ef345b01fbbf0a2052c7875ff7e0

... and follow-up commits will handle the remaining few checkers.

My goal is to ensure that the call description mode is always explicitly 
specified and eliminate (or strongly restrict) the vague "may be either a 
method or a simple function" mode that's the current default.

---
Full diff: https://github.com/llvm/llvm-project/pull/91331.diff


1 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (+13-13) 


``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
index 153a0a51e98024..9757a00f1fb2f6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -46,13 +46,13 @@ class MIGChecker : public Checker,
   // additionally an argument of a MIG routine, the checker keeps track of that
   // information and issues a warning when an error is returned from the
   // respective routine.
-  std::vector> Deallocators = {
+  CallDescriptionMap Deallocators = {
 #define CALL(required_args, deallocated_arg, ...)  
\
-  {{{__VA_ARGS__}, required_args}, deallocated_arg}
-  // E.g., if the checker sees a C function 'vm_deallocate' that is
-  // defined on class 'IOUserClient' that has exactly 3 parameters, it 
knows
-  // that argument #1 (starting from 0, i.e. the second argument) is going
-  // to be consumed in the sense of the MIG consume-on-success convention.
+  {{CDM::SimpleFunc, {__VA_ARGS__}, required_args}, deallocated_arg}
+  // E.g., if the checker sees a C function 'vm_deallocate' that has
+  // exactly 3 parameters, it knows that argument #1 (starting from 0, i.e.
+  // the second argument) is going to be consumed in the sense of the MIG
+  // consume-on-success convention.
   CALL(3, 1, "vm_deallocate"),
   CALL(3, 1, "mach_vm_deallocate"),
   CALL(2, 0, "mig_deallocate"),
@@ -78,6 +78,9 @@ class MIGChecker : public Checker,
   CALL(1, 0, "thread_inspect_deallocate"),
   CALL(1, 0, "upl_deallocate"),
   CALL(1, 0, "vm_map_deallocate"),
+#undef CALL
+#define CALL(required_args, deallocated_arg, ...)  
\
+  {{CDM::CXXMethod, {__VA_ARGS__}, required_args}, deallocated_arg}
   // E.g., if the checker sees a method 'releaseAsyncReference64()' that is
   // defined on class 'IOUserClient' that takes exactly 1 argument, it 
knows
   // that the argument is going to be consumed in the sense of the MIG
@@ -87,7 +90,7 @@ class MIGChecker : public Checker,
 #undef CALL
   };
 
-  CallDescription OsRefRetain{{"os_ref_retain"}, 1};
+  CallDescription OsRefRetain{CDM::SimpleFunc, {"os_ref_retain"}, 1};
 
   void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;
 
@@ -198,15 +201,12 @@ void MIGChecker::checkPostCall(const CallEvent &Call, 
CheckerContext &C) const {
   if (!isInMIGCall(C))
 return;
 
-  auto I = llvm::find_if(Deallocators,
- [&](const std::pair &Item) 
{
-   return Item.first.matches(Call);
- });
-  if (I == Deallocators.end())
+  const unsigned *ArgIdxPtr = Deallocators.lookup(Call);
+  if (!ArgIdxPtr)
 return;
 
   ProgramStateRef State = C.getState();
-  unsigned ArgIdx = I->second;
+  unsigned ArgIdx = *ArgIdxPtr;
   SVal Arg = Call.getArgSVal(ArgIdx);
   const ParmVarDecl *PVD = getOriginParam(Arg, C);
   if (!PVD || State->contains(PVD))

``




https://github.com/llvm/llvm-project/pull/91331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Use explicit call description mode in MIGChecker (PR #91331)

2024-05-07 Thread Donát Nagy via cfe-commits 

https://github.com/NagyDonat created 
https://github.com/llvm/llvm-project/pull/91331

This commit explicitly specifies the matching mode (C library function, any 
non-method function, or C++ method) for the `CallDescription`s constructed in 
the checker `osx.MIG`.

The code was simplified to use a `CallDescriptionMap` instead of a raw vector 
of pairs.

This change won't cause major functional changes, but isn't NFC because it 
ensures that e.g. call descriptions for a non-method function won't 
accidentally match a method that has the same name.

Separate commits have already performed this change in other checkers:
- easy cases: e2f1cbae45f81f3cd9a4d3c2bcf69a094eb060fa, 
6d64f8e1feee014e72730a78b62d9d415df112ff
- MallocChecker: d6d84b5d1448e4f2e24b467a0abcf42fe9d543e9
- iterator checkers: 06eedffe0d2782922e63cc25cb927f4acdaf7b30
- InvalidPtr checker: 024281d4d26344f9613b9115ea1fcbdbdba23235
- apiModeling.llvm.ReturnValue: 97dd8e3c4f38ef345b01fbbf0a2052c7875ff7e0

... and follow-up commits will handle the remaining few checkers.

My goal is to ensure that the call description mode is always explicitly 
specified and eliminate (or strongly restrict) the vague "may be either a 
method or a simple function" mode that's the current default.

From c4d8ecff61e0b4dee7429023dc965594be1b9b7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= 
Date: Tue, 7 May 2024 14:04:35 +0200
Subject: [PATCH] [analyzer] Use explicit call description mode in MIGChecker

This commit explicitly specifies the matching mode (C library function,
any non-method function, or C++ method) for the `CallDescription`s
constructed in the checker `osx.MIG`.

The code was simplified to use a `CallDescriptionMap` instead of a raw
vector of pairs.

This change won't cause major functional changes, but isn't NFC because
it ensures that e.g. call descriptions for a non-method function won't
accidentally match a method that has the same name.

Separate commits have already performed this change in other checkers:
- easy cases: e2f1cbae45f81f3cd9a4d3c2bcf69a094eb060fa,
6d64f8e1feee014e72730a78b62d9d415df112ff
- MallocChecker: d6d84b5d1448e4f2e24b467a0abcf42fe9d543e9
- iterator checkers: 06eedffe0d2782922e63cc25cb927f4acdaf7b30
- InvalidPtr checker: 024281d4d26344f9613b9115ea1fcbdbdba23235
- apiModeling.llvm.ReturnValue: 97dd8e3c4f38ef345b01fbbf0a2052c7875ff7e0

... and follow-up commits will handle the remaining few checkers.

My goal is to ensure that the call description mode is always explicitly
specified and eliminate (or strongly restrict) the vague "may be either
a method or a simple function" mode that's the current default.
---
 .../StaticAnalyzer/Checkers/MIGChecker.cpp| 26 +--
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
index 153a0a51e98024..9757a00f1fb2f6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -46,13 +46,13 @@ class MIGChecker : public Checker,
   // additionally an argument of a MIG routine, the checker keeps track of that
   // information and issues a warning when an error is returned from the
   // respective routine.
-  std::vector> Deallocators = {
+  CallDescriptionMap Deallocators = {
 #define CALL(required_args, deallocated_arg, ...)  
\
-  {{{__VA_ARGS__}, required_args}, deallocated_arg}
-  // E.g., if the checker sees a C function 'vm_deallocate' that is
-  // defined on class 'IOUserClient' that has exactly 3 parameters, it 
knows
-  // that argument #1 (starting from 0, i.e. the second argument) is going
-  // to be consumed in the sense of the MIG consume-on-success convention.
+  {{CDM::SimpleFunc, {__VA_ARGS__}, required_args}, deallocated_arg}
+  // E.g., if the checker sees a C function 'vm_deallocate' that has
+  // exactly 3 parameters, it knows that argument #1 (starting from 0, i.e.
+  // the second argument) is going to be consumed in the sense of the MIG
+  // consume-on-success convention.
   CALL(3, 1, "vm_deallocate"),
   CALL(3, 1, "mach_vm_deallocate"),
   CALL(2, 0, "mig_deallocate"),
@@ -78,6 +78,9 @@ class MIGChecker : public Checker,
   CALL(1, 0, "thread_inspect_deallocate"),
   CALL(1, 0, "upl_deallocate"),
   CALL(1, 0, "vm_map_deallocate"),
+#undef CALL
+#define CALL(required_args, deallocated_arg, ...)  
\
+  {{CDM::CXXMethod, {__VA_ARGS__}, required_args}, deallocated_arg}
   // E.g., if the checker sees a method 'releaseAsyncReference64()' that is
   // defined on class 'IOUserClient' that takes exactly 1 argument, it 
knows
   // that the argument is going to be consumed in the sense of the MIG
@@ -87,7 +90,7 @@ class MIGChecker : public Checker,
 #undef CALL
   };
 
-  CallDescription OsRefRetain{{"os_ref_retain"}, 1};
+  CallDescription Os