This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] tree-ssa-dom.c: Use VEC(vrp_element_p,heap) instead ofVARRAY.


Hi Jeff,

> This doesn't look right to me.  If you read the comments earlier in the
> code it states that two tests must be performed.  First you have to
> verify the existance of the varray (or VEC) then you have to look at
> the number of active entries.
> 
> Your change will cause a segfault in the first case as *vrp_records
> would be dereferencing a null pointer.
> 
> So my question to you would be is there some reason why it's no longer
> possible for vrp_records to be null?

*vrp_records may be NULL, but VEC_length (vrp_element_p, NULL)
evaluates to 0 because VEC_length is implemented as

static inline unsigned VEC_OP (T,base,length) (const VEC(T,base) *vec_)	  \
{									  \
  return vec_ ? vec_->num : 0;						  \
}									  \

Note that it tests vec_ against 0 first before dereferencing vec_.
The VEC API consistently treats the NULL pointer as an empty vector.
For example, you can also do

  VEC(tree,heap) *vec = NULL;
  VEC_safe_push (tree, heap, vec, integer_one_node);

because VEC_safe_push allocates a vector first if it is given a NULL
pointer.

> No, it means the vector does not exist.  There is a difference.

The VEC API absorbs the difference.  You might want to search for '?'.
There are a lot of checks of the form "vec_ ? ...".

> >  VEC_safe_push reserves memory if a pointer holding a vector is
> > null or we have not allocated any memory for the vector at all, so we
> > don't need to check *vrp_records_p.  It's included in VEC_safe_push.
> > 
> > Since we are now explicitly managing the memory for these hash
> > elements, I created a new function vec_free, which is responsible for
> > vrp_hash_elt as well as the vector in it.
> You may be trying to explain my objection above -- but I can't really
> parse what you're trying to say.  Can you try to reword this to be
> clearer?

Again, if we have

  VEC(tree,heap) *vec = NULL;

the VEC API considers vec as an empty vector, not a nonexistent
vector.  As such, you can ask the length like so

  VEC(tree,heap) *vec = NULL;
  printf ("length == %d\n, VEC_length (tree, vec));

This program will not segfault but print out 0.

Kazu Hirata


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]