options: Add EnumBitSet property support [PR104158]
Commit Message
On Sat, Jan 22, 2022 at 01:47:08AM +0100, Jakub Jelinek via Gcc-patches wrote:
> I think with the 2) patch I achieve what we want for Fortran, for 1)
> the only behavior from gcc 11 is that
> -fsanitize-coverage=trace-cmp,trace-cmp is now rejected.
> This is mainly from the desire to disallow
> -fconvert=big-endian,little-endian or -Wbidi-chars=bidirectional,any
> etc. where it would be confusing to users what exactly it means.
> But it is the only from these options that actually acts as an Enum
> bit set, each enumerator can be specified with all the others.
> So one option would be stop requiring the EnumSet implies Set properties
> must be specified and just require that either they are specified on all
> EnumValues, or on none of them; the latter case would be for
> -fsanitize-coverage= and the non-Set case would mean that all the
> EnumValues need to have disjoint Value bitmasks and that they can
> be all specified and unlike the Set case also repeated.
> Thoughts on this?
Here is an incremental patch to the first two patches of the series
that implements EnumBitSet that fully restores the -fsanitize-coverage
GCC 11 behavior.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-01-24 Jakub Jelinek <jakub@redhat.com>
PR sanitizer/104158
* opt-functions.awk (var_set): Handle EnumBitSet property.
* optc-gen.awk: Don't disallow RejectNegative if EnumBitSet is
specified.
* opts.h (enum cl_enum_var_value): New type.
* opts-common.cc (decode_cmdline_option): Use CLEV_* values.
Handle CLEV_BITSET.
(cmdline_handle_error): Handle CLEV_BITSET.
* opts.cc (test_enum_sets): Also test EnumBitSet requirements.
* doc/options.texi (EnumBitSet): Document.
* common.opt (fsanitize-coverage=): Use EnumBitSet instead of
EnumSet.
(trace-pc, trace-cmp): Drop Set properties.
* gcc.dg/sancov/pr104158-7.c: Adjust for repeating of arguments
being allowed.
Jakub
Comments
On Mon, Jan 24, 2022 at 10:01 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sat, Jan 22, 2022 at 01:47:08AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > I think with the 2) patch I achieve what we want for Fortran, for 1)
> > the only behavior from gcc 11 is that
> > -fsanitize-coverage=trace-cmp,trace-cmp is now rejected.
> > This is mainly from the desire to disallow
> > -fconvert=big-endian,little-endian or -Wbidi-chars=bidirectional,any
> > etc. where it would be confusing to users what exactly it means.
> > But it is the only from these options that actually acts as an Enum
> > bit set, each enumerator can be specified with all the others.
> > So one option would be stop requiring the EnumSet implies Set properties
> > must be specified and just require that either they are specified on all
> > EnumValues, or on none of them; the latter case would be for
> > -fsanitize-coverage= and the non-Set case would mean that all the
> > EnumValues need to have disjoint Value bitmasks and that they can
> > be all specified and unlike the Set case also repeated.
> > Thoughts on this?
>
> Here is an incremental patch to the first two patches of the series
> that implements EnumBitSet that fully restores the -fsanitize-coverage
> GCC 11 behavior.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2022-01-24 Jakub Jelinek <jakub@redhat.com>
>
> PR sanitizer/104158
> * opt-functions.awk (var_set): Handle EnumBitSet property.
> * optc-gen.awk: Don't disallow RejectNegative if EnumBitSet is
> specified.
> * opts.h (enum cl_enum_var_value): New type.
> * opts-common.cc (decode_cmdline_option): Use CLEV_* values.
> Handle CLEV_BITSET.
> (cmdline_handle_error): Handle CLEV_BITSET.
> * opts.cc (test_enum_sets): Also test EnumBitSet requirements.
> * doc/options.texi (EnumBitSet): Document.
> * common.opt (fsanitize-coverage=): Use EnumBitSet instead of
> EnumSet.
> (trace-pc, trace-cmp): Drop Set properties.
>
> * gcc.dg/sancov/pr104158-7.c: Adjust for repeating of arguments
> being allowed.
>
> --- gcc/opt-functions.awk.jj 2022-01-23 17:25:21.166588518 +0100
> +++ gcc/opt-functions.awk 2022-01-23 17:30:17.989443241 +0100
> @@ -298,9 +298,11 @@ function var_set(flags)
> if (flag_set_p("Enum.*", flags)) {
> en = opt_args("Enum", flags);
> if (flag_set_p("EnumSet", flags))
> - return enum_index[en] ", CLVC_ENUM, 1"
> + return enum_index[en] ", CLVC_ENUM, CLEV_SET"
> + else if (flag_set_p("EnumBitSet", flags))
> + return enum_index[en] ", CLVC_ENUM, CLEV_BITSET"
> else
> - return enum_index[en] ", CLVC_ENUM, 0"
> + return enum_index[en] ", CLVC_ENUM, CLEV_NORMAL"
> }
> if (var_type(flags) == "const char *")
> return "0, CLVC_STRING, 0"
> --- gcc/optc-gen.awk.jj 2022-01-23 17:25:21.000000000 +0100
> +++ gcc/optc-gen.awk 2022-01-23 17:26:38.920502407 +0100
> @@ -349,6 +349,7 @@ for (i = 0; i < n_opts; i++) {
> if (flag_set_p("Enum.*", flags[i])) {
> if (!flag_set_p("RejectNegative", flags[i]) \
> && !flag_set_p("EnumSet", flags[i]) \
> + && !flag_set_p("EnumBitSet", flags[i]) \
> && opts[i] ~ "^[Wfgm]")
> print "#error Enum allowing negative form"
> }
> --- gcc/opts.h.jj 2022-01-23 17:25:21.167588504 +0100
> +++ gcc/opts.h 2022-01-23 17:30:00.469687787 +0100
> @@ -52,6 +52,18 @@ enum cl_var_type {
> CLVC_DEFER
> };
>
> +/* Values for var_value member of CLVC_ENUM. */
> +enum cl_enum_var_value {
> + /* Enum without EnumSet or EnumBitSet. */
> + CLEV_NORMAL,
> +
> + /* EnumSet. */
> + CLEV_SET,
> +
> + /* EnumBitSet. */
> + CLEV_BITSET
> +};
> +
> struct cl_option
> {
> /* Text of the option, including initial '-'. */
> --- gcc/opts-common.cc.jj 2022-01-23 17:25:21.169588477 +0100
> +++ gcc/opts-common.cc 2022-01-23 17:38:00.508987332 +0100
> @@ -811,8 +811,8 @@ decode_cmdline_option (const char *const
> {
> const struct cl_enum *e = &cl_enums[option->var_enum];
>
> - gcc_assert (option->var_value || value == 1);
> - if (option->var_value)
> + gcc_assert (option->var_value != CLEV_NORMAL || value == 1);
> + if (option->var_value != CLEV_NORMAL)
> {
> const char *p = arg;
> HOST_WIDE_INT sum_value = 0;
> @@ -834,19 +834,30 @@ decode_cmdline_option (const char *const
> break;
> }
>
> - unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
> - gcc_checking_assert (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
> - if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
> + HOST_WIDE_INT this_mask = 0;
> + if (option->var_value == CLEV_SET)
> {
> - errors |= CL_ERR_ENUM_SET_ARG;
> - break;
> + unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
> + gcc_checking_assert (set >= 1
> + && set <= HOST_BITS_PER_WIDE_INT);
> + if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
> + {
> + errors |= CL_ERR_ENUM_SET_ARG;
> + break;
> + }
> + used_sets |= HOST_WIDE_INT_1U << (set - 1);
> +
> + for (int i = 0; e->values[i].arg != NULL; i++)
> + if (set == (e->values[i].flags >> CL_ENUM_SET_SHIFT))
> + this_mask |= e->values[i].value;
> + }
> + else
> + {
> + gcc_assert (option->var_value == CLEV_BITSET
> + && ((e->values[idx].flags >> CL_ENUM_SET_SHIFT)
> + == 0));
> + this_mask = this_value;
> }
> - used_sets |= HOST_WIDE_INT_1U << (set - 1);
> -
> - HOST_WIDE_INT this_mask = 0;
> - for (int i = 0; e->values[i].arg != NULL; i++)
> - if (set == (e->values[i].flags >> CL_ENUM_SET_SHIFT))
> - this_mask |= e->values[i].value;
>
> sum_value |= this_value;
> mask |= this_mask;
> @@ -1430,6 +1441,14 @@ cmdline_handle_error (location_t loc, co
> break;
> }
>
> + if (option->var_value == CLEV_BITSET)
> + {
> + if (q == NULL)
> + break;
> + p = q + 1;
> + continue;
> + }
> +
> unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
> gcc_checking_assert (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
> if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
> --- gcc/opts.cc.jj 2022-01-23 17:25:25.554527225 +0100
> +++ gcc/opts.cc 2022-01-23 17:48:55.505842425 +0100
> @@ -3705,13 +3705,14 @@ test_get_option_html_page ()
> #endif
> }
>
> -/* Verify EnumSet requirements. */
> +/* Verify EnumSet and EnumBitSet requirements. */
>
> static void
> test_enum_sets ()
> {
> for (unsigned i = 0; i < cl_options_count; ++i)
> - if (cl_options[i].var_type == CLVC_ENUM && cl_options[i].var_value)
> + if (cl_options[i].var_type == CLVC_ENUM
> + && cl_options[i].var_value != CLEV_NORMAL)
> {
> const struct cl_enum *e = &cl_enums[cl_options[i].var_enum];
> unsigned HOST_WIDE_INT used_sets = 0;
> @@ -3720,12 +3721,22 @@ test_enum_sets ()
> for (unsigned j = 0; e->values[j].arg; ++j)
> {
> unsigned set = e->values[j].flags >> CL_ENUM_SET_SHIFT;
> + if (cl_options[i].var_value == CLEV_BITSET)
> + {
> + /* For EnumBitSet Set shouldn't be used and Value should
> + be a power of two. */
> + ASSERT_TRUE (set == 0);
> + ASSERT_TRUE (pow2p_hwi (e->values[j].value));
> + continue;
> + }
> /* Test that enumerators referenced in EnumSet have all
> Set(n) on them within the valid range. */
> ASSERT_TRUE (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
> highest_set = MAX (set, highest_set);
> used_sets |= HOST_WIDE_INT_1U << (set - 1);
> }
> + if (cl_options[i].var_value == CLEV_BITSET)
> + continue;
> /* If there is just one set, no point to using EnumSet. */
> ASSERT_TRUE (highest_set >= 2);
> /* Test that there are no gaps in between the sets. */
> --- gcc/doc/options.texi.jj 2022-01-23 17:25:21.169588477 +0100
> +++ gcc/doc/options.texi 2022-01-23 17:52:19.005999122 +0100
> @@ -421,6 +421,14 @@ enumeration values with the same set bit
> Or option's argument can be a comma separated list of strings where
> each string is from a different @code{Set(@var{number})}.
>
> +@item EnumBitSet
> +Must be used together with the @code{Enum(@var{name})} property.
> +Similar to @samp{EnumBitSet}, but corresponding @samp{Enum} record must
similar to EnumSet?
Otherwise looks OK.
> +not use @code{Set} properties, each @code{EnumValue} should have
> +@code{Value} that is a power of 2, each value is treated as its own
> +set and its value as the set's mask, so there are no mutually
> +exclusive arguments.
> +
> @item Defer
> The option should be stored in a vector, specified with @code{Var},
> for later processing.
> --- gcc/common.opt.jj 2022-01-23 17:25:25.553527239 +0100
> +++ gcc/common.opt 2022-01-23 17:26:11.640883463 +0100
> @@ -1072,17 +1072,17 @@ Common Driver Joined
> Select what to sanitize.
>
> fsanitize-coverage=
> -Common Joined Enum(sanitize_coverage) Var(flag_sanitize_coverage) EnumSet
> +Common Joined Enum(sanitize_coverage) Var(flag_sanitize_coverage) EnumBitSet
> Select type of coverage sanitization.
>
> Enum
> Name(sanitize_coverage) Type(int)
>
> EnumValue
> -Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC) Set(1)
> +Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC)
>
> EnumValue
> -Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP) Set(2)
> +Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP)
>
> fasan-shadow-offset=
> Common Joined RejectNegative Var(common_deferred_options) Defer
> --- gcc/testsuite/gcc.dg/sancov/pr104158-7.c.jj 2022-01-23 17:56:44.179294117 +0100
> +++ gcc/testsuite/gcc.dg/sancov/pr104158-7.c 2022-01-23 17:55:58.659930113 +0100
> @@ -1,5 +1,11 @@
> /* PR sanitizer/104158 */
> /* { dg-do compile } */
> /* { dg-options "-fsanitize-coverage=trace-cmp,trace-cmp -fdump-tree-optimized" } */
> -/* { dg-error "invalid argument in option '-fsanitize-coverage=trace-cmp,trace-cmp'" "" { target *-*-* } 0 } */
> -/* { dg-message "'trace-cmp' specified multiple times in the same option" "" { target *-*-* } 0 } */
> +/* { dg-final { scan-tree-dump "__sanitizer_cov_trace_cmp" "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "__sanitizer_cov_trace_pc" "optimized" } } */
> +
> +int
> +foo (int a, int b)
> +{
> + return a == b;
> +}
>
>
> Jakub
>
@@ -298,9 +298,11 @@ function var_set(flags)
if (flag_set_p("Enum.*", flags)) {
en = opt_args("Enum", flags);
if (flag_set_p("EnumSet", flags))
- return enum_index[en] ", CLVC_ENUM, 1"
+ return enum_index[en] ", CLVC_ENUM, CLEV_SET"
+ else if (flag_set_p("EnumBitSet", flags))
+ return enum_index[en] ", CLVC_ENUM, CLEV_BITSET"
else
- return enum_index[en] ", CLVC_ENUM, 0"
+ return enum_index[en] ", CLVC_ENUM, CLEV_NORMAL"
}
if (var_type(flags) == "const char *")
return "0, CLVC_STRING, 0"
@@ -349,6 +349,7 @@ for (i = 0; i < n_opts; i++) {
if (flag_set_p("Enum.*", flags[i])) {
if (!flag_set_p("RejectNegative", flags[i]) \
&& !flag_set_p("EnumSet", flags[i]) \
+ && !flag_set_p("EnumBitSet", flags[i]) \
&& opts[i] ~ "^[Wfgm]")
print "#error Enum allowing negative form"
}
@@ -52,6 +52,18 @@ enum cl_var_type {
CLVC_DEFER
};
+/* Values for var_value member of CLVC_ENUM. */
+enum cl_enum_var_value {
+ /* Enum without EnumSet or EnumBitSet. */
+ CLEV_NORMAL,
+
+ /* EnumSet. */
+ CLEV_SET,
+
+ /* EnumBitSet. */
+ CLEV_BITSET
+};
+
struct cl_option
{
/* Text of the option, including initial '-'. */
@@ -811,8 +811,8 @@ decode_cmdline_option (const char *const
{
const struct cl_enum *e = &cl_enums[option->var_enum];
- gcc_assert (option->var_value || value == 1);
- if (option->var_value)
+ gcc_assert (option->var_value != CLEV_NORMAL || value == 1);
+ if (option->var_value != CLEV_NORMAL)
{
const char *p = arg;
HOST_WIDE_INT sum_value = 0;
@@ -834,19 +834,30 @@ decode_cmdline_option (const char *const
break;
}
- unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
- gcc_checking_assert (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
- if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
+ HOST_WIDE_INT this_mask = 0;
+ if (option->var_value == CLEV_SET)
{
- errors |= CL_ERR_ENUM_SET_ARG;
- break;
+ unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
+ gcc_checking_assert (set >= 1
+ && set <= HOST_BITS_PER_WIDE_INT);
+ if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
+ {
+ errors |= CL_ERR_ENUM_SET_ARG;
+ break;
+ }
+ used_sets |= HOST_WIDE_INT_1U << (set - 1);
+
+ for (int i = 0; e->values[i].arg != NULL; i++)
+ if (set == (e->values[i].flags >> CL_ENUM_SET_SHIFT))
+ this_mask |= e->values[i].value;
+ }
+ else
+ {
+ gcc_assert (option->var_value == CLEV_BITSET
+ && ((e->values[idx].flags >> CL_ENUM_SET_SHIFT)
+ == 0));
+ this_mask = this_value;
}
- used_sets |= HOST_WIDE_INT_1U << (set - 1);
-
- HOST_WIDE_INT this_mask = 0;
- for (int i = 0; e->values[i].arg != NULL; i++)
- if (set == (e->values[i].flags >> CL_ENUM_SET_SHIFT))
- this_mask |= e->values[i].value;
sum_value |= this_value;
mask |= this_mask;
@@ -1430,6 +1441,14 @@ cmdline_handle_error (location_t loc, co
break;
}
+ if (option->var_value == CLEV_BITSET)
+ {
+ if (q == NULL)
+ break;
+ p = q + 1;
+ continue;
+ }
+
unsigned set = e->values[idx].flags >> CL_ENUM_SET_SHIFT;
gcc_checking_assert (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
if ((used_sets & (HOST_WIDE_INT_1U << (set - 1))) != 0)
@@ -3705,13 +3705,14 @@ test_get_option_html_page ()
#endif
}
-/* Verify EnumSet requirements. */
+/* Verify EnumSet and EnumBitSet requirements. */
static void
test_enum_sets ()
{
for (unsigned i = 0; i < cl_options_count; ++i)
- if (cl_options[i].var_type == CLVC_ENUM && cl_options[i].var_value)
+ if (cl_options[i].var_type == CLVC_ENUM
+ && cl_options[i].var_value != CLEV_NORMAL)
{
const struct cl_enum *e = &cl_enums[cl_options[i].var_enum];
unsigned HOST_WIDE_INT used_sets = 0;
@@ -3720,12 +3721,22 @@ test_enum_sets ()
for (unsigned j = 0; e->values[j].arg; ++j)
{
unsigned set = e->values[j].flags >> CL_ENUM_SET_SHIFT;
+ if (cl_options[i].var_value == CLEV_BITSET)
+ {
+ /* For EnumBitSet Set shouldn't be used and Value should
+ be a power of two. */
+ ASSERT_TRUE (set == 0);
+ ASSERT_TRUE (pow2p_hwi (e->values[j].value));
+ continue;
+ }
/* Test that enumerators referenced in EnumSet have all
Set(n) on them within the valid range. */
ASSERT_TRUE (set >= 1 && set <= HOST_BITS_PER_WIDE_INT);
highest_set = MAX (set, highest_set);
used_sets |= HOST_WIDE_INT_1U << (set - 1);
}
+ if (cl_options[i].var_value == CLEV_BITSET)
+ continue;
/* If there is just one set, no point to using EnumSet. */
ASSERT_TRUE (highest_set >= 2);
/* Test that there are no gaps in between the sets. */
@@ -421,6 +421,14 @@ enumeration values with the same set bit
Or option's argument can be a comma separated list of strings where
each string is from a different @code{Set(@var{number})}.
+@item EnumBitSet
+Must be used together with the @code{Enum(@var{name})} property.
+Similar to @samp{EnumBitSet}, but corresponding @samp{Enum} record must
+not use @code{Set} properties, each @code{EnumValue} should have
+@code{Value} that is a power of 2, each value is treated as its own
+set and its value as the set's mask, so there are no mutually
+exclusive arguments.
+
@item Defer
The option should be stored in a vector, specified with @code{Var},
for later processing.
@@ -1072,17 +1072,17 @@ Common Driver Joined
Select what to sanitize.
fsanitize-coverage=
-Common Joined Enum(sanitize_coverage) Var(flag_sanitize_coverage) EnumSet
+Common Joined Enum(sanitize_coverage) Var(flag_sanitize_coverage) EnumBitSet
Select type of coverage sanitization.
Enum
Name(sanitize_coverage) Type(int)
EnumValue
-Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC) Set(1)
+Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC)
EnumValue
-Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP) Set(2)
+Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP)
fasan-shadow-offset=
Common Joined RejectNegative Var(common_deferred_options) Defer
@@ -1,5 +1,11 @@
/* PR sanitizer/104158 */
/* { dg-do compile } */
/* { dg-options "-fsanitize-coverage=trace-cmp,trace-cmp -fdump-tree-optimized" } */
-/* { dg-error "invalid argument in option '-fsanitize-coverage=trace-cmp,trace-cmp'" "" { target *-*-* } 0 } */
-/* { dg-message "'trace-cmp' specified multiple times in the same option" "" { target *-*-* } 0 } */
+/* { dg-final { scan-tree-dump "__sanitizer_cov_trace_cmp" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__sanitizer_cov_trace_pc" "optimized" } } */
+
+int
+foo (int a, int b)
+{
+ return a == b;
+}