From bc8fe4fb9704b1a9a0752e24d54d8bc19e5dc5ea Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 30 Jan 2018 08:25:56 -0800 Subject: [PATCH] [java] don't warn on unclosed subclasses of non-resource types Summary: We already knew not to warn when non-resource `Closeable`'s like `ByteArrayOutputStream` weren't closed, but we still warned on their subtypes. This diff fixes that problem by checking subclasses in the frontend. This also removes the need for Java source code models of non-resource types, so I removed them. Reviewed By: jeremydubreil Differential Revision: D6843413 fbshipit-source-id: 60fe7fb --- .../src/java/io/ByteArrayInputStream.java | 20 -------------- .../src/java/io/ByteArrayOutputStream.java | 26 ------------------- .../models/java/src/java/io/StringReader.java | 18 ------------- .../models/java/src/java/io/StringWriter.java | 19 -------------- infer/src/java/jTrans.ml | 15 ++++++++++- .../infer/CloseableAsResourceExample.java | 15 +++++++++++ 6 files changed, 29 insertions(+), 84 deletions(-) delete mode 100644 infer/models/java/src/java/io/ByteArrayInputStream.java delete mode 100644 infer/models/java/src/java/io/ByteArrayOutputStream.java delete mode 100644 infer/models/java/src/java/io/StringReader.java delete mode 100644 infer/models/java/src/java/io/StringWriter.java diff --git a/infer/models/java/src/java/io/ByteArrayInputStream.java b/infer/models/java/src/java/io/ByteArrayInputStream.java deleted file mode 100644 index 40c37d9fb..000000000 --- a/infer/models/java/src/java/io/ByteArrayInputStream.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package java.io; - -public class ByteArrayInputStream extends InputStream implements Closeable { - - public ByteArrayInputStream(byte[] buf) {} - - public ByteArrayInputStream(byte[] buf, int offset, int length) {} - - public void close() {} - -} diff --git a/infer/models/java/src/java/io/ByteArrayOutputStream.java b/infer/models/java/src/java/io/ByteArrayOutputStream.java deleted file mode 100644 index 0bfc6e2d1..000000000 --- a/infer/models/java/src/java/io/ByteArrayOutputStream.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package java.io; - -public class ByteArrayOutputStream extends OutputStream implements Closeable { - - public ByteArrayOutputStream() {} - - public ByteArrayOutputStream(int size) {} - - @Override - public void write(int b) {} - - @Override - public void write(byte[] b, int off, int len) {} - - public void close() {} - -} diff --git a/infer/models/java/src/java/io/StringReader.java b/infer/models/java/src/java/io/StringReader.java deleted file mode 100644 index 43b66dff8..000000000 --- a/infer/models/java/src/java/io/StringReader.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package java.io; - -public class StringReader implements Closeable { - - public StringReader(String s) {} - - public void close() {} - -} diff --git a/infer/models/java/src/java/io/StringWriter.java b/infer/models/java/src/java/io/StringWriter.java deleted file mode 100644 index 5858e1e23..000000000 --- a/infer/models/java/src/java/io/StringWriter.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright (c) 2015 - present Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package java.io; - -public class StringWriter implements Closeable{ - - public StringWriter() {} - public StringWriter(int initialSize) {} - - public void close() {} - -} diff --git a/infer/src/java/jTrans.ml b/infer/src/java/jTrans.ml index 72845469f..ae52d5609 100644 --- a/infer/src/java/jTrans.ml +++ b/infer/src/java/jTrans.ml @@ -656,6 +656,18 @@ let method_invocation (context: JContext.t) loc pc var_opt cn ms sil_obj_opt exp | _ -> false in + (* return true for classes that are a subclass of Closeable, but don't actually represent a + resource *) + let is_non_resource_closeable typename _ = + match Typ.Name.name typename with + | "java.io.ByteArrayInputStream" + | "java.io.ByteArrayOutputStream" + | "java.io.StringReader" + | "java.io.StringWriter" -> + true + | _ -> + false + in let instrs = match call_args with (* modeling a class bypasses the treatment of Closeable *) @@ -665,7 +677,8 @@ let method_invocation (context: JContext.t) loc pc var_opt cn ms sil_obj_opt exp | ((_, {Typ.desc= Typ.Tptr ({desc= Tstruct typename}, _)}) as exp) :: _ (* add a file attribute when calling the constructor of a subtype of Closeable *) when Typ.Procname.is_constructor callee_procname - && AndroidFramework.is_autocloseable tenv typename -> + && AndroidFramework.is_autocloseable tenv typename + && not (PatternMatch.supertype_exists tenv is_non_resource_closeable typename) -> let set_file_attr = let set_builtin = Exp.Const (Const.Cfun BuiltinDecl.__set_file_attribute) in Sil.Call (None, set_builtin, [exp], loc, CallFlags.default) diff --git a/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java b/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java index 5d17e32e1..04f886555 100644 --- a/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java +++ b/infer/tests/codetoanalyze/java/infer/CloseableAsResourceExample.java @@ -66,6 +66,16 @@ class ResourceWithException implements Closeable { } } +class ByteArrayOutputStreamWrapper extends ByteArrayOutputStream { +} + +class ByteArrayInputStreamWrapper extends ByteArrayInputStream { + + public ByteArrayInputStreamWrapper(byte[] arr) { + super(arr); + } +} + public class CloseableAsResourceExample { native static boolean star(); @@ -115,6 +125,11 @@ public class CloseableAsResourceExample { ByteArrayOutputStream stream = new ByteArrayOutputStream(42); } + void noCloseByteArrayWrappersOk(byte[] array) { + ByteArrayOutputStreamWrapper stream1 = new ByteArrayOutputStreamWrapper(); + ByteArrayInputStreamWrapper stream2 = new ByteArrayInputStreamWrapper(array); + } + void noNeedToCloseByteArrayInputStream(byte[] array) { ByteArrayInputStream stream = new ByteArrayInputStream(array); }