Open himanshujha199640 opened 5 years ago
Agree, you will need to look at all uses, this block is inlined anyway, so there is no need to annotate this.
As we can't annotate macros, the warning for mm.h
can't be silenced in any way.
Looking at the users in gup.c
above, the warning wouldn't have been generated if we have
pte_unmap_unlock();
macro to do the unlocking. But the problem is the obfuscation in the function
which leads to analysis to fail.
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:200:9: note: mutex acquired here
ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2013:2: note: expanded from macro 'pte_offset_map_lock'
spin_lock(__ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:216:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
pte_unmap_unlock(ptep, ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2018:2: note: expanded from macro 'pte_unmap_unlock'
spin_unlock(ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:223:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
pte_unmap_unlock(ptep, ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2018:2: note: expanded from macro 'pte_unmap_unlock'
spin_unlock(ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:259:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
pte_unmap_unlock(ptep, ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2018:2: note: expanded from macro 'pte_unmap_unlock'
spin_unlock(ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:313:2: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
pte_unmap_unlock(ptep, ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2018:2: note: expanded from macro 'pte_unmap_unlock'
spin_unlock(ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:316:2: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
pte_unmap_unlock(ptep, ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2018:2: note: expanded from macro 'pte_unmap_unlock'
spin_unlock(ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:320:1: warning: mutex '__ptl' is not held on every path through here [-Wthread-safety-analysis]
}
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:200:9: note: mutex acquired here
ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
^
/home/himanshu/clang-thread-safety-analysis/include/linux/mm.h:2013:2: note: expanded from macro 'pte_offset_map_lock'
spin_lock(__ptl); \
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:374:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:387:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:391:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:398:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:405:4: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:412:5: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:415:4: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:428:2: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
/home/himanshu/clang-thread-safety-analysis/mm/gup.c:463:3: warning: releasing mutex 'ptl' that was not held [-Wthread-safety-analysis]
spin_unlock(ptl);
^
There are many warnings on this:
himanshu@himanshu-Vostro-3559:~/clang-thread-safety-analysis$ grep 'pte_off' ~/build/logs/clang-analysis-allnoconfig-spinlock-1-morning.txt | wc -l
62
himanshu@himanshu-Vostro-3559:~/clang-thread-safety-analysis$ grep 'pte_unmap' ~/build/logs/clang-analysis-allnoconfig-spinlock-1-morning.txt | wc -l
74
It is really annoying that we cannot annotate macros and in this case, we cannot analyse the inlined block properly due to the assignment in the block. I decided to turn the macro into a function and annotate it. This change of course is unlikely to be accepted upstream.
At include/net/af_unix.h
:
46 #define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
47 #define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1092:1: warning: mutex 'unix_sk(sk1).lock' is not held on every path through here [-Wthread-safety-analysis]
}
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1086:3: note: mutex acquired here
unix_state_lock(sk1);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1092:1: warning: mutex 'unix_sk(sk2).lock' is not held on every path through here [-Wthread-safety-analysis]
}
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1089:3: note: mutex acquired here
unix_state_lock(sk2);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1092:1: warning: mutex 'unix_sk(sk1).lock' is not held on every path through here [-Wthread-safety-analysis]
}
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1082:3: note: mutex acquired here
unix_state_lock(sk1);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1097:3: warning: releasing mutex 'unix_sk(sk1).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk1);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1100:2: warning: releasing mutex 'unix_sk(sk1).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk1);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1101:2: warning: releasing mutex 'unix_sk(sk2).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk2);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1196:2: warning: releasing mutex 'unix_sk(other).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1250:10: warning: expecting mutex 'unix_sk(other).lock' to be held at start of each loop [-Wthread-safety-analysis]
other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, hash, &err);
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1255:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1313:3: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1321:3: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1370:2: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1386:2: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
kfree_skb(skb);
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1255:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1386:2: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
kfree_skb(skb);
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1255:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1650:7: warning: expecting mutex 'unix_sk(other).lock' to be held at start of each loop [-Wthread-safety-analysis]
if (!other) {
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1670:9: warning: expecting mutex 'unix_sk(other).lock' to be held at start of each loop [-Wthread-safety-analysis]
err = -EPERM;
^
/home/himanshu/clang-thread-safety-analysis/include/uapi/asm-generic/errno-base.h:5:17: note: expanded from macro 'EPERM'
#define EPERM 1 /* Operation not permitted */
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1685:9: warning: mutex 'unix_sk(sk).lock' is not held on every path through here [-Wthread-safety-analysis]
err = 0;
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1683:4: note: mutex acquired here
unix_state_lock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1690:4: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1696:4: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1736:7: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (unix_peer(sk) != other ||
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:185:24: note: expanded from macro 'unix_peer'
#define unix_peer(sk) (unix_sk(sk)->peer)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1749:6: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (unlikely(sk_locked))
^
/home/himanshu/clang-thread-safety-analysis/include/linux/compiler.h:78:22: note: expanded from macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1749:6: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (unlikely(sk_locked))
^
/home/himanshu/clang-thread-safety-analysis/include/linux/compiler.h:78:22: note: expanded from macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1750:3: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1756:2: warning: releasing mutex 'unix_sk(other).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1763:6: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (sk_locked)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1763:6: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (sk_locked)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1763:6: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
if (sk_locked)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1764:3: warning: releasing mutex 'unix_sk(sk).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(sk);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1765:2: warning: releasing mutex 'unix_sk(other).lock' that was not held [-Wthread-safety-analysis]
unix_state_unlock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:47:30: note: expanded from macro 'unix_state_unlock'
#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock)
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1767:2: warning: mutex 'unix_sk(other).lock' is not held on every path through here [-Wthread-safety-analysis]
kfree_skb(skb);
^
/home/himanshu/clang-thread-safety-analysis/net/unix/af_unix.c:1668:2: note: mutex acquired here
unix_state_lock(other);
^
/home/himanshu/clang-thread-safety-analysis/include/net/af_unix.h:46:28: note: expanded from macro 'unix_state_lock'
#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock)
^
At block/blk-mq-debugfs.c
:
641 #define CTX_RQ_SEQ_OPS(name, type) \
642 static void *ctx_##name##_rq_list_start(struct seq_file *m, loff_t *pos) \
643 __acquires(&ctx->lock) \
644 { \
645 struct blk_mq_ctx *ctx = m->private; \
646 \
647 **spin_lock(&ctx->lock);** \
648 return seq_list_start(&ctx->rq_lists[type], *pos); \
649 } \
650 \
651 static void *ctx_##name##_rq_list_next(struct seq_file *m, void *v, \
652 loff_t *pos) \
653 { \
654 struct blk_mq_ctx *ctx = m->private; \
655 \
656 return seq_list_next(v, &ctx->rq_lists[type], pos); \
657 } \
658 \
659 static void ctx_##name##_rq_list_stop(struct seq_file *m, void *v) \
660 __releases(&ctx->lock) \
661 { \
662 struct blk_mq_ctx *ctx = m->private; \
663 \
664 **spin_unlock(&ctx->lock);** \
665 } \
666 \
At block/mq-deadline.c
:
656 #define DEADLINE_DEBUGFS_DDIR_ATTRS(ddir, name) \
657 static void *deadline_##name##_fifo_start(struct seq_file *m, \
658 loff_t *pos) \
659 __acquires(&dd->lock) \
660 { \
661 struct request_queue *q = m->private; \
662 struct deadline_data *dd = q->elevator->elevator_data; \
663 \
664 spin_lock(&dd->lock); \
665 return seq_list_start(&dd->fifo_list[ddir], *pos); \
666 } \
667 \
668 static void *deadline_##name##_fifo_next(struct seq_file *m, void *v, \
669 loff_t *pos) \
670 { \
671 struct request_queue *q = m->private; \
672 struct deadline_data *dd = q->elevator->elevator_data; \
673 \
674 return seq_list_next(v, &dd->fifo_list[ddir], pos); \
675 } \
676 \
677 static void deadline_##name##_fifo_stop(struct seq_file *m, void *v) \
678 __releases(&dd->lock) \
679 { \
680 struct request_queue *q = m->private; \
681 struct deadline_data *dd = q->elevator->elevator_data; \
682 \
683 spin_unlock(&dd->lock); \
684 } \
685 \
...
This change of course is unlikely to be accepted upstream.
Maybe, but I still think it could be made a static inline
function no problem.
which will give warnings on subsequent users such as: