Fix i686-linux bootstrap (was Re: [PATCH 1/2] Add alloc_size for libiberty memory allocation functions.)

Message ID 20190610225141.GH19695@tucnak
State New
Headers show
Series
  • Fix i686-linux bootstrap (was Re: [PATCH 1/2] Add alloc_size for libiberty memory allocation functions.)
Related show

Commit Message

Jakub Jelinek June 10, 2019, 10:51 p.m.
On Fri, Jun 07, 2019 at 09:07:39AM +0200, Martin Liška wrote:
> 2019-06-07  Martin Liska  <mliska@suse.cz>

> 

> 	* ansidecl.h:

> 	(ATTRIBUTE_RESULT_SIZE_1): Define new macro.

> 	(ATTRIBUTE_RESULT_SIZE_2): Likewise.

> 	(ATTRIBUTE_RESULT_SIZE_1_2): Likewise.

> 	* libiberty.h (xmalloc): Add RESULT_SIZE attribute.

> 	(xrealloc): Likewise.

> 	(xcalloc): Likewise.


This change broke i686-linux bootstrap, we get:
error: argument 1 value '4294967292' exceeds maximum object size 2147483647 [-Werror=alloc-size-larger-than=]
error, false positive warning.
Because blocks.length () is
  unsigned length (void) const
  { return m_vec ? m_vec->length () : 0; }
we decide to jump-thread that and think the 0 value is somehow special and
the warning decides to warn, but the fact is that we don't really know
anything about it (and for such case don't warn).

The caller actually guarantees that blocks.length () is > 0 as it pushes one
basic block to the vector unconditionally, so we can work around this
warning by adding an assertion.  Not entirely happy about that, on the other
side nobody uses trans-mem and so the extra comparison and cold check
isn't that big a deal.

Bootstrapped/regtested on i686-linux, ok for trunk?

2019-06-10  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/90819
	* trans-mem.c (tm_memopt_compute_available): Add assertion
	that blocks is not empty.  Formatting fix.



	Jakub

Comments

Richard Biener June 11, 2019, 8:17 a.m. | #1
On Tue, 11 Jun 2019, Jakub Jelinek wrote:

> On Fri, Jun 07, 2019 at 09:07:39AM +0200, Martin Liška wrote:

> > 2019-06-07  Martin Liska  <mliska@suse.cz>

> > 

> > 	* ansidecl.h:

> > 	(ATTRIBUTE_RESULT_SIZE_1): Define new macro.

> > 	(ATTRIBUTE_RESULT_SIZE_2): Likewise.

> > 	(ATTRIBUTE_RESULT_SIZE_1_2): Likewise.

> > 	* libiberty.h (xmalloc): Add RESULT_SIZE attribute.

> > 	(xrealloc): Likewise.

> > 	(xcalloc): Likewise.

> 

> This change broke i686-linux bootstrap, we get:

> error: argument 1 value '4294967292' exceeds maximum object size 2147483647 [-Werror=alloc-size-larger-than=]

> error, false positive warning.

> Because blocks.length () is

>   unsigned length (void) const

>   { return m_vec ? m_vec->length () : 0; }

> we decide to jump-thread that and think the 0 value is somehow special and

> the warning decides to warn, but the fact is that we don't really know

> anything about it (and for such case don't warn).

> 

> The caller actually guarantees that blocks.length () is > 0 as it pushes one

> basic block to the vector unconditionally, so we can work around this

> warning by adding an assertion.  Not entirely happy about that, on the other

> side nobody uses trans-mem and so the extra comparison and cold check

> isn't that big a deal.

> 

> Bootstrapped/regtested on i686-linux, ok for trunk?


OK.

Richard.

> 2019-06-10  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR bootstrap/90819

> 	* trans-mem.c (tm_memopt_compute_available): Add assertion

> 	that blocks is not empty.  Formatting fix.

> 

> --- gcc/trans-mem.c.jj	2019-05-20 21:59:20.081717030 +0200

> +++ gcc/trans-mem.c	2019-06-10 23:34:31.725056038 +0200

> @@ -3708,9 +3708,9 @@ tm_memopt_compute_available (struct tm_r

>    /* Allocate a worklist array/queue.  Entries are only added to the

>       list if they were not already on the list.  So the size is

>       bounded by the number of basic blocks in the region.  */

> +  gcc_assert (!blocks.is_empty ());

>    qlen = blocks.length () - 1;

> -  qin = qout = worklist =

> -    XNEWVEC (basic_block, qlen);

> +  qin = qout = worklist = XNEWVEC (basic_block, qlen);

>  

>    /* Put every block in the region on the worklist.  */

>    for (i = 0; blocks.iterate (i, &bb); ++i)

> 

> 

> 	Jakub

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Patch

--- gcc/trans-mem.c.jj	2019-05-20 21:59:20.081717030 +0200
+++ gcc/trans-mem.c	2019-06-10 23:34:31.725056038 +0200
@@ -3708,9 +3708,9 @@  tm_memopt_compute_available (struct tm_r
   /* Allocate a worklist array/queue.  Entries are only added to the
      list if they were not already on the list.  So the size is
      bounded by the number of basic blocks in the region.  */
+  gcc_assert (!blocks.is_empty ());
   qlen = blocks.length () - 1;
-  qin = qout = worklist =
-    XNEWVEC (basic_block, qlen);
+  qin = qout = worklist = XNEWVEC (basic_block, qlen);
 
   /* Put every block in the region on the worklist.  */
   for (i = 0; blocks.iterate (i, &bb); ++i)