Re: [PATCH] squash spurious warnings in dominance.c

[Richard Biener <richard.guenther@gmail.com>]

On Sat, Apr 22, 2017 at 2:51 AM, Martin Sebor <msebor@gmail.com> wrote:
> Bug 80486 - spurious -Walloc-size-larger-than and
> -Wstringop-overflow in dominance.c during profiledbootstrap
> points out a number of warnings that show up in dominance.c
> during a profiledbootstrap.  I'm pretty sure the warnings
> are due to the size check the C++ new expression introduces
> to avoid unsigned overflow before calling operator new, and
> by some optimization like jump threading introducing a branch
> with the call to the allocation function and memset with
> the excessive constant size.
>
> Two ways to avoid it come to mind: 1) use the libiberty
> XCNEWVEC and XNEWVEC macros instead of C++ new expressions,
> and 2) constraining the size variable to a valid range.
>
> Either of these approaches should result in better code than
> the new expression because they both eliminate the test for
> the overflow.  Attached is a patch that implements (1). I
> chose it mainly because it seems in line with GCC's memory
> management policy and with avoiding exceptions.
>
> An alternate patch should be straightforward.  Either add
> an assert like the one below or change the type of
> m_n_basic_blocks from size_t to unsigned.  This approach,
> though less intrusive, will likely bring the warning back
> in ILP32 builds; I'm not sure if it matters.

Please change m_n_basic_blocks (and local copies) from size_t
to unsigned int.  This is an odd inconsistency that's worth fixing
in any case.

Richard.

> Martin
>
> diff --git a/gcc/dominance.c b/gcc/dominance.c
> index c76e62e..ebb0a8f 100644
> --- a/gcc/dominance.c
> +++ b/gcc/dominance.c
> @@ -161,6 +161,9 @@ void
>  dom_info::dom_init (void)
>  {
>    size_t num = m_n_basic_blocks;
> +
> +  gcc_assert (num < SIZE_MAX / sizeof (basic_block) / 2);
> +
>    m_dfs_parent = new_zero_array <TBB> (num);
>    m_dom = new_zero_array <TBB> (num);
>