From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 7040D3858016; Wed, 15 Jun 2022 21:47:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7040D3858016 From: "cvs-commit at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug analyzer/105962] Unhelpful diagnostics paths from analyzer in the face of inlining Date: Wed, 15 Jun 2022 21:47:30 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: analyzer X-Bugzilla-Version: 12.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: dmalcolm at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jun 2022 21:47:30 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D105962 --- Comment #2 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:63c073199491b7ec2261d39af51c02147c2f0daf commit r13-1119-g63c073199491b7ec2261d39af51c02147c2f0daf Author: David Malcolm Date: Wed Jun 15 17:44:14 2022 -0400 analyzer: fix up paths for inlining (PR analyzer/105962) -fanalyzer runs late compared to other code analysis tools, in that in runs on the partially-optimized gimple-ssa representation. I chose this point to run in the hope of easy integration with LTO. As PR analyzer/105962 notes, this means that function inlining can occur before the -fanalyzer "sees" the user's code. For example given: void foo (void *p) { __builtin_free (p); } void bar (void *q) { foo (q); foo (q); } Below -O2, -fanalyzer shows the calls and returns: inline-1.c: In function =C3=A2foo=C3=A2: inline-1.c:3:3: warning: double-=C3=A2free=C3=A2 of =C3=A2p=C3=A2 [CWE-= 415] [-Wanalyzer-double-free] 3 | __builtin_free (p); | ^~~~~~~~~~~~~~~~~~ =C3=A2bar=C3=A2: events 1-2 | | 6 | void bar (void *q) | | ^~~ | | | | | (1) entry to =C3=A2bar=C3=A2 | 7 | { | 8 | foo (q); | | ~~~~~~~ | | | | | (2) calling =C3=A2foo=C3=A2 from =C3=A2bar=C3=A2 | +--> =C3=A2foo=C3=A2: events 3-4 | | 1 | void foo (void *p) | | ^~~ | | | | | (3) entry to =C3=A2foo=C3=A2 | 2 | { | 3 | __builtin_free (p); | | ~~~~~~~~~~~~~~~~~~ | | | | | (4) first =C3=A2free=C3=A2 here | <------+ | =C3=A2bar=C3=A2: events 5-6 | | 8 | foo (q); | | ^~~~~~~ | | | | | (5) returning to =C3=A2bar=C3=A2 from =C3=A2foo=C3=A2 | 9 | foo (q); | | ~~~~~~~ | | | | | (6) passing freed pointer =C3=A2q=C3=A2 in call to =C3= =A2foo=C3=A2 from =C3=A2bar=C3=A2 | +--> =C3=A2foo=C3=A2: events 7-8 | | 1 | void foo (void *p) | | ^~~ | | | | | (7) entry to =C3=A2foo=C3=A2 | 2 | { | 3 | __builtin_free (p); | | ~~~~~~~~~~~~~~~~~~ | | | | | (8) second =C3=A2free=C3=A2 here; first =C3=A2fre= e=C3=A2 was at (4) | but at -O2, -fanalyzer "sees" this gimple: void bar (void * q) { [local count: 1073741824]: __builtin_free (q_2(D)); __builtin_free (q_2(D)); return; } where "foo" has been inlined away, leading to this unhelpful output: In function =C3=A2foo=C3=A2, inlined from =C3=A2bar=C3=A2 at inline-1.c:9:3: inline-1.c:3:3: warning: double-=C3=A2free=C3=A2 of =C3=A2q=C3=A2 [CWE-= 415] [-Wanalyzer-double-free] 3 | __builtin_free (p); | ^~~~~~~~~~~~~~~~~~ =C3=A2bar=C3=A2: events 1-2 | | 3 | __builtin_free (p); | | ^~~~~~~~~~~~~~~~~~ | | | | | (1) first =C3=A2free=C3=A2 here | | (2) second =C3=A2free=C3=A2 here; first =C3=A2free=C3=A2= was at (1) where the stack frame information in the execution path suggests that t= hese events are happening in "bar", in the top stack frame. This is what the analyzer sees, but I find it hard to decipher such output. Hence, as a workaround for the fact that -fanalyzer runs so late, this patch attempts to reconstruct the "true" stack frame information, and to inject events showing inline calls, based on the inlining chain information recorded in the location_t values for the events. Doing so leads to this output at -O2 on the above example (with -fdiagnostics-show-path-depths): In function =C3=A2foo=C3=A2, inlined from =C3=A2bar=C3=A2 at inline-1.c:9:3: inline-1.c:3:3: warning: double-=C3=A2free=C3=A2 of =C3=A2q=C3=A2 [CWE-= 415] [-Wanalyzer-double-free] 3 | __builtin_free (p); | ^~~~~~~~~~~~~~~~~~ =C3=A2bar=C3=A2: events 1-2 (depth 1) | | 6 | void bar (void *q) | | ^~~ | | | | | (1) entry to =C3=A2bar=C3=A2 | 7 | { | 8 | foo (q); | | ~ | | | | | (2) inlined call to =C3=A2foo=C3=A2 from =C3=A2bar=C3=A2 | +--> =C3=A2foo=C3=A2: event 3 (depth 2) | | 3 | __builtin_free (p); | | ^~~~~~~~~~~~~~~~~~ | | | | | (3) first =C3=A2free=C3=A2 here | <------+ | =C3=A2bar=C3=A2: event 4 (depth 1) | | 9 | foo (q); | | ^ | | | | | (4) inlined call to =C3=A2foo=C3=A2 from =C3=A2bar=C3=A2 | +--> =C3=A2foo=C3=A2: event 5 (depth 2) | | 3 | __builtin_free (p); | | ^~~~~~~~~~~~~~~~~~ | | | | | (5) second =C3=A2free=C3=A2 here; first =C3=A2fre= e=C3=A2 was at (3) | reconstructing the calls and returns. The patch also adds a new option, -fno-analyzer-undo-inlining, which can be used to disable this reconstruction, restoring the output listed above (this time with -fdiagnostics-show-path-depths): In function =C3=A2foo=C3=A2, inlined from =C3=A2bar=C3=A2 at inline-1.c:9:3: inline-1.c:3:3: warning: double-=C3=A2free=C3=A2 of =C3=A2q=C3=A2 [CWE-= 415] [-Wanalyzer-double-free] 3 | __builtin_free (p); | ^~~~~~~~~~~~~~~~~~ =C3=A2bar=C3=A2: events 1-2 (depth 1) | | 3 | __builtin_free (p); | | ^~~~~~~~~~~~~~~~~~ | | | | | (1) first =C3=A2free=C3=A2 here | | (2) second =C3=A2free=C3=A2 here; first =C3=A2free=C3=A2= was at (1) | gcc/analyzer/ChangeLog: PR analyzer/105962 * analyzer.opt (fanalyzer-undo-inlining): New option. * checker-path.cc: Include "diagnostic-core.h" and "inlining-iterator.h". (event_kind_to_string): Handle EK_INLINED_CALL. (class inlining_info): New class. (checker_event::checker_event): Move here from checker-path.h. Store original fndecl and depth, and calculate effective fndecl and depth based on inlining information. (checker_event::dump): Emit original depth as well as effective depth when they differ; likewise for fndecl. (region_creation_event::get_desc): Use m_effective_fndecl. (inlined_call_event::get_desc): New. (inlined_call_event::get_meaning): New. (checker_path::inject_any_inlined_call_events): New. * checker-path.h (enum event_kind): Add EK_INLINED_CALL. (checker_event::checker_event): Make protected, and move definition to checker-path.cc. (checker_event::get_fndecl): Use effective fndecl. (checker_event::get_stack_depth): Use effective stack depth. (checker_event::get_logical_location): Use effective stack dept= h. (checker_event::get_original_stack_depth): New. (checker_event::m_fndecl): Rename to... (checker_event::m_original_fndecl): ...this. (checker_event::m_depth): Rename to... (checker_event::m_original_depth): ...this. (checker_event::m_effective_fndecl): New field. (checker_event::m_effective_depth): New field. (class inlined_call_event): New checker_event subclass. (checker_path::inject_any_inlined_call_events): New decl. * diagnostic-manager.cc: Include "inlining-iterator.h". (diagnostic_manager::emit_saved_diagnostic): Call checker_path::inject_any_inlined_call_events. (diagnostic_manager::prune_for_sm_diagnostic): Handle EK_INLINED_CALL. * engine.cc (tainted_args_function_custom_event::get_desc): Use effective fndecl. * inlining-iterator.h: New file. gcc/testsuite/ChangeLog: PR analyzer/105962 * gcc.dg/analyzer/inlining-1-multiline.c: New test. * gcc.dg/analyzer/inlining-1-no-undo.c: New test. * gcc.dg/analyzer/inlining-1.c: New test. * gcc.dg/analyzer/inlining-2-multiline.c: New test. * gcc.dg/analyzer/inlining-2.c: New test. * gcc.dg/analyzer/inlining-3-multiline.c: New test. * gcc.dg/analyzer/inlining-3.c: New test. * gcc.dg/analyzer/inlining-4-multiline.c: New test. * gcc.dg/analyzer/inlining-4.c: New test. * gcc.dg/analyzer/inlining-5-multiline.c: New test. * gcc.dg/analyzer/inlining-5.c: New test. * gcc.dg/analyzer/inlining-6-multiline.c: New test. * gcc.dg/analyzer/inlining-6.c: New test. * gcc.dg/analyzer/inlining-7-multiline.c: New test. * gcc.dg/analyzer/inlining-7.c: New test. gcc/ChangeLog: PR analyzer/105962 * doc/invoke.texi: Add -fno-analyzer-undo-inlining. * tree-diagnostic-path.cc (default_tree_diagnostic_path_printer= ): Extend -fdiagnostics-path-format=3Dseparate-events so that with -fdiagnostics-show-path-depths it prints fndecls as well as sta= ck depths. Signed-off-by: David Malcolm =