#1350 ValueNumberAnalysis: available loads are not removed on exception

[Tagir Valeev]

Currently in ValueNumberAnalysis when invoke instruction is encountered, loads of fields which might be changed by this call are removed so when they are loaded again, they receive a new ValueNumber. The problem is that they are not removed on caught exception edge. It's possible that the invoked method throw an exception after changing the field or don't throw it at all under given circumstances. Going through exception edge preserves the available load which may result in various false positives. The following test case illustrates the problem:

public class VnaExceptionProblem {
    Integer field;

    public String get() {
        if(field == null) {
            try {
                initField();
            } catch (IllegalStateException e) {
                // ignore
            }
        }
        return field.toString(); // NP_NULL_ON_SOME_PATH_EXCEPTION here
    }

    private void initField() {
        if(field != null)
            throw new IllegalStateException();
        field = 0;
    }
}

Here false-positive NP_NULL_ON_SOME_PATH_EXCEPTION occurs, though field is always initialized regardless of exception.

Real life example of this problem is UrlResource, line#97:

private Boolean exists;

public InputStream openStream() throws IOException
{
    if (exists == null) 
    {
        try
        {
            inputStream = url.openStream();
            exists = true;
        }
        catch (IOException e)
        {
            exists = false;
            throw e;
        }
    }
    return inputStream;
}

public boolean exists()
{
    if (exists == null)
    {
        try
        {
            openStream();
        }
        catch (IOException e)
        {
            // openStream() will always set the exists field.
        }
    }
    return exists; // NP_NULL_ON_SOME_PATH_EXCEPTION fp is here
}

Also several false positives were discovered for my new pattern UC_USELESS_CONDITION due to this problem.

I patched ValueNumberAnalysis, so it kills available loads for invoke instructions inside the exception thrower basic block which precedes the normal invokation block. Will commit after final testing.

[Tagir Valeev]

Fixed:
https://code.google.com/p/findbugs/source/detail?r=1c30a7ffca206fd8f2054103e154c78a27178673