From patchwork Wed Nov 20 21:13:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1198530 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-514241-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="OQgQ641I"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="XNZX5WmP"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47JFg40m2jz9sPL for ; Thu, 21 Nov 2019 08:09:03 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :content-type:content-transfer-encoding; q=dns; s=default; b=B+2 Fjx0oOIiF4GeBN8rWWgLn9s49UgC1V/Q6Wotl753/btTA10CESF/UyWVAEAEEMAg Afu9ZUQ8RwoI/OBjondPjsPl1NJW0Wm6z4jMF6WBbVWgZqn++4RY6bUKQ1/w85pU J2FBDJRNm/tL1t9HOEd2pAk9Vy4m8RS7MpLKWtcs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :content-type:content-transfer-encoding; s=default; bh=8ywP626Nj udUeCf+dzUeNDVnMnY=; b=OQgQ641IVro98LsZW15/sOney5PXb8O+prtksGe91 gVTXdUFqprQY+eSOVl9t4LBemaD3mai/jbnE55lxqmIeDInE2isS/xZGV6Oz6exE TzeM03kHl/9QikSW8CnQXxag55wgudbc7HzVXKCic/aHPW9liOlWbi5HMZZAtpcF tg= Received: (qmail 87057 invoked by alias); 20 Nov 2019 21:07:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 86971 invoked by uid 89); 20 Nov 2019 21:07:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, T_FILL_THIS_FORM_SHORT autolearn=ham version=3.3.1 spammy=association, POINT, sk:new_reg X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Nov 2019 21:07:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574284038; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ozwD/WzuQhC7jt1vTlAX10RkaNP7F7CduQ5yLAr0gV8=; b=XNZX5WmPU9FK1GScTuPnIh1vAASdU3/bYFnM6mrNvgreupDmSQPI3qX/R5JZK3xsINMKSI ZnLo1T87oIhAde4LHmxsPz2eqXgDadK28Mt1aydTzs/+JhmIydasT/1zREet2h5KWz4Dbb vPD3a5XfbWvkZtn2KWuIo06PXzEW/jg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-409-czjzrlaoNW-kzWwg0B2daQ-1; Wed, 20 Nov 2019 16:07:16 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 907C0800A02 for ; Wed, 20 Nov 2019 21:07:15 +0000 (UTC) Received: from c64.redhat.com (ovpn-112-19.phx2.redhat.com [10.3.112.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9D6EE106427B; Wed, 20 Nov 2019 21:07:14 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH 08/11] [analyzer] Show rewind destination for leaks due to longjmp Date: Wed, 20 Nov 2019 16:13:47 -0500 Message-Id: <1574284430-8776-9-git-send-email-dmalcolm@redhat.com> In-Reply-To: <1574284430-8776-1-git-send-email-dmalcolm@redhat.com> References: <1574284430-8776-1-git-send-email-dmalcolm@redhat.com> X-Mimecast-Spam-Score: 0 X-IsSubscribed: yes This patch adds some special-case logic to how paths are generated so that leaks due to longjmp rewinding past a frame now show where the longjmp rewinds to (longjmp and setjmp are already something of a special-case so this seems reasonable). A colorized LTO example of the output can be seen at: https://dmalcolm.fedorapeople.org/gcc/2019-11-18/lto-longjmp-leak-demo.html gcc/ChangeLog: * analyzer/diagnostic-manager.cc (saved_diagnostic::saved_diagnostic): Initialize new field m_trailing_eedge. (diagnostic_manager::emit_saved_diagnostic): If m_trailing_eedge is set, use it to add extra events after the "final" event. * analyzer/diagnostic-manager.h (saved_diagnostic::operator==): Compare m_trailing_eedge fields. (saved_diagnostic::m_trailing_eedge): New field. (diagnostic_manager::get_num_diagnostics): New accessor. (diagnostic_manager::get_saved_diagnostic): New accessor. * analyzer/engine.cc (exploded_node::on_longjmp): Set m_trailing_edge on any diagnostics saved when handling region_model::on_longjmp. (exploded_graph::add_edge): Return the newly-created eedge. * analyzer/exploded-graph.h (exploded_graph::add_edge): Convert return type from void to exploded_edge *. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/setjmp-7a.c: Update expected multiline output to include a final pair of events showing the longjmp and the setjmp that it rewinds to. --- gcc/analyzer/diagnostic-manager.cc | 9 ++++- gcc/analyzer/diagnostic-manager.h | 13 ++++++- gcc/analyzer/engine.cc | 56 ++++++++++++++++++++++++++++--- gcc/analyzer/exploded-graph.h | 8 ++--- gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c | 13 +++++-- 5 files changed, 86 insertions(+), 13 deletions(-) diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index fae38c7..926900b 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -47,7 +47,7 @@ saved_diagnostic::saved_diagnostic (const state_machine *sm, outlive that. */ m_stmt_finder (stmt_finder ? stmt_finder->clone () : NULL), m_var (var), m_state (state), - m_d (d) + m_d (d), m_trailing_eedge (NULL) { gcc_assert (m_stmt || m_stmt_finder); @@ -427,6 +427,13 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg, emission_path.add_final_event (sd.m_sm, epath.get_final_enode (), stmt, sd.m_var, sd.m_state); + /* The "final" event might not be final; if the saved_diagnostic has a + trailing eedge stashed, add any events for it. This is for use + in handling longjmp, to show where a longjmp is rewinding to. */ + if (sd.m_trailing_eedge) + add_events_for_eedge (*sd.m_trailing_eedge, eg.get_ext_state (), + &emission_path); + emission_path.prepare_for_emission (sd.m_d); gcc_rich_location rich_loc (stmt->location); diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h index 0f4444c..8591d2e 100644 --- a/gcc/analyzer/diagnostic-manager.h +++ b/gcc/analyzer/diagnostic-manager.h @@ -45,7 +45,8 @@ public: /* We don't compare m_stmt_finder. */ && m_var == other.m_var && m_state == other.m_state - && m_d->equal_p (*other.m_d)); + && m_d->equal_p (*other.m_d) + && m_trailing_eedge == other.m_trailing_eedge); } //private: @@ -57,6 +58,7 @@ public: tree m_var; state_machine::state_t m_state; pending_diagnostic *m_d; + exploded_edge *m_trailing_eedge; }; /* A class with responsibility for saving pending diagnostics, so that @@ -93,6 +95,15 @@ public: const gimple *stmt, int num_dupes); + unsigned get_num_diagnostics () const + { + return m_saved_diagnostics.length (); + } + saved_diagnostic *get_saved_diagnostic (unsigned idx) + { + return m_saved_diagnostics[idx]; + } + private: void build_emission_path (const exploded_graph &eg, const exploded_path &epath, diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 9936e1e..aa4a359 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -1083,6 +1083,13 @@ exploded_node::on_longjmp (exploded_graph &eg, >= setjmp_point.get_stack_depth ()); /* Update the state for use by the destination node. */ + + /* Stash the current number of diagnostics so that we can update + any that this adds to show where the longjmp is rewinding to. */ + + diagnostic_manager *dm = &eg.get_diagnostic_manager (); + unsigned prev_num_diagnostics = dm->get_num_diagnostics (); + new_region_model->on_longjmp (longjmp_call, setjmp_call, setjmp_point.get_stack_depth (), ctxt); @@ -1095,9 +1102,46 @@ exploded_node::on_longjmp (exploded_graph &eg, /* Create custom exploded_edge for a longjmp. */ if (next) - eg.add_edge (const_cast (this), next, NULL, - change, - new rewind_info_t (enode_origin)); + { + exploded_edge *eedge + = eg.add_edge (const_cast (this), next, NULL, + change, + new rewind_info_t (enode_origin)); + + /* For any diagnostics that were queued here (such as leaks) we want + the checker_path to show the rewinding events after the "final event" + so that the user sees where the longjmp is rewinding to (otherwise the + path is meaningless). + + For example, we want to emit something like: + | NN | { + | NN | longjmp (env, 1); + | | ~~~~~~~~~~~~~~~~ + | | | + | | (10) 'ptr' leaks here; was allocated at (7) + | | (11) rewinding from 'longjmp' in 'inner'... + | + <-------------+ + | + 'outer': event 12 + | + | NN | i = setjmp(env); + | | ^~~~~~ + | | | + | | (12) ...to 'setjmp' in 'outer' (saved at (2)) + + where the "final" event above is event (10), but we want to append + events (11) and (12) afterwards. + + Do this by setting m_trailing_eedge on any diagnostics that were + just saved. */ + unsigned num_diagnostics = dm->get_num_diagnostics (); + for (unsigned i = prev_num_diagnostics; i < num_diagnostics; i++) + { + saved_diagnostic *sd = dm->get_saved_diagnostic (i); + sd->m_trailing_eedge = eedge; + } + } } /* Subroutine of exploded_graph::process_node for finding the successors @@ -1768,9 +1812,10 @@ exploded_graph::get_or_create_node (const program_point &point, /* Add an exploded_edge from SRC to DEST, recording its association with SEDGE (which may be NULL), and, if non-NULL, taking ownership - of REWIND_INFO. */ + of REWIND_INFO. + Return the newly-created eedge. */ -void +exploded_edge * exploded_graph::add_edge (exploded_node *src, exploded_node *dest, const superedge *sedge, const state_change &change, @@ -1778,6 +1823,7 @@ exploded_graph::add_edge (exploded_node *src, exploded_node *dest, { exploded_edge *e = new exploded_edge (src, dest, sedge, change, rewind_info); digraph::add_edge (e); + return e; } /* Ensure that this graph has per-program_point-data for POINT; diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h index f97d2b6..97e1005 100644 --- a/gcc/analyzer/exploded-graph.h +++ b/gcc/analyzer/exploded-graph.h @@ -627,10 +627,10 @@ public: exploded_node *get_or_create_node (const program_point &point, const program_state &state, state_change *change); - void add_edge (exploded_node *src, exploded_node *dest, - const superedge *sedge, - const state_change &change, - rewind_info_t *rewind_info = NULL); + exploded_edge *add_edge (exploded_node *src, exploded_node *dest, + const superedge *sedge, + const state_change &change, + rewind_info_t *rewind_info = NULL); per_program_point_data * get_or_create_per_program_point_data (const program_point &); diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c b/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c index 1d5fc2e..6f99df5 100644 --- a/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c +++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c @@ -85,7 +85,7 @@ void outer (void) | | | | | (8) calling 'inner' from 'middle' | - +--> 'inner': events 9-10 + +--> 'inner': events 9-11 | | NN | static void inner (void) | | ^~~~~ @@ -96,6 +96,15 @@ void outer (void) | | ~~~~~~~~~~~~~~~~ | | | | | (10) 'ptr' leaks here; was allocated at (7) + | | (11) rewinding from 'longjmp' in 'inner'... | + <-------------+ + | + 'outer': event 12 + | + | NN | i = setjmp(env); + | | ^~~~~~ + | | | + | | (12) ...to 'setjmp' in 'outer' (saved at (2)) + | { dg-end-multiline-output "" } */ -// TODO: show the rewind to the setjmp