[match-and-simplify] set simplify::capture_max to 0 if pattern contains no captures

[match-and-simplify] set simplify::capture_max to 0 if pattern contains no captures

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

Caused segfault for pattern containing no captures at:
info.safe_grow_cleared(capture_max + 1); in capture_info::capture_info

artificial test-case:
(define_predicates integer_zerop)
(simplify
  (bit_not integer_zerop)
  { build_zero_cst (type); })

* genmatch.c
  (simplify::simplify): Set simplify::capture_max to 0 if pattern
contains no captures.

Thanks,
Prathamesh

[-- Attachment #2: capture_max.patch --]
[-- Type: text/x-patch, Size: 646 bytes --]

Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c	(revision 218664)
+++ gcc/genmatch.c	(working copy)
@@ -576,7 +576,7 @@
       : match (match_), match_location (match_location_),
       result (result_), result_location (result_location_),
       ifexpr_vec (ifexpr_vec_), for_vec (for_vec_),
-      capture_ids (capture_ids_), capture_max (capture_ids_->elements () - 1) {}
+      capture_ids (capture_ids_), capture_max (capture_ids_->elements () ? capture_ids_->elements () - 1 : 0) {}
 
   /* The expression that is matched against the GENERIC or GIMPLE IL.  */
   operand *match;

Re: [match-and-simplify] set simplify::capture_max to 0 if pattern contains no captures

[Richard Biener]

On Mon, 15 Dec 2014, Prathamesh Kulkarni wrote:

> Caused segfault for pattern containing no captures at:
> info.safe_grow_cleared(capture_max + 1); in capture_info::capture_info
> 
> artificial test-case:
> (define_predicates integer_zerop)
> (simplify
>   (bit_not integer_zerop)
>   { build_zero_cst (type); })
> 
> * genmatch.c
>   (simplify::simplify): Set simplify::capture_max to 0 if pattern
> contains no captures.

Hmm, I think I've seen this before and I think that vec should be
more robust.  In fact

  vec<int> v = vNULL;
  v.quick_grow (0);

will segfault as called via

1572    vec<T, va_heap, vl_ptr>::safe_grow (unsigned len MEM_STAT_DECL)
1573    {       
1574      unsigned oldlen = length ();
1575      gcc_checking_assert (oldlen <= len);
1576      reserve_exact (len - oldlen PASS_MEM_STAT);
1577      m_vec->quick_grow (len);

in fact it is documented here:

/* Allocator for heap memory.  Ensure there are at least RESERVE free
   slots in V.  If EXACT is true, grow exactly, else grow
   exponentially.  As a special case, if the vector had not been
   allocated and and RESERVE is 0, no vector will be created.  */

template<typename T>
inline void
va_heap::reserve (vec<T, va_heap, vl_embed> *&v, unsigned reserve, bool 
exact
                  MEM_STAT_DECL)
{


if we don't want to do that we should simply guard the safe_grow_cleared
call, not artificially increase the number of elements.

But I think the following is the best and also matches vec::truncate.
 
Boostrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2014-12-15  Richard Biener  <rguenther@suse.de>

	* vec.h (vec::safe_grow): Guard against a grow to zero size.

Index: gcc/vec.h
===================================================================
--- gcc/vec.h	(revision 218746)
+++ gcc/vec.h	(working copy)
@@ -1574,7 +1574,10 @@ vec<T, va_heap, vl_ptr>::safe_grow (unsi
   unsigned oldlen = length ();
   gcc_checking_assert (oldlen <= len);
   reserve_exact (len - oldlen PASS_MEM_STAT);
-  m_vec->quick_grow (len);
+  if (m_vec)
+    m_vec->quick_grow (len);
+  else
+    gcc_checking_assert (len == 0);
 }