Re: [Valgrind-developers] Current upstream Valgrind gets SIGSEGV running java

[Florian K. <fl...@ei...>]

On 08/14/2013 10:31 AM, Julian Seward wrote:
> On 08/13/2013 10:03 PM, Florian Krohm wrote:
> 
>> I bet that e is IRExprP__BBPTR  which is ((IRExpr*)17)  which is  0x11,
>> the faulting address. Looks as if deepCopyIRExprVec does not handle the
>> special expressions introduced recently.
> 
> I bet you are exactly right about that.  Also shallowCopyIRExprVec has the
> same problem.

Not really, as that function just copies the pointers of the vector
elements. Nor does deepCopyIRExprVec, as I said earlier.
The function that has the problem is deepCopyIRExpr. It needs

   if (UNLIKELY(is_IRExprP__VECRET_or_BBPTR(e))) return e;

at the beginning.
Certain utility functions for IRExpr need adjustment, too. ppIRExpr
comes to mind and typeOfIRExpr.

> I just had the unpleasant realisation that adding IRExprP__* has had an
> undesired side effect: it is possible to put these constants at the leaf
> of an expression tree, and get something which is completely meaningless,
> eg IRExpr_Binop(Iop_Add64, IRExpr_RdTmp(t), IRExprP__BBPTR).  Urr.

True. And you can do similar nonsense with Iex_Binder expressions, btw.

It might be better actually, to introduce new IRExpr node kinds for
VECRET and BBPTR. Doing that makes it easier to find all the places that
need adjustment. E.g. GCC has options to find unhandled enumerators in
switch statements...  And we would never have to worry about e->tag
possibly segfaulting as long as e != 0.

> In hindsight it would have been cleaner to have left IRExpr unchanged but
> instead changed the type of the IRExprVec components to be a type which is
> basically "all IRExpr*s, plus IRExprP__BBPTR, plus IRExpr__VECRET".  Then
> it wouldn't be possible to construct nonsense trees using the new constants.

Yes.

   Florian