* options generation and language count
@ 2011-08-30 6:57 Gary Funck
2011-08-30 16:50 ` Joseph S. Myers
0 siblings, 1 reply; 8+ messages in thread
From: Gary Funck @ 2011-08-30 6:57 UTC (permalink / raw)
To: Gcc Patches
Recently, we ran "make check" on the GCC built from the GUPC branck
and compared it against "make check" run against the GCC trunk version
of the most recent merge with the trunk. The following failure
was detected on the GUPC branch.
cc1: internal compiler error: in print_specific_help, at opts.c:1128
The following command generates the error.
gcc/xgcc -Bgcc/ test-18810.c --help=optimizers -lm -o test-18810.x
The assertion check is here:
1126 /* Sanity check: Make sure that we do not have more
1127 languages than we have bits available to enumerate them. */
1128 gcc_assert ((1U << cl_lang_count) < CL_MIN_OPTION_CLASS);
The value of 'cl_lang_count' is calculated by an AWK script that
processes the .opt files and generates .c and .h files. The value of
CL_MIN_OPTION_CLASS is defined in opts.h.
130 #define CL_PARAMS (1U << 11) /* Fake entry. Used to display --param info with --help. */
131 #define CL_WARNING (1U << 12) /* Enables an (optional) warning message. */
132 #define CL_OPTIMIZATION (1U << 13) /* Enables an (optional) optimization. */
133 #define CL_DRIVER (1U << 14) /* Driver option. */
134 #define CL_TARGET (1U << 15) /* Target-specific option. */
135 #define CL_COMMON (1U << 16) /* Language-independent. */
136
137 #define CL_MIN_OPTION_CLASS CL_PARAMS
138 #define CL_MAX_OPTION_CLASS CL_COMMON
Above, we can see that CL_MIN_OPTION_CLASS is equal to CL_PARAMS,
which is a fixed value: (1U << 11).
The options.h file is generated by the AWK scripts and is present in
the 'gcc' build directory. Here's the relevant values.
#define CL_UPC (1U << 10)
#define CL_LANG_ALL ((1U << 11) - 1)
The cl_lang_count value is in the generated options.c file.
const unsigned int cl_lang_count = 11;
Although I haven't reviewed the source code in detail it looks to me
like there might be two issues here:
1) The sanity check should probably read:
gcc_assert ((1U << cl_lang_count) <= CL_MIN_OPTION_CLASS);
In other words, it is off-by-one.
2) The fixed shift counts starting at CL_PARAMS probably need to be
adjusted upwards to allow for more languages.
A third problem, is that this sort of error would be better detected
at build time rather than waiting to run a DejaGNU test to find it.
Also, the use of fixed masks is problematic. Perhaps the AWK script
could be changed to also generate values for CL_PARAMS, etc., ensuring
that will not conflict with the language mask values?
- Gary
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: options generation and language count 2011-08-30 6:57 options generation and language count Gary Funck @ 2011-08-30 16:50 ` Joseph S. Myers 2011-08-30 16:50 ` Gary Funck 0 siblings, 1 reply; 8+ messages in thread From: Joseph S. Myers @ 2011-08-30 16:50 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches On Mon, 29 Aug 2011, Gary Funck wrote: > 1) The sanity check should probably read: > gcc_assert ((1U << cl_lang_count) <= CL_MIN_OPTION_CLASS); > In other words, it is off-by-one. Please send a patch. > 2) The fixed shift counts starting at CL_PARAMS probably need to be > adjusted upwards to allow for more languages. Plese send a patch. I moved lots of flags to bit-fields so there is now plenty of room to move flags up. > A third problem, is that this sort of error would be better detected > at build time rather than waiting to run a DejaGNU test to find it. Please send a patch. Making optc-gen.awk or opth-gen.awk output #if conditions surrounding a #error would be the usual approach. > Also, the use of fixed masks is problematic. Perhaps the AWK script > could be changed to also generate values for CL_PARAMS, etc., ensuring > that will not conflict with the language mask values? That sounds riskier (and does everywhere using opts.h actually need the generated options.h as well?), although in principle it ought to be possible to assign this automatically (and actually only CL_DRIVER, CL_TARGET and CL_COMMON should really need bits assigned, though avoiding the others would require a --help rework). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: options generation and language count 2011-08-30 16:50 ` Joseph S. Myers @ 2011-08-30 16:50 ` Gary Funck 2011-08-30 19:59 ` Joseph S. Myers 0 siblings, 1 reply; 8+ messages in thread From: Gary Funck @ 2011-08-30 16:50 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Gcc Patches On 08/30/11 15:52:12, Joseph S. Myers wrote: > Please send a patch. [... on points 1, 2, and 3]0 OK, will do. > GF: Also, the use of fixed masks is problematic. Perhaps the AWK script > GF: could be changed to also generate values for CL_PARAMS, etc., ensuring > GF: that will not conflict with the language mask values? > > That sounds riskier (and does everywhere using opts.h actually need the > generated options.h as well?) It looks like many files include opts.h, but do not include options.h. > although in principle it ought to be > possible to assign this automatically (and actually only CL_DRIVER, > CL_TARGET and CL_COMMON should really need bits assigned, though avoiding > the others would require a --help rework). An alternative might be to move the fixed assignments (CL_DRIVER, CL_TARGET, CL_COMMON ...) down to start at the beginning of the range, (leaving room for a few more) and to start the auto-generated language mask bits above that? - Gary ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: options generation and language count 2011-08-30 16:50 ` Gary Funck @ 2011-08-30 19:59 ` Joseph S. Myers 2011-09-01 21:49 ` [Patch, C] " Gary Funck 0 siblings, 1 reply; 8+ messages in thread From: Joseph S. Myers @ 2011-08-30 19:59 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches On Tue, 30 Aug 2011, Gary Funck wrote: > On 08/30/11 15:52:12, Joseph S. Myers wrote: > > Please send a patch. [... on points 1, 2, and 3]0 > > OK, will do. > > > GF: Also, the use of fixed masks is problematic. Perhaps the AWK script > > GF: could be changed to also generate values for CL_PARAMS, etc., ensuring > > GF: that will not conflict with the language mask values? > > > > That sounds riskier (and does everywhere using opts.h actually need the > > generated options.h as well?) > > It looks like many files include opts.h, but do not include options.h. options.h is also included via flags.h or tm.h. (The dependencies between these headers are rather a mess.) > > although in principle it ought to be > > possible to assign this automatically (and actually only CL_DRIVER, > > CL_TARGET and CL_COMMON should really need bits assigned, though avoiding > > the others would require a --help rework). > > An alternative might be to move the fixed assignments (CL_DRIVER, > CL_TARGET, CL_COMMON ...) down to start at the beginning of the range, > (leaving room for a few more) and to start the auto-generated > language mask bits above that? You could do that - again, you'd need to watch for existing dependencies on the ordering. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch, C] options generation and language count 2011-08-30 19:59 ` Joseph S. Myers @ 2011-09-01 21:49 ` Gary Funck 2011-09-02 13:42 ` Joseph S. Myers 0 siblings, 1 reply; 8+ messages in thread From: Gary Funck @ 2011-09-01 21:49 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Gcc Patches [-- Attachment #1: Type: text/plain, Size: 896 bytes --] 2011-09-01 Gary Funck <gary@intrepid.com> * opts.c (print_specific_help): Fix off-by-one compare in assertion check. * opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED): Increase by +5 to allow for more languages. * Makefile.in (options.c): Extract max. number of languages value from opts.h, and pass to optc-gen.awk script. * optc-gen.awk: Use max_lang value and issue error if number of languages exceeds implementation-defined limit. This patch extracts the shift count used in the definition of CL_PARAMS to determine the maximum number of language supported by the implementation. optc-gen.awk implements the check and will exit with an error if the number of languages exceeds the limit. Patch, attached. Please review. Thanks, - Gary [-- Attachment #2: optc-gen-lang-check.patch --] [-- Type: text/plain, Size: 5208 bytes --] Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 178389) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,15 @@ +2011-09-01 Gary Funck <gary@intrepid.com> + + * opts.c (print_specific_help): Fix off-by-one compare in + assertion check. + * opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, + CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED): + Increase by +5 to allow for more languages. + * Makefile.in (options.c): Extract max. number of languages value + from opts.h, and pass to optc-gen.awk script. + * optc-gen.awk: Use max_lang value and issue error if number of + languages exceeds implementation-defined limit. + 2011-08-31 Richard Sandiford <rdsandiford@googlemail.com> * config/i386/i386.md: Use (match_test ...) for attribute tests. Index: gcc/opts.c =================================================================== --- gcc/opts.c (revision 178389) +++ gcc/opts.c (working copy) @@ -1125,7 +1125,7 @@ print_specific_help (unsigned int includ /* Sanity check: Make sure that we do not have more languages than we have bits available to enumerate them. */ - gcc_assert ((1U << cl_lang_count) < CL_MIN_OPTION_CLASS); + gcc_assert ((1U << cl_lang_count) <= CL_MIN_OPTION_CLASS); /* If we have not done so already, obtain the desired maximum width of the output. */ Index: gcc/opts.h =================================================================== --- gcc/opts.h (revision 178389) +++ gcc/opts.h (working copy) @@ -127,12 +127,12 @@ extern const unsigned int cl_options_cou extern const char *const lang_names[]; extern const unsigned int cl_lang_count; -#define CL_PARAMS (1U << 11) /* Fake entry. Used to display --param info with --help. */ -#define CL_WARNING (1U << 12) /* Enables an (optional) warning message. */ -#define CL_OPTIMIZATION (1U << 13) /* Enables an (optional) optimization. */ -#define CL_DRIVER (1U << 14) /* Driver option. */ -#define CL_TARGET (1U << 15) /* Target-specific option. */ -#define CL_COMMON (1U << 16) /* Language-independent. */ +#define CL_PARAMS (1U << 16) /* Fake entry. Used to display --param info with --help. */ +#define CL_WARNING (1U << 17) /* Enables an (optional) warning message. */ +#define CL_OPTIMIZATION (1U << 18) /* Enables an (optional) optimization. */ +#define CL_DRIVER (1U << 19) /* Driver option. */ +#define CL_TARGET (1U << 20) /* Target-specific option. */ +#define CL_COMMON (1U << 21) /* Language-independent. */ #define CL_MIN_OPTION_CLASS CL_PARAMS #define CL_MAX_OPTION_CLASS CL_COMMON @@ -142,9 +142,9 @@ extern const unsigned int cl_lang_count; This distinction is important because --help will not list options which only have these higher bits set. */ -#define CL_JOINED (1U << 17) /* If takes joined argument. */ -#define CL_SEPARATE (1U << 18) /* If takes a separate argument. */ -#define CL_UNDOCUMENTED (1U << 19) /* Do not output with --help. */ +#define CL_JOINED (1U << 22) /* If takes joined argument. */ +#define CL_SEPARATE (1U << 23) /* If takes a separate argument. */ +#define CL_UNDOCUMENTED (1U << 24) /* Do not output with --help. */ /* Flags for an enumerated option argument. */ #define CL_ENUM_CANONICAL (1 << 0) /* Canonical for this value. */ Index: gcc/optc-gen.awk =================================================================== --- gcc/optc-gen.awk (revision 178389) +++ gcc/optc-gen.awk (working copy) @@ -30,6 +30,15 @@ # Dump that array of options into a C file. END { +# MAX_LANG is the maximum number of languages that can be defined. +# Its value is extracted from the value of CL_PARAMS in opts.h +# and is passed on the command line as '-v max_lang=...'. +if (n_langs > max_lang) { + print "Error: the number of defined languages (" n_langs ") " \ + "exceeds the maximum supported by this implementation " \ + "(" max_lang ")" > "/dev/stderr" + exit 2 +} print "/* This file is auto-generated by optc-gen.awk. */" print "" n_headers = split(header_name, headers, " ") Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in (revision 178389) +++ gcc/Makefile.in (working copy) @@ -2216,10 +2216,18 @@ s-options: $(ALL_OPT_FILES) Makefile $(s $(STAMP) s-options options.c: optionlist $(srcdir)/opt-functions.awk $(srcdir)/opt-read.awk \ - $(srcdir)/optc-gen.awk + $(srcdir)/optc-gen.awk $(srcdir)/opts.h + max_lang=`$(AWK) '/^#define +CL_PARAMS +\(1U +<< +[0-9]+\)/ \ + {x=$$5; sub(")","",x); print +x; exit}' $(srcdir)/opts.h`; \ + if test -z "$${max_lang}"; then \ + echo "Could not find a valid CL_PARAMS definition in $(srcdir)/opts.h"; \ + exit 2; \ + fi; \ $(AWK) -f $(srcdir)/opt-functions.awk -f $(srcdir)/opt-read.awk \ -f $(srcdir)/optc-gen.awk \ - -v header_name="config.h system.h coretypes.h tm.h" < $< > $@ + -v max_lang="$$max_lang" \ + -v header_name="config.h system.h coretypes.h tm.h" < $< > $@.tmp + mv $@.tmp $@ options-save.c: optionlist $(srcdir)/opt-functions.awk $(srcdir)/opt-read.awk \ $(srcdir)/optc-save-gen.awk ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, C] options generation and language count 2011-09-01 21:49 ` [Patch, C] " Gary Funck @ 2011-09-02 13:42 ` Joseph S. Myers 2011-09-02 16:27 ` Gary Funck 0 siblings, 1 reply; 8+ messages in thread From: Joseph S. Myers @ 2011-09-02 13:42 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches On Thu, 1 Sep 2011, Gary Funck wrote: > +# MAX_LANG is the maximum number of languages that can be defined. > +# Its value is extracted from the value of CL_PARAMS in opts.h > +# and is passed on the command line as '-v max_lang=...'. > +if (n_langs > max_lang) { > + print "Error: the number of defined languages (" n_langs ") " \ > + "exceeds the maximum supported by this implementation " \ > + "(" max_lang ")" > "/dev/stderr" > + exit 2 Are you sure this /dev/stderr reference is portable? I think this is trying to be too clever and you should just generate #if/#error in the output just like all the other error checks, and so not need to extract a value from a header with awk at all. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, C] options generation and language count 2011-09-02 13:42 ` Joseph S. Myers @ 2011-09-02 16:27 ` Gary Funck 2011-09-02 19:53 ` Joseph S. Myers 0 siblings, 1 reply; 8+ messages in thread From: Gary Funck @ 2011-09-02 16:27 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Gcc Patches [-- Attachment #1: Type: text/plain, Size: 727 bytes --] On 09/02/11 13:42:32, Joseph S. Myers wrote: > [..] you should just generate #if/#error in the output [...] OK, take two, attached. (Confirmed that the #if works for the (<, ==, >) relationships between n_langs and the max. number of languages supported.) - Gary 2011-09-02 Gary Funck <gary@intrepid.com> * opts.c (print_specific_help): Fix off-by-one compare in assertion check. * opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED): Increase by +5 to allow for more languages. * optc-gen.awk: Generate #if that ensures that the number of languages is within the implementation-defined limit. [-- Attachment #2: optc-gen-lang-check.patch --] [-- Type: text/plain, Size: 3802 bytes --] Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 178389) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,13 @@ +2011-09-02 Gary Funck <gary@intrepid.com> + + * opts.c (print_specific_help): Fix off-by-one compare in + assertion check. + * opts.h (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, + CL_TARGET, CL_COMMON, CL_JOINED, CL_SEPARATE, CL_UNDOCUMENTED): + Increase by +5 to allow for more languages. + * optc-gen.awk: Generate #if that ensures that the number of + languages is within the implementation-defined limit. + 2011-08-31 Richard Sandiford <rdsandiford@googlemail.com> * config/i386/i386.md: Use (match_test ...) for attribute tests. Index: gcc/opts.c =================================================================== --- gcc/opts.c (revision 178389) +++ gcc/opts.c (working copy) @@ -1125,7 +1125,7 @@ print_specific_help (unsigned int includ /* Sanity check: Make sure that we do not have more languages than we have bits available to enumerate them. */ - gcc_assert ((1U << cl_lang_count) < CL_MIN_OPTION_CLASS); + gcc_assert ((1U << cl_lang_count) <= CL_MIN_OPTION_CLASS); /* If we have not done so already, obtain the desired maximum width of the output. */ Index: gcc/opts.h =================================================================== --- gcc/opts.h (revision 178389) +++ gcc/opts.h (working copy) @@ -127,12 +127,12 @@ extern const unsigned int cl_options_cou extern const char *const lang_names[]; extern const unsigned int cl_lang_count; -#define CL_PARAMS (1U << 11) /* Fake entry. Used to display --param info with --help. */ -#define CL_WARNING (1U << 12) /* Enables an (optional) warning message. */ -#define CL_OPTIMIZATION (1U << 13) /* Enables an (optional) optimization. */ -#define CL_DRIVER (1U << 14) /* Driver option. */ -#define CL_TARGET (1U << 15) /* Target-specific option. */ -#define CL_COMMON (1U << 16) /* Language-independent. */ +#define CL_PARAMS (1U << 16) /* Fake entry. Used to display --param info with --help. */ +#define CL_WARNING (1U << 17) /* Enables an (optional) warning message. */ +#define CL_OPTIMIZATION (1U << 18) /* Enables an (optional) optimization. */ +#define CL_DRIVER (1U << 19) /* Driver option. */ +#define CL_TARGET (1U << 20) /* Target-specific option. */ +#define CL_COMMON (1U << 21) /* Language-independent. */ #define CL_MIN_OPTION_CLASS CL_PARAMS #define CL_MAX_OPTION_CLASS CL_COMMON @@ -142,9 +142,9 @@ extern const unsigned int cl_lang_count; This distinction is important because --help will not list options which only have these higher bits set. */ -#define CL_JOINED (1U << 17) /* If takes joined argument. */ -#define CL_SEPARATE (1U << 18) /* If takes a separate argument. */ -#define CL_UNDOCUMENTED (1U << 19) /* Do not output with --help. */ +#define CL_JOINED (1U << 22) /* If takes joined argument. */ +#define CL_SEPARATE (1U << 23) /* If takes a separate argument. */ +#define CL_UNDOCUMENTED (1U << 24) /* Do not output with --help. */ /* Flags for an enumerated option argument. */ #define CL_ENUM_CANONICAL (1 << 0) /* Canonical for this value. */ Index: gcc/optc-gen.awk =================================================================== --- gcc/optc-gen.awk (revision 178389) +++ gcc/optc-gen.awk (working copy) @@ -169,6 +169,9 @@ for (i = 0; i < n_langs; i++) { print " 0\n};\n" print "const unsigned int cl_options_count = N_OPTS;\n" +print "#if (1U << " n_langs ") > CL_MIN_OPTION_CLASS" +print " #error the number of languages exceeds the implementation limit" +print "#endif" print "const unsigned int cl_lang_count = " n_langs ";\n" print "const struct cl_option cl_options[] =\n{" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, C] options generation and language count 2011-09-02 16:27 ` Gary Funck @ 2011-09-02 19:53 ` Joseph S. Myers 0 siblings, 0 replies; 8+ messages in thread From: Joseph S. Myers @ 2011-09-02 19:53 UTC (permalink / raw) To: Gary Funck; +Cc: Gcc Patches On Fri, 2 Sep 2011, Gary Funck wrote: > On 09/02/11 13:42:32, Joseph S. Myers wrote: > > [..] you should just generate #if/#error in the output [...] > > OK, take two, attached. (Confirmed that the #if works for > the (<, ==, >) relationships between n_langs and the max. > number of languages supported.) This version is OK. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-09-02 19:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-30 6:57 options generation and language count Gary Funck 2011-08-30 16:50 ` Joseph S. Myers 2011-08-30 16:50 ` Gary Funck 2011-08-30 19:59 ` Joseph S. Myers 2011-09-01 21:49 ` [Patch, C] " Gary Funck 2011-09-02 13:42 ` Joseph S. Myers 2011-09-02 16:27 ` Gary Funck 2011-09-02 19:53 ` Joseph S. Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).