> Am 12.12.2024 um 11:28 schrieb Jakub Jelinek <ja...@redhat.com>:
>
> Hi!
>
> As the following testcase shows, -fsanitize=builtin instruments the
> builtins in the ubsan pass which is done shortly after going into
> SSA, but if optimizations optimize the builtins away before that,
> nothing is instrumented. Now, I think it is just fine if the
> result of the builtins isn't used in any way and we just DCE them,
> but in the following optimizations the result is used.
> So, the following patch for -fsanitize=builtin only defers the
> optimizations that might turn single argument CLZ/CTZ (aka undefined
> at zero) until the ubsan pass is done.
> Now, we don't have PROP_ubsan and am not sure it is worth adding it,
> there is PROP_ssa set by the ssa pass which is 3 passes before
> ubsan, but there are only 2 warning passes in between, so PROP_ssa
> looked good enough to me.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
Ok
Richard
> 2024-12-12 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/115127
> * match.pd (clz (X) == C, ctz (X) == C, ctz (X) >= C): Don't
> optimize if -fsanitize=builtin and not yet in SSA form.
>
> * c-c++-common/ubsan/builtin-2.c: New test.
>
> --- gcc/match.pd.jj 2024-12-06 11:00:27.937579733 +0100
> +++ gcc/match.pd 2024-12-11 19:55:08.978334222 +0100
> @@ -9636,13 +9636,17 @@ (define_operator_list SYNC_FETCH_AND_AND
> cmp (lt ge)
> (simplify
> (op (clz:s@2 @0) INTEGER_CST@1)
> - (if (integer_zerop (@1) && single_use (@2))
> - /* clz(X) == 0 is (int)X < 0 and clz(X) != 0 is (int)X >= 0. */
> - (with { tree stype = signed_type_for (TREE_TYPE (@0)); }
> - (cmp (convert:stype @0) { build_zero_cst (stype); }))
> - /* clz(X) == (prec-1) is X == 1 and clz(X) != (prec-1) is X != 1. */
> - (if (wi::to_wide (@1) == TYPE_PRECISION (TREE_TYPE (@0)) - 1)
> - (op @0 { build_one_cst (TREE_TYPE (@0)); }))))))
> + (if (!sanitize_flags_p (SANITIZE_BUILTIN)
> + /* For -fsanitize=builtin give ubsan pass a chance
> + to instrument it first. */
> + || (cfun && (cfun->curr_properties & PROP_ssa) != 0))
> + (if (integer_zerop (@1) && single_use (@2))
> + /* clz(X) == 0 is (int)X < 0 and clz(X) != 0 is (int)X >= 0. */
> + (with { tree stype = signed_type_for (TREE_TYPE (@0)); }
> + (cmp (convert:stype @0) { build_zero_cst (stype); }))
> + /* clz(X) == (prec-1) is X == 1 and clz(X) != (prec-1) is X != 1. */
> + (if (wi::to_wide (@1) == TYPE_PRECISION (TREE_TYPE (@0)) - 1)
> + (op @0 { build_one_cst (TREE_TYPE (@0)); })))))))
> (for op (eq ne)
> cmp (lt ge)
> (simplify
> @@ -9682,7 +9686,13 @@ (define_operator_list SYNC_FETCH_AND_AND
> (op (ctz:s @0) INTEGER_CST@1)
> (with { bool ok = true;
> HOST_WIDE_INT val = 0;
> - if (!tree_fits_shwi_p (@1))
> + if (sanitize_flags_p (SANITIZE_BUILTIN)
> + /* For -fsanitize=builtin give ubsan pass a chance
> + to instrument it first. */
> + && (!cfun
> + || (cfun->curr_properties & PROP_ssa) == 0))
> + ok = false;
> + else if (!tree_fits_shwi_p (@1))
> ok = false;
> else
> {
> @@ -9713,8 +9723,15 @@ (define_operator_list SYNC_FETCH_AND_AND
> (op (ctz:s @0) INTEGER_CST@1)
> (with { tree type0 = TREE_TYPE (@0);
> int prec = TYPE_PRECISION (type0);
> + bool ok = true;
> + if (sanitize_flags_p (SANITIZE_BUILTIN)
> + /* For -fsanitize=builtin give ubsan pass a chance
> + to instrument it first. */
> + && (!cfun
> + || (cfun->curr_properties & PROP_ssa) == 0))
> + ok = false;
> }
> - (if (prec <= MAX_FIXED_MODE_SIZE)
> + (if (ok && prec <= MAX_FIXED_MODE_SIZE)
> (if (tree_int_cst_sgn (@1) < 0 || wi::to_widest (@1) >= prec)
> { constant_boolean_node (op == EQ_EXPR ? false : true, type); }
> (op (bit_and @0 { wide_int_to_tree (type0,
> @@ -9815,7 +9832,13 @@ (define_operator_list SYNC_FETCH_AND_AND
> else if (TYPE_PRECISION (type0)
> == TYPE_PRECISION (long_long_unsigned_type_node))
> cfn = CFN_BUILT_IN_CTZLL;
> - } }
> + }
> + if (sanitize_flags_p (SANITIZE_BUILTIN)
> + /* For -fsanitize=builtin give ubsan pass a chance
> + to instrument it first. */
> + && (!cfun
> + || (cfun->curr_properties & PROP_ssa) == 0))
> + cfn = CFN_LAST; }
> (if (cfn == CFN_CTZ)
> (IFN_CTZ (convert:type0 @0))
> (if (cfn == CFN_BUILT_IN_CTZ)
> --- gcc/testsuite/c-c++-common/ubsan/builtin-2.c.jj 2024-12-11
> 19:49:42.072941749 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/builtin-2.c 2024-12-11
> 19:51:21.503540338 +0100
> @@ -0,0 +1,89 @@
> +/* PR sanitizer/115127 */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=undefined" } */
> +
> +#include <stdio.h>
> +
> +__attribute__((noipa)) int
> +f1 (unsigned a)
> +{
> + return __builtin_clz (a) == 0;
> +}
> +
> +__attribute__((noipa)) int
> +f2 (unsigned long a)
> +{
> + return __builtin_clzl (a) != 0;
> +}
> +
> +__attribute__((noipa)) int
> +f3 (unsigned long long a)
> +{
> + return __builtin_clzll (a) == __CHAR_BIT__ * __SIZEOF_LONG_LONG__ - 1;
> +}
> +
> +__attribute__((noipa)) int
> +f4 (unsigned a)
> +{
> + return __builtin_clz (a) != __CHAR_BIT__ * __SIZEOF_INT__ - 1;
> +}
> +
> +__attribute__((noipa)) int
> +f5 (unsigned long a)
> +{
> + return __builtin_ctzl (a) == 0;
> +}
> +
> +__attribute__((noipa)) int
> +f6 (unsigned long long a)
> +{
> + return __builtin_ctzll (a) != 0;
> +}
> +
> +__attribute__((noipa)) int
> +f7 (unsigned a)
> +{
> + return __builtin_ctz (a) == 4;
> +}
> +
> +__attribute__((noipa)) int
> +f8 (unsigned long a)
> +{
> + return __builtin_ctzl (a) != 4;
> +}
> +
> +__attribute__((noipa)) int
> +f9 (unsigned long long a)
> +{
> + return __builtin_ctzll (a) >= 4;
> +}
> +
> +__attribute__((noipa)) int
> +f10 (unsigned a)
> +{
> + return __builtin_ctz (a) < 4;
> +}
> +
> +int
> +main ()
> +{
> + fprintf (stderr, "FOO MARKER1\n");
> + f1 (0);
> + f2 (0);
> + f3 (0);
> + f4 (0);
> + fprintf (stderr, "FOO MARKER2\n");
> + f5 (0);
> + f6 (0);
> + f7 (0);
> + f8 (0);
> + f9 (0);
> + f10 (0);
> + fprintf (stderr, "FOO MARKER3\n");
> +}
> +
> +/* { dg-output "FOO MARKER1(\n|\r\n|\r)" } */
> +/* { dg-output "(\[^\n\r]*runtime error: passing zero to
> __builtin_clz\\\(\\\), which is not a valid
> argument\[^\n\r]*(\n|\r\n|\r)){4}" } */
> +/* { dg-output "FOO MARKER2(\n|\r\n|\r)" } */
> +/* { dg-output "(\[^\n\r]*runtime error: passing zero to
> __builtin_ctz\\\(\\\), which is not a valid
> argument\[^\n\r]*(\n|\r\n|\r)){6}" } */
> +/* { dg-output "FOO MARKER3" } */
>
> Jakub
>
