On Tue, 13 Mar 2018, Jakub Jelinek wrote:
> Hi!
>
> The sanopt maybe_optimize_ubsan_ptr_ifn optimization behaves differently
> on 32-bit and on 64-bit targets when using similar arguments maximum or
> minimum of ptrdiff_t or values close to it.
>
> The problem is that UHWI is 64-bit, regardless of whether addresses are
> 64-bit or 32-bit, so for 32-bit targets get_inner_reference returns
> NULL offset and all the offset is in pbitpos, while on 64-bit targets
> where such large offsets in bytes would fit into shwi, but in bits won't
> fit, we return INTEGER_CST offset and often 0 pbitpos.
>
> The following patch handles such offset, so that the sanopt behaves the
> same way, and adjusts the testcase (which really should have just 14
> matches, the 3 lines with comment changes are already covered by the
> overflow check on p = b - SMAX;).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2018-03-13 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/83392
> * sanopt.c (maybe_optimize_ubsan_ptr_ifn): Handle also
> INTEGER_CST offset, add it together with bitpos / 8 and
> sign extend based on POINTER_SIZE.
>
> * c-c++-common/ubsan/ptr-overflow-sanitization-1.c: Adjust expected
> check count from 17 to 14.
>
> --- gcc/sanopt.c.jj 2018-03-02 00:15:54.670780980 +0100
> +++ gcc/sanopt.c 2018-03-13 16:54:49.905621373 +0100
> @@ -486,12 +486,17 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
> HOST_WIDE_INT bitpos;
> base = get_inner_reference (base, &bitsize, &pbitpos, &offset, &mode,
> &unsignedp, &reversep, &volatilep);
> - if (offset == NULL_TREE
> + if ((offset == NULL_TREE || TREE_CODE (offset) == INTEGER_CST)
> && DECL_P (base)
> && pbitpos.is_constant (&bitpos))
> {
> gcc_assert (!DECL_REGISTER (base));
> - offset_int expr_offset = bitpos / BITS_PER_UNIT;
> + offset_int expr_offset;
> + if (offset)
> + expr_offset = wi::to_offset (offset) + bitpos / BITS_PER_UNIT;
> + else
> + expr_offset = bitpos / BITS_PER_UNIT;
> + expr_offset = wi::sext (expr_offset, POINTER_SIZE);
> offset_int total_offset = expr_offset + cur_offset;
> if (total_offset != wi::sext (total_offset, POINTER_SIZE))
> {
> @@ -511,7 +516,7 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
> && (!is_global_var (base) || decl_binds_to_current_def_p (base)))
> {
> offset_int base_size = wi::to_offset (DECL_SIZE_UNIT (base));
> - if (bitpos >= 0
> + if (!wi::neg_p (expr_offset)
> && wi::les_p (total_offset, base_size))
> {
> if (!wi::neg_p (total_offset)
> @@ -532,7 +537,7 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx
> */
>
> bool sign_cur_offset = !wi::neg_p (cur_offset);
> - bool sign_expr_offset = bitpos >= 0;
> + bool sign_expr_offset = !wi::neg_p (expr_offset);
>
> tree base_addr
> = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (base)), base);
> --- gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c.jj
> 2017-10-11 22:37:52.798901780 +0200
> +++ gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
> 2018-03-13 17:00:18.947808006 +0100
> @@ -25,9 +25,9 @@ void foo(void)
> p2 = p + 2;
>
> p = b - SMAX; /* pointer overflow check is needed */
> - p2 = p + (SMAX - 2); /* b - 2: pointer overflow check is needed */
> - p2 = p + (SMAX - 1); /* b - 1: pointer overflow check is needed */
> - p2 = p + SMAX; /* b: pointer overflow check is needed */
> + p2 = p + (SMAX - 2); /* b - 2: no need to check this */
> + p2 = p + (SMAX - 1); /* b - 1: no need to check this */
> + p2 = p + SMAX; /* b: no need to check this */
> p2++; /* b + 1 */
>
> p = c;
> @@ -75,4 +75,4 @@ void negative_to_negative (char *ptr)
> p2 += 5;
> }
>
> -/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 17
> "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_pointer_overflow" 14
> "optimized" } } */
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
