add option to singlejar to double-check correct default and static interface method desugaring for Android.

RELNOTES: none

PiperOrigin-RevId: 171891682

9 files changed, 310 insertions, 17 deletions

diff --git a/WORKSPACE b/WORKSPACE
index f9af7af640..a86032d907 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -79,6 +79,12 @@ new_local_repository(
 )
 
 new_local_repository(
+    name = "com_google_protobuf_cc",
+    build_file = "./third_party/protobuf/3.4.0/BUILD",
+    path = "./third_party/protobuf/3.4.0/",
+)
+
+new_local_repository(
     name = "com_google_protobuf_java",
     build_file = "./third_party/protobuf/3.4.0/com_google_protobuf_java.BUILD",
     path = "./third_party/protobuf/3.4.0/",
diff --git a/src/tools/singlejar/BUILD b/src/tools/singlejar/BUILD
index 35b0c50e3b..e986ab06c6 100644
--- a/src/tools/singlejar/BUILD
+++ b/src/tools/singlejar/BUILD
@@ -285,7 +285,10 @@ cc_library(
         ":zip_headers",
     ],
     hdrs = ["combiners.h"],
-    deps = ["//third_party/zlib"],
+    deps = [
+        "//src/main/protobuf:desugar_deps_cc_proto",
+        "//third_party/zlib",
+    ],
 )
 
 cc_library(
diff --git a/src/tools/singlejar/combiners.cc b/src/tools/singlejar/combiners.cc
index 8fd2a0fdfe..aaddd79aaa 100644
--- a/src/tools/singlejar/combiners.cc
+++ b/src/tools/singlejar/combiners.cc
@@ -14,6 +14,7 @@
 
 #include "src/tools/singlejar/combiners.h"
 #include "src/tools/singlejar/diag.h"
+#include "src/main/protobuf/desugar_deps.pb.h"
 
 Combiner::~Combiner() {}
 
@@ -174,3 +175,139 @@ PropertyCombiner::~PropertyCombiner() {}
 bool PropertyCombiner::Merge(const CDH *cdh, const LH *lh) {
   return false;  // This should not be called.
 }
+
+bool Java8DesugarDepsChecker::Merge(const CDH *cdh, const LH *lh) {
+  // Throw away anything previously read, no need to concatenate
+  buffer_.reset(new TransientBytes());
+  if (Z_NO_COMPRESSION == lh->compression_method()) {
+    buffer_->ReadEntryContents(lh);
+  } else if (Z_DEFLATED == lh->compression_method()) {
+    if (!inflater_.get()) {
+      inflater_.reset(new Inflater());
+    }
+    buffer_->DecompressEntryContents(cdh, lh, inflater_.get());
+  } else {
+    errx(2, "META-INF/desugar_deps is neither stored nor deflated");
+  }
+
+  // TODO(kmb): Wrap buffer_ as ZeroCopyInputStream to avoid copying out.
+  // Note we only copy one file at a time, so overhead should be modest.
+  uint32_t checksum;
+  const size_t data_size = buffer_->data_size();
+  uint8_t *buf = reinterpret_cast<uint8_t *>(malloc(data_size));
+  buffer_->CopyOut(reinterpret_cast<uint8_t *>(buf), &checksum);
+  buffer_.reset();  // release buffer eagerly
+
+  bazel::tools::desugar::DesugarDepsInfo deps_info;
+  google::protobuf::io::CodedInputStream content(buf, data_size);
+  if (!deps_info.ParseFromCodedStream(&content)) {
+    errx(2, "META-INF/desugar_deps: unable to parse");
+  }
+  if (!content.ConsumedEntireMessage()) {
+    errx(2, "META-INF/desugar_deps: unexpected trailing content");
+  }
+  free(buf);
+
+  for (const auto &assume_present : deps_info.assume_present()) {
+    // This means we need file named <target>.class in the output.  Remember
+    // the first origin of this requirement for error messages, drop others.
+    needed_deps_.emplace(assume_present.target().binary_name() + ".class",
+                         assume_present.origin().binary_name());
+  }
+
+  for (const auto &missing : deps_info.missing_interface()) {
+    // Remember the first origin of this requirement for error messages, drop
+    // subsequent ones.
+    missing_interfaces_.emplace(missing.target().binary_name(),
+                                missing.origin().binary_name());
+  }
+
+  for (const auto &extends : deps_info.interface_with_supertypes()) {
+    // Remember interface hierarchy the first time we see this interface, drop
+    // subsequent ones for consistency with how singlejar will keep the first
+    // occurrence of the file defining the interface.  We'll lazily derive
+    // whether missing_interfaces_ inherit default methods with this data later.
+    if (extends.extended_interface_size() > 0) {
+      std::vector<std::string> extended;
+      extended.reserve(extends.extended_interface_size());
+      for (const auto &itf : extends.extended_interface()) {
+        extended.push_back(itf.binary_name());
+      }
+      extended_interfaces_.emplace(extends.origin().binary_name(),
+                                   std::move(extended));
+    }
+  }
+
+  for (const auto &companion : deps_info.interface_with_companion()) {
+    // Only remember interfaces that definitely have default methods for now.
+    // For all other interfaces we'll transitively check extended interfaces
+    // in HasDefaultMethods.
+    if (companion.num_default_methods() > 0) {
+      has_default_methods_[companion.origin().binary_name()] = true;
+    }
+  }
+  return true;
+}
+
+void *Java8DesugarDepsChecker::OutputEntry(bool compress) {
+  if (verbose_) {
+    fprintf(stderr, "Needed deps: %lu\n", needed_deps_.size());
+    fprintf(stderr, "Interfaces to check: %lu\n", missing_interfaces_.size());
+    fprintf(stderr, "Sub-interfaces: %lu\n", extended_interfaces_.size());
+    fprintf(stderr, "Interfaces w/ default methods: %lu\n",
+            has_default_methods_.size());
+  }
+  for (auto needed : needed_deps_) {
+    if (verbose_) {
+      fprintf(stderr, "Looking for %s\n", needed.first.c_str());
+    }
+    if (!known_member_(needed.first)) {
+      if (fail_on_error_) {
+        errx(2, "%s referenced by %s but not found.  Is the former defined in "
+                "a neverlink library?",
+                needed.first.c_str(), needed.second.c_str());
+      } else {
+        error_ = true;
+      }
+    }
+  }
+
+  for (auto missing : missing_interfaces_) {
+    if (verbose_) {
+      fprintf(stderr, "Checking %s\n", missing.first.c_str());
+    }
+    if (HasDefaultMethods(missing.first)) {
+      if (fail_on_error_) {
+        errx(2, "%s needed on the classpath for desugaring %s.  Please add the "
+                "missing dependency to the target containing the latter.",
+                missing.first.c_str(), missing.second.c_str());
+      } else {
+        error_ = true;
+      }
+    }
+  }
+
+  // We don't want these files in the output, just check them for consistency
+  return nullptr;
+}
+
+bool Java8DesugarDepsChecker::HasDefaultMethods(
+    const std::string &interface_name) {
+  auto cached = has_default_methods_.find(interface_name);
+  if (cached != has_default_methods_.end()) {
+    return cached->second;
+  }
+
+  // Prime with false in case there's a cycle.  We'll update with the true value
+  // (ignoring the cycle) below.
+  has_default_methods_.emplace(interface_name, false);
+
+  for (const std::string &extended : extended_interfaces_[interface_name]) {
+    if (HasDefaultMethods(extended)) {
+      has_default_methods_[interface_name] = true;
+      return true;
+    }
+  }
+  has_default_methods_[interface_name] = false;
+  return false;
+}
diff --git a/src/tools/singlejar/combiners.h b/src/tools/singlejar/combiners.h
index 98c2df47c5..f7d4cc7be9 100644
--- a/src/tools/singlejar/combiners.h
+++ b/src/tools/singlejar/combiners.h
@@ -15,8 +15,11 @@
 #ifndef SRC_TOOLS_SINGLEJAR_COMBINERS_H_
 #define SRC_TOOLS_SINGLEJAR_COMBINERS_H_ 1
 
+#include <map>
 #include <memory>
 #include <string>
+#include <unordered_map>
+#include <vector>
 
 #include "src/tools/singlejar/transient_bytes.h"
 #include "src/tools/singlejar/zip_headers.h"
@@ -133,4 +136,61 @@ class PropertyCombiner : public Concatenator {
   }
 };
 
+// Combiner that checks META-INF/desugar_deps files (b/65645388) to ensure
+// correct bytecode desugaring, specifically of default and static interface
+// methods, across an entire binary.  Two checks are performed:
+// 1. Make sure that any dependency assumed by the desugaring process is in
+//    fact part of the binary.  This protects against ill-advised uses of
+//    neverlink, where a library is only on the compile-time classpath but not
+//    the runtime classpath.
+// 2. To paper over incomplete classpaths during desugaring (b/65211436), check
+//    that interfaces that couldn't be found don't declare or inherit default
+//    methods.  Desugar emits extra metadata to avoid us having to open up and
+//    parse .class files for this purpose.
+class Java8DesugarDepsChecker : public Combiner {
+ public:
+  Java8DesugarDepsChecker(std::function<bool(const std::string &)> known_member,
+                          bool verbose)
+      : Java8DesugarDepsChecker(std::move(known_member), verbose, true) {}
+  ~Java8DesugarDepsChecker() override {}
+
+  bool Merge(const CDH *cdh, const LH *lh) override;
+
+  void *OutputEntry(bool compress) override;
+
+ private:
+  Java8DesugarDepsChecker(std::function<bool(const std::string &)> known_member,
+                          bool verbose, bool fail_on_error)
+      : known_member_(std::move(known_member)),
+        verbose_(verbose),
+        fail_on_error_(fail_on_error),
+        error_(false) {}
+  /// Computes and caches whether the given interface has default methods.
+  /// \param interface_name interface name as it would appear in bytecode, e.g.,
+  ///        "java/lang/Runnable"
+  bool HasDefaultMethods(const std::string &interface_name);
+
+  const std::function<bool(const std::string &)> known_member_;
+  const bool verbose_;
+  const bool fail_on_error_;  // For testing
+
+  std::unique_ptr<TransientBytes> buffer_;
+  std::unique_ptr<Inflater> inflater_;
+  /// Reverse mapping from needed dependencies to one of the users.
+  std::map<std::string, std::string> needed_deps_;
+  /// Reverse mapping from missing interfaces to one of the classes that missed
+  /// them.
+  std::map<std::string, std::string> missing_interfaces_;
+  std::unordered_map<std::string, std::vector<std::string> >
+      extended_interfaces_;
+  /// Cache of interfaces known to definitely define or inherit default methods
+  /// or definitely not define and not inherit default methods.  Merge()
+  /// populates initial entries and HasDefaultMethods() adds to the cache as
+  /// needed.
+  std::unordered_map<std::string, bool> has_default_methods_;
+  bool error_;
+
+  friend class CombinersTest;
+};
+
 #endif  //  SRC_TOOLS_SINGLEJAR_COMBINERS_H_
diff --git a/src/tools/singlejar/combiners_test.cc b/src/tools/singlejar/combiners_test.cc
index b4358ffa68..effdf265be 100644
--- a/src/tools/singlejar/combiners_test.cc
+++ b/src/tools/singlejar/combiners_test.cc
@@ -19,9 +19,6 @@
 #include "src/tools/singlejar/zlib_interface.h"
 #include "gtest/gtest.h"
 
-namespace {
-using std::string;
-
 static const char kTag1Contents[] = "<tag1>Contents1</tag1>";
 static const char kTag2Contents[] = "<tag2>Contents2</tag2>";
 static const char kCombinedXmlContents[] =
@@ -52,6 +49,67 @@ class CombinersTest : public ::testing::Test {
     }
     return true;
   }
+
+  static void TestJava8DesugarDepsChecker_HasDefaultMethods() {
+    Java8DesugarDepsChecker checker([](const std::string &) { return false; },
+                                    /*verbose=*/false);
+    checker.has_default_methods_["a"] = true;
+    checker.extended_interfaces_["c"] = {"b", "a"};
+
+    // Induce cycle (shouldn't happen but make sure we don't crash)
+    checker.extended_interfaces_["d"] = {"e"};
+    checker.extended_interfaces_["e"] = {"d", "a"};
+
+    EXPECT_TRUE(checker.HasDefaultMethods("a"));
+    EXPECT_FALSE(checker.HasDefaultMethods("b"));
+    EXPECT_TRUE(checker.HasDefaultMethods("c"));  // Transitivly through a
+    EXPECT_TRUE(checker.HasDefaultMethods("d"));  // Transitivly through a
+    EXPECT_FALSE(checker.error_);
+  }
+
+  static void TestJava8DesugarDepsChecker_OutputEntry() {
+    bool checkedA = false;
+    Java8DesugarDepsChecker checker(
+        [&checkedA](const std::string &binary_name) {
+          checkedA = true;
+          return binary_name == "a$$CC.class";
+        },
+        /*verbose=*/false);
+    checker.has_default_methods_["a"] = true;
+    checker.extended_interfaces_["b"] = {"c", "d"};
+    checker.extended_interfaces_["c"] = {"e"};
+    checker.needed_deps_["a$$CC.class"] = "f";
+    checker.missing_interfaces_["b"] = "g";
+    EXPECT_EQ(nullptr, checker.OutputEntry(/*compress=*/true));
+    EXPECT_TRUE(checkedA);
+
+    // Make sure we checked b and its extended interfaces for default methods
+    EXPECT_FALSE(checker.has_default_methods_.at("b"));  // should be cached
+    EXPECT_FALSE(checker.has_default_methods_.at("c"));  // should be cached
+    EXPECT_FALSE(checker.has_default_methods_.at("d"));  // should be cached
+    EXPECT_FALSE(checker.has_default_methods_.at("e"));  // should be cached
+    EXPECT_FALSE(checker.error_);
+  }
+
+  static void TestJava8DesugarDepsChecker_NeededDepMissing() {
+    Java8DesugarDepsChecker checker([](const std::string &) { return false; },
+                                    /*verbose=*/false,
+                                    /*fail_on_error=*/false);
+    checker.needed_deps_["a$$CC.class"] = "b";
+    EXPECT_EQ(nullptr, checker.OutputEntry(/*compress=*/true));
+    EXPECT_TRUE(checker.error_);
+  }
+
+  static void TestJava8DesugarDepsChecker_MissedDefaultMethods() {
+    Java8DesugarDepsChecker checker([](const std::string &) { return true; },
+                                    /*verbose=*/false,
+                                    /*fail_on_error=*/false);
+    checker.has_default_methods_["b"] = true;
+    checker.extended_interfaces_["a"] = {"b", "a"};
+    checker.missing_interfaces_["a"] = "g";
+    EXPECT_EQ(nullptr, checker.OutputEntry(/*compress=*/true));
+    EXPECT_TRUE(checker.error_);
+  }
 };
 
 // Test Concatenator.
@@ -87,7 +145,7 @@ TEST_F(CombinersTest, ConcatenatorSmall) {
   ASSERT_EQ(Z_STREAM_END, inflater.Inflate((buffer), sizeof(buffer)));
   EXPECT_EQ(kPoison, buffer[original_size]);
   EXPECT_EQ(kConcatenatedContents,
-            string(reinterpret_cast<char *>(buffer), original_size));
+            std::string(reinterpret_cast<char *>(buffer), original_size));
   free(reinterpret_cast<void *>(entry));
 
   // And if we just copy instead of compress:
@@ -98,8 +156,9 @@ TEST_F(CombinersTest, ConcatenatorSmall) {
   original_size = entry->uncompressed_file_size();
   compressed_size = entry->compressed_file_size();
   EXPECT_EQ(compressed_size, original_size);
-  EXPECT_EQ(kConcatenatedContents,
-            string(reinterpret_cast<char *>(entry->data()), original_size));
+  EXPECT_EQ(
+      kConcatenatedContents,
+      std::string(reinterpret_cast<char *>(entry->data()), original_size));
   EXPECT_TRUE(entry->file_name_is("concat"));
   EXPECT_EQ(0, entry->extra_fields_length());
   free(reinterpret_cast<void *>(entry));
@@ -175,7 +234,7 @@ TEST_F(CombinersTest, XmlCombiner) {
   ASSERT_EQ(Z_STREAM_END, inflater.Inflate((buffer), sizeof(buffer)));
   EXPECT_EQ(kPoison, buffer[original_size]);
   EXPECT_EQ(kCombinedXmlContents,
-            string(reinterpret_cast<char *>(buffer), original_size));
+            std::string(reinterpret_cast<char *>(buffer), original_size));
   free(reinterpret_cast<void *>(entry));
 
   // And for the combiner that just copies out:
@@ -186,8 +245,9 @@ TEST_F(CombinersTest, XmlCombiner) {
   original_size = entry->uncompressed_file_size();
   compressed_size = entry->compressed_file_size();
   EXPECT_EQ(compressed_size, original_size);
-  EXPECT_EQ(kCombinedXmlContents,
-            string(reinterpret_cast<char *>(entry->data()), original_size));
+  EXPECT_EQ(
+      kCombinedXmlContents,
+      std::string(reinterpret_cast<char *>(entry->data()), original_size));
   EXPECT_TRUE(entry->file_name_is("combined2.xml"));
   EXPECT_EQ(0, entry->extra_fields_length());
   free(reinterpret_cast<void *>(entry));
@@ -200,7 +260,8 @@ TEST_F(CombinersTest, PropertyCombiner) {
       "name_str=value_str\n";
   PropertyCombiner property_combiner("properties");
   property_combiner.AddProperty("name", "value");
-  property_combiner.AddProperty(string("name_str"), string("value_str"));
+  property_combiner.AddProperty(std::string("name_str"),
+                                std::string("value_str"));
 
   // Merge should not be called.
   ASSERT_FALSE(property_combiner.Merge(nullptr, nullptr));
@@ -225,7 +286,7 @@ TEST_F(CombinersTest, PropertyCombiner) {
   ASSERT_EQ(Z_STREAM_END, inflater.Inflate((buffer), sizeof(buffer)));
   EXPECT_EQ(kPoison, buffer[original_size]);
   EXPECT_EQ(kProperties,
-            string(reinterpret_cast<char *>(buffer), original_size));
+            std::string(reinterpret_cast<char *>(buffer), original_size));
   free(reinterpret_cast<void *>(entry));
 
   // Create output, verify Local Header contents.
@@ -236,11 +297,18 @@ TEST_F(CombinersTest, PropertyCombiner) {
   original_size = entry->uncompressed_file_size();
   compressed_size = entry->compressed_file_size();
   EXPECT_EQ(compressed_size, original_size);
-  EXPECT_EQ(kProperties,
-            string(reinterpret_cast<char *>(entry->data()), original_size));
+  EXPECT_EQ(
+      kProperties,
+      std::string(reinterpret_cast<char *>(entry->data()), original_size));
   EXPECT_EQ("properties", entry->file_name_string());
   EXPECT_EQ(0, entry->extra_fields_length());
   free(reinterpret_cast<void *>(entry));
 }
 
-}  // namespace
+TEST_F(CombinersTest, Java8DesugarDepsChecker) {
+  // Tests are instance methods of CombinersTest to avoid gUnit dep in .h file.
+  TestJava8DesugarDepsChecker_HasDefaultMethods();
+  TestJava8DesugarDepsChecker_OutputEntry();
+  TestJava8DesugarDepsChecker_NeededDepMissing();
+  TestJava8DesugarDepsChecker_MissedDefaultMethods();
+}
diff --git a/src/tools/singlejar/options.cc b/src/tools/singlejar/options.cc
index 11484c4124..86c35fe142 100644
--- a/src/tools/singlejar/options.cc
+++ b/src/tools/singlejar/options.cc
@@ -38,7 +38,8 @@ void Options::ParseCommandLine(int argc, const char * const argv[]) {
         tokens.MatchAndSet("--verbose", &verbose) ||
         tokens.MatchAndSet("--warn_duplicate_resources",
                            &warn_duplicate_resources) ||
-        tokens.MatchAndSet("--nocompress_suffixes", &nocompress_suffixes)) {
+        tokens.MatchAndSet("--nocompress_suffixes", &nocompress_suffixes) ||
+        tokens.MatchAndSet("--check_desugar_deps", &check_desugar_deps)) {
       continue;
     } else if (tokens.MatchAndSet("--build_info_file", &optarg)) {
       build_info_files.push_back(optarg);
diff --git a/src/tools/singlejar/options.h b/src/tools/singlejar/options.h
index cc01cd8547..c228fc6892 100644
--- a/src/tools/singlejar/options.h
+++ b/src/tools/singlejar/options.h
@@ -29,7 +29,8 @@ class Options {
         no_duplicate_classes(false),
         preserve_compression(false),
         verbose(false),
-        warn_duplicate_resources(false) {}
+        warn_duplicate_resources(false),
+        check_desugar_deps(false) {}
 
   // Parses command line arguments into the fields of this instance.
   void ParseCommandLine(int argc, const char * const argv[]);
@@ -53,6 +54,7 @@ class Options {
   bool preserve_compression;
   bool verbose;
   bool warn_duplicate_resources;
+  bool check_desugar_deps;
 };
 
 #endif  // THIRD_PARTY_BAZEL_SRC_TOOLS_SINGLEJAR_OPTIONS_H_
diff --git a/src/tools/singlejar/options_test.cc b/src/tools/singlejar/options_test.cc
index 14e1d4e5c3..a16c134161 100644
--- a/src/tools/singlejar/options_test.cc
+++ b/src/tools/singlejar/options_test.cc
@@ -36,6 +36,7 @@ TEST(OptionsTest, Flags1) {
   EXPECT_FALSE(options.preserve_compression);
   EXPECT_FALSE(options.verbose);
   EXPECT_FALSE(options.warn_duplicate_resources);
+  EXPECT_FALSE(options.check_desugar_deps);
   EXPECT_EQ("output_jar", options.output_jar);
 }
 
@@ -43,6 +44,7 @@ TEST(OptionsTest, Flags2) {
   const char *args[] = {"--dont_change_compression",
                         "--verbose",
                         "--warn_duplicate_resources",
+                        "--check_desugar_deps",
                         "--output", "output_jar"};
   Options options;
   options.ParseCommandLine(arraysize(args), args);
@@ -54,6 +56,7 @@ TEST(OptionsTest, Flags2) {
   ASSERT_TRUE(options.preserve_compression);
   ASSERT_TRUE(options.verbose);
   ASSERT_TRUE(options.warn_duplicate_resources);
+  ASSERT_TRUE(options.check_desugar_deps);
 }
 
 TEST(OptionsTest, SingleOptargs) {
diff --git a/src/tools/singlejar/output_jar.cc b/src/tools/singlejar/output_jar.cc
index 7fd97abb6e..3674d27aee 100644
--- a/src/tools/singlejar/output_jar.cc
+++ b/src/tools/singlejar/output_jar.cc
@@ -89,6 +89,17 @@ int OutputJar::Doit(Options *options) {
                            EntryInfo{&build_properties_});
   }
 
+  // Process or drop Java 8 desugaring metadata, see b/65645388.  We don't want
+  // or need these files afterwards so make sure we drop them either way.
+  Combiner *desugar_checker = options_->check_desugar_deps
+                                  ? new Java8DesugarDepsChecker(
+                                        [this](const std::string &filename) {
+                                          return !NewEntry(filename);
+                                        },
+                                        options_->verbose)
+                                  : (Combiner *)new NullCombiner();
+  ExtraCombiner("META-INF/desugar_deps", desugar_checker);
+
   build_properties_.AddProperty("build.target", options_->output_jar.c_str());
   if (options_->verbose) {
     fprintf(stderr, "combined_file_name=%s\n", options_->output_jar.c_str());
@@ -352,6 +363,8 @@ bool OutputJar::AddJar(int jar_path_index) {
       auto &entry_info = got.first->second;
       // Handle special entries (the ones that have a combiner).
       if (entry_info.combiner_ != nullptr) {
+        // TODO(kmb,asmundak): Should be checking Merge() return value but fails
+        // for build-data.properties when merging deploy jars into deploy jars.
         entry_info.combiner_->Merge(jar_entry, lh);
         continue;
       }