On Thu, 23 Jul 2020, Alexandre Oliva wrote: > On Jul 23, 2020, Richard Biener <rguent...@suse.de> wrote: > > > On Thu, 23 Jul 2020, Zhanghaijian (A) wrote: > >> # This option restores naming of aux and dump output files > >> # after input files when multiple input files are named, > >> # instead of getting them combined with the output name. > > > Does it actually do what the above comment suggests? > > It should. When there are multiple inputs, it clearly does. I guess > it's just as clear I haven't tried it with a single input. > > > I'd have naiively assumed that for gcc -S t.c -fdump-tree-optimized > > -dumpbase "" I get a ".238t.optimized" file (thus empty dumpbase). > > Uhh, I guess that would be a reasonable expectation, more so if combined > with -dumpdir, but the documentation suggests -dumpbase "" is to restore > pre-revamp behavior. Should we change that?
No, after reading it it seems like a useful feature. > Here's a patch that adds some more tests, fixes the misbehavior, and > adds comments to conditions that led me down investigations that wasted > a lot of time while looking into this problem and trying to make for > more widespread changes. > > Tested on x86_64-linux-gnu. Ok to install? OK. Thanks, Richard. > > [PR96230] some -dumpbase-ext fixes > > From: Alexandre Oliva <ol...@adacore.com> > > The initial bug report was that compiling (-c) with -dumpbase "" > -dumpbase-ext .<ext> crashes the driver. > > The verification of -dumpbase-ext against -dumpbase doesn't cover the > case in which -dumpbase activates backward-compatibility mode. > > I added a test for that, and for -dumpbase-ext without -dumpbase, > trying to make it work in a sensible way, as if applied to the default > -dumpbase for each file. It turned out that this made for too much > complexity in dealing with suffixes derived from input filenames, so I > gave that up and returned to discarding -dumpbase-ext as documented, > ending up with a change identical to that in the original bug report. > > I also thought I caught an off-by-one error in the initial > verification, that caused dumpbase_ext to be discarded if it was > identical to the specified dumpbase, but that turned out to be > intentional as well, so I put in comments and a test to reflect it. > > Finally, an earlier version of the newly-added tests used "$var.ext" > in an expected output list, which showed me the handling of string > expansion was incorrect. Reworked the expr into an eval to make that > work, and, absent any reliance on post-eval adjustments to so-expanded > output names, I arranged for the adjustments to be skipped after eval. > > > Co-Authored-By: "Zhanghaijian (A)" <z.zhanghaij...@huawei.com> > for gcc/ChangeLog > > PR driver/96230 > * gcc.c (process_command): Adjust and document conditions to > reset dumpbase_ext. > > for gcc/testsuite/ChangeLog > > PR driver/96230 > * gcc.misc-tests/outputs.exp: Add tests with -dumpbase-ext, > with identical -dumpbase, with -dumpbase "", and without any > -dumpbase. > (outest): Fix "" expansion in expected outputs, skip > adjustments. > --- > gcc/gcc.c | 15 +++++++++++++-- > gcc/testsuite/gcc.misc-tests/outputs.exp | 23 ++++++++++++++++++----- > 2 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/gcc/gcc.c b/gcc/gcc.c > index c0eb3c1..10bc988 100644 > --- a/gcc/gcc.c > +++ b/gcc/gcc.c > @@ -4907,6 +4907,9 @@ process_command (unsigned int decoded_options_count, > int lendb = strlen (dumpbase); > int lendbx = strlen (dumpbase_ext); > > + /* -dumpbase-ext must be a suffix proper; discard it if it > + matches all of -dumpbase, as that would make for an empty > + basename. */ > if (lendbx >= lendb > || strcmp (dumpbase + lendb - lendbx, dumpbase_ext) != 0) > { > @@ -5083,10 +5086,18 @@ process_command (unsigned int decoded_options_count, > /* Check that dumpbase_ext, if still present, still matches the end > of dumpbase, if present, and drop it otherwise. We only retained > it above when dumpbase was absent to maybe use it to drop the > - extension from output_name before combining it with dumpdir. */ > + extension from output_name before combining it with dumpdir. We > + won't deal with -dumpbase-ext when -dumpbase is not explicitly > + given, even if just to activate backward-compatible dumpbase: > + dropping it on the floor is correct, expected and documented > + behavior. Attempting to deal with a -dumpbase-ext that might > + match the end of some input filename, or of the combination of > + the output basename with the suffix of the input filename, > + possible with an intermediate .gk extension for -fcompare-debug, > + is just calling for trouble. */ > if (dumpbase_ext) > { > - if (!dumpbase) > + if (!dumpbase || !*dumpbase) > { > free (dumpbase_ext); > dumpbase_ext = NULL; > diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp > b/gcc/testsuite/gcc.misc-tests/outputs.exp > index 0784a8e..1e3cd41 100644 > --- a/gcc/testsuite/gcc.misc-tests/outputs.exp > +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp > @@ -137,11 +137,9 @@ proc outest { test sources opts dirs outputs } { > > if { [string match "\[\$\"\]" [string index $og 0]] } { > global aout > - set og [expr $og] > - if { "$og" == "" } { continue } > - } > - > - if { [string index $og 0] == "-" } then { > + eval set o $og > + if { "$o" == "" } { continue } > + } elseif { [string index $og 0] == "-" } then { > if { [string index $og 1] == "-" \ > || [string index $og 1] == "." } then { > set o "$b-$b[string range $og 1 end]" > @@ -723,6 +721,21 @@ outest "$b lto st mult namedb" $mult "-o dir/$b.exe > -save-temps -O2 -flto -flto- > # !$skip_lto > } > > +# PR96230 - -dumpbase "" with -dumpbase-ext, not linking > +outest "$b single -c -o -db'' -dbx.c" $sing "-c -o dir/$b.o -dumpbase \"\" > -dumpbase-ext .c -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} > {{-0.c.???r.final -0.su !!$gspd -0.dwo !0 .o} {}} > +outest "$b mult -c -dd -db'' -dbx.c" $mult "-c -dumpdir dir/ -dumpbase > \"\" -dumpbase-ext .c -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} > {{-1.c.???r.final -1.su !!$gspd -1.dwo !0 -2.c.???r.final -2.su !!$gspd > -2.dwo} {-1.o -2.o}} > +outest "$b single -c -o -db'' -dbx.x" $sing "-c -o dir/$b.o -dumpbase \"\" > -dumpbase-ext .x -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} > {{-0.c.???r.final -0.su !!$gspd -0.dwo !0 .o} {}} > +outest "$b mult -c -dd -db'' -dbx.x" $mult "-c -dumpdir dir/ -dumpbase > \"\" -dumpbase-ext .x -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} > {{-1.c.???r.final -1.su !!$gspd -1.dwo !0 -2.c.???r.final -2.su !!$gspd > -2.dwo} {-1.o -2.o}} > +# Test -dumpbase-ext without an explicit -dumpbase too. > +outest "$b single -c -o -dbx.c" $sing "-c -o dir/$b.o -dumpbase-ext .c > -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} {{.c.???r.final .su > !!$gspd .dwo !0 .o} {}} > +outest "$b mult -c -dd -dbx.c" $mult "-c -dumpdir dir/ -dumpbase-ext .c > -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} {{-1.c.???r.final -1.su > !!$gspd -1.dwo !0 -2.c.???r.final -2.su !!$gspd -2.dwo} {-1.o -2.o}} > +outest "$b single -c -o -dbx.x" $sing "-c -o dir/$b.o -dumpbase-ext .x > -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} {{.c.???r.final .su > !!$gspd .dwo !0 .o} {}} > +outest "$b mult -c -dd -dbx.x" $mult "-c -dumpdir dir/ -dumpbase-ext .x > -fdump-rtl-final -fstack-usage $gsplit_dwarf" {dir/} {{-1.c.???r.final -1.su > !!$gspd -1.dwo !0 -2.c.???r.final -2.su !!$gspd -2.dwo} {-1.o -2.o}} > +outest "$b obj compare-debug save-temps -dbx.x" $sing "-c -fcompare-debug > -save-temps -fdump-rtl-final -fstack-usage $gsplit_dwarf -fdump-final-insns > -dumpbase-ext .x" {} {{-0.c.???r.final -0.su -0.i -0.c.gkd -0.s -0.gk.i > -0.gk.c.???r.final -0.gk.c.gkd !!$gspd -0.dwo !0 -0.o}} > +# -dumpbase-ext is dropped if identical to -dumpbase. > +outest "$b asm db=dbext 1" $sing "-S -fdump-rtl-final -fstack-usage > $gsplit_dwarf -dumpbase a -dumpbase-ext a" {} {{a.???r.final a.su -0.s}} > + > + > # Below are examples taken from the documentation. > # They are likely redundant with earlier test, > # but we want to make sure behavior matches the docs. > > > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)