From patchwork Tue Sep 11 14:11:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 29308 Received: (qmail 90460 invoked by alias); 11 Sep 2018 14:12:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 90436 invoked by uid 89); 11 Sep 2018 14:12:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.221.66, Hx-spam-relays-external:209.85.221.66 X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Sep 2018 14:11:58 +0000 Received: by mail-wr1-f66.google.com with SMTP id v17-v6so26208150wrr.9 for ; Tue, 11 Sep 2018 07:11:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id; bh=jbMHdnYYvUt5OnIz27O1xleuR5+Sj1QPCHDFP9BMs2U=; b=EECSmamKICfCx3/o7C0mb6FQijeNM8h9xe63PtO6GccjXAKHExk3mcOHZji7qh3gQt F8+9XQX0MzYcJ1T3c+Zps/fuBIA2YlmKsDuPSbf1o0jr/brwSN9sr2C3FQA3SysVuqRz aLPq1iH9He9uaA+xo01uHd1IzVe+21k9Vq9vUIdqvJXKY0zQslKjZP/lx9Nmo14ce7Ox ubwNyx4AXtIET0yhnTUulj3cQwY0hIDy/PBdLHv656ZL6aWGRbNF/be6eaCLUOrlmXpB 73jJqeQa4PzrdEolTmthOJu4vMG8PdS0HjI4/Lvu3omOuB6QJaS2Po//tY0RekHHjIbi XuMw== Return-Path: Received: from localhost (host86-160-178-11.range86-160.btcentralplus.com. [86.160.178.11]) by smtp.gmail.com with ESMTPSA id j6-v6sm16243930wrq.25.2018.09.11.07.11.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 11 Sep 2018 07:11:54 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb: Don't leak memory with TYPE_ALLOC / TYPE_ZALLOC Date: Tue, 11 Sep 2018 15:11:47 +0100 Message-Id: <20180911141147.14107-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes I ran into the following issue after runnig valgrind on GDB. Calling TYPE_ALLOC or TYPE_ZALLOC currently allocates memory from either the objfile obstack or by using malloc. The problem with this is that types are allocated either on the objfile obstack, or on the gdbarch obstack. As a result, if we discard a type associated with an objfile then auxiliary data allocated with TYPE_(Z)ALLOC will be correctly discarded. But, if we were ever to discard a gdbarch then any auxiliary type data would be leaked. This commit ensures that auxiliary type data is allocated from the same obstack as the type itself, which should reduce leaked memory. The one problem case that I found with this change was in eval.c, where in one place we allocate a local type structure, and then used TYPE_ZALLOC to allocate some space for the type. This local type is neither object file owned, nor gdbarch owned, and so the updated TYPE_ALLOC code is unable to find an objstack to allocate space on. My proposed solution for this issue is that the space should be allocated with a direct call to xzalloc. We could extend TYPE_ALLOC to check for type->gdbarch being null, and then fall back to a direct call to xzalloc, however, I think making the rare case of a local type requiring special handling is not a bad thing, this serves to highlight that clearing up the memory will require special handling too. Tested on X86-64 GNU/Linux with no regressions. gdb/ChangeLog: * gdbtypes.h (TYPE_ALLOC): Allocate space on either the objfile obstack, or the gdbarch obstack. (TYPE_ZALLOC): Rewrite using TYPE_ALLOC. * eval.c (fake_method::fake_method): Call xzalloc directly for a type that is neither object file owned, nor gdbarch owned. --- gdb/ChangeLog | 8 ++++++++ gdb/eval.c | 2 +- gdb/gdbtypes.h | 36 ++++++++++++++++++++---------------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/gdb/eval.c b/gdb/eval.c index 2e08e9355f5..baa98e17545 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -685,7 +685,7 @@ fake_method::fake_method (type_instance_flags flags, TYPE_NFIELDS (type) = num_types; TYPE_FIELDS (type) = (struct field *) - TYPE_ZALLOC (type, sizeof (struct field) * num_types); + xzalloc (sizeof (struct field) * num_types); while (num_types-- > 0) TYPE_FIELD_TYPE (type, num_types) = param_types[num_types]; diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index eb7c365b71d..3778b46839c 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -49,6 +49,7 @@ #include "common/enum-flags.h" #include "common/underlying.h" #include "common/print-utils.h" +#include "gdbarch.h" /* Forward declarations for prototypes. */ struct field; @@ -1717,26 +1718,29 @@ extern const struct floatformat *floatformats_vax_d[BFD_ENDIAN_UNKNOWN]; extern const struct floatformat *floatformats_ibm_long_double[BFD_ENDIAN_UNKNOWN]; -/* * Allocate space for storing data associated with a particular +/* Allocate space for storing data associated with a particular type. We ensure that the space is allocated using the same mechanism that was used to allocate the space for the type structure itself. I.e. if the type is on an objfile's objfile_obstack, then the space for data associated with that type - will also be allocated on the objfile_obstack. If the type is not - associated with any particular objfile (such as builtin types), - then the data space will be allocated with xmalloc, the same as for - the type structure. */ - -#define TYPE_ALLOC(t,size) \ - (TYPE_OBJFILE_OWNED (t) \ - ? obstack_alloc (&TYPE_OBJFILE (t) -> objfile_obstack, size) \ - : xmalloc (size)) - -#define TYPE_ZALLOC(t,size) \ - (TYPE_OBJFILE_OWNED (t) \ - ? memset (obstack_alloc (&TYPE_OBJFILE (t)->objfile_obstack, size), \ - 0, size) \ - : xzalloc (size)) + will also be allocated on the objfile_obstack. If the type is + associated with a gdbarch, then the space for data associated with that + type will also be allocated on the gdbarch_obstack. + + If a type is not associated with neither an objfile or a gdbarch then + you should not use this macro to allocate space for data, instead you + should call xmalloc directly, and ensure the memory is correctly freed + when it is no longer needed. */ + +#define TYPE_ALLOC(t,size) \ + (obstack_alloc ((TYPE_OBJFILE_OWNED (t) \ + ? &TYPE_OBJFILE (t)->objfile_obstack \ + : gdbarch_obstack (TYPE_OWNER (t).gdbarch)), \ + size)) + +/* See comment on TYPE_ALLOC. */ + +#define TYPE_ZALLOC(t,size) (memset (TYPE_ALLOC (t, size), 0, size)) /* Use alloc_type to allocate a type owned by an objfile. Use alloc_type_arch to allocate a type owned by an architecture. Use