options generation and language count

options generation and language count

[Gary Funck]

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

Re: options generation and language count

[Joseph S. Myers]

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

Re: options generation and language count

[Gary Funck]

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

Re: options generation and language count

[Joseph S. Myers]

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

[Patch, C] options generation and language count

[Gary Funck]

[-- 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

Re: [Patch, C] options generation and language count

[Joseph S. Myers]

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

Re: [Patch, C] options generation and language count

[Gary Funck]

[-- 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{"

Re: [Patch, C] options generation and language count

[Joseph S. Myers]

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