| Message ID | AM6PR03MB51704B4EEAFBF3BB1915529EE4130@AM6PR03MB5170.eurprd03.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix -save-temp leaking lto files in /tmp | expand |
On Thu, 20 Feb 2020, Bernd Edlinger wrote: > Hi, > > this is the remaining issue that happens when -flto and -save-temps > is used together, it leaks several files in /tmp. > > I try to give more background to how these temp files are > created, and in all likelihood the leaking of these > files is wanted, and certainly very helpful for debugging > lto issues, that's for sure. It is just not helpful > that they need to be looked up in the /tmp folder, even > if you want to debug something with lto. > > The short story is... > > They are created here: > > if (parallel) > { > makefile = make_temp_file (".mk"); > mstream = fopen (makefile, "w"); > > and here: > > /* Note: we assume argv contains at least one element; this is > checked above. */ > > response_file = make_temp_file (""); > > f = fopen (response_file, "w"); > > And in a few other places as well, it depends a bit > if -o is used or not (i.e. linker_output != NULL or not). > > and not removed here: > > > if (response_file && !save_temps) > { > unlink (response_file); > response_file = NULL; > } > > and here: > > do_wait (new_argv[0], pex); > maybe_unlink (makefile); > makefile = NULL; > > > the code with the response_file is executed both in > lto-wrapper and collect2, but in collect2 only when > if is invoked from lto-wrapper, triggered by the @file > argument list. > > Therefore I figured that the best possible > solution is just let lto-wrapper create a temp-file > for those problem cases, and use TMPDIR to have > all make_temp_file that follow use that to folder to > place the those response files and other stuff in > there. > > > So that is what I split out from the previous patch, > which focused on collect2. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? I don't think this is an improvement. The files still will be (correctly) retained and now in addition to that there's temporary directories piling up? A better improvement would be to selectively decide which files might not be needed to be preserved and/or give them names in the build directory in more cases. Richard. > > Thanks > Bernd. > >
On 2/20/20 2:34 PM, Richard Biener wrote: > On Thu, 20 Feb 2020, Bernd Edlinger wrote: > >> Hi, >> >> this is the remaining issue that happens when -flto and -save-temps >> is used together, it leaks several files in /tmp. >> >> I try to give more background to how these temp files are >> created, and in all likelihood the leaking of these >> files is wanted, and certainly very helpful for debugging >> lto issues, that's for sure. It is just not helpful >> that they need to be looked up in the /tmp folder, even >> if you want to debug something with lto. >> >> The short story is... >> >> They are created here: >> >> if (parallel) >> { >> makefile = make_temp_file (".mk"); >> mstream = fopen (makefile, "w"); >> >> and here: >> >> /* Note: we assume argv contains at least one element; this is >> checked above. */ >> >> response_file = make_temp_file (""); >> >> f = fopen (response_file, "w"); >> >> And in a few other places as well, it depends a bit >> if -o is used or not (i.e. linker_output != NULL or not). >> >> and not removed here: >> >> >> if (response_file && !save_temps) >> { >> unlink (response_file); >> response_file = NULL; >> } >> >> and here: >> >> do_wait (new_argv[0], pex); >> maybe_unlink (makefile); >> makefile = NULL; >> >> >> the code with the response_file is executed both in >> lto-wrapper and collect2, but in collect2 only when >> if is invoked from lto-wrapper, triggered by the @file >> argument list. >> >> Therefore I figured that the best possible >> solution is just let lto-wrapper create a temp-file >> for those problem cases, and use TMPDIR to have >> all make_temp_file that follow use that to folder to >> place the those response files and other stuff in >> there. >> >> >> So that is what I split out from the previous patch, >> which focused on collect2. >> >> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >> Is it OK for trunk? > > I don't think this is an improvement. The files still > will be (correctly) retained and now in addition to that > there's temporary directories piling up? > Well, the temp. directory has a known name, in case the command line is gcc -save-temps -lto -o test test.c there are those 4 in a *known* place, test.lto_tmpdir: test.lto_tmpdir: insgesamt 24 drwxrwxr-x 2 ed ed 4096 Feb 20 16:14 . drwxrwxr-x 3 ed ed 4096 Feb 20 16:14 .. -rw------- 1 ed ed 15 Feb 20 16:14 cc8FwPQ1 -rw------- 1 ed ed 237 Feb 20 16:14 cchROrdh -rw------- 1 ed ed 197 Feb 20 16:14 cclbBAUp -rw------- 1 ed ed 7 Feb 20 16:14 ccMlfh1g plus -rw-rw-r-- 1 ed ed 164 Feb 20 16:14 test.i -rw-rw-r-- 1 ed ed 50 Feb 20 16:14 test.lto_wrapper_args -rw-rw-r-- 1 ed ed 17 Feb 20 16:14 test.ltrans.out -rw-rw-r-- 1 ed ed 1232 Feb 20 16:14 test.ltrans0.ltrans.o -rw-rw-r-- 1 ed ed 2539 Feb 20 16:14 test.ltrans0.o -rw-rw-r-- 1 ed ed 374 Feb 20 16:14 test.ltrans0.s -rw-rw-r-- 1 ed ed 52 Feb 20 16:14 test.res -rw-rw-r-- 1 ed ed 4267 Feb 20 16:14 test.s if the command line is gcc -save-temps -lto test.c there are a few more but also in a *known* place, a.out.lto_tmpdir: a.out.lto_tmpdir/ total 36 drwxrwxr-x 2 ed ed 4096 Feb 20 16:18 . drwxrwxr-x 3 ed ed 4096 Feb 20 16:18 .. -rw------- 1 ed ed 7 Feb 20 16:18 cc2hY8SH -rw------- 1 ed ed 227 Feb 20 16:18 ccDafyit -rw------- 1 ed ed 262 Feb 20 16:18 ccglzNAe -rw------- 1 ed ed 36 Feb 20 16:18 ccnAU7NG -rw------- 1 ed ed 38 Feb 20 16:18 ccoLFY2H.ltrans.out -rw-rw-r-- 1 ed ed 1232 Feb 20 16:18 ccoLFY2H.ltrans0.ltrans.o -rw-rw-r-- 1 ed ed 2560 Feb 20 16:18 ccoLFY2H.ltrans0.o plus -rw-rw-r-- 1 ed ed 50 Feb 20 16:18 a.out.lto_wrapper_args -rw-rw-r-- 1 ed ed 160 Feb 20 16:18 test.i -rw-rw-r-- 1 ed ed 3104 Feb 20 16:18 test.o -rw-rw-r-- 1 ed ed 52 Feb 20 16:18 test.res -rw-rw-r-- 1 ed ed 4267 Feb 20 16:18 test.s > A better improvement would be to selectively decide > which files might not be needed to be preserved and/or > give them names in the build directory in more cases. > Ah, well, it just felt like that will probably need a rather complicated patch which is probably not worth it, since the are a lot of different code pathes involved, (with -g or not, with -flto=jobserver or not, flto_partition=none, or with offload or not, with -o or not etc.) in two or more independent forked processes, which will need to coordinate somehow, not to clobber each other's tempfiles. Bernd. > Richard. > >> >> Thanks >> Bernd. >> >> >
Hi Bernd, hi all, note that you can get even more files: If you do offloading, LTO is additionally run for each offloading target (there can be more than one), cf. https://gcc.gnu.org/wiki/Offloading Some of those files are probably also from mkoffload etc. See below. On 2/20/20 4:33 PM, Bernd Edlinger wrote: > On 2/20/20 2:34 PM, Richard Biener wrote: >> I don't think this is an improvement. The files still >> will be (correctly) retained and now in addition to that >> there's temporary directories piling up? > Well, the temp. directory has a known name, > in case the command line is > > gcc -save-temps -lto -o test test.c > > there are those 4 in a *known* place, test.lto_tmpdir: Here, with offloading (compiler only supports one offloading target), I get for gfortran -fopenacc -save-temps -o test kernels-map-2.F90 (1) in the current directory: kernels-map-2.f90 kernels-map-2.o test.lto_wrapper_args kernels-map-2.res kernels-map-2.s ccv97orQ.i ccv97orQ.s (2) in /tmp: ccXtFBKP.ofldlist cckm9TaT cc1aPJVX ccwnCSfQ ccEwCVRX ccv97orQ.c ccgtnzbU.mkoffload ccsZzs01 ccouoZqP.target.o cc6o87mX.crtoffloadtable.o test And with "-flto" in addition: (1) Current directory: kernels-map-2.f90 kernels-map-2.o test.lto_wrapper_args kernels-map-2.res kernels-map-2.s ccdBFFxF.i ccdBFFxF.s test.ltrans0.o test.ltrans.out test.ltrans0.s test.ltrans0.ltrans.o test (2) in /tmp: ccX5EDTE.ofldlist ccpAKbE7 ccxnuNsz ccq5sdmF ccWUsloz ccZ5rev7.mkoffload ccdBFFxF.c cc3PGOK1 ccW4hTGF.target.o ccWH1ft2 ccsKJG3z.crtoffloadtable.o ccmE0bPK ccB54fLv cckO6O6c Cheers, Tobias
Hi Tobias, On 2/20/20 5:09 PM, Tobias Burnus wrote: > Hi Bernd, hi all, > > note that you can get even more files: If you do offloading, > LTO is additionally run for each offloading target (there > can be more than one), cf. https://gcc.gnu.org/wiki/Offloading > > Some of those files are probably also from mkoffload etc. > See below. > > On 2/20/20 4:33 PM, Bernd Edlinger wrote: > >> On 2/20/20 2:34 PM, Richard Biener wrote: >>> I don't think this is an improvement. The files still >>> will be (correctly) retained and now in addition to that >>> there's temporary directories piling up? >> Well, the temp. directory has a known name, >> in case the command line is >> >> gcc -save-temps -lto -o test test.c >> >> there are those 4 in a *known* place, test.lto_tmpdir: > > Here, with offloading (compiler only supports one > offloading target), I get for > gfortran -fopenacc -save-temps -o test kernels-map-2.F90 > > (1) in the current directory: > kernels-map-2.f90 > kernels-map-2.o > test.lto_wrapper_args > kernels-map-2.res > kernels-map-2.s > ccv97orQ.i > ccv97orQ.s > > (2) in /tmp: > ccXtFBKP.ofldlist > cckm9TaT > cc1aPJVX > ccwnCSfQ > ccEwCVRX > ccv97orQ.c > ccgtnzbU.mkoffload > ccsZzs01 > ccouoZqP.target.o > cc6o87mX.crtoffloadtable.o > test > > And with "-flto" in addition: > > (1) Current directory: > kernels-map-2.f90 > kernels-map-2.o > test.lto_wrapper_args > kernels-map-2.res > kernels-map-2.s > ccdBFFxF.i > ccdBFFxF.s > test.ltrans0.o > test.ltrans.out > test.ltrans0.s > test.ltrans0.ltrans.o > test > > (2) in /tmp: > ccX5EDTE.ofldlist > ccpAKbE7 > ccxnuNsz > ccq5sdmF > ccWUsloz > ccZ5rev7.mkoffload > ccdBFFxF.c > cc3PGOK1 > ccW4hTGF.target.o > ccWH1ft2 > ccsKJG3z.crtoffloadtable.o > ccmE0bPK > ccB54fLv > cckO6O6c > I cannot test anything with offload, because that setup is out of my reach. I believe this patch will improve the situation significantly, but I would be curious to know if the temp files move away from /tmp if you might give this patch a try. It will not prevent the ccXXXXX.i/s in the $PWD, unfortunately. Thanks Bernd.
On Thu, 20 Feb 2020, Bernd Edlinger wrote: > On 2/20/20 2:34 PM, Richard Biener wrote: > > On Thu, 20 Feb 2020, Bernd Edlinger wrote: > > > >> Hi, > >> > >> this is the remaining issue that happens when -flto and -save-temps > >> is used together, it leaks several files in /tmp. > >> > >> I try to give more background to how these temp files are > >> created, and in all likelihood the leaking of these > >> files is wanted, and certainly very helpful for debugging > >> lto issues, that's for sure. It is just not helpful > >> that they need to be looked up in the /tmp folder, even > >> if you want to debug something with lto. > >> > >> The short story is... > >> > >> They are created here: > >> > >> if (parallel) > >> { > >> makefile = make_temp_file (".mk"); > >> mstream = fopen (makefile, "w"); > >> > >> and here: > >> > >> /* Note: we assume argv contains at least one element; this is > >> checked above. */ > >> > >> response_file = make_temp_file (""); > >> > >> f = fopen (response_file, "w"); > >> > >> And in a few other places as well, it depends a bit > >> if -o is used or not (i.e. linker_output != NULL or not). > >> > >> and not removed here: > >> > >> > >> if (response_file && !save_temps) > >> { > >> unlink (response_file); > >> response_file = NULL; > >> } > >> > >> and here: > >> > >> do_wait (new_argv[0], pex); > >> maybe_unlink (makefile); > >> makefile = NULL; > >> > >> > >> the code with the response_file is executed both in > >> lto-wrapper and collect2, but in collect2 only when > >> if is invoked from lto-wrapper, triggered by the @file > >> argument list. > >> > >> Therefore I figured that the best possible > >> solution is just let lto-wrapper create a temp-file > >> for those problem cases, and use TMPDIR to have > >> all make_temp_file that follow use that to folder to > >> place the those response files and other stuff in > >> there. > >> > >> > >> So that is what I split out from the previous patch, > >> which focused on collect2. > >> > >> > >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > >> Is it OK for trunk? > > > > I don't think this is an improvement. The files still > > will be (correctly) retained and now in addition to that > > there's temporary directories piling up? > > > > Well, the temp. directory has a known name, > in case the command line is > > gcc -save-temps -lto -o test test.c > > there are those 4 in a *known* place, test.lto_tmpdir: > > test.lto_tmpdir: > insgesamt 24 > drwxrwxr-x 2 ed ed 4096 Feb 20 16:14 . > drwxrwxr-x 3 ed ed 4096 Feb 20 16:14 .. > -rw------- 1 ed ed 15 Feb 20 16:14 cc8FwPQ1 > -rw------- 1 ed ed 237 Feb 20 16:14 cchROrdh > -rw------- 1 ed ed 197 Feb 20 16:14 cclbBAUp > -rw------- 1 ed ed 7 Feb 20 16:14 ccMlfh1g > > plus > -rw-rw-r-- 1 ed ed 164 Feb 20 16:14 test.i > -rw-rw-r-- 1 ed ed 50 Feb 20 16:14 test.lto_wrapper_args > -rw-rw-r-- 1 ed ed 17 Feb 20 16:14 test.ltrans.out > -rw-rw-r-- 1 ed ed 1232 Feb 20 16:14 test.ltrans0.ltrans.o > -rw-rw-r-- 1 ed ed 2539 Feb 20 16:14 test.ltrans0.o > -rw-rw-r-- 1 ed ed 374 Feb 20 16:14 test.ltrans0.s > -rw-rw-r-- 1 ed ed 52 Feb 20 16:14 test.res > -rw-rw-r-- 1 ed ed 4267 Feb 20 16:14 test.s > > if the command line is > gcc -save-temps -lto test.c > > there are a few more but also in a *known* place, a.out.lto_tmpdir: > > a.out.lto_tmpdir/ > total 36 > drwxrwxr-x 2 ed ed 4096 Feb 20 16:18 . > drwxrwxr-x 3 ed ed 4096 Feb 20 16:18 .. > -rw------- 1 ed ed 7 Feb 20 16:18 cc2hY8SH > -rw------- 1 ed ed 227 Feb 20 16:18 ccDafyit > -rw------- 1 ed ed 262 Feb 20 16:18 ccglzNAe > -rw------- 1 ed ed 36 Feb 20 16:18 ccnAU7NG > -rw------- 1 ed ed 38 Feb 20 16:18 ccoLFY2H.ltrans.out > -rw-rw-r-- 1 ed ed 1232 Feb 20 16:18 ccoLFY2H.ltrans0.ltrans.o > -rw-rw-r-- 1 ed ed 2560 Feb 20 16:18 ccoLFY2H.ltrans0.o > > plus > -rw-rw-r-- 1 ed ed 50 Feb 20 16:18 a.out.lto_wrapper_args > -rw-rw-r-- 1 ed ed 160 Feb 20 16:18 test.i > -rw-rw-r-- 1 ed ed 3104 Feb 20 16:18 test.o > -rw-rw-r-- 1 ed ed 52 Feb 20 16:18 test.res > -rw-rw-r-- 1 ed ed 4267 Feb 20 16:18 test.s > > > > A better improvement would be to selectively decide > > which files might not be needed to be preserved and/or > > give them names in the build directory in more cases. > > > > Ah, well, it just felt like that will probably need a > rather complicated patch which is probably not > worth it, since the are a lot of different > code pathes involved, (with -g or not, with -flto=jobserver > or not, flto_partition=none, or with offload or not, with -o > or not etc.) in two or more independent forked processes, > which will need to coordinate somehow, not to clobber each > other's tempfiles. IIRC this definitely clashes with Alex work to overhaul -auxdir/-dumpdir queued for GCC 11 where some of the above is improved. So whatever we do we should do it for GCC 11 after Alex patches land. Richard. > > Bernd. > > > > Richard. > > > >> > >> Thanks > >> Bernd. > >> > >> > > >
Hi, On 2/21/20 8:35 AM, Richard Biener wrote: > > IIRC this definitely clashes with Alex work to overhaul > -auxdir/-dumpdir queued for GCC 11 where some of the above > is improved. > > So whatever we do we should do it for GCC 11 after Alex patches > land. > > Richard. > Okay, I think this patch was applied in the mean time. Just some 20-30 temp files are left from a full run of the testsuite. So I rewrote my patch, and found this time it looks feasible to avoid all remaining temp files with unpredictable random names, so that is an improvement over the state earlier this year. Attached is my new patch, to clean up the rest of the -save-temps files. That consist just of a couple of @file parameters. I added a few test cases, and the testsuite runs without any temp files leaking. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd.
Hi, I'd like to ping for this patch below: https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561811.html Thanks Bernd. On 12/14/20 4:31 PM, Bernd Edlinger wrote: > Hi, > > On 2/21/20 8:35 AM, Richard Biener wrote: >> >> IIRC this definitely clashes with Alex work to overhaul >> -auxdir/-dumpdir queued for GCC 11 where some of the above >> is improved. >> >> So whatever we do we should do it for GCC 11 after Alex patches >> land. >> >> Richard. >> > > Okay, I think this patch was applied in the mean time. > Just some 20-30 temp files are left from a full run of the testsuite. > > So I rewrote my patch, and found this time it looks > feasible to avoid all remaining temp files with unpredictable > random names, so that is an improvement over the state earlier > this year. > > > Attached is my new patch, to clean up the rest of the -save-temps > files. That consist just of a couple of @file parameters. > > I added a few test cases, and the testsuite runs without any > temp files leaking. > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. >
On Mon, 14 Dec 2020, Bernd Edlinger wrote: > Hi, > > On 2/21/20 8:35 AM, Richard Biener wrote: > > > > IIRC this definitely clashes with Alex work to overhaul > > -auxdir/-dumpdir queued for GCC 11 where some of the above > > is improved. > > > > So whatever we do we should do it for GCC 11 after Alex patches > > land. > > > > Richard. > > > > Okay, I think this patch was applied in the mean time. > Just some 20-30 temp files are left from a full run of the testsuite. > > So I rewrote my patch, and found this time it looks > feasible to avoid all remaining temp files with unpredictable > random names, so that is an improvement over the state earlier > this year. > > > Attached is my new patch, to clean up the rest of the -save-temps > files. That consist just of a couple of @file parameters. > > I added a few test cases, and the testsuite runs without any > temp files leaking. > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? OK. Thanks, Richard. > > Thanks > Bernd. >
From d6dc826c63dc881fe41dbf0c3a461008afdef8b3 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Mon, 17 Feb 2020 17:40:07 +0100 Subject: [PATCH] Fix -save-temp leaking lto files in /tmp When linking with -flto and -save-temps, various temporary files are created in /tmp. They would normally be deleted without -save-temps, but are retained for debugging purposes, which is good. So this just creates a folder named as output-file.lto_tmpdir, which makes this feature even more useful, as the temporary files do not linger in the /tmp directoy but instead are more easy to locate this way. 2020-02-20 Bernd Edlinger <bernd.edlinger@hotmail.de> * lto-wrapper.c (run_gcc): Create an lto tmpdir and use it when -save-temps is used. --- gcc/lto-wrapper.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c index fe8f292..fdc9565 100644 --- a/gcc/lto-wrapper.c +++ b/gcc/lto-wrapper.c @@ -1423,6 +1423,17 @@ run_gcc (unsigned argc, char *argv[]) fputc ('\n', stderr); } + if (save_temps) + { + char *tmpdir = concat (linker_output ? linker_output : "a.out", + ".lto_tmpdir", NULL); + /* Make directory if necessary, but expect no race here. */ + if (access (tmpdir, F_OK) == 0 + || mkdir (tmpdir, S_IRWXU | S_IRWXG | S_IRWXO) == 0) + setenv ("TMPDIR", tmpdir, 1); + free (tmpdir); + } + if (linker_output_rel) no_partition = true; -- 1.9.1
Hi, this is the remaining issue that happens when -flto and -save-temps is used together, it leaks several files in /tmp. I try to give more background to how these temp files are created, and in all likelihood the leaking of these files is wanted, and certainly very helpful for debugging lto issues, that's for sure. It is just not helpful that they need to be looked up in the /tmp folder, even if you want to debug something with lto. The short story is... They are created here: if (parallel) { makefile = make_temp_file (".mk"); mstream = fopen (makefile, "w"); and here: /* Note: we assume argv contains at least one element; this is checked above. */ response_file = make_temp_file (""); f = fopen (response_file, "w"); And in a few other places as well, it depends a bit if -o is used or not (i.e. linker_output != NULL or not). and not removed here: if (response_file && !save_temps) { unlink (response_file); response_file = NULL; } and here: do_wait (new_argv[0], pex); maybe_unlink (makefile); makefile = NULL; the code with the response_file is executed both in lto-wrapper and collect2, but in collect2 only when if is invoked from lto-wrapper, triggered by the @file argument list. Therefore I figured that the best possible solution is just let lto-wrapper create a temp-file for those problem cases, and use TMPDIR to have all make_temp_file that follow use that to folder to place the those response files and other stuff in there. So that is what I split out from the previous patch, which focused on collect2. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd.