From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) by sourceware.org (Postfix) with ESMTPS id 4ABB13858401; Sat, 6 Nov 2021 19:23:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4ABB13858401 X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from gluon.fritz.box ([93.207.80.195]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1M3UV8-1mit3Q1Dms-000g0q; Sat, 06 Nov 2021 20:23:30 +0100 Subject: Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800] To: Bernhard Reutner-Fischer , gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org Cc: Bernhard Reutner-Fischer Newsgroups: gmane.comp.gcc.patches,gmane.comp.gcc.fortran References: <20211105211718.2261686-1-rep.dot.nop@gmail.com> <20211105211718.2261686-3-rep.dot.nop@gmail.com> From: Harald Anlauf Message-ID: Date: Sat, 6 Nov 2021 20:22:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20211105211718.2261686-3-rep.dot.nop@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:+31vdX3yEEwFG7RRe7/COT1b/rpVFuyKgKbw8pZQZkBOGKulAv7 bo6r+QJMeFZUEHQdbkKxVYzbktxcNjTRml9hQIbWdVCvQZlSMENQyhSvEOYDXljdkRluckk j57PI5slgVIbYl09wsiqQST5G59AjkpN89PbmhOBm/x6+dsY7kfjvIOa0TAEvM3JxMc1kcU hParNt32dKTFn7sWP1AfQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:SdVbJpk0NHc=:sEhqJU3Zv3k2OfsKDLaTqC Fkei+yJR0W2+yvlxku461l6jdgAvV6v3S1asPsh+0ynGfhckFDdyuJxmdRKSgOa4+T+btvWFv OdeJ4tFp6gpgo8admkC829dJrrYIY0mMLcjPCtWD3/RxlzEEun2UCfev4SiXRbzViM2dla/Ia dCF+FB+I3wQQY1YTQIJwVeSj3WB8yiMVpa1CZuD+/dS6z3L83jk+cjX8vsT4o7gApfTLn/gvP Gd43vN9ymJ1xBLIUsTvhcSaFj2tZFFiEgPnfvmmx1UOJD59tkzGnoA3xTVAlT9dj5zkIuOSe8 32tHwYBnLo5Ovd6yoD26zy+dbxk7qTGaFb7IidqNsXozEtLxUt/DAh4nzrE0QcvdZrDpUMr6U igf3IrYxgppqJVVUeYcVCjFxbInQ2KluCbLhIP4RSgOeVbC8/9KH1FC+M2ex9N6bwXrFRNsxf 70Ii4NB1drm7DfdfKOGI/5YLCG4M0z2n4ODTEdKix+npDrL+H5Yr1Y+7BDAxVYplXlIZ9QpQH dAf93+EJ0YR5wRjhWXR1uvXX1yQTLr6GuvO/0a8a3KA6ENS13mLnH3PP4B0+GTse4i2YFQ9Pj a8nRga3g54S+MsohuOWwfkCTyMA4U+yuQyLYKT5eUkbmRE9AKAavpD5TqEUF9Lp9s+knoOaVq hLFQ/Gf/yEMZdVuiswEsQp1TkZGKRMoJnevUEfsBZyiGORKMXqb+6w6tWq8yx7yZupUQhkONk YBwNsohY2m5VP7r05f4RoGWFy8n8GzsnWslgSZAOIEIUd95wqxPKcDU/CvogOF0Lc8zsjpbzG TtO4kczEf51d1KUa+LsuxdlC+BH8cCB9xiYjfk4YdGIq7oXN5RSLeR0huWPsFOu4qYUJYgXtb Du+mjOGiflkwGZmF2q5eRQahKc38HDWhrpLvsjMqMeIKQIOkQWvm5+gIz1D7g4aXWqg81FiZf LFE286v7jdlotugLnnCBDA4CBqCeyQHeEMMXvkfyxt9FrLkXCurONslp5WannHHKWapc0uanO PfyGvPU0UvhNNoCjbxdobLbGccgLcq3E64fyYzINL9IWnuOwH6273zIY5CiEl3LbPK0F6hedR F3bjeplKPexk/0= X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_MSPIKE_H2, 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: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Nov 2021 19:23:34 -0000 Hi Bernhard, I cannot comment on the gcc/ parts, but Am 05.11.21 um 22:17 schrieb Bernhard Reutner-Fischer via Gcc-patches: > From: Bernhard Reutner-Fischer > > gcc/fortran/ChangeLog: > > PR fortran/68800 > * cpp.h (gfc_cpp_free_cpp_dirs): New declaration. > * cpp.c (gfc_cpp_free_cpp_dirs): New definition. > (gfc_cpp_add_include_path, gfc_cpp_add_include_path_after): Add > comment. > * f95-lang.c (gfc_finish): Call gfc_cpp_free_cpp_dirs. > * scanner.c (add_path_to_list): Allocate unzeroed memory. > (gfc_add_include_path): Add comment and fix formatting. > > --- > Bootstrapped and regtested without regressions on x86_64-unknown-linux. > Ok for trunk? > > Note: in add_path_to_list i changed dir->path =3D XCNEWVEC to XNEWVEC > since strcpy and strcat add a terminating null byte for us anyway. > Fixes: > > -=3D=3D 68 bytes in 1 blocks are still reachable in loss record 228 of 3= 71 > -=3D=3D at : malloc (vg_replace_malloc.c:380) > -=3D=3D by : xmalloc (xmalloc.c:149) > -=3D=3D by : xstrdup (xstrdup.c:34) > -=3D=3D by : gfc_add_include_path(char const*, bool, bool, bool, bool= ) (scanner.c > :428) > -=3D=3D by : gfc_handle_option(unsigned long, char const*, long, int,= unsigned in > t, cl_option_handlers const*) (options.c:702) > -=3D=3D by : handle_option(gcc_options*, gcc_options*, cl_decoded_opt= ion const*, > unsigned int, int, unsigned int, cl_option_handlers const*, bool, diagno= stic_con > text*) (opts-common.c:1181) > -=3D=3D by : read_cmdline_option(gcc_options*, gcc_options*, cl_decod= ed_option*, > unsigned int, unsigned int, cl_option_handlers const*, diagnostic_contex= t*) (opt > s-common.c:1431) > -=3D=3D by : read_cmdline_options (opts-global.c:237) > -=3D=3D by : decode_options(gcc_options*, gcc_options*, cl_decoded_op= tion*, unsig > ned int, unsigned int, diagnostic_context*, void (*)()) (opts-global.c:3= 19) > -=3D=3D by : toplev::main(int, char**) (toplev.c:2273) > -=3D=3D by : main (main.c:39) > --- > gcc/fortran/cpp.c | 13 +++++++++++-- > gcc/fortran/cpp.h | 1 + > gcc/fortran/f95-lang.c | 2 +- > gcc/fortran/scanner.c | 7 ++++--- > 4 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c > index e86386c8b17..04fe8fe460b 100644 > --- a/gcc/fortran/cpp.c > +++ b/gcc/fortran/cpp.c > @@ -728,12 +728,20 @@ gfc_cpp_done (void) > cpp_clear_file_cache (cpp_in); > } why do you introduce a wrapper for something outside of fortran that is used only once, > -/* PATH must be malloc-ed and NULL-terminated. */ > +/* Free all cpp include dirs. */ > +void > +gfc_cpp_free_cpp_dirs (void) > +{ > + free_cpp_dirs (); > +} > + > +/* PATH must be NULL-terminated. */ > void > gfc_cpp_add_include_path (char *path, bool user_supplied) > { > /* CHAIN sets cpp_dir->sysp which differs from 0 if PATH is a system > - include path. Fortran does not define any system include paths. *= / > + include path. Fortran does not define any system include paths. > + incpath.c manages the incoming, xstrdup()ed path. */ > int cxx_aware =3D 0; > > add_path (path, INC_BRACKET, cxx_aware, user_supplied); > @@ -742,6 +750,7 @@ gfc_cpp_add_include_path (char *path, bool user_supp= lied) > void > gfc_cpp_add_include_path_after (char *path, bool user_supplied) > { > + /* incpath.c manages the incoming, xstrdup()ed path. */ > int cxx_aware =3D 0; > add_path (path, INC_AFTER, cxx_aware, user_supplied); > } > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h > index 44644a2a333..963b9a9c89e 100644 > --- a/gcc/fortran/cpp.h > +++ b/gcc/fortran/cpp.h > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool); > bool gfc_cpp_preprocess (const char *source_file); > > void gfc_cpp_done (void); > +void gfc_cpp_free_cpp_dirs (void); > > void gfc_cpp_add_include_path (char *path, bool user_supplied); > void gfc_cpp_add_include_path_after (char *path, bool user_supplied); > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c > index 58dcaf01d75..ec4c2cf01d9 100644 > --- a/gcc/fortran/f95-lang.c > +++ b/gcc/fortran/f95-lang.c > @@ -275,7 +275,7 @@ gfc_finish (void) > gfc_cpp_done (); > gfc_done_1 (); > gfc_release_include_path (); > - return; namely here? > + gfc_cpp_free_cpp_dirs (); > } Why not call free_cpp_dirs () here directly, omit all unnecessary stuff, and maybe only add a brief comment here? > /* These functions and variables deal with binding contours. We only > diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c > index 69b81ab97f8..268d1e3e7fb 100644 > --- a/gcc/fortran/scanner.c > +++ b/gcc/fortran/scanner.c > @@ -370,7 +370,7 @@ add_path_to_list (gfc_directorylist **list, const ch= ar *path, > char *q; > size_t len; > int i; > - > + > p =3D path; > while (*p =3D=3D ' ' || *p =3D=3D '\t') /* someone might do "-I inc= lude" */ > if (*p++ =3D=3D '\0') > @@ -409,7 +409,7 @@ add_path_to_list (gfc_directorylist **list, const ch= ar *path, > *list =3D dir; > dir->use_for_modules =3D use_for_modules; > dir->warn =3D warn; > - dir->path =3D XCNEWVEC (char, strlen (p) + = 2); > + dir->path =3D XNEWVEC (char, strlen (p) + 2); > strcpy (dir->path, p); > strcat (dir->path, "/"); /* make '/' last character */ > } > @@ -424,8 +424,9 @@ gfc_add_include_path (const char *path, bool use_for= _modules, bool file_dir, > defer_warn); > > /* For '#include "..."' these directories are automatically searched= . */ > + /* CPP manages 'path' on it's own via incpath.c so give it it's own c= opy. */ > if (!file_dir) > - gfc_cpp_add_include_path (xstrdup(path), true); > + gfc_cpp_add_include_path (xstrdup (path), true); > } > > > Harald