[Soot-list] ThrowableSet comparison with "!="

[John Jorgensen]

>>>>> "eric.bodden" == Eric Bodden <eric.bodden at ec-spride.de> writes:

    eric.bodden> Thanks for reporting this Thomas.  Indeed this
    eric.bodden> looks bogus. I actually wonder why this check
    eric.bodden> never failed before. I have now changed to using
    eric.bodden> equals(..).

I can tell you the intent that was supposed to keep this from
failing before, but won't pretend that the intent might not be
flawed or misimplemented.

The implementation of ThrowableSet is supposed to ensure that
each ThrowableSet object is an immutable constant, and that for
any given set of exceptions, there is only a single ThrowableSet
object that represents that set. That's why the constructor is
private (or at least it was back in 2003); you're only supposed
to be able to create new ThrowableSets by add()ing to an
existing one (starting from ThrowableSet.Manager.v().EMPTY), and
ThrowableSet.Manager is supposed to keep a record of the
ThrowableSets that exist so far, so that add() can provide a
reference to a single copy of already existing sets, rather than
creating new ones.

There's more about this in Section 3.3.2 of
http://www.sable.mcgill.ca/publications/techreports/sable-tr-2003-3.pdf,
especially the paragraph which begins "Second, efficiency."

So if two ThrowableSets are "equals()" without also being "==",
then something in the original design is broken. Substituting
"! equals()" for "!=" may just be camouflaging the underlying flaw,
rather than fixing it.

On 4 September 2012 16:42, Thomas Pilot <t_pilot at rbg.informatik.tu-darmstadt.de> wrote:

    t_pilot> 
    t_pilot> on the current master branch of soot, at the end of
    t_pilot> soot.toolkits.graph.ExceptionalUnitGraph.buildExceptionDests(ThrowAnalysis)
    t_pilot> two ThrowableSets are compared with "!=" multiple times.
    t_pilot> 
    t_pilot> At that point, for some reason, I managed to have an empty
    t_pilot> thrownSet which is not
    t_pilot> ThrowableSet.Manager.v().EMPTY. This leads to the "!="
    t_pilot> comparison being false and the IllegalStateException in
    t_pilot> line 367 being thrown, although the ThrowableSet's
    t_pilot> contents are equal.
    t_pilot> 
    t_pilot> I suggest implementing the equals() method in ThrowableSet
    t_pilot> and using it for comparison at least there.
    t_pilot> 

The intriguing question is how you managed to get an empty
ThrowableSet that wasn't a reference to EMPTY, since that might
reveal what's wrong with ThrowableSet.

Have you called the private constructor, or maybe implemented an
operation to remove exceptions from an existing ThrowableSet? (I
was never able to come up with a workable, general implementation
of remove().)

Unfortunately, my knowledge of soot is very rusty, and I'm
unlikely to have a chance to look at your test case until
October, but if you will put up with tardiness,  I'd be curious
to know how you provoke the failure.

It occurs to me that back when I contributed to soot, soot was
entirely single-threaded. Maybe it is multi-threaded now,
exposing flaws in ThrowableSet's memoization.

-- 
John Jorgensen    <jorgnsn at cs.uregina.ca>    306.337.2344