analyzer: fix missing leak after call to strsep [PR100615]
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 18 May 2021 16:29:58 +0000 (12:29 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Tue, 18 May 2021 16:29:58 +0000 (12:29 -0400)
PR analyzer/100615 reports a missing leak diagnostic.
The issue is that the code calls strsep which the analyzer doesn't
have special knowledge of, and so conservatively assumes that it
could free the pointer, so drops malloc state for it.

Properly "teaching" the analyzer about strsep would require it
to support bifurcating state at a call, which is currently fiddly to
do, so for now this patch notes that strsep doesn't affect the
malloc state machine, allowing the analyzer to correctly detect the leak.

gcc/analyzer/ChangeLog:
PR analyzer/100615
* sm-malloc.cc: Include "analyzer/function-set.h".
(malloc_state_machine::on_stmt): Call unaffected_by_call_p and
bail on the functions it recognizes.
(malloc_state_machine::unaffected_by_call_p): New.

gcc/testsuite/ChangeLog:
PR analyzer/100615
* gcc.dg/analyzer/pr100615.c: New test.

gcc/analyzer/sm-malloc.cc
gcc/testsuite/gcc.dg/analyzer/pr100615.c [new file with mode: 0644]

index f02b73a..a1582ca 100644 (file)
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/region-model.h"
 #include "stringpool.h"
 #include "attribs.h"
+#include "analyzer/function-set.h"
 
 #if ENABLE_ANALYZER
 
@@ -384,6 +385,8 @@ public:
   bool reset_when_passed_to_unknown_fn_p (state_t s,
                                          bool is_mutable) const FINAL OVERRIDE;
 
+  static bool unaffected_by_call_p (tree fndecl);
+
   standard_deallocator_set m_free;
   standard_deallocator_set m_scalar_delete;
   standard_deallocator_set m_vector_delete;
@@ -1569,6 +1572,9 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
            return true;
          }
 
+       if (unaffected_by_call_p (callee_fndecl))
+         return true;
+
        /* Cast away const-ness for cache-like operations.  */
        malloc_state_machine *mutable_this
          = const_cast <malloc_state_machine *> (this);
@@ -1925,6 +1931,28 @@ malloc_state_machine::reset_when_passed_to_unknown_fn_p (state_t s,
   return is_mutable;
 }
 
+/* Return true if calls to FNDECL are known to not affect this sm-state.  */
+
+bool
+malloc_state_machine::unaffected_by_call_p (tree fndecl)
+{
+  /* A set of functions that are known to not affect allocation
+     status, even if we haven't fully modelled the rest of their
+     behavior yet.  */
+  static const char * const funcnames[] = {
+    /* This array must be kept sorted.  */
+    "strsep",
+  };
+  const size_t count
+    = sizeof(funcnames) / sizeof (funcnames[0]);
+  function_set fs (funcnames, count);
+
+  if (fs.contains_decl_p (fndecl))
+    return true;
+
+  return false;
+}
+
 /* Shared logic for handling GIMPLE_ASSIGNs and GIMPLE_PHIs that
    assign zero to LHS.  */
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr100615.c b/gcc/testsuite/gcc.dg/analyzer/pr100615.c
new file mode 100644 (file)
index 0000000..7a06f98
--- /dev/null
@@ -0,0 +1,53 @@
+/* Adapted from
+   https://github.com/stackpath/rxtxcpu/blob/816d86c5d49c4db2ea5649f6b87e96da5af660f1/cpu.c
+   which is MIT-licensed.  */
+
+typedef __SIZE_TYPE__ size_t;
+#define NULL ((void *)0)
+
+extern size_t strlen (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__pure__))
+  __attribute__ ((__nonnull__ (1)));
+extern char *strdup (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__malloc__))
+  __attribute__ ((__nonnull__ (1)));
+extern char *strsep (char **__restrict __stringp,
+                    const char *__restrict __delim)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__nonnull__ (1, 2)));
+extern long int strtol (const char *__restrict __nptr,
+                       char **__restrict __endptr, int __base)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__nonnull__ (1)));
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+#define CPU_LIST_BASE 10
+
+int parse_cpu_list(char *cpu_list) {
+  if (strlen(cpu_list) == 0) {
+    return 0;
+  }
+
+  char *endptr;
+  char *tofree, *string, *range;
+
+  tofree = string = strdup(cpu_list); /* { dg-message "allocated here" } */
+
+  while ((range = strsep(&string, ",")) != NULL) {
+    int first = strtol(range, &endptr, CPU_LIST_BASE);
+    if (!*endptr) {
+      continue;
+    }
+    char *save = endptr;
+    endptr++;
+    int last = strtol(endptr, &endptr, CPU_LIST_BASE);
+    if (save[0] != '-' || *endptr || last < first) {
+      return -1; /* { dg-warning "leak of 'tofree'" } */
+    }
+  }
+  free(tofree);
+  return 0;
+}