> Please do send such a patch (with an explanation in each case of how > "validated" still gets set with that redundant setting removed). >
'validated' can only be removed for user --specs switches, this is why I think it doesn't need to be propagated to do_specs. My mistake from the previous mail: It can't be split out of the whole patch. For the explanation, We should see a difference in the way define a flag as a .opt internal, and a option in a spec file. %{S: as a substitute doesn't really define a new flag. But we want to allow them. This why it works. a --spec file option is validated as soon as a user switch matches a substitute spec string. It can't possibly be an invalid option and a user substitute rule in the same time, so there is no need to check in do_specs. validate_all_switches is enough. Of course for .opt flags it is different, and the current handling is unchanged. But I need the .known guard, because we must make sure that we are not caught by any * matching. If the switch is defined by a substitute option in a user spec rule, then the switch is validated when we see it. Since there is necessarily a spec rule, it is necessarily used in a spec. So all other checks in do_specs are not needed. The only risk is that we might mark a switch as SWITCH_FALSE even if it's not valid. Which would be the case for instance if we use a %<flag rule in a internal GCC spec that is only defined in a --spec file..., but that would be a forbidden use, caught by an error. So I tested the following semantics, with the ones that you pointed. 1) undefined switch =================== unchanged: passing a switch that is not declared in a .opt internal file and not defined in a --spec file is an error. If it is not declared in a .opt but defined in a --spec file it is accepted 2) defined unchanged ============ a) passing a switch that is declared in a .opt internal file, but is not in a spec is an error b) passing a switch that is declared in a .opt internal file, and used in any spec rule is accepted. even it it is just to ignore it %< such switch are marked with the .know field of the 'struct switchstr' when it is not decoded as OPT_SPECIAL_unknown. 3) defined changed ========== a) passing a switch that is defined in a --spec file is accepted. Because the driver will necessary process the spec definition rule at some point (starting from EXTRA_SPECS). It there is no definition, then it is an error b) if the switch is used in the <* case it is accepted * A switch handled in the <* case that is defined in a --spec file doesn't not need to be validated at this point, because it will be validated when processing the spec option. * If there is no spec rule for this switch this will be an error. This implementation should change the current semantics when --spec is not used, or when it is used only new substitutes should be allowed. I'm attaching here a revised version of the patch that contains the removal of the 'validated' field in do_specs for non internal flags, thanks for your feedback. Christian
Index: gcc/gcc.c =================================================================== --- gcc/gcc.c (revision 187927) +++ 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); @@ -1171,11 +1171,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[] = @@ -1479,7 +1480,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; @@ -1531,7 +1532,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. */ @@ -1687,7 +1689,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; @@ -1736,7 +1738,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) @@ -1757,7 +1759,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; @@ -1834,7 +1836,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))); @@ -1900,7 +1902,7 @@ if (! strcmp (suffix, "*link_command")) link_command_spec = spec; else - set_spec (suffix + 1, spec); + set_spec (suffix + 1, spec, user_p); } else { @@ -2820,8 +2822,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; @@ -3087,11 +3090,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; @@ -3106,6 +3109,7 @@ switches[n_switches].live_cond = 0; switches[n_switches].validated = validated; + switches[n_switches].known = known; switches[n_switches].ordering = 0; n_switches++; } @@ -3124,9 +3128,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; } @@ -3151,7 +3163,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; @@ -3294,7 +3306,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_: @@ -3379,12 +3391,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: @@ -3427,7 +3439,7 @@ user_specs_head = user; user_specs_tail = user; } - do_save = false; + validated = true; break; case OPT__sysroot_: @@ -3506,7 +3518,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: @@ -3529,7 +3541,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; } @@ -3956,7 +3968,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; @@ -4331,7 +4343,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: @@ -5203,8 +5215,12 @@ if (!strncmp (switches[i].part1, p, len - have_wildcard) && (have_wildcard || switches[i].part1[len] == '\0')) { + /* User switch be validated from validate_all_switches. + when the definition is seen from the spec file. + If not defined anywhere, will be rejected. */ + if (switches[i].known) + switches[i].validated = true; switches[i].live_cond |= switch_option; - switches[i].validated = 1; } p += len; @@ -5831,7 +5847,9 @@ if (switches[i].part1[0] == name[0] && ! strcmp (&switches[i].part1[1], &name[4])) { - switches[switchnum].validated = 1; + /* --specs are validated with the validate_switches mechanism. */ + if (switches[switchnum].known) + switches[switchnum].validated = true; switches[switchnum].live_cond = SWITCH_FALSE; return 0; } @@ -5846,7 +5864,9 @@ && switches[i].part1[3] == '-' && !strcmp (&switches[i].part1[4], &name[1])) { - switches[switchnum].validated = 1; + /* --specs are validated with the validate_switches mechanism. */ + if (switches[switchnum].known) + switches[switchnum].validated = true; switches[switchnum].live_cond = SWITCH_FALSE; return 0; } @@ -6278,7 +6298,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 (); @@ -6291,7 +6311,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. */ @@ -6335,7 +6355,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 @@ -6411,7 +6431,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. */ @@ -7050,14 +7070,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 @@ -7067,20 +7087,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; @@ -7117,8 +7137,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++; @@ -7133,9 +7154,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++; @@ -8065,7 +8086,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/ChangeLog =================================================================== --- gcc/ChangeLog (revision 187927) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,20 @@ +<<<<<<< .mine +2012-05-18 Christian Bruel <christian.br...@st.com> + + * gcc.c (save_switch) Add user_p parameter. Set live_cond. + (read_specs): Likewise. Call record_file_spec. + (main): Call read_specs with user_p. + (record_file_spec): New function. + (file_spec_p): Likewise. + (SWITCH_USER): New flag. + (driver_unknown_option_callback): Test OPT_SPECIAL_unknown. + Add user_p parameter. + (set_collect_gcc_options): Don't ignore SWITCH_USER. + (check_live_switch): Likewise. + (validate_switches): Validate SWITCH_USER options. + (driver_handle_option): Propagate OPT_specs to collect2. + +======= 2012-05-27 Nathan Sidwell <nat...@acm.org> * tree.c (build_constructor): Propagate TREE_SIDE_EFFECTS. @@ -883,6 +900,7 @@ * tree-vrp.c (extract_range_from_binary_expr_1): Handle LSHIFT_EXPRs by constants. +>>>>>>> .r187927 2012-05-15 Tristan Gingold <ging...@adacore.com> * tree-ssa-strlen.c (get_string_length): Convert lhs if needed. 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,2 @@ +*cc1runtime: ++ %{mfoo|tfoo: -DFOO} %<mfoo