This revision was automatically updated to reflect the committed changes.
Closed by commit rL304159: [analyzer] PthreadLockChecker: model failed
pthread_mutex_destroy() calls. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D32449?vs=99970&id=100615#toc
Repository:
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I'd commit your patch without the .gitignore change, as it deserves a separate
commit and more attention; will have a look at it myself - llvm's and clang's
.gitignores have diverged quite a bit.
T
malhar1995 updated this revision to Diff 99970.
malhar1995 added a comment.
Applied clang-format only to the changed lines in the final code.
Repository:
rL LLVM
https://reviews.llvm.org/D32449
Files:
.gitignore
lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
test/Analysis/pthreadlo
NoQ added a comment.
No-no, all i was trying to say is that there's code in `PthreadLockChecker.cpp`
that you haven't changed, but accidentally reformatted - and this is something
we normally try to avoid. Like, for example, changing `enum LockingSemantics
{...}` from vertical to horizontal - t
malhar1995 added a comment.
In https://reviews.llvm.org/D32449#761303, @NoQ wrote:
> Thanks, this is great! Two more things:
>
> - You have touched other code, unrelated to your patch, with clang-format;
> we're usually trying to avoid that, because it creates merge conflicts out of
> nowhere,
NoQ added a comment.
Thanks, this is great! Two more things:
- You have touched other code, unrelated to your patch, with clang-format;
we're usually trying to avoid that, because it creates merge conflicts out of
nowhere, and because some of that code actually seems formatted by hand
intentio
malhar1995 updated this revision to Diff 99750.
malhar1995 added a comment.
Addressed previous comments (removed Schrodinger from lock state names, changed
method name setAppropriateLockState to resolvePossiblyDestroyedMutex, added an
assert in resolvePossiblyDestroyedMutex, formatted the code u
malhar1995 added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:143-146
+if(lstate->isSchrodingerUntouched())
+ state = state->remove(lockR);
+else if(lstate->isSchrodingerUnlocked())
+ state = state->set(lockR, LockState::getUnl
malhar1995 marked 2 inline comments as done.
malhar1995 added a comment.
In https://reviews.llvm.org/D32449#760141, @NoQ wrote:
> Thanks! Your code looks very clear now, and it seems correct to me.
>
> One last thing we definitely should do here would be add regression tests for
> the new functi
NoQ added a comment.
Thanks! Your code looks very clear now, and it seems correct to me.
One last thing we definitely should do here would be add regression tests for
the new functionality. I guess you already have your tests, otherwise you
wouldn't know if your code works, so you'd just need t
malhar1995 updated this revision to Diff 99388.
malhar1995 marked an inline comment as done.
malhar1995 added a comment.
Cleaned up the previous patch.
Added checking of LockState before initializing a mutex as well.
Added separate branches of execution for PthreadSemantics and XNUSemantics.
Add
malhar1995 updated this revision to Diff 99179.
malhar1995 added a comment.
Added context.
Also, I removed the inclusion of iostream and also added the repetitive code to
the function setAppropriateLockState.
Currently working on finding various corner cases and invariants.
Repository:
rL LL
zaks.anna added a comment.
Thank you for the patch! Could you please re-submit the patch with context?
Instructions on how to do that can be found here:
http://llvm.org/docs/Phabricator.html
Repository:
rL LLVM
https://reviews.llvm.org/D32449
__
NoQ added a comment.
Thanks for uploading this to phabricator and sorry again that i was lost for a
while.
As we already discussed in the mailing lists, i agree with your point that the
locked-and-possibly-destroyed state should be removed, and we also had a
thought of making it explicit that
NoQ edited reviewers, added: NoQ; removed: dergachev.a.
NoQ added a comment.
Uhm, i need to do something about this duplicate account. Sorry, I have
completely forgotten that the review is already up...
Repository:
rL LLVM
https://reviews.llvm.org/D32449
__
malhar1995 created this revision.
I am currently working on to avoid false positives which currently occur as the
return values of mutex functions like pthread_mutex_destroy() are not taken
into consideration.
The precise description of the bug can be found here:
https://bugs.llvm.org/show_bug
16 matches
Mail list logo
