On May 26, 2020, Richard Biener <rguent...@suse.de> wrote:
> All visible by testing on x86_64-linux.
Interesting bug. You wouldn't have hit it if you enabled Ada. That's
because I'd missed the %< directives that validate_switches mishandled
in the Ada specs, so validate_switches handled the %{d*} and validated
-dA, -dumpbase, and any other option starting with -d.
Without those Ada specs, they wouldn't be validated, and we'd end up
reporting them as unrecognized.
So we have two bugs:
1. the Ada specs should have included %<dumpdir, %<dumpbase and
%<dumpbase-ext, so that %{d*} would not pass these flags on to gnat1, so
that %:dumps(...) can drop or override these flags without risk of
accidents.
Once I fixed that, I got the same symptoms that you did with a compiler
built without Ada.
2. validate_switches mishandled %< directives. Unlike %{, %W{ and %@{,
%< is terminated by whitespace, and it can't have multiple options
separated by '|' or '&', nor does it have sub-specs to process after a
':'. As it tried to handle %<dumpdir as %{dumpdir} or %{dumpdir| or
%{dumpdir:specs}, it skipped the whitespace after %<dumpdir, then
skipped what, for braced specs, would have been a separator or
terminator, and then proceeded to look at what the skipped character
was. Finding none of the expected separators, it would just assume it
had skipped the closing brace and be done with it. Problem is that,
instead of skipping a separator, it skipped the first character of the
next spec atom. In our case, a '%'. So validate_switches_from_spec
would skip <dumpbase, validate %<dumpbase-ext and get the % of %{d*}
skipped, so validate_switches_from_spec would skip rather than handle
{d*}. And that's why none of the d* options got validated. Oops.
I've arranged for callers of validate_switches to specify whether it's
to handle all the possibilities of a braced spec, or a single switch
name.
This is the resulting patch, that, though I'm still testing it, appears
to fix the problem. I'll report back with a shorter summary and a
ChangeLog entry once I get it more thoroughly tested.
do not skip validation of switch after %<opt
From: Alexandre Oliva <ol...@gnu.org>
---
gcc/ada/gcc-interface/lang-specs.h | 8 +++++---
gcc/gcc.c | 22 +++++++++++++++-------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/gcc/ada/gcc-interface/lang-specs.h
b/gcc/ada/gcc-interface/lang-specs.h
index 12b7cf5e..b72ab29 100644
--- a/gcc/ada/gcc-interface/lang-specs.h
+++ b/gcc/ada/gcc-interface/lang-specs.h
@@ -23,6 +23,8 @@
* *
****************************************************************************/
+#define ADA_DUMPS_OPTIONS DUMPS_OPTIONS ("%{!.adb:%{!.ads:.ada}}")
+
/* This is the contribution to the `default_compilers' array in gcc.c for
GNAT. */
@@ -35,7 +37,7 @@
gnat1 %{I*} %{k8:-gnatk8} %{Wall:-gnatwa} %{w:-gnatws} %{!Q:-quiet}\
%{nostdinc*} %{nostdlib*}\
%{fcompare-debug-second:-gnatd_A} \
- %{O*} %{W*} %{w} %{p} %{pg:-p} %{d*} %:dumps(%{!.adb:%{!.ads:.ada}}) \
+ %{O*} %{W*} %{w} %{p} %{pg:-p} " ADA_DUMPS_OPTIONS " \
%{coverage:-fprofile-arcs -ftest-coverage} "
#if defined(TARGET_VXWORKS_RTP)
"%{fRTS=rtp|fRTS=rtp-smp|fRTS=ravenscar-cert-rtp:-mrtp} "
@@ -51,7 +53,7 @@
%{!c:%e-c required for gnat2why}\
gnat1why %{I*} %{k8:-gnatk8} %{!Q:-quiet}\
%{nostdinc*} %{nostdlib*}\
- %{a} %{d*} %:dumps(%{!.adb:%{!.ads:.ada}}) \
+ %{a} " ADA_DUMPS_OPTIONS " \
%{gnatea:-gnatez} %{g*&m*&f*} \
%1 %{o*:%w%*-gnatO} \
%i \
@@ -62,7 +64,7 @@
%{!c:%e-c required for gnat2scil}\
gnat1scil %{I*} %{k8:-gnatk8} %{!Q:-quiet}\
%{nostdinc*} %{nostdlib*}\
- %{a} %{d*} %:dumps(%{!.adb:%{!.ads:.ada}}) \
+ %{a} " ADA_DUMPS_OPTIONS " \
%{gnatea:-gnatez} %{g*&m*&f*} \
%1 %{o*:%w%*-gnatO} \
%i \
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 8c851d7..49b7020 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -388,7 +388,7 @@ static void do_option_spec (const char *, const char *);
static void do_self_spec (const char *);
static const char *find_file (const char *);
static int is_directory (const char *, bool);
-static const char *validate_switches (const char *, bool);
+static const char *validate_switches (const char *, bool, bool);
static void validate_all_switches (void);
static inline void validate_switches_from_spec (const char *, bool);
static void give_switch (int, int);
@@ -1176,9 +1176,14 @@ static const char *cpp_options =
%{!fno-working-directory:-fworking-directory}}} %{O*}\
%{undef} %{save-temps*:-fpch-preprocess}";
+/* Make it easy for Ada to override the arguments for the %:dumps
+ specs function call. */
+#define DUMPS_OPTIONS(EXTS) \
+ "%<dumpdir %<dumpbase %<dumpbase-ext %{d*} %:dumps(" EXTS ")"
+
/* This contains cpp options which are not passed when the preprocessor
output will be used by another program. */
-static const char *cpp_debug_options = "%<dumpdir %<dumpbase %<dumpbase-ext
%{d*} %:dumps()";
+static const char *cpp_debug_options = DUMPS_OPTIONS ("");
/* NB: This is shared amongst all front-ends, except for Ada. */
static const char *cc1_options =
@@ -9061,7 +9066,7 @@ validate_switches_from_spec (const char *spec, bool user)
|| (*p == 'W' && *++p == '{')
|| (*p == '@' && *++p == '{')))
/* We have a switch spec. */
- p = validate_switches (p + 1, user);
+ p = validate_switches (p + 1, user, *p == '{');
}
static void
@@ -9084,7 +9089,7 @@ validate_all_switches (void)
and mark as valid all supplied switches that match it. */
static const char *
-validate_switches (const char *start, bool user_spec)
+validate_switches (const char *start, bool user_spec, bool braced)
{
const char *p = start;
const char *atom;
@@ -9126,6 +9131,9 @@ next_member:
switches[i].validated = true;
}
+ if (!braced)
+ return p;
+
if (*p) p++;
if (*p && (p[-1] == '|' || p[-1] == '&'))
goto next_member;
@@ -9138,11 +9146,11 @@ next_member:
{
p++;
if (*p == '{' || *p == '<')
- p = validate_switches (p+1, user_spec);
+ p = validate_switches (p+1, user_spec, *p == '{');
else if (p[0] == 'W' && p[1] == '{')
- p = validate_switches (p+2, user_spec);
+ p = validate_switches (p+2, user_spec, true);
else if (p[0] == '@' && p[1] == '{')
- p = validate_switches (p+2, user_spec);
+ p = validate_switches (p+2, user_spec, true);
}
else
p++;
--
Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo/
Free Software Evangelist Stallman was right, but he's left :(
GNU Toolchain Engineer Live long and free, and prosper ethically
