From: Jakub Jelinek Date: Mon, 24 Feb 2025 08:18:27 +0000 (+0100) Subject: analyzer: Handle nonnull_if_nonzero attribute [PR117023] X-Git-Tag: basepoints/gcc-16~1871 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=206cb6c10589bef4afc90f4df993fc3bdb031e27;p=thirdparty%2Fgcc.git analyzer: Handle nonnull_if_nonzero attribute [PR117023] On top of the https://gcc.gnu.org/pipermail/gcc-patches/2024-November/668554.html patch which introduces the nonnull_if_nonzero attribute (because C2Y is allowing NULL arguments on various calls like memcpy, memset, strncpy etc. as long as the count is 0) the following patch adds just limited handling of the attribute in the analyzer. For nonnull attribute(s) we have the get_nonnull_args helper which returns a bitmap, for nonnull_if_nonzero a function would need to return a hash_map or something similar, I think it is better to handle the attributes one by one. This patch just handles the non-zero INTEGER_CST (integer_nonzerop) count arguments, in other places the above patch uses ranger to some extent, but I'm not familiar enough with the analyzer to know if one can use the ranger, or should somehow explain in data structures the conditional nature of the nonnull property, the argument is nonnull only if some other argument is nonzero. Also, analyzer uses get_nonnull_args in another spot when entering a frame, not sure if anything can be done there (note the conditional nonnull somehow, pass from callers if the argument is nonzero, ...). Note, the testsuite changes aren't strictly necessary with just the above and this patch, but will be with a patch I'm going to post soon. 2025-02-24 Jakub Jelinek PR c/117023 gcc/analyzer/ * sm-malloc.cc (malloc_state_machine::handle_nonnull): New private method. (malloc_state_machine::on_stmt): Use it for nonnull attribute arguments. Handle also nonnull_if_nonzero attributes. gcc/testsuite/ * c-c++-common/analyzer/call-summaries-malloc.c (test_use_without_check): Pass 4 rather than sz to memset. * c-c++-common/analyzer/strncpy-1.c (test_null_dst, test_null_src): Pass 42 rather than count to strncpy. --- diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 2bf957ed289..6972a55d7fd 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -501,6 +501,12 @@ private: void on_zero_assignment (sm_context &sm_ctxt, const gimple *stmt, tree lhs) const; + void handle_nonnull (sm_context &sm_ctx, + const supernode *node, + const gimple *stmt, + tree fndecl, + tree arg, + unsigned i) const; /* A map for consolidating deallocators so that they are unique per deallocator FUNCTION_DECL. */ @@ -2004,6 +2010,43 @@ malloc_state_machine::maybe_assume_non_null (sm_context &sm_ctxt, } } +/* Helper method for malloc_state_machine::on_stmt. Handle a single + argument (Ith argument ARG) if it is nonnull or nonnull_if_nonzero + and size is nonzero. */ + +void +malloc_state_machine::handle_nonnull (sm_context &sm_ctxt, + const supernode *node, + const gimple *stmt, + tree fndecl, + tree arg, + unsigned i) const +{ + state_t state = sm_ctxt.get_state (stmt, arg); + /* Can't use a switch as the states are non-const. */ + /* Do use the fndecl that caused the warning so that the + misused attributes are printed and the user not confused. */ + if (unchecked_p (state)) + { + tree diag_arg = sm_ctxt.get_diagnostic_tree (arg); + sm_ctxt.warn (node, stmt, arg, + make_unique (*this, diag_arg, fndecl, + i)); + const allocation_state *astate + = as_a_allocation_state (state); + sm_ctxt.set_next_state (stmt, arg, astate->get_nonnull ()); + } + else if (state == m_null) + { + tree diag_arg = sm_ctxt.get_diagnostic_tree (arg); + sm_ctxt.warn (node, stmt, arg, + make_unique (*this, diag_arg, fndecl, i)); + sm_ctxt.set_next_state (stmt, arg, m_stop); + } + else if (state == m_start) + maybe_assume_non_null (sm_ctxt, arg, stmt); +} + /* Implementation of state_machine::on_stmt vfunc for malloc_state_machine. */ bool @@ -2118,37 +2161,37 @@ malloc_state_machine::on_stmt (sm_context &sm_ctxt, just the specified pointers. */ if (bitmap_empty_p (nonnull_args) || bitmap_bit_p (nonnull_args, i)) - { - state_t state = sm_ctxt.get_state (stmt, arg); - /* Can't use a switch as the states are non-const. */ - /* Do use the fndecl that caused the warning so that the - misused attributes are printed and the user not - confused. */ - if (unchecked_p (state)) - { - tree diag_arg = sm_ctxt.get_diagnostic_tree (arg); - sm_ctxt.warn (node, stmt, arg, - make_unique - (*this, diag_arg, fndecl, i)); - const allocation_state *astate - = as_a_allocation_state (state); - sm_ctxt.set_next_state (stmt, arg, - astate->get_nonnull ()); - } - else if (state == m_null) - { - tree diag_arg = sm_ctxt.get_diagnostic_tree (arg); - sm_ctxt.warn (node, stmt, arg, - make_unique - (*this, diag_arg, fndecl, i)); - sm_ctxt.set_next_state (stmt, arg, m_stop); - } - else if (state == m_start) - maybe_assume_non_null (sm_ctxt, arg, stmt); - } + handle_nonnull (sm_ctxt, node, stmt, fndecl, arg, i); } BITMAP_FREE (nonnull_args); } + /* Handle __attribute__((nonnull_if_nonzero (x, y))). */ + if (fntype) + for (tree attrs = TYPE_ATTRIBUTES (fntype); + (attrs = lookup_attribute ("nonnull_if_nonzero", attrs)); + attrs = TREE_CHAIN (attrs)) + { + tree args = TREE_VALUE (attrs); + unsigned int idx = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1; + unsigned int idx2 + = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (args))) - 1; + if (idx < gimple_call_num_args (stmt) + && idx2 < gimple_call_num_args (stmt)) + { + tree arg = gimple_call_arg (stmt, idx); + tree arg2 = gimple_call_arg (stmt, idx2); + if (TREE_CODE (TREE_TYPE (arg)) != POINTER_TYPE + || !INTEGRAL_TYPE_P (TREE_TYPE (arg2)) + || integer_zerop (arg2)) + continue; + if (integer_nonzerop (arg2)) + ; + else + /* FIXME: Use ranger here to query arg2 range? */ + continue; + handle_nonnull (sm_ctxt, node, stmt, fndecl, arg, idx); + } + } } /* Check for this after nonnull, so that if we have both diff --git a/gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c b/gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c index 6522c37e108..d9d9ced8d95 100644 --- a/gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c +++ b/gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c @@ -67,7 +67,7 @@ void test_use_after_free (void) void test_use_without_check (size_t sz) { char *buf = (char *) wrapped_malloc (sz); /* { dg-message "this call could return NULL" } */ - memset (buf, 'x', sz); /* { dg-warning "use of possibly-NULL 'buf' where non-null expected" } */ + memset (buf, 'x', 4); /* { dg-warning "use of possibly-NULL 'buf' where non-null expected" } */ wrapped_free (buf); } diff --git a/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c b/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c index 8edaf26654d..f8b38a58d4b 100644 --- a/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c +++ b/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c @@ -20,13 +20,13 @@ test_passthrough (char *dst, const char *src, size_t count) char * test_null_dst (const char *src, size_t count) { - return strncpy (NULL, src, count); /* { dg-warning "use of NULL where non-null expected" } */ + return strncpy (NULL, src, 42); /* { dg-warning "use of NULL where non-null expected" } */ } char * test_null_src (char *dst, size_t count) { - return strncpy (dst, NULL, count); /* { dg-warning "use of NULL where non-null expected" } */ + return strncpy (dst, NULL, 42); /* { dg-warning "use of NULL where non-null expected" } */ } void