================
@@ -0,0 +1,80 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify %s
+//
+
+typedef unsigned long size_t;
+
+typedef enum memory_order {
+  memory_order_relaxed = __ATOMIC_RELAXED,
+} memory_order;
+
+void *calloc(size_t, size_t);
+void free(void *);
+
+struct SomeData {
+  int i;
+  _Atomic int ref;
+};
+
+static struct SomeData *alloc_data(void)
+{
+  struct SomeData *data = calloc(sizeof(*data), 1);
+
+  __c11_atomic_store(&data->ref, 2, memory_order_relaxed);
+  return data;
+}
+
+static void put_data(struct SomeData *data)
+{
+ if (__c11_atomic_fetch_sub(&data->ref, 1, memory_order_relaxed) == 1)
+   free(data);
+}
+
+static int dec_refcounter(struct SomeData *data)
+{
+  return __c11_atomic_fetch_sub(&data->ref, 1, memory_order_relaxed) == 1;
+}
+
+static void put_data_nested(struct SomeData *data)
+{
+  if (dec_refcounter(data))
+    free(data);
+}
+
+static void put_data_uncond(struct SomeData *data)
+{
+  free(data);
+}
+
+static void put_data_unrelated_atomic(struct SomeData *data)
+{
+  free(data);
+  __c11_atomic_fetch_sub(&data->ref, 1, memory_order_relaxed);
----------------
NagyDonat wrote:

No, `LocationContext::isParentOf` is not relevant here. (Its purpose is to 
handle cases like `put_data_nested` where the atomic operation is found within 
a location context that is a descendant of the `ReleaseFunctionLC`.)

The suppression logic is very simple -- `MallocBugVisitor::VisitNode` just 
visits _each_ exploded graph node along the execution path that led to creating 
the bug report, and performs the suppression when it finds an atomic add/sub 
that is (A) in the stack frame corresponding to `ReleaseFunctionLC` or (B) in a 
stack frame of a function that is a descendant of `ReleaseFunctionLC`. [Part 
(B) is facilitated by the `LocationContext::isParentOf` call.]

This means that the suppression can be triggered by
- atomic add/sub on totally unrelated operands;
- atomic add/sub whose result is ignored;
- atomic add/sub that happens _after_ the free call and is a clear 
use-after-free error.

To avoid suppressing TPs in cases like the one bothering you, it would be 
possible refine the suppression logic to
- only look for atomic operations that are performed in the condition of a 
conditional statement; and/or
- only look for atomic operations that happen before the `free()` call [because 
operations that happen after the `free()` are either use-after-free errors or 
they are almost surely unrelated to the current object].

(This could be a nice follow-up commit if you're interested and have time for 
it.)

https://github.com/llvm/llvm-project/pull/104599
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to