spotBugs and JDK-8194978: Javac produces dead code for try-with-resource

Enrico Olivelli eolivelli at gmail.com
Mon Jan 7 13:28:17 UTC 2019


Two questions/ideas:
1) Wouldn't it be possible for javac to annotate such code blocks and
write into the class file that that code is "synthetic" ? I really
don't know where to store such flags, but maybe someone could
elaborate more this idea
2) Why isn't javac adding  the "$closeResource" method ?

Enrico

Il giorno ven 9 nov 2018 alle ore 23:01 Alex Buckley
<alex.buckley at oracle.com> ha scritto:
>
> On 11/9/2018 12:31 PM, Vicente Romero wrote:
> > My evaluation on this issue is that javac is generating code
> > according to the spec, see JLS11 14.20.3.1 [1]. There is an explicit
> > check for null on the resource before invoking the close() method.
> > This is the null check javac is generating.
>
> I confirm that the explicit null check on the resource variable is
> required, per the translation in
> https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-14.20.3.1-140
> -- it has been specified that way since JLS7.
>
> > Fix for JDK-8194978 went a bit further trying to eliminate dead code
> > for some cases for which javac could statically determine that the
> > resource would be different from null. In particular if the resource
> > was initialized with a new class expression.
>
> I confirm it's acceptable to not perform the null check in generated
> code if you can prove the resource variable is non-null. Frankly,
> though, there is so much that can occur in a "new class expression" --
> `new Outer(a(b(c)))<>.new <String>Inner(d(e(f)))` -- that I would be
> wary of trying to prove anything.
>
> > It could be argued that we could try to analyze the body and if we
> > find out that a NPE must be thrown in the body of the try, then the
> > null check on the resource would be deemed unnecessary. But then we
> > can get to an implementation that will be out of sync with the spec
> > plus it probably won't cover all cases.
>
> Right, let's not be too clever.
>
> Alex
>
> > Thanks,
> > Vicente
> >
> > [1]
> > https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-14.20.3.1
> >
> > On 11/9/18 10:22 AM, Ismael Juma wrote:
> >> Any comments on this? Many people are disabling spotBugs when
> >> compiling with Java 11 due to this issue:
> >>
> >> https://github.com/spotbugs/spotbugs/issues/756
> >>
> >> Ismael
> >>
> >> On Thu, Sep 13, 2018 at 10:05 PM Ismael Juma <mlists at juma.me.uk
> >> <mailto:mlists at juma.me.uk>> wrote:
> >>
> >>     Hi all,
> >>
> >>     JDK-8194978 introduced some changes to the bytecode generated by
> >>     javac for the try with resource construct. In the following code,
> >>     it seems to generate a null check on a reference after invoking a
> >>     method on it:
> >>
> >>         public static void readFileAsString(String path) throws
> >>     IOException {
> >>             try (FileChannel fc = FileChannel.open(Paths.get(path))) {
> >>                 fc.size();
> >>             }
> >>
> >>         }
> >>
> >>     In line 16 to 22 of the bytecode, it looks like we check for null
> >>     after calling a method on the fc reference:
> >>
> >>           16: aload_1
> >>           17: invokevirtual #6                  // Method
> >>     java/nio/channels/FileChannel.size:()J
> >>           20: pop2
> >>           21: aload_1
> >>           22: ifnull        52
> >>           25: aload_1
> >>           26: invokevirtual #7                  // Method
> >>     java/nio/channels/FileChannel.close:()V
> >>
> >>     Is this intentional? I ask because this pattern triggers a
> >>     spotBugs warning since it often implies a bug in user's code:
> >>
> >>     RCN | Nullcheck of fc at line 10 of value previously dereferenced
> >>     in TryTest.readFileAsString(String, Charset)
> >>
> >>     Note that this works fine in Java versions older than Java 11.
> >>     Since this spotBugs warning is generally useful, it would be handy
> >>     if javac did not trigger it. Alternatively, if there's a good way
> >>     to detect the code that was generated by javac, spotBugs could be
> >>     updated to ignore it. For reference, this was discussed in the
> >>     spotBugs issue tracker:
> >>
> >>     https://github.com/spotbugs/spotbugs/issues/756
> >>
> >>     And method bytecode in full:
> >>
> >>       public static void readFileAsString(java.lang.String) throws
> >>     java.io.IOException;
> >>         Code:
> >>            0: aload_0
> >>            1: iconst_0
> >>            2: anewarray     #2                  // class java/lang/String
> >>            5: invokestatic  #3                  // Method
> >>     java/nio/file/Paths.get:(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;
> >>            8: iconst_0
> >>            9: anewarray     #4                  // class
> >>     java/nio/file/OpenOption
> >>           12: invokestatic  #5                  // Method
> >>     java/nio/channels/FileChannel.open:(Ljava/nio/file/Path;[Ljava/nio/file/OpenOption;)Ljava/nio/channels/FileChannel;
> >>           15: astore_1
> >>           16: aload_1
> >>           17: invokevirtual #6                  // Method
> >>     java/nio/channels/FileChannel.size:()J
> >>           20: pop2
> >>           21: aload_1
> >>           22: ifnull        52
> >>           25: aload_1
> >>           26: invokevirtual #7                  // Method
> >>     java/nio/channels/FileChannel.close:()V
> >>           29: goto          52
> >>           32: astore_2
> >>           33: aload_1
> >>           34: ifnull        50
> >>           37: aload_1
> >>           38: invokevirtual #7                  // Method
> >>     java/nio/channels/FileChannel.close:()V
> >>           41: goto          50
> >>           44: astore_3
> >>           45: aload_2
> >>           46: aload_3
> >>           47: invokevirtual #9                  // Method
> >>     java/lang/Throwable.addSuppressed:(Ljava/lang/Throwable;)V
> >>           50: aload_2
> >>           51: athrow
> >>           52: return
> >>         Exception table:
> >>            from    to  target type
> >>               16    21    32   Class java/lang/Throwable
> >>               37    41    44   Class java/lang/Throwable
> >>
> >>     Thanks,
> >>     Ismael
> >>
> >


More information about the compiler-dev mailing list