From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb31.google.com (mail-yb1-xb31.google.com [IPv6:2607:f8b0:4864:20::b31]) by sourceware.org (Postfix) with ESMTPS id 6A8573858D28 for ; Sat, 18 Dec 2021 17:50:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6A8573858D28 Received: by mail-yb1-xb31.google.com with SMTP id d10so15996507ybn.0 for ; Sat, 18 Dec 2021 09:50:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=tT+Vkz0S50c01v8S1+Dn8DWsd3msU1AnVPjSAp3YGw4=; b=UWlpwDasEDVDMXQ0OLKWI8ZlptKjl976zUYuDln138TYSmIq5ef/NVPHkYrI6ii2w5 +q432lV3x0ZtDImMzMZJM4B1h2wTLgmSybz6w5ILxiAJy5FTRrymAyp5c4lY4c6Agz6k 0z6LLoeTa7mUwL/b0GtTRSiOVFq9JYxHG4Na/t0nYhiADBw/H3EW3kuE2cl60nu41723 g0I8pX2d+X4Wi4oIM8xCdFIGEbxN9IZek27r0ztyEYZcYNWixT8Z4I0NH1zhv9dzCOgm pfOHVtH7CdbeCDhAEGsZSs2wqjbzZ1qcVw5MfYnhjFWE7XUhxVJcbPC0keTMTD4/s4Xl /2nw== X-Gm-Message-State: AOAM532W7C2rUTEAtbo0O69GKkoL8T9RHv5H5ZKsXcKvIN8MgeGJld5G SsKwRYxt21ykwl2e+Z9U5t2B3Exx88yhVIMVXN4= X-Google-Smtp-Source: ABdhPJwhG/e7qWGnfz47sPzzU8jPPNlCYp6iMFK0BqDQBxMm8gUWGGFyodMDm4URM1PNhlZKx1bzpHN79xGdzaz4WOs= X-Received: by 2002:a25:ced1:: with SMTP id x200mr12393157ybe.211.1639849838966; Sat, 18 Dec 2021 09:50:38 -0800 (PST) MIME-Version: 1.0 References: <07714a8d9583dd2ebfe548f5c066d6d2213434d7.camel@redhat.com> In-Reply-To: <07714a8d9583dd2ebfe548f5c066d6d2213434d7.camel@redhat.com> From: =?UTF-8?Q?Marc_Nieper=2DWi=C3=9Fkirchen?= Date: Sat, 18 Dec 2021 18:50:27 +0100 Message-ID: Subject: Re: Memory leaks (detected by Valgrind) To: David Malcolm Cc: =?UTF-8?Q?Marc_Nieper=2DWi=C3=9Fkirchen?= , Alex Coplan , Mark Wielaard , "jit@gcc.gnu.org" , Joseph Myers Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Sat, 18 Dec 2021 17:50:41 -0000 Am Sa., 18. Dez. 2021 um 17:45 Uhr schrieb David Malcolm : > > On Sat, 2021-12-18 at 14:57 +0100, Marc Nieper-Wi=C3=9Fkirchen wrote: > > Am Sa., 18. Dez. 2021 um 00:23 Uhr schrieb David Malcolm < > > dmalcolm@redhat.com>: > > > > > > On Fri, 2021-12-17 at 10:52 +0000, Alex Coplan via Jit wrote: > > > > Hi, > > > > > > > > > -----Original Message----- > > > > > From: Jit On > > > > > Behalf > > > > > Of Marc > > > > > Nieper-Wi=C3=9Fkirchen via Jit > > > > > Sent: 17 December 2021 10:29 > > > > > To: Mark Wielaard > > > > > Cc: Marc Nieper-Wi=C3=9Fkirchen ; > > > > > jit@gcc.gnu.org > > > > > Subject: Re: Memory leaks (detected by Valgrind) > > > > > > > > > > Thanks! > > > > > > > > > > With `--enable-valgrind-annotations`, the "uses of > > > > > uninitialized > > > > > values" > > > > > have gone away, but a lot of small leaks are still present: > > > > > > > > Memory leaks with libgccjit are a known issue, see > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D63854 > > > > > > FWIW, it looks like all of the remaining leaks are in gcc.c (the > > > "driver" code, for the "gcc" binary). IIRC most of this relates > > > to > > > long-standing code that was written with the assumption that it > > > will > > > only be run once and not persist in memory, and so it tends to mix > > > up > > > string literals and dynamically-allocated strings without bothering > > > to > > > free them (the pointer values persist to the end of "main" when run > > > from gcc, but get cleared when run from libgccjit.so). > > > > > > Patches to clean these up would be great. That said, I'm not the > > > driver/gcc.c maintainer, so I can't formally approve them (but can > > > post > > > +1 emails if they look good to me) [1] > > > > I am currently trying to find my way through the code in gcc.c. > > During > > my experiments, I fixed a single leak where a dynamically allocated > > string was assigned in place of a statically allocated one by moving > > the dynamically allocated string onto the obstack. I hope that such > > an > > approach is acceptable. > > IIRC I tried that approach, and the maintainer (Joseph Myers, I think), > wasn't keen on it, but this was a few years ago. I've tried finding > the discussion in the archives, but couldn't. IIRC, he preferred > adding a flag to strings in gcc.c, tracking if they needed to be freed. > I'm CCing Joseph. FWIW there's a class in libcpp/include/line-map.h: > class label_text which does this. Perhaps it could be used for this > (and maybe renamed???) Okay, no problem to do it that way. > Commit 9376dd63e6a2d94823f6faf8212c9f37bef5a656 added the code to run > gcc.c in-process (before it was run in a subprocess), so maybe the > discussion was some time around then? (2015-08-25) > > Note that it's still possible to run this code in a subprocess rather > than in-process (and thus workaround the leak) using > gcc_jit_context_set_bool_use_external_driver. There are still leaks using the external driver as far as I can tell. In passes.c, the pass map is not deleted. It seems that the following fixes it: diff --git a/gcc/passes.c b/gcc/passes.c index 4bea6ae5b6a..0c70ece5321 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -903,7 +903,7 @@ void pass_manager::register_pass_name (opt_pass *pass, const char *name) { if (!m_name_to_pass_map) - m_name_to_pass_map =3D new hash_map (2= 56); + m_name_to_pass_map =3D new hash_map (256= ); if (m_name_to_pass_map->get (name)) return; /* Ignore plugin passes. */ @@ -1674,6 +1674,7 @@ pass_manager::~pass_manager () GCC_PASS_LISTS #undef DEF_PASS_LIST + delete m_name_to_pass_map; } diff --git a/gcc/pass_manager.h b/gcc/pass_manager.h index aaf72cf6803..f5615e1fda8 100644 --- a/gcc/pass_manager.h +++ b/gcc/pass_manager.h @@ -106,7 +106,7 @@ private: private: context *m_ctxt; - hash_map *m_name_to_pass_map; + hash_map *m_name_to_pass_map; /* References to all of the individual passes. These fields are generated via macro expansion. @@ -146,4 +146,3 @@ private: } // namespace gcc diff --git a/gcc/hash-traits.h b/gcc/hash-traits.h index 6f0373ec27f..1c08d6874ab 100644 --- a/gcc/hash-traits.h +++ b/gcc/hash-traits.h @@ -28,6 +28,11 @@ struct typed_free_remove static inline void remove (Type *p); }; +template +struct typed_const_free_remove +{ + static inline void remove (const Type *p); +}; /* Remove with free. */ @@ -38,6 +43,13 @@ typed_free_remove ::remove (Type *p) free (p); } +template +inline void +typed_const_free_remove ::remove (const Type *p) +{ + free (const_cast (p)); +} + /* Helpful type for removing with delete. */ template @@ -305,6 +317,11 @@ struct ggc_ptr_hash : pointer_hash , ggc_remove {}; template struct ggc_cache_ptr_hash : pointer_hash , ggc_cache_remove {}; +/* Traits for string elements that should be freed when an element is + deleted. */ + +struct free_string_hash : string_hash, typed_const_free_remove {}; + /* Traits for string elements that should not be freed when an element is deleted. */ -- Marc