pthread_mutex_lock() modeling error

[llvmbot]

Bugzilla Link	8395
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@belkadan

Extended Description

$ clang -v clang version 2.9 (trunk 116687) Target: x86_64-unknown-linux-gnu Thread model: posix s$ cat pthread_mutex_lock.c

include

static pthread_mutex_t m;

void foo() { int err = 0; err = pthread_mutex_lock(&m); if(err) return; pthread_mutex_unlock(&m); } $ clang -cc1 pthread_mutex_lock.c -analyze -analyzer-check-objc-mem -analyzer-experimental-checks pthread_mutex_lock.c:10:2: warning: This statement is never executed return; ^~ 1 warning generated.

[belkadan]

This passed because we turned off the analyzer's dead code checker for now. It still fails with

% clang -cc1 -analyze -analyzer-checker=alpha.unix.PthreadLock,alpha.deadcode pthread_mutex_lock.c

Arguably there's a policy decision here: many people do not expect pthread locks to fail. We probably need to do something similar to realloc -- most people just assume it succeeds, but if someone was defensive enough to test it then we can offer leak warnings on the original buffer.

[llvmbot]

$ clang --version clang version 3.2 (trunk 165709) Target: x86_64-unknown-linux-gnu Thread model: posix

PASS for me. Thanks!

[llvmbot]

Both functions -- pthread_mutex_lock() and pthread_mutex_trylock() can:

• acquire lock
• return error

Then we can process them equally. And the following naive patch will solve PR

• remove now useless 'bool isTryLock' from AcquireLock()

• remove now useless else-part from 'if(isTryLock)'

• Patch was tested with PR testcase only

• Patch is against rev.120936

$ clang --version clang version 2.9 (trunk 120936) Target: x86_64-unknown-linux-gnu Thread model: posix

Checker$ svn diff Index: PthreadLockChecker.cpp

--- PthreadLockChecker.cpp (revision 120936) +++ PthreadLockChecker.cpp (working copy) @@ -32,8 +32,7 @@ } void PostVisitCallExpr(CheckerContext &C, const CallExpr *CE);

• void AcquireLock(CheckerContext &C, const CallExpr *CE,

• SVal lock, bool isTryLock);

• void AcquireLock(CheckerContext &C, const CallExpr *CE, SVal lock);

  void ReleaseLock(CheckerContext &C, const CallExpr *CE, SVal lock); @@ -73,12 +72,12 @@ if (FName == "pthread_mutex_lock") { if (CE->getNumArgs() != 1) return;

• AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false);

• AcquireLock(C, CE, state->getSVal(CE->getArg(0))); } else if (FName == "pthread_mutex_trylock") { if (CE->getNumArgs() != 1) return;

• AcquireLock(C, CE, state->getSVal(CE->getArg(0)), true);

• AcquireLock(C, CE, state->getSVal(CE->getArg(0))); } else if (FName == "pthread_mutex_unlock") { if (CE->getNumArgs() != 1) @@ -88,7 +87,7 @@ }

  void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE,

• SVal lock, bool isTryLock) {

• SVal lock) {

  const MemRegion lockR = lock.getAsRegion(); if (!lockR) @@ -103,18 +102,11 @@ DefinedSVal retVal = cast(X); const GRState lockSucc = state;

• if (isTryLock) {

• // Bifurcate the state, and allow a mode where the lock acquisition fails.

• const GRState *lockFail;

• llvm::tie(lockFail, lockSucc) = state->assume(retVal);

• assert(lockFail && lockSucc);

• C.addTransition(C.GenerateNode(CE, lockFail));

• }

• else {

• // Assume that the return value was 0.

• lockSucc = state->assume(retVal, false);

• assert(lockSucc);

• }

• // Bifurcate the state, and allow a mode where the lock acquisition fails.

• const GRState *lockFail;

• llvm::tie(lockFail, lockSucc) = state->assume(retVal);

• assert(lockFail && lockSucc);

• C.addTransition(C.GenerateNode(CE, lockFail));

  // Record that the lock was acquired. lockSucc = lockSucc->add(lockR);

[llvmbot]

assigned to @tkremenek