commit 94383b4038970838ded1f0a482f16d8b612a4701
author Stefano Cianciulli <scianciulli@google.com> Fri Sep 02 10:54:44 2022 +0000
committer Stefano Cianciulli <scianciulli@google.com> Mon Sep 05 08:20:55 2022 +0000
tree 3ff85ad8a91458308bbb42ea9d5f5e0e79566564
parent 7023bf8227b3f46f4c3cc98e18889f54a584cd16

Fix bugprone-unused-return-value clang-tidy issues

This CL fixed the last remaining bugprone-unused-return-value
issue reported by clang-tidy: instead of calling the method
Runtime::AppendToBootClassPath (which does not get ownership
of the pointers to the DEX files), a new method called
Runtime::AddExtraBootDexFiles has been created, which works
similarly to the previous one, but calls internally the method
ClassLinker::AddExtraBootDexFiles, where pointers are
transferred ownership, therefore eliminating the for loop
with the calls to std::unique_ptr::release, which was most
likely causing a memory leak.

Bug: 213953102
Test: art/test/testrunner/run_build_test_target.py -j80 art-test | grep "936-search-onload"
Change-Id: I4b4237f3fb4fbe399e8cd3154e0465b9cfb0e05c

build/Android.bp

diff --git a/build/Android.bp b/build/Android.bp
index c0323b4..5f9c1fa 100644
--- a/build/Android.bp
+++ b/build/Android.bp
@@ -33,6 +33,7 @@
     "bugprone-lambda-function-name",
     "bugprone-macro-parentheses",
     "bugprone-unused-raii", // Protect scoped things like MutexLock.
+    "bugprone-unused-return-value",
     "bugprone-virtual-near-miss",
     "misc-unused-using-decls",
     "modernize-use-bool-literals",
@@ -49,7 +50,6 @@
     // Many files have these warnings. Move them to art_clang_tidy_errors
     // when all files are free of these warnings.
     "android-cloexec-dup",
-    "bugprone-unused-return-value",
     "modernize-use-using", // TODO: move to art_clang_tidy_errors after b/236243696 is done
 ]
 

openjdkjvmti/ti_search.cc

diff --git a/openjdkjvmti/ti_search.cc b/openjdkjvmti/ti_search.cc
index 3ddf4ab..5c1677f 100644
--- a/openjdkjvmti/ti_search.cc
+++ b/openjdkjvmti/ti_search.cc
@@ -247,12 +247,8 @@
     return ERR(ILLEGAL_ARGUMENT);
   }
 
-  current->AppendToBootClassPath(segment, segment, dex_files);
-  for (std::unique_ptr<const art::DexFile>& dex_file : dex_files) {
-    dex_file.release();
-  }
-
-  return ERR(NONE);
+  current->AddExtraBootDexFiles(segment, segment, std::move(dex_files));
+  return OK;
 }
 
 jvmtiError SearchUtil::AddToDexClassLoaderInMemory(jvmtiEnv* jvmti_env,

runtime/runtime.cc

diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 642e0bc..390fcf0 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -3491,4 +3491,15 @@
   }
 }
 
+void Runtime::AddExtraBootDexFiles(const std::string& filename,
+                                   const std::string& location,
+                                   std::vector<std::unique_ptr<const art::DexFile>>&& dex_files) {
+  boot_class_path_.push_back(filename);
+  if (!boot_class_path_locations_.empty()) {
+    boot_class_path_locations_.push_back(location);
+  }
+  ScopedObjectAccess soa(Thread::Current());
+  GetClassLinker()->AddExtraBootDexFiles(Thread::Current(), std::move(dex_files));
+}
+
 }  // namespace art

runtime/runtime.h

diff --git a/runtime/runtime.h b/runtime/runtime.h
index 6f15fce..395128d 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -307,6 +307,10 @@
                              const std::string& location,
                              const std::vector<std::unique_ptr<const art::DexFile>>& dex_files);
 
+  void AddExtraBootDexFiles(const std::string& filename,
+                            const std::string& location,
+                            std::vector<std::unique_ptr<const art::DexFile>>&& dex_files);
+
   const std::vector<int>& GetBootClassPathFds() const {
     return boot_class_path_fds_;
   }