[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2021-02-18 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

@steakhal

> Could you please give a few links to some documentation about the functions 
> you are trying to model?

Yes, you a re right. I've edited the summary with the link.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2021-02-16 Thread Balázs Benics via Phabricator via cfe-commits 
steakhal added a comment.

Could you please give a few links to some documentation about the functions you 
are trying to model?
Is 
https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/synchronization/synchronization.html
 a legitimate resource for this?

It's really important to bake the proper semantic rules into the modeling.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2021-01-25 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

Just want to load this revision in scope of the stack. I need you review for 
this. Thanks.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-12-16 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

Hey, folk. Please, look at this revision.
This is the **2nd** revision from the //stack of 3//. Its aim is preparing the 
field for the **3rd** revision (also welcome to review). The **1st** one has 
been closed.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-12-01 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

Just a ping. Let me, please, do smth with this revision.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-28 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

@baloghadamsoftware

> Sorry, absolutely no competence.

That's OK. I've added a clarification section to the summary to make it easier 
to understand my intentions.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-25 Thread Balogh , Ádám via Phabricator via cfe-commits 
baloghadamsoftware added a comment.

In D87146#2294514 , 
@baloghadamsoftware wrote:

> In D87146#2294423 , @ASDenysPetrov 
> wrote:
>
>> It would be nice if someone had time to look at this. Thanks.
>
> I am just looking, but I am not a `pthread` expert.

Sorry, absolutely no competence.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-25 Thread Balogh , Ádám via Phabricator via cfe-commits 
baloghadamsoftware added a comment.

In D87146#2294423 , @ASDenysPetrov 
wrote:

> It would be nice if someone had time to look at this. Thanks.

I am just looking, but I am not a `pthread` expert.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-25 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov added a comment.

It would be nice if someone had time to look at this. Thanks.


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

https://reviews.llvm.org/D87146

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

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-06 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov updated this revision to Diff 290134.
ASDenysPetrov added a comment.

Test fix.


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

https://reviews.llvm.org/D87146

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
  clang/test/Analysis/pthreadlock.c

Index: clang/test/Analysis/pthreadlock.c
===
--- clang/test/Analysis/pthreadlock.c
+++ clang/test/Analysis/pthreadlock.c
@@ -242,6 +242,16 @@
   pthread_mutex_destroy(_mtx);
 }
 
+void ok32(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_exclusive();  // no-warning
+}
+
+void ok33(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_unlock_shared();  // no-warning
+}
+
 void
 bad1(void)
 {
@@ -502,14 +512,47 @@
 }
 
 void bad32(void) {
+  pthread_mutex_lock(pmtx);
+  fake_system_function();
+  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad33(void) {
   lck_rw_lock_shared();
-  lck_rw_unlock_exclusive(); // FIXME: warn - should be shared?
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad34(void) {
   lck_rw_lock_exclusive();
-  lck_rw_unlock_shared(); // FIXME: warn - should be exclusive?
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
 }
 
-void bad33(void) {
-  pthread_mutex_lock(pmtx);
-  fake_system_function();
-  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+void bad35(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad36(void) {
+  lck_rw_lock_shared();
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad37(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad38(void) {
+  lck_rw_lock_shared();
+  lck_rw_unlock_exclusive(); // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
+}
+
+void bad39(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad40(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_unlock_exclusive();   // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
 }
Index: clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -34,6 +34,8 @@
 extern void lck_mtx_lock(lck_mtx_t *);
 extern void lck_mtx_unlock(lck_mtx_t *);
 extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
+extern boolean_t lck_rw_try_lock_shared(lck_rw_t *);
+extern boolean_t lck_rw_try_lock_exclusive(lck_rw_t *);
 extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
 
 extern void lck_rw_lock_exclusive(lck_rw_t *lck);
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -33,7 +33,9 @@
   enum Kind {
 Destroyed,
 Locked,
+SharedLocked,
 Unlocked,
+SharedUnlocked,
 UntouchedAndPossiblyDestroyed,
 UnlockedAndPossiblyDestroyed
   } K;
@@ -43,7 +45,9 @@
 
 public:
   static LockState getLocked() { return LockState(Locked); }
+  static LockState getSharedLocked() { return LockState(SharedLocked); }
   static LockState getUnlocked() { return LockState(Unlocked); }
+  static LockState getSharedUnlocked() { return LockState(SharedUnlocked); }
   static LockState getDestroyed() { return LockState(Destroyed); }
   static LockState getUntouchedAndPossiblyDestroyed() {
 return LockState(UntouchedAndPossiblyDestroyed);
@@ -55,7 +59,11 @@
   bool operator==(const LockState ) const { return K == X.K; }
 
   bool isLocked() const { return K == Locked; }
+  bool isSharedLocked() const { return K == SharedLocked; }
+  bool isAnyLocked() const { return isLocked() || isSharedLocked(); }
   bool isUnlocked() const { return K == Unlocked; }
+  bool isSharedUnlocked() const { return K == SharedUnlocked; }
+  bool isAnyUnlocked() const { return isUnlocked() || isSharedUnlocked(); }
   bool isDestroyed() const { return K == Destroyed; }
   bool isUntouchedAndPossiblyDestroyed() const {
 return K == UntouchedAndPossiblyDestroyed;
@@ -99,7 +107,7 @@
   {{"pthread_rwlock_wrlock", 1}, ::AcquirePthreadLock},

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-06 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov updated this revision to Diff 290133.
ASDenysPetrov added a comment.

Minor fixes.


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

https://reviews.llvm.org/D87146

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
  clang/test/Analysis/pthreadlock.c

Index: clang/test/Analysis/pthreadlock.c
===
--- clang/test/Analysis/pthreadlock.c
+++ clang/test/Analysis/pthreadlock.c
@@ -242,6 +242,16 @@
   pthread_mutex_destroy(_mtx);
 }
 
+void ok32(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_exclusive();  // no-warning
+}
+
+void ok33(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_lock_shared();// no-warning
+}
+
 void
 bad1(void)
 {
@@ -502,14 +512,47 @@
 }
 
 void bad32(void) {
+  pthread_mutex_lock(pmtx);
+  fake_system_function();
+  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad33(void) {
   lck_rw_lock_shared();
-  lck_rw_unlock_exclusive(); // FIXME: warn - should be shared?
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad34(void) {
   lck_rw_lock_exclusive();
-  lck_rw_unlock_shared(); // FIXME: warn - should be exclusive?
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
 }
 
-void bad33(void) {
-  pthread_mutex_lock(pmtx);
-  fake_system_function();
-  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+void bad35(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad36(void) {
+  lck_rw_lock_shared();
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad37(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad38(void) {
+  lck_rw_lock_shared();
+  lck_rw_unlock_exclusive(); // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
+}
+
+void bad39(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad40(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_unlock_exclusive();   // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
 }
Index: clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -34,6 +34,8 @@
 extern void lck_mtx_lock(lck_mtx_t *);
 extern void lck_mtx_unlock(lck_mtx_t *);
 extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
+extern boolean_t lck_rw_try_lock_shared(lck_rw_t *);
+extern boolean_t lck_rw_try_lock_exclusive(lck_rw_t *);
 extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
 
 extern void lck_rw_lock_exclusive(lck_rw_t *lck);
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -33,7 +33,9 @@
   enum Kind {
 Destroyed,
 Locked,
+SharedLocked,
 Unlocked,
+SharedUnlocked,
 UntouchedAndPossiblyDestroyed,
 UnlockedAndPossiblyDestroyed
   } K;
@@ -43,7 +45,9 @@
 
 public:
   static LockState getLocked() { return LockState(Locked); }
+  static LockState getSharedLocked() { return LockState(SharedLocked); }
   static LockState getUnlocked() { return LockState(Unlocked); }
+  static LockState getSharedUnlocked() { return LockState(SharedUnlocked); }
   static LockState getDestroyed() { return LockState(Destroyed); }
   static LockState getUntouchedAndPossiblyDestroyed() {
 return LockState(UntouchedAndPossiblyDestroyed);
@@ -55,7 +59,11 @@
   bool operator==(const LockState ) const { return K == X.K; }
 
   bool isLocked() const { return K == Locked; }
+  bool isSharedLocked() const { return K == SharedLocked; }
+  bool isAnyLocked() const { return isLocked() || isSharedLocked(); }
   bool isUnlocked() const { return K == Unlocked; }
+  bool isSharedUnlocked() const { return K == SharedUnlocked; }
+  bool isAnyUnlocked() const { return isUnlocked() || isSharedUnlocked(); }
   bool isDestroyed() const { return K == Destroyed; }
   bool isUntouchedAndPossiblyDestroyed() const {
 return K == UntouchedAndPossiblyDestroyed;
@@ -99,7 +107,7 @@
   {{"pthread_rwlock_wrlock", 1}, ::AcquirePthreadLock},

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

2020-09-06 Thread Denys Petrov via Phabricator via cfe-commits 
ASDenysPetrov updated this revision to Diff 290131.
ASDenysPetrov added a comment.

Fixed some missed conditions. Added more tests.


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

https://reviews.llvm.org/D87146

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
  clang/test/Analysis/pthreadlock.c

Index: clang/test/Analysis/pthreadlock.c
===
--- clang/test/Analysis/pthreadlock.c
+++ clang/test/Analysis/pthreadlock.c
@@ -242,6 +242,16 @@
   pthread_mutex_destroy(_mtx);
 }
 
+void ok32(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_exclusive();  // no-warning
+}
+
+void ok33(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_lock_shared();// no-warning
+}
+
 void
 bad1(void)
 {
@@ -502,14 +512,47 @@
 }
 
 void bad32(void) {
+  pthread_mutex_lock(pmtx);
+  fake_system_function();
+  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad33(void) {
   lck_rw_lock_shared();
-  lck_rw_unlock_exclusive(); // FIXME: warn - should be shared?
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad34(void) {
   lck_rw_lock_exclusive();
-  lck_rw_unlock_shared(); // FIXME: warn - should be exclusive?
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
 }
 
-void bad33(void) {
-  pthread_mutex_lock(pmtx);
-  fake_system_function();
-  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+void bad35(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_lock_shared(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad36(void) {
+  lck_rw_lock_shared();
+  lck_rw_lock_exclusive(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad37(void) {
+  lck_rw_lock_exclusive();
+  lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad38(void) {
+  lck_rw_lock_shared();
+  lck_rw_unlock_exclusive(); // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
+}
+
+void bad39(void) {
+  if (lck_rw_try_lock_exclusive() != 0) // no-warning
+lck_rw_unlock_shared(); // expected-warning{{This lock has been acquired exclusively, but shared unlock used}}
+}
+
+void bad40(void) {
+  if (lck_rw_try_lock_shared() != 0) // no-warning
+lck_rw_unlock_exclusive();   // expected-warning{{This lock has been acquired as shared, but exclusive unlock used}}
 }
Index: clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -19,6 +19,7 @@
   void *foo;
 } lck_rw_t;
 
+typedef pthread_mutex_t lck_mtx_t;
 typedef pthread_mutex_t lck_mtx_t;
 
 extern void fake_system_function();
@@ -34,6 +35,8 @@
 extern void lck_mtx_lock(lck_mtx_t *);
 extern void lck_mtx_unlock(lck_mtx_t *);
 extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
+extern boolean_t lck_rw_try_lock_shared(lck_rw_t *lck);
+extern boolean_t lck_rw_try_lock_exclusive(lck_rw_t *lck);
 extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
 
 extern void lck_rw_lock_exclusive(lck_rw_t *lck);
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -33,7 +33,9 @@
   enum Kind {
 Destroyed,
 Locked,
+SharedLocked,
 Unlocked,
+SharedUnlocked,
 UntouchedAndPossiblyDestroyed,
 UnlockedAndPossiblyDestroyed
   } K;
@@ -43,7 +45,9 @@
 
 public:
   static LockState getLocked() { return LockState(Locked); }
+  static LockState getSharedLocked() { return LockState(SharedLocked); }
   static LockState getUnlocked() { return LockState(Unlocked); }
+  static LockState getSharedUnlocked() { return LockState(SharedUnlocked); }
   static LockState getDestroyed() { return LockState(Destroyed); }
   static LockState getUntouchedAndPossiblyDestroyed() {
 return LockState(UntouchedAndPossiblyDestroyed);
@@ -55,7 +59,11 @@
   bool operator==(const LockState ) const { return K == X.K; }
 
   bool isLocked() const { return K == Locked; }
+  bool isSharedLocked() const { return K == SharedLocked; }
+  bool isAnyLocked() const { return isLocked() || isSharedLocked(); }
   bool isUnlocked() const { return K == Unlocked; }
+  bool isSharedUnlocked() const { return K == SharedUnlocked; }
+  bool isAnyUnlocked() const { return isUnlocked() || isSharedUnlocked(); }
   bool isDestroyed() const { return K == Destroyed;