This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [driver, LTO Patch]: Resurrect user specs support
Hello
On 05/22/2012 03:52 PM, Joseph S. Myers wrote:
> On Mon, 21 May 2012, Christian Bruel wrote:
>
>> 1) Lazily check the flag validation until all command line spec files
>> are read. For this purpose, 'read_specs' records specs, to be analyzed
>> with 'file_spec_p'. Such flags have 'live_cond' = SWITCH_USER
>
> I like the idea of allowing flags mentioned in user specs but not other
> specs - but not the implementation using this new live_cond.
OK, I have removed the SWITCH_USER flag and replaced it by a bool in the
struct spec_list. This field is now passed as parameter to
validate_switches. Maybe less central, but more flexible as you proposed.
> There are a
> lot of places in gcc.c that set "validated", and I can't convince myself
> that this implementation will ensure they all take correct account of
> where the relevant spec (if any) came from without causing any other
> change to how the driver behaves. For example, I don't see any change to
> the setting of "validated" for %< and %> in do_spec_1 to account for where
> the spec came from.
>
> Instead, I think that any function that sets "validated" based on a spec
> should be passed the information about whether it's a user spec or not.
> So validate_all_switches would need to pass that down to
> validate_switches_from_spec, for example - and do_spec_1 would also need
> to get that information.
I shared the same concern, however, after playing bits with spec toys, I
couldn't a find a way to get a %< switch recognition failure, since the
switches passed on the command line at this point are already validated
if necessary.
I put a simple example of this in attachment to illustrate this. But I
might lack imagination to make up a regression.
We indeed now have all the information to pass down to the do_specs
interfaces, but this would be very intrusive, I'm reluctant to do it if
not strictly necessary. Do you see a way an invalid option could be
accidentally validated ? I would have thought that with the current
implementation invalid flags are detected earlier.
Cheers
Christian
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c (revision 187500)
+++ gcc/gcc.c (working copy)
@@ -190,8 +190,8 @@
static void store_arg (const char *, int, int);
static void insert_wrapper (const char *);
static char *load_specs (const char *);
-static void read_specs (const char *, int);
-static void set_spec (const char *, const char *);
+static void read_specs (const char *, bool, bool);
+static void set_spec (const char *, const char *, bool);
static struct compiler *lookup_compiler (const char *, size_t, const char *);
static char *build_search_list (const struct path_prefix *, const char *,
bool, bool);
@@ -227,9 +227,9 @@
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 *);
+static const char *validate_switches (const char *, bool);
static void validate_all_switches (void);
-static inline void validate_switches_from_spec (const char *);
+static inline void validate_switches_from_spec (const char *, bool);
static void give_switch (int, int);
static int used_arg (const char *, int);
static int default_arg (const char *, int);
@@ -1170,11 +1170,12 @@
const char **ptr_spec; /* pointer to the spec itself. */
struct spec_list *next; /* Next spec in linked list. */
int name_len; /* length of the name */
- int alloc_p; /* whether string was allocated */
+ bool user_p; /* whether string come from file spec. */
+ bool alloc_p; /* whether string was allocated */
};
#define INIT_STATIC_SPEC(NAME,PTR) \
-{ NAME, NULL, PTR, (struct spec_list *) 0, sizeof (NAME) - 1, 0 }
+ { NAME, NULL, PTR, (struct spec_list *) 0, sizeof (NAME) - 1, false, false }
/* List of statically defined specs. */
static struct spec_list static_specs[] =
@@ -1478,7 +1479,7 @@
current spec. */
static void
-set_spec (const char *name, const char *spec)
+set_spec (const char *name, const char *spec, bool user_p)
{
struct spec_list *sl;
const char *old_spec;
@@ -1530,7 +1531,8 @@
if (old_spec && sl->alloc_p)
free (CONST_CAST(char *, old_spec));
- sl->alloc_p = 1;
+ sl->user_p = user_p;
+ sl->alloc_p = true;
}
/* Accumulate a command (program name and args), and run it. */
@@ -1686,7 +1688,7 @@
Anything invalid in the file is a fatal error. */
static void
-read_specs (const char *filename, int main_p)
+read_specs (const char *filename, bool main_p, bool user_p)
{
char *buffer;
char *p;
@@ -1735,7 +1737,7 @@
p[-2] = '\0';
new_filename = find_a_file (&startfile_prefixes, p1, R_OK, true);
- read_specs (new_filename ? new_filename : p1, FALSE);
+ read_specs (new_filename ? new_filename : p1, false, user_p);
continue;
}
else if (!strncmp (p1, "%include_noerr", sizeof "%include_noerr" - 1)
@@ -1756,7 +1758,7 @@
p[-2] = '\0';
new_filename = find_a_file (&startfile_prefixes, p1, R_OK, true);
if (new_filename)
- read_specs (new_filename, FALSE);
+ read_specs (new_filename, false, user_p);
else if (verbose_flag)
fnotice (stderr, "could not find specs file %s\n", p1);
continue;
@@ -1833,7 +1835,7 @@
#endif
}
- set_spec (p2, *(sl->ptr_spec));
+ set_spec (p2, *(sl->ptr_spec), user_p);
if (sl->alloc_p)
free (CONST_CAST (char *, *(sl->ptr_spec)));
@@ -1899,7 +1901,7 @@
if (! strcmp (suffix, "*link_command"))
link_command_spec = spec;
else
- set_spec (suffix + 1, spec);
+ set_spec (suffix + 1, spec, user_p);
}
else
{
@@ -2819,8 +2821,9 @@
const char *part1;
const char **args;
unsigned int live_cond;
- unsigned char validated;
- unsigned char ordering;
+ bool known;
+ bool validated;
+ bool ordering;
};
static struct switchstr *switches;
@@ -3086,11 +3089,11 @@
}
/* Save an option OPT with N_ARGS arguments in array ARGS, marking it
- as validated if VALIDATED. */
+ as validated if VALIDATED and KNOWN if it is an internal switch. */
static void
save_switch (const char *opt, size_t n_args, const char *const *args,
- bool validated)
+ bool validated, bool known)
{
alloc_switch ();
switches[n_switches].part1 = opt + 1;
@@ -3105,6 +3108,7 @@
switches[n_switches].live_cond = 0;
switches[n_switches].validated = validated;
+ switches[n_switches].known = known;
switches[n_switches].ordering = 0;
n_switches++;
}
@@ -3123,9 +3127,17 @@
diagnosed only if there are warnings. */
save_switch (decoded->canonical_option[0],
decoded->canonical_option_num_elements - 1,
- &decoded->canonical_option[1], false);
+ &decoded->canonical_option[1], false, true);
return false;
}
+ if (decoded->opt_index == OPT_SPECIAL_unknown)
+ {
+ /* Give it a chance to define it a a spec file. */
+ save_switch (decoded->canonical_option[0],
+ decoded->canonical_option_num_elements - 1,
+ &decoded->canonical_option[1], false, false);
+ return false;
+ }
else
return true;
}
@@ -3150,7 +3162,7 @@
else
save_switch (decoded->canonical_option[0],
decoded->canonical_option_num_elements - 1,
- &decoded->canonical_option[1], false);
+ &decoded->canonical_option[1], false, true);
}
static const char *spec_lang = 0;
@@ -3293,7 +3305,7 @@
compare_debug_opt = NULL;
else
compare_debug_opt = arg;
- save_switch (compare_debug_replacement_opt, 0, NULL, validated);
+ save_switch (compare_debug_replacement_opt, 0, NULL, validated, true);
return true;
case OPT_Wa_:
@@ -3378,12 +3390,12 @@
case OPT_L:
/* Similarly, canonicalize -L for linkers that may not accept
separate arguments. */
- save_switch (concat ("-L", arg, NULL), 0, NULL, validated);
+ save_switch (concat ("-L", arg, NULL), 0, NULL, validated, true);
return true;
case OPT_F:
/* Likewise -F. */
- save_switch (concat ("-F", arg, NULL), 0, NULL, validated);
+ save_switch (concat ("-F", arg, NULL), 0, NULL, validated, true);
return true;
case OPT_save_temps:
@@ -3426,7 +3438,7 @@
user_specs_head = user;
user_specs_tail = user;
}
- do_save = false;
+ validated = true;
break;
case OPT__sysroot_:
@@ -3505,7 +3517,7 @@
save_temps_prefix = xstrdup (arg);
/* On some systems, ld cannot handle "-o" without a space. So
split the option from its argument. */
- save_switch ("-o", 1, &arg, validated);
+ save_switch ("-o", 1, &arg, validated, true);
return true;
case OPT_static_libgcc:
@@ -3528,7 +3540,7 @@
if (do_save)
save_switch (decoded->canonical_option[0],
decoded->canonical_option_num_elements - 1,
- &decoded->canonical_option[1], validated);
+ &decoded->canonical_option[1], validated, true);
return true;
}
@@ -3955,7 +3967,7 @@
NULL);
switches[n_switches].args = 0;
switches[n_switches].live_cond = 0;
- switches[n_switches].validated = 0;
+ switches[n_switches].validated = false;
switches[n_switches].ordering = 0;
n_switches++;
compare_debug = 1;
@@ -4330,7 +4342,7 @@
save_switch (decoded_options[j].canonical_option[0],
(decoded_options[j].canonical_option_num_elements
- 1),
- &decoded_options[j].canonical_option[1], false);
+ &decoded_options[j].canonical_option[1], false, true);
break;
default:
@@ -5195,7 +5207,7 @@
&& (have_wildcard || switches[i].part1[len] == '\0'))
{
switches[i].live_cond |= switch_option;
- switches[i].validated = 1;
+ switches[i].validated = true;
}
p += len;
@@ -5808,7 +5820,7 @@
for (i = switchnum + 1; i < n_switches; i++)
if (switches[i].part1[0] == 'O')
{
- switches[switchnum].validated = 1;
+ switches[switchnum].validated = true;
switches[switchnum].live_cond = SWITCH_FALSE;
return 0;
}
@@ -5822,7 +5834,7 @@
if (switches[i].part1[0] == name[0]
&& ! strcmp (&switches[i].part1[1], &name[4]))
{
- switches[switchnum].validated = 1;
+ switches[switchnum].validated = true;
switches[switchnum].live_cond = SWITCH_FALSE;
return 0;
}
@@ -5837,7 +5849,7 @@
&& switches[i].part1[3] == '-'
&& !strcmp (&switches[i].part1[4], &name[1]))
{
- switches[switchnum].validated = 1;
+ switches[switchnum].validated = true;
switches[switchnum].live_cond = SWITCH_FALSE;
return 0;
}
@@ -5901,7 +5913,7 @@
}
do_spec_1 (" ", 0, NULL);
- switches[switchnum].validated = 1;
+ switches[switchnum].validated = true;
}
/* Search for a file named NAME trying various prefixes including the
@@ -6269,7 +6281,7 @@
specs_file = find_a_file (&startfile_prefixes, "specs", R_OK, true);
/* Read the specs file unless it is a default one. */
if (specs_file != 0 && strcmp (specs_file, "specs"))
- read_specs (specs_file, TRUE);
+ read_specs (specs_file, true, false);
else
init_spec ();
@@ -6282,7 +6294,7 @@
strcat (specs_file, just_machine_suffix);
strcat (specs_file, "specs");
if (access (specs_file, R_OK) == 0)
- read_specs (specs_file, TRUE);
+ read_specs (specs_file, true, false);
/* Process any configure-time defaults specified for the command line
options, via OPTION_DEFAULT_SPECS. */
@@ -6326,7 +6338,7 @@
{
obstack_grow (&obstack, "%(sysroot_spec) ", strlen ("%(sysroot_spec) "));
obstack_grow0 (&obstack, link_spec, strlen (link_spec));
- set_spec ("link", XOBFINISH (&obstack, const char *));
+ set_spec ("link", XOBFINISH (&obstack, const char *), false);
}
#endif
@@ -6402,7 +6414,7 @@
{
char *filename = find_a_file (&startfile_prefixes, uptr->filename,
R_OK, true);
- read_specs (filename ? filename : uptr->filename, FALSE);
+ read_specs (filename ? filename : uptr->filename, false, true);
}
/* Process any user self specs. */
@@ -7041,14 +7053,14 @@
}
static inline void
-validate_switches_from_spec (const char *spec)
+validate_switches_from_spec (const char *spec, bool user)
{
const char *p = spec;
char c;
while ((c = *p++))
if (c == '%' && (*p == '{' || *p == '<' || (*p == 'W' && *++p == '{')))
/* We have a switch spec. */
- p = validate_switches (p + 1);
+ p = validate_switches (p + 1, user);
}
static void
@@ -7058,20 +7070,20 @@
struct spec_list *spec;
for (comp = compilers; comp->spec; comp++)
- validate_switches_from_spec (comp->spec);
+ validate_switches_from_spec (comp->spec, false);
/* Look through the linked list of specs read from the specs file. */
for (spec = specs; spec; spec = spec->next)
- validate_switches_from_spec (*spec->ptr_spec);
+ validate_switches_from_spec (*spec->ptr_spec, spec->user_p);
- validate_switches_from_spec (link_command_spec);
+ validate_switches_from_spec (link_command_spec, false);
}
/* Look at the switch-name that comes after START
and mark as valid all supplied switches that match it. */
static const char *
-validate_switches (const char *start)
+validate_switches (const char *start, bool user_spec)
{
const char *p = start;
const char *atom;
@@ -7108,8 +7120,9 @@
/* Mark all matching switches as valid. */
for (i = 0; i < n_switches; i++)
if (!strncmp (switches[i].part1, atom, len)
- && (starred || switches[i].part1[len] == 0))
- switches[i].validated = 1;
+ && (starred || switches[i].part1[len] == '\0')
+ && (switches[i].known || user_spec))
+ switches[i].validated = true;
}
if (*p) p++;
@@ -7124,9 +7137,9 @@
{
p++;
if (*p == '{' || *p == '<')
- p = validate_switches (p+1);
+ p = validate_switches (p+1, user_spec);
else if (p[0] == 'W' && p[1] == '{')
- p = validate_switches (p+2);
+ p = validate_switches (p+2, user_spec);
}
else
p++;
@@ -8056,7 +8069,7 @@
abort ();
file = find_a_file (&startfile_prefixes, argv[0], R_OK, true);
- read_specs (file ? file : argv[0], FALSE);
+ read_specs (file ? file : argv[0], false, false);
return NULL;
}
Index: gcc/testsuite/gcc.dg/spec-options.c
===================================================================
--- gcc/testsuite/gcc.dg/spec-options.c (revision 0)
+++ gcc/testsuite/gcc.dg/spec-options.c (revision 0)
@@ -0,0 +1,16 @@
+/* Check that -mfoo is accepted if defined in a user spec
+ and that it is not passed on the command line. */
+/* { dg-do compile } */
+/* { dg-do run { target sh*-*-* } } */
+/* { dg-options "-B${srcdir}/gcc.dg --specs=foo.specs -mfoo" } */
+
+extern void abort(void);
+
+int main(void)
+{
+#ifdef FOO
+ return 0;
+#else
+ abort();
+#endif
+}
Index: gcc/testsuite/gcc.dg/spec-options-2.c
===================================================================
--- gcc/testsuite/gcc.dg/spec-options-2.c (revision 0)
+++ gcc/testsuite/gcc.dg/spec-options-2.c (revision 0)
@@ -0,0 +1,15 @@
+/* Check that -tfoo is accepted if defined in a user spec. */
+/* { dg-do compile } */
+/* { dg-do run { target sh*-*-* } } */
+/* { dg-options "-B${srcdir}/gcc.dg --specs=foo.specs -tfoo" } */
+
+extern void abort(void);
+
+int main(void)
+{
+#ifdef FOO
+ return 0;
+#else
+ abort();
+#endif
+}
Index: gcc/testsuite/gcc.dg/foo.specs
===================================================================
--- gcc/testsuite/gcc.dg/foo.specs (revision 0)
+++ gcc/testsuite/gcc.dg/foo.specs (revision 0)
@@ -0,0 +1,3 @@
+*cc1runtime:
++ %{mfoo|tfoo: -DFOO} %<mfoo
+