From c863b538ad551b1bb651d9bc463a03772ef627d7 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 15 Dec 2025 15:27:40 -0500 Subject: [PATCH 21/30] analyzer: add -Wanalyzer-missing-return [PR123146] After r16-6063-g0b786d961d4426 the analyzer now complains about execution paths that "fall off the bottom" of non-void-returning functions, albeit with messages like this: test.c:23:1: warning: use of uninitialized value '' [CWE-457] [-Wanalyzer-use-of-uninitialized-value] This happens due to handling a gimple return stmt with a non-NULL retval, where DECL_RESULT (fndecl), a result_decl, is uninitialized. This patch special-cases this, so that uninit diagnostics about result_decl nodes gain a better wording (taken from that of -Wreturn-type outside the analyzer), and a new command-line option "-Wanalyzer-missing-return" to better express to the user what is going on, so that the above becomes: test.c:23:1: warning: control reaches end of non-void function [CWE-457] [-Wanalyzer-missing-return] In doing so, I found that the state-merging code was being rather overzealous in merging states where a cluster is bound in one state and uninitialized in another. I tried a general fix for this using m_base_region->can_have_initial_svalue_p, but this led to state explosions in loops, so instead the patch does a more targeted fix that only prevents state merger for the case where a RESULT_DECL node has a value in one state and is uninitialized in the other. Doing so uncovered various pre-existing path-specific missing return statements in the testsuite, which the patch adds dg directives for. gcc/analyzer/ChangeLog: PR analyzer/123146 * analyzer.opt (Wanalyzer-missing-return): New option. * analyzer.opt.urls: Regenerate. * region-model.cc (poisoned_value_diagnostic::poisoned_value_diagnostic): Assert that m_expr is non-null. (poisoned_value_diagnostic::get_controlling_option): Use OPT_Wanalyzer_missing_return for uninit of RESULT_DECL. (poisoned_value_diagnostic::emit): Customize wording of uninit of RESULT_DECL. (poisoned_value_diagnostic::describe_final_event): Likewise. (poisoned_value_diagnostic::mark_interesting_stuff): Don't show region creation if it's a RESULT_DECL. * store.cc (binding_cluster::can_merge_p): Don't merge uninit clusters for RESULT_DECL nodes with initialized clusters. gcc/ChangeLog: PR analyzer/123146 * doc/invoke.texi: Document -Wanalyzer-missing-return. gcc/testsuite/ChangeLog: PR analyzer/123146 * c-c++-common/analyzer/missing-return.c: New test. * gcc.dg/analyzer/call-summaries-pr114473.c: Expect missing return warning. * gcc.dg/analyzer/compound-assignment-2.c: Update expected warning message. * gcc.dg/analyzer/compound-assignment-3.c: Likewise. * gcc.dg/analyzer/explode-4.c: Expect missing return warning. * gcc.dg/analyzer/pr101837.c: Update expected warning message. * gcc.dg/analyzer/pr94579.c: Likewise. * gcc.dg/analyzer/switch-enum-taint-1.c: Likewise. * gcc.dg/analyzer/taint-CVE-2020-13143-1.c: Expect missing return warning. * gcc.dg/analyzer/zlib-6.c: Likewise. * gcc.dg/analyzer/zlib-6a.c: Likewise. Signed-off-by: David Malcolm --- gcc/analyzer/analyzer.opt | 4 ++ gcc/analyzer/analyzer.opt.urls | 3 ++ gcc/analyzer/region-model.cc | 29 +++++++++---- gcc/analyzer/store.cc | 8 ++++ gcc/doc/invoke.texi | 15 +++++++ .../c-c++-common/analyzer/missing-return.c | 41 +++++++++++++++++++ .../gcc.dg/analyzer/call-summaries-pr114473.c | 2 +- .../gcc.dg/analyzer/compound-assignment-2.c | 2 +- .../gcc.dg/analyzer/compound-assignment-3.c | 2 +- gcc/testsuite/gcc.dg/analyzer/explode-4.c | 2 +- gcc/testsuite/gcc.dg/analyzer/pr101837.c | 2 +- gcc/testsuite/gcc.dg/analyzer/pr94579.c | 2 +- .../gcc.dg/analyzer/switch-enum-taint-1.c | 2 +- .../gcc.dg/analyzer/taint-CVE-2020-13143-1.c | 2 +- gcc/testsuite/gcc.dg/analyzer/zlib-6.c | 2 +- gcc/testsuite/gcc.dg/analyzer/zlib-6a.c | 2 +- 16 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/analyzer/missing-return.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 1e55003dd2e..bed2f3506fc 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -158,6 +158,10 @@ Wanalyzer-mismatching-deallocation Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning Warn about code paths in which the wrong deallocation function is called. +Wanalyzer-missing-return +Common Var(warn_analyzer_missing_return) Init(1) Warning +Warn about code paths in which control reaches the end of a non-void function. + Wanalyzer-out-of-bounds Common Var(warn_analyzer_out_of_bounds) Init(1) Warning Warn about code paths in which a write or read to a buffer is out-of-bounds. diff --git a/gcc/analyzer/analyzer.opt.urls b/gcc/analyzer/analyzer.opt.urls index a0789445d30..a078856a350 100644 --- a/gcc/analyzer/analyzer.opt.urls +++ b/gcc/analyzer/analyzer.opt.urls @@ -66,6 +66,9 @@ UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-malloc-leak) Wanalyzer-mismatching-deallocation UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-mismatching-deallocation) +Wanalyzer-missing-return +UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-missing-return) + Wanalyzer-out-of-bounds UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-out-of-bounds) diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index df9d5b5b86e..9253c674c4d 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -755,7 +755,9 @@ public: : m_expr (expr), m_pkind (pkind), m_src_region (src_region), m_check_expr (check_expr) - {} + { + gcc_assert (m_expr); + } const char *get_kind () const final override { return "poisoned_value_diagnostic"; } @@ -778,7 +780,10 @@ public: default: gcc_unreachable (); case poison_kind::uninit: - return OPT_Wanalyzer_use_of_uninitialized_value; + if (TREE_CODE (m_expr) == RESULT_DECL) + return OPT_Wanalyzer_missing_return; + else + return OPT_Wanalyzer_use_of_uninitialized_value; case poison_kind::freed: case poison_kind::deleted: return OPT_Wanalyzer_use_after_free; @@ -798,8 +803,12 @@ public: case poison_kind::uninit: { ctxt.add_cwe (457); /* "CWE-457: Use of Uninitialized Variable". */ - return ctxt.warn ("use of uninitialized value %qE", - m_expr); + if (TREE_CODE (m_expr) == RESULT_DECL) + /* Mimic the message from -Wreturn-type. */ + return ctxt.warn ("control reaches end of non-void function"); + else + return ctxt.warn ("use of uninitialized value %qE", + m_expr); } break; case poison_kind::freed: @@ -837,9 +846,12 @@ public: gcc_unreachable (); case poison_kind::uninit: { - pp_printf (&pp, - "use of uninitialized value %qE here", - m_expr); + if (TREE_CODE (m_expr) == RESULT_DECL) + pp_printf (&pp, "control reaches end of non-void function here"); + else + pp_printf (&pp, + "use of uninitialized value %qE here", + m_expr); return true; } case poison_kind::freed: @@ -868,7 +880,8 @@ public: void mark_interesting_stuff (interesting_t *interest) final override { - if (m_src_region) + if (m_src_region + && TREE_CODE (m_expr) != RESULT_DECL) interest->add_region_creation (m_src_region); } diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc index 878536eb42b..8b66a35f63c 100644 --- a/gcc/analyzer/store.cc +++ b/gcc/analyzer/store.cc @@ -2248,6 +2248,10 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a, { gcc_assert (cluster_b != nullptr); gcc_assert (cluster_b->m_base_region == out_cluster->m_base_region); + /* Don't merge with uninit for RESULT_DECL. */ + if (tree decl = out_cluster->m_base_region->maybe_get_decl ()) + if (TREE_CODE (decl) == RESULT_DECL) + return false; out_cluster->make_unknown_relative_to (cluster_b, out_store, mgr); return true; } @@ -2255,6 +2259,10 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a, { gcc_assert (cluster_a != nullptr); gcc_assert (cluster_a->m_base_region == out_cluster->m_base_region); + /* Don't merge with uninit for RESULT_DECL. */ + if (tree decl = out_cluster->m_base_region->maybe_get_decl ()) + if (TREE_CODE (decl) == RESULT_DECL) + return false; out_cluster->make_unknown_relative_to (cluster_a, out_store, mgr); return true; } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index dcb6cfbab23..e5a4592411f 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -528,6 +528,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-jump-through-null -Wno-analyzer-malloc-leak -Wno-analyzer-mismatching-deallocation +-Wno-analyzer-missing-return -Wno-analyzer-null-argument -Wno-analyzer-null-dereference -Wno-analyzer-out-of-bounds @@ -11572,6 +11573,7 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-jump-through-null -Wanalyzer-malloc-leak -Wanalyzer-mismatching-deallocation +-Wanalyzer-missing-return -Wanalyzer-null-argument -Wanalyzer-null-dereference -Wanalyzer-out-of-bounds @@ -11976,6 +11978,19 @@ pairs using attribute @code{malloc}. See @uref{https://cwe.mitre.org/data/definitions/762.html, CWE-762: Mismatched Memory Management Routines}. +@opindex Wanalyzer-missing-return +@opindex Wno-analyzer-missing-return +@item -Wno-analyzer-missing-return +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-missing-return} +to disable it. + +This diagnostic warns for paths through the code which ``fall off the +bottom'' of a non-void function without a @code{return} statement. +This is similar to @option{-Wreturn-type}, but is implemented in a +different way, and will show any control flow needed to reach the +end of the function. + @opindex Wanalyzer-out-of-bounds @opindex Wno-analyzer-out-of-bounds @item -Wno-analyzer-out-of-bounds diff --git a/gcc/testsuite/c-c++-common/analyzer/missing-return.c b/gcc/testsuite/c-c++-common/analyzer/missing-return.c new file mode 100644 index 00000000000..b560198aaa0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/analyzer/missing-return.c @@ -0,0 +1,41 @@ +int +test_1 (void) +{ +} /* { dg-warning "control reaches end of non-void function \\\[CWE-457\\\] \\\[-Wanalyzer-missing-return\\\]" } */ + +int +test_2 (int flag) +{ + if (flag) + 42; + else + 1066; +} /* { dg-warning "control reaches end of non-void function" } */ + +int +test_3 (int flag) +{ + if (flag) /* { dg-message "following 'false' branch" } */ + return 42; +} /* { dg-warning "control reaches end of non-void function" } */ + +int +test_4 (int x) +{ + switch (x) /* { dg-message "following 'default:' branch" } */ + { + case 0: + return 42; + case 1: + return 1066; + case 2: + return 1776; + } +} /* { dg-warning "control reaches end of non-void function" } */ + +int +test_5 (void) +{ + int i; + return i; /* { dg-warning "use of uninitialized value 'i'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/call-summaries-pr114473.c b/gcc/testsuite/gcc.dg/analyzer/call-summaries-pr114473.c index 4598840f0df..6c3227ae078 100644 --- a/gcc/testsuite/gcc.dg/analyzer/call-summaries-pr114473.c +++ b/gcc/testsuite/gcc.dg/analyzer/call-summaries-pr114473.c @@ -11,7 +11,7 @@ baz (int *src) src && a; return src; } -} +} /* { dg-warning "control reaches end of non-void function" } */ void bar (int **src) diff --git a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c index 4b1f27854cc..8e79f18b005 100644 --- a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-2.c @@ -22,4 +22,4 @@ test_2 (void) aw2.ptrs[1] = malloc (512); } /* { dg-warning "leak of 'aw2.ptrs.0.'" "leak of element 0" } */ /* { dg-warning "leak of 'aw2.ptrs.1.'" "leak of element 1" { target *-*-* } .-1 } */ -/* { dg-warning "use of uninitialized value ''" "missing return" { target *-*-* } .-2 } */ +/* { dg-warning "control reaches end of non-void function" "missing return" { target *-*-* } .-2 } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c index 86e7c69ba6f..0697a1b2491 100644 --- a/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c +++ b/gcc/testsuite/gcc.dg/analyzer/compound-assignment-3.c @@ -23,4 +23,4 @@ test_2 (void) struct union_wrapper uw2; uw2.u.ptr = malloc (1024); } /* { dg-warning "leak of 'uw2.u.ptr'" } */ -/* { dg-warning "use of uninitialized value ''" "missing return" { target *-*-* } .-1 } */ +/* { dg-warning "control reaches end of non-void function" "missing return" { target *-*-* } .-1 } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/explode-4.c b/gcc/testsuite/gcc.dg/analyzer/explode-4.c index a98dfb56bf5..2e6f70a7207 100644 --- a/gcc/testsuite/gcc.dg/analyzer/explode-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/explode-4.c @@ -126,7 +126,7 @@ fail: libusb_reset_device(s->dh); usb_host_attach_kernel(s); } -} +} /* { dg-warning "control reaches end of non-void function" } */ static QEMUTimer *usb_auto_timer; static void usb_host_vm_state(void *unused, _Bool running) {} static void usb_host_auto_check(void *unused) { diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101837.c b/gcc/testsuite/gcc.dg/analyzer/pr101837.c index a61f1b5ec07..fe32ba670e2 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr101837.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr101837.c @@ -7,6 +7,6 @@ void memcheck(void *ptr) { } int emalloc(int size) { memcheck(__builtin_malloc(size)); } /* { dg-message "allocated here" } */ -/* { dg-warning "use of uninitialized value ''" "missing return" { target *-*-* } .-1 } */ +/* { dg-warning "control reaches end of non-void function" "missing return" { target *-*-* } .-1 } */ void main() { int max_envvar_len = emalloc(max_envvar_len + 1); } /* { dg-message "use of uninitialized value 'max_envvar_len'" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94579.c b/gcc/testsuite/gcc.dg/analyzer/pr94579.c index 96d588cb475..017e2317577 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr94579.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr94579.c @@ -2,7 +2,7 @@ struct a *c; struct a { int b; } d() { -} /* { dg-warning "use of uninitialized value ''" } */ +} /* { dg-warning "control reaches end of non-void function" } */ void e() diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-enum-taint-1.c b/gcc/testsuite/gcc.dg/analyzer/switch-enum-taint-1.c index 0c3bbf675e4..983fe3fdb03 100644 --- a/gcc/testsuite/gcc.dg/analyzer/switch-enum-taint-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/switch-enum-taint-1.c @@ -26,7 +26,7 @@ test_all_values_covered_implicit_default_1 (enum e x) return 1945; } __analyzer_dump_path (); /* { dg-message "path" } */ -} /* { dg-warning "use of uninitialized value ''" } */ +} /* { dg-warning "control reaches end of non-void function" } */ int __attribute__((tainted_args)) test_all_values_covered_implicit_default_2 (enum e x) diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c index 1b81c1bb1f8..12e2db7d2b2 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c @@ -30,6 +30,6 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, if (name[len - 1] == '\n') /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ name[len - 1] = '\0'; /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ /* [...snip...] */ \ -} +} /* { dg-warning "control reaches end of non-void function" } */ CONFIGFS_ATTR(gadget_dev_desc_, UDC); /* { dg-message "\\(2\\) function 'gadget_dev_desc_UDC_store' used as initializer for field 'store' marked with '__attribute__\\(\\(tainted_args\\)\\)'" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-6.c b/gcc/testsuite/gcc.dg/analyzer/zlib-6.c index c8e06c61313..b63b0d24b39 100644 --- a/gcc/testsuite/gcc.dg/analyzer/zlib-6.c +++ b/gcc/testsuite/gcc.dg/analyzer/zlib-6.c @@ -37,4 +37,4 @@ int inflate_blocks(inflate_blocks_statef *s, z_stream *z, int r, b |= ((uLong)(n--, *p++)) << k; k += 8; } -} +} /* { dg-warning "control reaches end of non-void function" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-6a.c b/gcc/testsuite/gcc.dg/analyzer/zlib-6a.c index 8c83de4863b..cee2870e243 100644 --- a/gcc/testsuite/gcc.dg/analyzer/zlib-6a.c +++ b/gcc/testsuite/gcc.dg/analyzer/zlib-6a.c @@ -46,4 +46,4 @@ int inflate_blocks(inflate_blocks_statef *s, z_stream *z, int r) { b |= ((uLong)(n--, *p++)) << k; /* { dg-warning "use of uninitialized value" } */ k += 8; /* { dg-warning "use of uninitialized value 'k'" } */ } -} +} /* { dg-warning "control reaches end of non-void function" } */ -- 2.49.0