From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 44D623858C3A for ; Thu, 6 Jan 2022 13:57:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 44D623858C3A Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-448-lpU17abUOimM9POZ8mJqKQ-1; Thu, 06 Jan 2022 08:57:09 -0500 X-MC-Unique: lpU17abUOimM9POZ8mJqKQ-1 Received: by mail-qk1-f200.google.com with SMTP id bq9-20020a05620a468900b004681cdb3483so1710455qkb.23 for ; Thu, 06 Jan 2022 05:57:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=unTxBQIfhlsZDzp8EshkusuJE5t10tZyQLU4F7dOUkE=; b=UTvPK/iXB2h+IuYLd0LT/l8ks1QKRmMls79ewrqtG6YjCU+u+R1Pw+Ix7bK+D/vLFf I8ZI+Lz0pwe+l+T82w7wAPu42EPro/HnRKqKRB/nfvZ1Ms3Nh+XHyRO10ICEfjjPXtBx qHmXV9Yf7KceQcr9Re7y7VCKIHuk4y2tx9aXcCAUoh9M7v2T+xrPLaAbAQjq0a0vrclI d7BaNCLGoT0/qcwh9X9gn3n30exiJBOaaHolMV0ugO94IL39dGK3eZTebcVyKSeTamc6 WgZTkbIF/LWT0sJeeZFfeXmhuAcqfj3x1ZZT6MHwlZvVg0+1j+7I70gD5H2x/1kBZ5MG 0GeA== X-Gm-Message-State: AOAM530iECeDsQQvcs5qUKvQPsXshfP7JVDkdA41qNuQRtyXxnv/03ub MJk8I7v+tcIQVYVvMIFV+j1ljtkBcV0x5pgbhLtUUqvvv5BvnQTQTrJ8w1NIKGm8TbvM06KlbxX wD+pdbDg= X-Received: by 2002:a05:620a:4485:: with SMTP id x5mr40212620qkp.277.1641477428480; Thu, 06 Jan 2022 05:57:08 -0800 (PST) X-Google-Smtp-Source: ABdhPJy3sTO4llfpsWhbfLc37NGu0EzP3bmQc8RvAdQ5WxzuW2NxQWzE6dNKoQNCIjIh8CJsuMkKOw== X-Received: by 2002:a05:620a:4485:: with SMTP id x5mr40212612qkp.277.1641477428303; Thu, 06 Jan 2022 05:57:08 -0800 (PST) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id bs30sm1478755qkb.87.2022.01.06.05.57.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jan 2022 05:57:07 -0800 (PST) Message-ID: <88abc7840520b561e3f8e524cfdd2b2f0b3928f6.camel@redhat.com> Subject: Re: [PATCH] gcc: pass-manager: Fix memory leak. [PR jit/63854] From: David Malcolm To: Marc =?ISO-8859-1?Q?Nieper-Wi=DFkirchen?= , gcc-patches@gcc.gnu.org Cc: jit@gcc.gnu.org Date: Thu, 06 Jan 2022 08:57:06 -0500 In-Reply-To: <2a54fac9b37d87afb009b8eb339d5ad6927454dd.camel@redhat.com> References: <20211219213010.17113-1-marc@nieper-wisskirchen.de> <2a54fac9b37d87afb009b8eb339d5ad6927454dd.camel@redhat.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Jan 2022 13:57:11 -0000 On Thu, 2022-01-06 at 08:53 -0500, David Malcolm wrote: > On Sun, 2021-12-19 at 22:30 +0100, Marc Nieper-Wißkirchen wrote: > > This patch fixes a memory leak in the pass manager. In the existing > > code, > > the m_name_to_pass_map is allocated in > > pass_manager::register_pass_name, but > > never deallocated.  This is fixed by adding a deletion in > > pass_manager::~pass_manager.  Moreover the string keys in > > m_name_to_pass_map are > > all dynamically allocated.  To free them, this patch adds a new hash > > trait for > > string hashes that are to be freed when the corresponding hash entry > > is removed. > > > > This fix is particularly relevant for using GCC as a library through > > libgccjit. > > The memory leak also occurs when libgccjit is instructed to use an > > external > > driver. > > > > Before the patch, compiling the hello world example of libgccjit with > > the external driver under Valgrind shows a loss of 12,611 (48 direct) > > bytes.  After the patch, no memory leaks are reported anymore. > > (Memory leaks occurring when using the internal driver are mostly in > > the driver code in gcc/gcc.c and have to be fixed separately.) > > > > The patch has been tested by fully bootstrapping the compiler with > > the > > frontends C, C++, Fortran, LTO, ObjC, JIT and running the test suite > > under a x86_64-pc-linux-gnu host. > > Thanks for the patch. > > It looks correct to me, given that pass_manager::register_pass_name > does an xstrdup and puts the result in the map. > > That said: > - I'm not officially a reviewer for this part of gcc (though I probably > touched this code last) > - is it cleaner to instead change m_name_to_pass_map's key type from > const char * to char *, to convey that the map "owns" the name?  That > way we probably wouldn't need struct typed_const_free_remove, and (I > hope) works better with the type system. [...snip...] > > > diff --git a/gcc/passes.c b/gcc/passes.c > > index 4bea6ae5b6a..0c70ece5321 100644 > > --- a/gcc/passes.c > > +++ b/gcc/passes.c [...snip...] > > @@ -1943,7 +1944,7 @@ pass_manager::dump_profile_report () const > >            "                                 |in count     |out > > prob     " > >            "|in count                  |out prob                  " > >            "|size               |time                      |\n"); > > -          > > + > >    for (int i = 1; i < passes_by_id_size; i++) > >      if (profile_record[i].run) > >        { > ...and there's a stray whitespace change here (in pass_manager::dump_profile_report), which probably shouldn't be in the patch. Dave