Consider: extern int pure_p (int) __attribute__ ((pure)); extern void do_stuff (void); void test (int a) { void *p; if (pure_p (a)) { p = __builtin_malloc (1024); if (!p) return; } do_stuff (); if (pure_p (a)) __builtin_free (p); } Currently trunk -fanalyzer emits these warnings: /tmp/foo.c: In function ‘test’: /tmp/foo.c:14:6: warning: leak of ‘p’ [CWE-401] [-Wanalyzer-malloc-leak] 14 | if (pure_p (a)) | ^ ‘test’: events 1-7 | | 7 | if (pure_p (a)) | | ^ | | | | | (1) following ‘true’ branch... | 8 | { | 9 | p = __builtin_malloc (1024); | | ~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) ...to here | | (3) allocated here | 10 | if (!p) | | ~ | | | | | (4) assuming ‘p’ is non-NULL | | (5) following ‘false’ branch (when ‘p’ is non-NULL)... |...... | 13 | do_stuff (); | | ~~~~~~~~~~~ | | | | | (6) ...to here | 14 | if (pure_p (a)) | | ~ | | | | | (7) following ‘false’ branch... | ‘test’: event 8 | |cc1: | (8): ...to here | ‘test’: event 9 | | 14 | if (pure_p (a)) | | ^ | | | | | (9) ‘p’ leaks here; was allocated at (3) | /tmp/foo.c:15:5: warning: use of uninitialized value ‘p’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value] 15 | __builtin_free (p); | ^~~~~~~~~~~~~~~~~~ ‘test’: events 1-6 | | 6 | void *p; | | ^ | | | | | (1) region created on stack here | 7 | if (pure_p (a)) | | ~ | | | | | (2) following ‘false’ branch... |...... | 13 | do_stuff (); | | ~~~~~~~~~~~ | | | | | (3) ...to here | 14 | if (pure_p (a)) | | ~ | | | | | (4) following ‘true’ branch... | 15 | __builtin_free (p); | | ~~~~~~~~~~~~~~~~~~ | | | | | (5) ...to here | | (6) use of uninitialized value ‘p’ here | by considering the execution paths where pure_p (a) is true then false (the false leak diagnostic), and false then true (the false uninit diagnostic) Presumably the analyzer should consider that the result of a pure/const function doesn't change, and thus only considers the true/true and false/false paths.
Seen on https://github.com/xianyi/OpenBLAS/blob/c5f280a7f0e875d83833d895b2b8b0e341efabf4/lapack-netlib/LAPACKE/src/lapacke_cgbbrd_work.c where the code has: if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' ) ) { pt_t = (lapack_complex_float*) LAPACKE_malloc( sizeof(lapack_complex_float) * ldpt_t * MAX(1,n) ); ...snip... } [...snip lots of code...] if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' ) ) { LAPACKE_free( pt_t ); } where the analyzer considers the execution path where the conditions guarding the malloc and the free are first true, and then false. LAPACKE_lsame is a case-insensitive comparison, implemented in its own source file. I think if it were marked as "pure", the analyzer could fix this without needing LTO.
On rereading https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html I think that "pure" isn't strong enough for the above example: the result of a pure function is allowed to change between invocations with the same inputs. I think the function needs to be "const".
OpenBLAS issue filed as https://github.com/xianyi/OpenBLAS/issues/3543 suggesting the use of __attribute__((const)) on LAPACKE_lsame.
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>: https://gcc.gnu.org/g:aee1adf2cdc1cf4e116e5c05b6e7c92b0fbb264b commit r12-7364-gaee1adf2cdc1cf4e116e5c05b6e7c92b0fbb264b Author: David Malcolm <dmalcolm@redhat.com> Date: Wed Feb 23 09:14:58 2022 -0500 analyzer: handle __attribute__((const)) [PR104434] When testing -fanalyzer on openblas-0.3, I noticed slightly over 2000 false positives from -Wanalyzer-malloc-leak on code like this: if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' ) ) { pt_t = (lapack_complex_float*) LAPACKE_malloc( sizeof(lapack_complex_float) * ldpt_t * MAX(1,n) ); [...snip...] } [...snip lots of code...] if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'q' ) ) { LAPACKE_free( pt_t ); } where LAPACKE_lsame is a char-comparison function implemented in a different TU. The analyzer naively considers the execution path where: LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' ) is true at the malloc guard, but then false at the free guard, which is thus a memory leak. This patch makes -fanalyer respect __attribute__((const)), so that the analyzer treats such functions as returning the same value when given the same inputs. I've filed https://github.com/xianyi/OpenBLAS/issues/3543 suggesting that LAPACKE_lsame be annotated with __attribute__((const)); with that, and with this patch, the false positives seem to be fixed. gcc/analyzer/ChangeLog: PR analyzer/104434 * analyzer.h (class const_fn_result_svalue): New decl. * region-model-impl-calls.cc (call_details::get_manager): New. * region-model-manager.cc (region_model_manager::get_or_create_const_fn_result_svalue): New. (region_model_manager::log_stats): Log m_const_fn_result_values_map. * region-model.cc (const_fn_p): New. (maybe_get_const_fn_result): New. (region_model::on_call_pre): Handle fndecls with __attribute__((const)) by calling the above rather than making a conjured_svalue. * region-model.h (visitor::visit_const_fn_result_svalue): New. (region_model_manager::get_or_create_const_fn_result_svalue): New decl. (region_model_manager::const_fn_result_values_map_t): New typedef. (region_model_manager::m_const_fn_result_values_map): New field. (call_details::get_manager): New decl. * svalue.cc (svalue::cmp_ptr): Handle SK_CONST_FN_RESULT. (const_fn_result_svalue::dump_to_pp): New. (const_fn_result_svalue::dump_input): New. (const_fn_result_svalue::accept): New. * svalue.h (enum svalue_kind): Add SK_CONST_FN_RESULT. (svalue::dyn_cast_const_fn_result_svalue): New. (class const_fn_result_svalue): New. (is_a_helper <const const_fn_result_svalue *>::test): New. (template <> struct default_hash_traits<const_fn_result_svalue::key_t>): New. gcc/testsuite/ChangeLog: PR analyzer/104434 * gcc.dg/analyzer/attr-const-1.c: New test. * gcc.dg/analyzer/attr-const-2.c: New test. * gcc.dg/analyzer/attr-const-3.c: New test. * gcc.dg/analyzer/pr104434-const.c: New test. * gcc.dg/analyzer/pr104434-nonconst.c: New test. * gcc.dg/analyzer/pr104434.h: New test. Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Should be fixed by the above commit for GCC 12.
OpenBLAS commit adding __attribute__((const)) to the decl: https://github.com/xianyi/OpenBLAS/commit/1c1ffb0591186e50311670369dee2cb450980d9a