Re: [PATCH] ada-lang, possible_user_operator_p, null pointer

[Joel Brobecker <brobecker@adacore.com>]

> > 2007-08-11  Michael Snyder  <msnyder@access-company.com>
> > 
> > 	* ada-lang.c (possible_user_operator_p): Guard against NULL.
> 
> This is another case where I'd prefer to have the check for a NULL
> type moved to the top of the function. But I'm having a tough time
> determining how this might actually happen. So I'd like to sit on it
> for a little longer before we proceed.

I'm glad I sat on it :). I overlooked the fact that part of this function
is not limited to binary operators. I also handles unary operators, so
it would make sense that type1 might actually be NULL.

However, the expression evaluation should always make sure that
we have two operands before trying to evaluate it, so this should
never happen, right?

Unfortunately, digging deeper, I found elsewhere something that looks
suspicious to me, and it actually allowed me to build an example that
trigger a dereference of a NULL type1!!!

I am currently discussing all this with Paul Hilfinger, who wrote
most of this code, and I've suggested some other ways of rewriting
the code that I think will make it more consistent: In short, define
a separate function that matches arrays or pointer to arrays, with
a guard against null pointers, the same as numeric_type_p for instance,
and simplify the condition by using that function. We'd end up with
something like this:

    case BINOP_CONCAT:
      return !(array_type_p (type0) && array_type_p (type1));

We also need to address the bug that this check guards against!

In the meantime, I think it's fine to check in your patch.
We'll probably end up rewriting this part of the code, but
that won't actually change the actual logic, so....

> > Index: ada-lang.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/ada-lang.c,v
> > retrieving revision 1.100
> > diff -p -r1.100 ada-lang.c
> > *** ada-lang.c	6 Aug 2007 20:07:44 -0000	1.100
> > --- ada-lang.c	11 Aug 2007 20:39:20 -0000
> > *************** possible_user_operator_p (enum exp_opcod
> > *** 3536,3542 ****
> >           ((TYPE_CODE (type0) != TYPE_CODE_ARRAY
> >             && (TYPE_CODE (type0) != TYPE_CODE_PTR
> >                 || TYPE_CODE (TYPE_TARGET_TYPE (type0)) != TYPE_CODE_ARRAY))
> > !          || (TYPE_CODE (type1) != TYPE_CODE_ARRAY
> >                && (TYPE_CODE (type1) != TYPE_CODE_PTR
> >                    || (TYPE_CODE (TYPE_TARGET_TYPE (type1)) 
> >   		     != TYPE_CODE_ARRAY))));
> > --- 3536,3542 ----
> >           ((TYPE_CODE (type0) != TYPE_CODE_ARRAY
> >             && (TYPE_CODE (type0) != TYPE_CODE_PTR
> >                 || TYPE_CODE (TYPE_TARGET_TYPE (type0)) != TYPE_CODE_ARRAY))
> > !          || (type1 != NULL && TYPE_CODE (type1) != TYPE_CODE_ARRAY
> >                && (TYPE_CODE (type1) != TYPE_CODE_PTR
> >                    || (TYPE_CODE (TYPE_TARGET_TYPE (type1)) 
> >   		     != TYPE_CODE_ARRAY))));

-- 
Joel