| Message ID | eb812563-055b-1054-66b0-ca51963a559f@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 24, 2017 at 11:07 PM, Martin Sebor <msebor@gmail.com> wrote: > On 04/24/2017 01:32 AM, Richard Biener wrote: >> >> 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. > > > Attached is this version of the patch. It also eliminates > the warnings and passes profiledbootstrap/regression test > on x86_64. Ok. Thanks, Richard. > Martin >
PR bootstrap/80486 - spurious -Walloc-size-larger-than and -Wstringop-overflow in dominance.c during profiledbootstrap gcc/ChangeLog: PR bootstrap/80486 * dominance.c (dom_info::m_n_basic_blocks): Change type to unsigned. (new_zero_array): Adjust signature. (dom_info::dom_init): Used unsigned rather that size_t. (dom_info::dom_info): Same. diff --git a/gcc/dominance.c b/gcc/dominance.c index c76e62e..1d4bd54 100644 --- a/gcc/dominance.c +++ b/gcc/dominance.c @@ -125,7 +125,7 @@ private: bitmap m_fake_exit_edge; /* Number of basic blocks in the function being compiled. */ - size_t m_n_basic_blocks; + unsigned m_n_basic_blocks; /* True, if we are computing postdominators (rather than dominators). */ bool m_reverse; @@ -148,7 +148,7 @@ void debug_dominance_tree (cdi_direction, basic_block); `x = new T[num] {};'. */ template<typename T> -inline T *new_zero_array (size_t num) +inline T *new_zero_array (unsigned num) { T *result = new T[num]; memset (result, 0, sizeof (T) * num); @@ -160,14 +160,15 @@ inline T *new_zero_array (size_t num) void dom_info::dom_init (void) { - size_t num = m_n_basic_blocks; + unsigned num = m_n_basic_blocks; + m_dfs_parent = new_zero_array <TBB> (num); m_dom = new_zero_array <TBB> (num); m_path_min = new TBB[num]; m_key = new TBB[num]; m_set_size = new unsigned int[num]; - for (size_t i = 0; i < num; i++) + for (unsigned i = 0; i < num; i++) { m_path_min[i] = m_key[i] = i; m_set_size[i] = 1; @@ -221,13 +222,13 @@ dom_info::dom_info (function *fn, cdi_direction dir) dom_info::dom_info (vec<basic_block> region, cdi_direction dir) { m_n_basic_blocks = region.length (); - unsigned int nm1 = m_n_basic_blocks - 1; + unsigned nm1 = m_n_basic_blocks - 1; dom_init (); /* Determine max basic block index in region. */ int max_index = region[0]->index; - for (size_t i = 1; i <= nm1; i++) + for (unsigned i = 1; i <= nm1; i++) if (region[i]->index > max_index) max_index = region[i]->index; max_index += 1; /* set index on the first bb out of region. */