From patchwork Fri Dec 13 18:11:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1209376 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-515958-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="vDIzbeo3"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="JCgLnU1f"; 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 47ZJwc2jWGz9sPJ for ; Sat, 14 Dec 2019 05:24:32 +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 :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=QK2AEutb1vv4itilbxkf3ujtQKJy4c18HA7yU1FaZ16onlfP89zya Pywja+EzeJM076Fjpjs2UsHrAOIrZmioEuaYKWSrj2qUEjc3MCxGdqQDM0isS1ux 9X+yRvo650b6R8RT4RgUQhDxJrXVQKnsg345rawd/+9Bwtkqp7qOS0= 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 :mime-version:content-type:content-transfer-encoding; s=default; bh=9eETNTiNIa7uFsuJ2EFXcaERr9M=; b=vDIzbeo38mxUmsetSQFmxiknmKDY qcJ9lGCjlw0CiMTgl8f3zmYpks7fq7gpKd90NVqkXVda1JrnKKK1/EpZLpDGvUqg 6NcDeCP8EiYeoJgtKwrsWyr4ARSjXroycwrpGz6buMia+o4SCdzr+RpvC/esvAiD LhQBeeiTJZ5Wjds= Received: (qmail 128233 invoked by alias); 13 Dec 2019 18:15:48 -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 108638 invoked by uid 89); 13 Dec 2019 18:13:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=Effective X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Dec 2019 18:13:12 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1576260790; 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: in-reply-to:in-reply-to:references:references; bh=/g+vzvGPCnBLUVJM1gDp3hJPbziZdhQv8HPgiORvx/A=; b=JCgLnU1ff8J39oC3HpFY86bgAEaJgExJAe8B511533nMGyPJJFB7yBgnOcHRUcPjnOVKwO epIQ7HwHMRLJq2PRg+yu82jwb70rmFYmWUZhSrQA5jUVBc217vacf9TG+6IBIX+UAwVKQn ueVI1HWH6dkNnYOSBGflUaLvlsmuJP8= 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-197-0zcnYkrzMSKuyoG7xY_vSg-1; Fri, 13 Dec 2019 13:11:58 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2384EDBE5 for ; Fri, 13 Dec 2019 18:11:57 +0000 (UTC) Received: from t470.redhat.com (ovpn-117-164.phx2.redhat.com [10.3.117.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF45D5C241; Fri, 13 Dec 2019 18:11:54 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH 30/45] analyzer: new file: sm-file.cc Date: Fri, 13 Dec 2019 13:11:19 -0500 Message-Id: <20191213181134.1830-31-dmalcolm@redhat.com> In-Reply-To: <20191213181134.1830-1-dmalcolm@redhat.com> References: <20191213181134.1830-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-IsSubscribed: yes Changed in v4: - Remove include of gcc-plugin.h, reworking includes accordingly. - Wrap everything in #if ENABLE_ANALYZER - Remove /// comment lines - Rework on_leak vfunc: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html - Rework for changes to is_named_call_p, resolving function pointers: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html This patch adds a state machine checker for stdio's FILE stream API. gcc/ChangeLog: * analyzer/sm-file.cc: New file. --- gcc/analyzer/sm-file.cc | 334 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 334 insertions(+) create mode 100644 gcc/analyzer/sm-file.cc diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc new file mode 100644 index 000000000000..20963c3e59b4 --- /dev/null +++ b/gcc/analyzer/sm-file.cc @@ -0,0 +1,334 @@ +/* A state machine for detecting misuses of 's FILE * API. + Copyright (C) 2019 Free Software Foundation, Inc. + Contributed by David Malcolm . + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it +under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "tree.h" +#include "function.h" +#include "basic-block.h" +#include "gimple.h" +#include "options.h" +#include "diagnostic-path.h" +#include "diagnostic-metadata.h" +#include "analyzer/analyzer.h" +#include "analyzer/pending-diagnostic.h" +#include "analyzer/sm.h" + +#if ENABLE_ANALYZER + +namespace { + +/* A state machine for detecting misuses of 's FILE * API. */ + +class fileptr_state_machine : public state_machine +{ +public: + fileptr_state_machine (logger *logger); + + bool inherited_state_p () const FINAL OVERRIDE { return false; } + + bool on_stmt (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt) const FINAL OVERRIDE; + + void on_condition (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt, + tree lhs, + enum tree_code op, + tree rhs) const FINAL OVERRIDE; + + bool can_purge_p (state_t s) const FINAL OVERRIDE; + pending_diagnostic *on_leak (tree var) const FINAL OVERRIDE; + + /* Start state. */ + state_t m_start; + + /* State for a FILE * returned from fopen that hasn't been checked for + NULL. + It could be an open stream, or could be NULL. */ + state_t m_unchecked; + + /* State for a FILE * that's known to be NULL. */ + state_t m_null; + + /* State for a FILE * that's known to be a non-NULL open stream. */ + state_t m_nonnull; + + /* State for a FILE * that's had fclose called on it. */ + state_t m_closed; + + /* Stop state, for a FILE * we don't want to track any more. */ + state_t m_stop; +}; + +/* Base class for diagnostics relative to fileptr_state_machine. */ + +class file_diagnostic : public pending_diagnostic +{ +public: + file_diagnostic (const fileptr_state_machine &sm, tree arg) + : m_sm (sm), m_arg (arg) + {} + + bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE + { + return m_arg == ((const file_diagnostic &)base_other).m_arg; + } + + label_text describe_state_change (const evdesc::state_change &change) + OVERRIDE + { + if (change.m_old_state == m_sm.m_start + && change.m_new_state == m_sm.m_unchecked) + // TODO: verify that it's the fopen stmt, not a copy + return label_text::borrow ("opened here"); + if (change.m_old_state == m_sm.m_unchecked + && change.m_new_state == m_sm.m_nonnull) + return change.formatted_print ("assuming %qE is non-NULL", + change.m_expr); + if (change.m_new_state == m_sm.m_null) + return change.formatted_print ("assuming %qE is NULL", + change.m_expr); + return label_text (); + } + +protected: + const fileptr_state_machine &m_sm; + tree m_arg; +}; + +class double_fclose : public file_diagnostic +{ +public: + double_fclose (const fileptr_state_machine &sm, tree arg) + : file_diagnostic (sm, arg) + {} + + const char *get_kind () const FINAL OVERRIDE { return "double_fclose"; } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + return warning_at (rich_loc, OPT_Wanalyzer_double_fclose, + "double % of FILE %qE", + m_arg); + } + + label_text describe_state_change (const evdesc::state_change &change) + OVERRIDE + { + if (change.m_new_state == m_sm.m_closed) + { + m_first_fclose_event = change.m_event_id; + return change.formatted_print ("first %qs here", "fclose"); + } + return file_diagnostic::describe_state_change (change); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + if (m_first_fclose_event.known_p ()) + return ev.formatted_print ("second %qs here; first %qs was at %@", + "fclose", "fclose", + &m_first_fclose_event); + return ev.formatted_print ("second %qs here", "fclose"); + } + +private: + diagnostic_event_id_t m_first_fclose_event; +}; + +class file_leak : public file_diagnostic +{ +public: + file_leak (const fileptr_state_machine &sm, tree arg) + : file_diagnostic (sm, arg) + {} + + const char *get_kind () const FINAL OVERRIDE { return "file_leak"; } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + diagnostic_metadata m; + /* CWE-775: "Missing Release of File Descriptor or Handle after + Effective Lifetime". */ + m.add_cwe (775); + return warning_at (rich_loc, m, OPT_Wanalyzer_file_leak, + "leak of FILE %qE", + m_arg); + } + + label_text describe_state_change (const evdesc::state_change &change) + FINAL OVERRIDE + { + if (change.m_new_state == m_sm.m_unchecked) + { + m_fopen_event = change.m_event_id; + return label_text::borrow ("opened here"); + } + return file_diagnostic::describe_state_change (change); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + if (m_fopen_event.known_p ()) + return ev.formatted_print ("%qE leaks here; was opened at %@", + ev.m_expr, &m_fopen_event); + else + return ev.formatted_print ("%qE leaks here", ev.m_expr); + } + +private: + diagnostic_event_id_t m_fopen_event; +}; + +/* fileptr_state_machine's ctor. */ + +fileptr_state_machine::fileptr_state_machine (logger *logger) +: state_machine ("file", logger) +{ + m_start = add_state ("start"); + m_unchecked = add_state ("unchecked"); + m_null = add_state ("null"); + m_nonnull = add_state ("nonnull"); + m_closed = add_state ("closed"); + m_stop = add_state ("stop"); +} + +/* Implementation of state_machine::on_stmt vfunc for fileptr_state_machine. */ + +bool +fileptr_state_machine::on_stmt (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt) const +{ + if (const gcall *call = dyn_cast (stmt)) + if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call)) + { + if (is_named_call_p (callee_fndecl, "fopen", call, 2)) + { + tree lhs = gimple_call_lhs (call); + if (lhs) + { + lhs = sm_ctxt->get_readable_tree (lhs); + sm_ctxt->on_transition (node, stmt, lhs, m_start, m_unchecked); + } + else + { + /* TODO: report leak. */ + } + return true; + } + + if (is_named_call_p (callee_fndecl, "fclose", call, 1)) + { + tree arg = gimple_call_arg (call, 0); + arg = sm_ctxt->get_readable_tree (arg); + + sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed); + + // TODO: is it safe to call fclose (NULL) ? + sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_null, m_closed); + + sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed); + + sm_ctxt->warn_for_state (node, stmt, arg, m_closed, + new double_fclose (*this, arg)); + sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop); + return true; + } + + // TODO: operations on closed file + // etc + } + + return false; +} + +/* Implementation of state_machine::on_condition vfunc for + fileptr_state_machine. + Potentially transition state 'unchecked' to 'nonnull' or to 'null'. */ + +void +fileptr_state_machine::on_condition (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt, + tree lhs, + enum tree_code op, + tree rhs) const +{ + if (!zerop (rhs)) + return; + + // TODO: has to be a FILE *, specifically + if (TREE_CODE (TREE_TYPE (lhs)) != POINTER_TYPE) + return; + + // TODO: has to be a FILE *, specifically + if (TREE_CODE (TREE_TYPE (rhs)) != POINTER_TYPE) + return; + + if (op == NE_EXPR) + { + log ("got 'ARG != 0' match"); + sm_ctxt->on_transition (node, stmt, + lhs, m_unchecked, m_nonnull); + } + else if (op == EQ_EXPR) + { + log ("got 'ARG == 0' match"); + sm_ctxt->on_transition (node, stmt, + lhs, m_unchecked, m_null); + } +} + +/* Implementation of state_machine::can_purge_p vfunc for fileptr_state_machine. + Don't allow purging of pointers in state 'unchecked' or 'nonnull' + (to avoid false leak reports). */ + +bool +fileptr_state_machine::can_purge_p (state_t s) const +{ + return s != m_unchecked && s != m_nonnull; +} + +/* Implementation of state_machine::on_leak vfunc for + fileptr_state_machine, for complaining about leaks of FILE * in + state 'unchecked' and 'nonnull'. */ + +pending_diagnostic * +fileptr_state_machine::on_leak (tree var) const +{ + return new file_leak (*this, var); +} + +} // anonymous namespace + +/* Internal interface to this file. */ + +state_machine * +make_fileptr_state_machine (logger *logger) +{ + return new fileptr_state_machine (logger); +} + +#endif /* #if ENABLE_ANALYZER */