[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd01280587d97: [analyzer][StdLibraryFunctionsChecker] Add 
POSIX pthread handling functions (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D84415?vs=289970=290296#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -108,6 +108,20 @@
 // CHECK: Loaded summary for: struct tm *gmtime(const time_t *tp)
 // CHECK: Loaded summary for: int clock_gettime(clockid_t clock_id, struct timespec *tp)
 // CHECK: Loaded summary for: int getitimer(int which, struct itimerval *curr_value)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -259,6 +273,34 @@
 struct itimerval;
 int getitimer(int which, struct itimerval *curr_value);
 
+typedef union {
+  int x;
+} pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union {
+  int x;
+} pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union {
+  int x;
+} pthread_mutex_t;
+typedef union {
+  int x;
+} pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -951,6 +951,8 @@
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -2182,6 +2184,99 @@
 Summary(ArgTypes{IntTy, StructItimervalPtrTy},
 RetType{IntTy}, NoEvalCall)
 .ArgConstraint(NotNull(ArgNo(1;
+
+Optional Pthread_cond_tTy = lookupTy("pthread_cond_t");
+Optional Pthread_cond_tPtrTy = getPointerTy(Pthread_cond_tTy);
+Optional Pthread_tTy = 

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko accepted this revision.
vsavchenko added a comment.

LGTM!
Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

@balazske seems to be very involved, he might have some closing words -- from 
my end, LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Polite ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 289970.
martong added a comment.

- Rebase to master


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,34 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union {
+  int x;
+} pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union {
+  int x;
+} pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union {
+  int x;
+} pthread_mutex_t;
+typedef union {
+  int x;
+} pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -931,6 +931,8 @@
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -2036,6 +2038,99 @@
 BufferSize(/*Buffer=*/ArgNo(4), /*BufSize=*/ArgNo(5)))
 .ArgConstraint(
 ArgumentCondition(5, WithinRange, Range(0, Socklen_tMax;
+
+Optional Pthread_cond_tTy = lookupTy("pthread_cond_t");
+Optional Pthread_cond_tPtrTy = 

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 287640.
martong added a comment.

- Rebase to parent patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,34 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union {
+  int x;
+} pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union {
+  int x;
+} pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union {
+  int x;
+} pthread_mutex_t;
+typedef union {
+  int x;
+} pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -903,6 +903,8 @@
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -978,6 +980,12 @@
   for (const Summary  : Summaries)
 operator()(Name, S);
 }
+// Add the same summary for different names with the Signature explicitly
+// given.
+void operator()(std::vector Names, Signature Sign, Summary Sum) {
+  for (StringRef Name : 

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

balazske wrote:
> martong wrote:
> > balazske wrote:
> > > Another idea: Have a class that handles all the variants (simple, 
> > > pointer, const pointer, const restrict pointer, restrict). It can have 
> > > get functions that compute the type if not done yet, or get every variant 
> > > at first time even if it is later not used (or has no sense).
> > > For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and 
> > > then call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, 
> > > ... .
> > > ```
> > I'd rather keep the current form, because I think it makes it easier to 
> > compose types from different base types (and it is similar to the 
> > ASTMatchers in this sense).
> If I got it correctly, the problem with my approach above is that it has 
> separate functions for every special type variant. The current is acceptable 
> but looks not fully uniform to me (the type has a `withConst` and for getting 
> a pointer or restrict there is a function call). We could have something like 
> a "`TypeBuilder`" that has methods `withConst()`, `withRestrict()`, 
> `withPointerTo()` (these can return a TypeBuilder too) and finally a 
> (omittable) `getType()` that returns `Optional`.
> ```
> const QualType ConstVoidPtrRestrictTy = TypeBuilder{ACtx, 
> VoidTy}.withConst().withPointerTo().withRestrict().getType();
> ```
> 
Yeah, `.withConst()` is making it nonuniform currently, you are right.
What if we had `getConstTy()` that receives an optional? Composability would be 
just perfect in this case :)
```
Optional ConstPthread_attr_tPtrTy = 
getPointerTy(getConstTy(Pthread_attr_tTy));
```
WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

martong wrote:
> balazske wrote:
> > Another idea: Have a class that handles all the variants (simple, pointer, 
> > const pointer, const restrict pointer, restrict). It can have get functions 
> > that compute the type if not done yet, or get every variant at first time 
> > even if it is later not used (or has no sense).
> > For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and 
> > then call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, 
> > ... .
> > ```
> I'd rather keep the current form, because I think it makes it easier to 
> compose types from different base types (and it is similar to the ASTMatchers 
> in this sense).
If I got it correctly, the problem with my approach above is that it has 
separate functions for every special type variant. The current is acceptable 
but looks not fully uniform to me (the type has a `withConst` and for getting a 
pointer or restrict there is a function call). We could have something like a 
"`TypeBuilder`" that has methods `withConst()`, `withRestrict()`, 
`withPointerTo()` (these can return a TypeBuilder too) and finally a 
(omittable) `getType()` that returns `Optional`.
```
const QualType ConstVoidPtrRestrictTy = TypeBuilder{ACtx, 
VoidTy}.withConst().withPointerTo().withRestrict().getType();
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong planned changes to this revision.
martong added a comment.

Planning to rebase this on a dependent patch that uses an API with Optionals.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks @balazske for your comments, you always make an assiduous review!




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:350
+  } else {
+*this = Signature(Args, *RetTy);
   }

balazske wrote:
> Not very bad but I do not like to call another constructor and assignment 
> here, this can be many redundant operations. It is better to fill the 
> internal arrays here and have a separate assert function that checks for 
> validness (of the internal arrays) instead of the private constructor.
Ok, makes sense, thanks!



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:378
 
   static QualType getArgType(const FunctionDecl *FD, ArgNo ArgN) {
 assert(FD && "Function must be set");

balazske wrote:
> Check for isInvalid here (with assert)?
We cannot do that, this is not a member function of the Signature, this 
operates on a FuncitonDecl. But it makes sense to add the assert to the 
`mathces` member function.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

balazske wrote:
> Another idea: Have a class that handles all the variants (simple, pointer, 
> const pointer, const restrict pointer, restrict). It can have get functions 
> that compute the type if not done yet, or get every variant at first time 
> even if it is later not used (or has no sense).
> For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and 
> then call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, ... 
> .
> ```
I'd rather keep the current form, because I think it makes it easier to compose 
types from different base types (and it is similar to the ASTMatchers in this 
sense).



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1026
   Optional FilePtrTy, FilePtrRestrictTy;
   if (FileTy) {
 // FILE *

balazske wrote:
> These `if`s can be removed too (in another patch).
Yep, I am going to create another base patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 287409.
martong marked 4 inline comments as done.
martong added a comment.

- Remove private ctor of Signature
- Assert a valid signature in Signature::matces


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,7 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,17 +324,36 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  class Signature {
+  

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

It is better now, but the refactoring change should be in a separate revision 
(it affects the other already added functions too).




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:350
+  } else {
+*this = Signature(Args, *RetTy);
   }

Not very bad but I do not like to call another constructor and assignment here, 
this can be many redundant operations. It is better to fill the internal arrays 
here and have a separate assert function that checks for validness (of the 
internal arrays) instead of the private constructor.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:378
 
   static QualType getArgType(const FunctionDecl *FD, ArgNo ArgN) {
 assert(FD && "Function must be set");

Check for isInvalid here (with assert)?



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

Another idea: Have a class that handles all the variants (simple, pointer, 
const pointer, const restrict pointer, restrict). It can have get functions 
that compute the type if not done yet, or get every variant at first time even 
if it is later not used (or has no sense).
For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and then 
call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, ... .
```



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1026
   Optional FilePtrTy, FilePtrRestrictTy;
   if (FileTy) {
 // FILE *

These `if`s can be removed too (in another patch).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Now I've update the patch to remove all the `if`s. And the optional types are 
now implicitly handled, so no such mistakes can be done that I had done 
previously.
However, the patch is getting to affect the core of this Checker, so, perhaps I 
should split this into two patches? One NFC for the core changes and another 
that involves only the addition of the pthread functions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 285682.
martong added a comment.

- Use Optionals through the Signature


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,7 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,20 +324,45 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-   

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong marked an inline comment as done.
martong added a comment.

In D84415#2218084 , @balazske wrote:

> Even some macro-like construct can improve readability. The current way of 
> adding functions is source of copy-paste errors because more things are 
> repeated, these should be written only once. And the `if`s for the existence 
> of types can be eliminated (automated) somehow. So the final way of 
> specifying the functions is to only list (get) the types first, then add the 
> functions with name, arguments and constraints in a way that nothing needs to 
> be repeated and computable parts are done automatically.

Yes, this totally makes sense, thank you guys for this suggestion!

Now I've started working on to reach this form, what do you think?

  // Taking an optional as arg.
  Optional Pthread_cond_tPtrTy = getPointerTy(Pthread_cond_tTy);
  addToFunctionSummaryMap(
  {"pthread_cond_signal", "pthread_cond_broadcast"},
  // Create a signature with Optionals, addToFunctionSummaryMap will skip
  // the summary if the optional is not set.
  Signature(ArgTypes{Pthread_cond_tPtrTy}, RetType{IntTy}),
  Summary(NoEvalCall)
  .ArgConstraint(NotNull(ArgNo(0;

Side note, one day hopefully we could have the same API to fill a 
`CallDescriptionMap`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Even some macro-like construct can improve readability. The current way of 
adding functions is source of copy-paste errors because more things are 
repeated, these should be written only once. And the `if`s for the existence of 
types can be eliminated (automated) somehow. So the final way of specifying the 
functions is to only list (get) the types first, then add the functions with 
name, arguments and constraints in a way that nothing needs to be repeated and 
computable parts are done automatically.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2013
+if (Pthread_tTy) {
+  Pthread_tPtrTy = ACtx.getPointerType(*Pthread_tTy);
+  Pthread_tPtrRestrictTy = getRestrictTy(*Pthread_tPtrTy);

martong wrote:
> vsavchenko wrote:
> > It feels like the readability of the code here can be drastically improved 
> > by introducing functions `getPointerType`, `getRestrictType`, and similar 
> > accepting `Optional` arguments. 
> Yeah, we already have `getRestrictTy`, but never though to have 
> `getPointerTy`, which indeed simplifies the code, thanks!
I think the improvement would be if these `get` functions accept `Optional` 
arguments and if the argument is empty return an empty value. This can make the 
`if`s unnecessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 285626.
martong marked an inline comment as done.
martong added a comment.

- Use the shorter 'getPointerTy'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -781,6 +781,7 @@
   auto getRestrictTy = [](QualType Ty) {
 return ACtx.getLangOpts().C99 ? ACtx.getRestrictType(Ty) : Ty;
   };
+  auto getPointerTy = [](QualType Ty) { return ACtx.getPointerType(Ty); };
 
   // These types are useful for writing specifications quickly,
   // New specifications should probably introduce more types.
@@ -797,21 +798,23 @@
   const QualType SizeTy = ACtx.getSizeType();
 
   const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
-  const QualType IntPtrTy = ACtx.getPointerType(IntTy); // int *
+  const QualType IntPtrTy = getPointerTy(IntTy); // int *
   const QualType UnsignedIntPtrTy =
-  ACtx.getPointerType(UnsignedIntTy); 

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D84415#2215780 , @vsavchenko wrote:

> Off-topic: I really think that we should have some sort of DSL for that kind 
> of stuff.

In a sense the API provided in the Checker itself is an (embedded) DSL. Or do 
you think about being able to provide the summaries by describing them in a 
JSON or a YAML file?




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2013
+if (Pthread_tTy) {
+  Pthread_tPtrTy = ACtx.getPointerType(*Pthread_tTy);
+  Pthread_tPtrRestrictTy = getRestrictTy(*Pthread_tPtrTy);

vsavchenko wrote:
> It feels like the readability of the code here can be drastically improved by 
> introducing functions `getPointerType`, `getRestrictType`, and similar 
> accepting `Optional` arguments. 
Yeah, we already have `getRestrictTy`, but never though to have `getPointerTy`, 
which indeed simplifies the code, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Off-topic: I really think that we should have some sort of DSL for that kind of 
stuff.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2013
+if (Pthread_tTy) {
+  Pthread_tPtrTy = ACtx.getPointerType(*Pthread_tTy);
+  Pthread_tPtrRestrictTy = getRestrictTy(*Pthread_tPtrTy);

It feels like the readability of the code here can be drastically improved by 
introducing functions `getPointerType`, `getRestrictType`, and similar 
accepting `Optional` arguments. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added reviewers: NoQ, vsavchenko.
martong added subscribers: vsavchenko, NoQ.
martong added a comment.

@NoQ, @vsavchenko My apologies, somehow I forgot to add you as reviewers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84415/new/

https://reviews.llvm.org/D84415

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: Szelethus, gamesh411, balazske.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,26 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union { int x; } pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union { int x; } pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union { int x; } pthread_mutex_t;
+typedef union { int x; } pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -812,6 +812,8 @@
   const QualType ConstWchar_tPtrTy =
   ACtx.getPointerType(ACtx.WCharTy.withConst()); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = ACtx.getPointerType(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -883,6 +885,11 @@
   for (const Summary  : Summaries)
 operator()(Name, S);
 }
+// Add the same summary for different names.
+