ClangBuiltLinux / thread-safety-analysis

A research project into applying Clang's Thread Safety Analysis to the Linux Kernel
Other
6 stars 0 forks source link

fs/dcache.c: can't annotate dentry_unlink_inode precisely #104

Open himanshujha199640 opened 5 years ago

himanshujha199640 commented 5 years ago

https://elixir.bootlin.com/linux/v5.2-rc2/source/fs/dcache.c#L368 I was going to mark it as __no_thread_safety_analysis for the following warning:

/home/himanshu/clang-thread-safety-analysis/fs/dcache.c:369:2: warning: releasing mutex 'inode->i_lock' that was not held
      [-Wthread-safety-analysis]
        spin_unlock(&inode->i_lock);
        ^

But instead decided to try:

diff --git a/fs/dcache.c b/fs/dcache.c
index 48eff20b34f7..0b7754842ffa 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -357,7 +357,7 @@ static void dentry_free(struct dentry *dentry)
 static void dentry_unlink_inode(struct dentry * dentry) 
        __releases_spinlock(&dentry->d_lock)
        __releases(dentry->d_lock)
-       __releases(dentry->d_inode->i_lock)
+       __releases(dentry->d_inode->i_lock) __releases_spinlock(dentry->d_inode->i_lock)
 {
        struct inode *inode = dentry->d_inode;

And it doesn't seem to work which appears to be a simple fix. I get weird warning:

/home/himanshu/clang-thread-safety-analysis/fs/dcache.c:369:2: warning: releasing mutex 'inode->i_lock' that was not held
      [-Wthread-safety-analysis]
        spin_unlock(&inode->i_lock);
        ^
/home/himanshu/clang-thread-safety-analysis/fs/dcache.c:376:1: warning: mutex 'dentry->d_inode->i_lock' is still held at the end of
      function [-Wthread-safety-analysis]
}
^
/home/himanshu/clang-thread-safety-analysis/fs/dcache.c:360:38: note: mutex acquired here
        __releases(dentry->d_inode->i_lock) __releases_spinlock((dentry->d_inode->i_lock))
                                            ^
/home/himanshu/clang-thread-safety-analysis/include/linux/compiler_attributes.h:264:50: note: expanded from macro '__releases_spinlock'
# define __releases_spinlock(x)                 __attribute__((release_capability(x)))
                                                               ^
himanshujha199640 commented 5 years ago

Ah, maybe the analysis fails to understand that:

dentry->d_inode->i_lock == inode->i_lock

Context:

struct inode *inode = dentry->d_inode;
....
spin_unlock(&inode->i_lock);
himanshujha199640 commented 5 years ago

Looks like a aliasing limitation: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-alias-analysis

himanshu@himanshu-Vostro-3559:~/gtbit/gsoc19/thread-safety-demo$ cat -n aliasing.c; clang -c -Wthread-safety aliasing.c
     1  struct Mutex {
     2          int mutex;
     3  } __attribute__((capability("mutex")));
     4  
     5  
     6  void acquire(struct Mutex *lock) __attribute__((acquire_capability(lock)));
     7  void release(struct Mutex *lock) __attribute__((release_capability(lock)));
     8  
     9  struct priv {
    10      struct Mutex lock;
    11  };
    12  
    13  struct a {
    14      struct priv *p;
    15  };
    16  
    17  struct b {
    18      struct a *bar;
    19  };
    20  
    21  void test (struct b *foo) __attribute__((release_capability(&foo->bar->p->lock))) {
    22      struct a *check = foo->bar;
    23  
    24      release(&check->p->lock);
    25  }
aliasing.c:24:2: warning: releasing mutex 'check->p->lock' that was not held [-Wthread-safety-analysis]
        release(&check->p->lock);
        ^
aliasing.c:25:1: warning: mutex 'foo->bar->p->lock' is still held at the end of function [-Wthread-safety-analysis]
}
^
aliasing.c:21:42: note: mutex acquired here
void test (struct b *foo) __attribute__((release_capability(&foo->bar->p->lock))) {
                                         ^
2 warnings generated.