From patchwork Wed Jan 22 19:56:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1227406 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-518060-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.a=rsa-sha1 header.s=default header.b=suubMb6P; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ia2MZCxA; 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 482x462xtfz9sPK for ; Thu, 23 Jan 2020 06:56:20 +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:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=CEwKDYNsVrgdaLRY /zaiZc3iypjkKJtiMcQQ/FL76inCwz/Xmq+lwClcy1gPI2x30NesTwwmsavxeR3K CiY6zpRn88g+paGf1t7OfwaTMIqGZ7TMSVxJ5BsU+3aflcsapIfXM50Xkq0S021a nSlc8oPWgIC7/npuf1/MargWI8Y= 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:mime-version:content-type :content-transfer-encoding; s=default; bh=WZL+X6mSuV2LZBkxnoodqj gVEQI=; b=suubMb6PRlenZ8RnyssBw4esMjTNxxsJ+1JWKI06W4zeIqcpVoIH+k 2J6F65F4cwOxIvMesSJxMt5c2xi3HpRAHYTscuS6uSKi5xjYAKi9wVMWJ/9pDPtq vypMt4Ju6O3A5wRpT/llu2xvKgjGE8kMS+bGQ7gdyVxBZftVDmhdA= Received: (qmail 74881 invoked by alias); 22 Jan 2020 19:56:12 -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 74873 invoked by uid 89); 22 Jan 2020 19:56:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Jan 2020 19:56:10 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579722968; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=iVF6fuh1TAluOZL2G1J6vsYvIuZpwsXvvFlaf+LZQcg=; b=Ia2MZCxAEGXodVx+l5MLrdWJvxvL2jOvAJax++8MGOPvEQdJMx93LZTtjPSzM78uXYu34w IXCfDrCJ9Y293ufZphnja+bnmqxRviHxX/lTi1R0irA/HmtEZ8Hnhf3vuY1yitKvA5gxQR GKVLDj32Vfi4Af4knYCGF/rM9T0Fy3U= 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-226-4aXXTgT9NtuifHToGTQ9WA-1; Wed, 22 Jan 2020 14:56:06 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C7E308017CC for ; Wed, 22 Jan 2020 19:56:05 +0000 (UTC) Received: from t470.redhat.com (ovpn-117-41.phx2.redhat.com [10.3.117.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id 494C28BE3E; Wed, 22 Jan 2020 19:56:05 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed] analyzer: fix setjmp handling with -g (PR 93378) Date: Wed, 22 Jan 2020 14:56:02 -0500 Message-Id: <20200122195602.24445-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-IsSubscribed: yes PR analyzer/93378 reports an ICE at -O1 -g when analyzing a rewind via longjmp to a setjmp call with. The root cause is that the rewind_info_t::get_setjmp_call attempts to locate the setjmp GIMPLE_CALL via within the exploded_node containing it, but the exploded_node has two stmts: a GIMPLE_DEBUG, then the GIMPLE_CALL, and so erroneously picks the GIMPLE_DEBUG, leading to a failed as_a . This patch reworks how the analyzer stores information about a setjmp so that instead of storing an exploded_node *, it instead introduces a "setjmp_record" struct, for use by both setjmp_svalue and rewind_info_t. Hence we store the information directly, rather than attempting to reconstruct it, fixing the bug. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; pushed to master as r10-6153-gfd9982bb0051d1a678191b684bb907d1ac177991. gcc/analyzer/ChangeLog: PR analyzer/93378 * engine.cc (setjmp_svalue::compare_fields): Update for replacement of m_enode with m_setjmp_record. (setjmp_svalue::add_to_hash): Likewise. (setjmp_svalue::get_index): Rename... (setjmp_svalue::get_enode_index): ...to this. (setjmp_svalue::print_details): Update for replacement of m_enode with m_setjmp_record. (exploded_node::on_longjmp): Likewise. * exploded-graph.h (rewind_info_t::m_enode_origin): Replace... (rewind_info_t::m_setjmp_record): ...with this. (rewind_info_t::rewind_info_t): Update for replacement of m_enode with m_setjmp_record. (rewind_info_t::get_setjmp_point): Likewise. (rewind_info_t::get_setjmp_call): Likewise. * region-model.cc (region_model::dump_summary_of_map): Likewise. (region_model::on_setjmp): Likewise. * region-model.h (struct setjmp_record): New struct. (setjmp_svalue::m_enode): Replace... (setjmp_svalue::m_setjmp_record): ...with this. (setjmp_svalue::setjmp_svalue): Update for replacement of m_enode with m_setjmp_record. (setjmp_svalue::clone): Likewise. (setjmp_svalue::get_index): Rename... (setjmp_svalue::get_enode_index): ...to this. (setjmp_svalue::get_exploded_node): Replace... (setjmp_svalue::get_setjmp_record): ...with this. gcc/testsuite/ChangeLog: PR analyzer/93378 * gcc.dg/analyzer/setjmp-pr93378.c: New test. --- gcc/analyzer/engine.cc | 18 +++++----- gcc/analyzer/exploded-graph.h | 15 ++++---- gcc/analyzer/region-model.cc | 7 ++-- gcc/analyzer/region-model.h | 34 +++++++++++++++---- .../gcc.dg/analyzer/setjmp-pr93378.c | 15 ++++++++ 5 files changed, 65 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 53c93791a07..737ea1dd6e4 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -155,7 +155,7 @@ impl_region_model_context::on_unknown_change (svalue_id sid) bool setjmp_svalue::compare_fields (const setjmp_svalue &other) const { - return m_enode == other.m_enode; + return m_setjmp_record == other.m_setjmp_record; } /* Implementation of svalue::add_to_hash vfunc for setjmp_svalue. */ @@ -163,15 +163,15 @@ setjmp_svalue::compare_fields (const setjmp_svalue &other) const void setjmp_svalue::add_to_hash (inchash::hash &hstate) const { - hstate.add_int (m_enode->m_index); + hstate.add_int (m_setjmp_record.m_enode->m_index); } /* Get the index of the stored exploded_node. */ int -setjmp_svalue::get_index () const +setjmp_svalue::get_enode_index () const { - return m_enode->m_index; + return m_setjmp_record.m_enode->m_index; } /* Implementation of svalue::print_details vfunc for setjmp_svalue. */ @@ -181,7 +181,7 @@ setjmp_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED, svalue_id this_sid ATTRIBUTE_UNUSED, pretty_printer *pp) const { - pp_printf (pp, "setjmp: EN: %i", m_enode->m_index); + pp_printf (pp, "setjmp: EN: %i", get_enode_index ()); } /* Concrete implementation of sm_context, wiring it up to the rest of this @@ -1172,11 +1172,11 @@ exploded_node::on_longjmp (exploded_graph &eg, if (!setjmp_sval) return; + const setjmp_record tmp_setjmp_record = setjmp_sval->get_setjmp_record (); + /* Build a custom enode and eedge for rewinding from the longjmp call back to the setjmp. */ - - const exploded_node *enode_origin = setjmp_sval->get_exploded_node (); - rewind_info_t rewind_info (enode_origin); + rewind_info_t rewind_info (tmp_setjmp_record); const gcall *setjmp_call = rewind_info.get_setjmp_call (); const program_point &setjmp_point = rewind_info.get_setjmp_point (); @@ -1217,7 +1217,7 @@ exploded_node::on_longjmp (exploded_graph &eg, exploded_edge *eedge = eg.add_edge (const_cast (this), next, NULL, change, - new rewind_info_t (enode_origin)); + new rewind_info_t (tmp_setjmp_record)); /* For any diagnostics that were queued here (such as leaks) we want the checker_path to show the rewinding events after the "final event" diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h index 57636fbe07c..3d1445c87ad 100644 --- a/gcc/analyzer/exploded-graph.h +++ b/gcc/analyzer/exploded-graph.h @@ -307,8 +307,8 @@ private: class rewind_info_t : public exploded_edge::custom_info_t { public: - rewind_info_t (const exploded_node *enode_origin) - : m_enode_origin (enode_origin) + rewind_info_t (const setjmp_record &setjmp_record) + : m_setjmp_record (setjmp_record) {} void print (pretty_printer *pp) FINAL OVERRIDE @@ -324,7 +324,7 @@ public: const program_point &get_setjmp_point () const { - const program_point &origin_point = m_enode_origin->get_point (); + const program_point &origin_point = get_enode_origin ()->get_point (); /* "origin_point" ought to be before the call to "setjmp". */ gcc_assert (origin_point.get_kind () == PK_BEFORE_STMT); @@ -336,13 +336,16 @@ public: const gcall *get_setjmp_call () const { - return as_a (get_setjmp_point ().get_stmt ()); + return m_setjmp_record.m_setjmp_call; } - const exploded_node *get_enode_origin () const { return m_enode_origin; } + const exploded_node *get_enode_origin () const + { + return m_setjmp_record.m_enode; + } private: - const exploded_node *m_enode_origin; + setjmp_record m_setjmp_record; }; /* Statistics about aspects of an exploded_graph. */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 0bca5c0fd71..25a22f8fc65 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -3660,7 +3660,7 @@ region_model::dump_summary_of_map (pretty_printer *pp, case SK_SETJMP: dump_separator (pp, is_first); pp_printf (pp, "setjmp: EN: %i", - sval->dyn_cast_setjmp_svalue ()->get_index ()); + sval->dyn_cast_setjmp_svalue ()->get_enode_index ()); break; } } @@ -4493,10 +4493,11 @@ region_model::on_setjmp (const gcall *call, const exploded_node *enode, region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt); region *buf = get_region (buf_rid); - /* Create a setjmp_svalue for ENODE and store it in BUF_RID's region. */ + /* Create a setjmp_svalue for this call and store it in BUF_RID's region. */ if (buf) { - svalue *sval = new setjmp_svalue (enode, buf->get_type ()); + setjmp_record r (enode, call); + svalue *sval = new setjmp_svalue (r, buf->get_type ()); svalue_id new_sid = add_svalue (sval); set_value (buf_rid, new_sid, ctxt); } diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 1090b96bfaa..f7fb7b0b6d0 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -718,20 +718,42 @@ is_a_helper ::test (svalue *sval) namespace ana { +/* A bundle of information recording a setjmp call, corresponding roughly + to a jmp_buf. */ + +struct setjmp_record +{ + setjmp_record (const exploded_node *enode, + const gcall *setjmp_call) + : m_enode (enode), m_setjmp_call (setjmp_call) + { + } + + bool operator== (const setjmp_record &other) const + { + return (m_enode == other.m_enode + && m_setjmp_call == other.m_setjmp_call); + } + + const exploded_node *m_enode; + const gcall *m_setjmp_call; +}; + /* Concrete subclass of svalue representing setjmp buffers, so that longjmp can potentially "return" to an entirely different function. */ class setjmp_svalue : public svalue { public: - setjmp_svalue (const exploded_node *enode, tree type) - : svalue (type), m_enode (enode) + setjmp_svalue (const setjmp_record &setjmp_record, + tree type) + : svalue (type), m_setjmp_record (setjmp_record) {} bool compare_fields (const setjmp_svalue &other) const; svalue *clone () const FINAL OVERRIDE - { return new setjmp_svalue (m_enode, get_type ()); } + { return new setjmp_svalue (m_setjmp_record, get_type ()); } enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_SETJMP; } @@ -739,9 +761,9 @@ public: setjmp_svalue *dyn_cast_setjmp_svalue () FINAL OVERRIDE { return this; } - int get_index () const; + int get_enode_index () const; - const exploded_node *get_exploded_node () const { return m_enode; } + const setjmp_record &get_setjmp_record () const { return m_setjmp_record; } private: void print_details (const region_model &model, @@ -749,7 +771,7 @@ public: pretty_printer *pp) const FINAL OVERRIDE; - const exploded_node *m_enode; + setjmp_record m_setjmp_record; }; /* An enum for discriminating between the different concrete subclasses diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c new file mode 100644 index 00000000000..7934a40301d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-pr93378.c @@ -0,0 +1,15 @@ +/* { dg-additional-options "-O1 -g" } */ + +#include + +jmp_buf buf; + +int +test (void) +{ + if (_setjmp (buf) != 0) + return 0; + + longjmp (buf, 1); + return 1; +}