On Fri, Feb 14, 2025 at 09:14:48AM -0500, David Malcolm wrote:
> I haven't used ranger yet within the analyzer, and I wonder if there is
> a philosophical divide here between the goals of optimization versus
> bug finding: the optimizer makes use of undefined behavior in order to
> add assumptions about the state of the code ("X can't happen"), whereas
> the analyzer ought to report on places where undefined behavior could
> happen ("if X happened here, it would be bad, let's warn the user").
You're right, that is why also using the ranger in the __builtin_*object_size
handling and sanitizers is problematic.
Maybe we'll need a ranger mode which only looks at clearly guaranteed things
rather than exploiting undefined behavior (including whatever is saved in
SSA_NAME_RANGE_INFO).
> One way of expressing the conditional nature of this property in the
> analyzer would be to "bifurcate" the analysis so that there are two
> out-edges in the exploded graph, covering the ptr == NULL vs ptr !=
> NULL conditions explicitly and separately. But that's fiddly, alas,
> and not something to be doing for GCC 15 at this stage (sorry).
Indeed.
> I see that the code from
> state_t state = sm_ctxt.get_state (stmt, arg);
> onwards seems to be a copy-and-paste of the existing code from within
> the
> /* Handle "__attribute__((nonnull))". */
> suite. malloc_state_machine::on_stmt is already longer than I'm
> comfortable with, so please can you introduce a subroutine to avoid the
> copy-and-paste (probably a private member function of
> malloc_state_machine).
>
> OK for trunk with that change (assuming usual testing, of course)
Here is what I've committed after another bootstrap/regtest on
x86_64-linux and i686-linux.
2025-02-24 Jakub Jelinek <ja...@redhat.com>
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.
--- gcc/analyzer/sm-malloc.cc.jj 2024-10-24 22:56:14.111158433 +0200
+++ gcc/analyzer/sm-malloc.cc 2025-02-24 00:28:51.111155033 +0100
@@ -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_n
}
}
+/* 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<possible_null_arg> (*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<null_arg> (*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_contex
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<possible_null_arg>
- (*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<null_arg>
- (*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
--- gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c.jj
2024-09-18 15:03:34.578092119 +0200
+++ gcc/testsuite/c-c++-common/analyzer/call-summaries-malloc.c 2024-11-14
10:54:35.445125885 +0100
@@ -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);
}
--- gcc/testsuite/c-c++-common/analyzer/strncpy-1.c.jj 2024-01-18
22:04:50.983005923 +0100
+++ gcc/testsuite/c-c++-common/analyzer/strncpy-1.c 2024-11-14
10:55:26.035410085 +0100
@@ -20,13 +20,13 @@ test_passthrough (char *dst, const char
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
