Add O_CREAT to ProfileCompilationInfo::SaveFallback.

Before this change, ProfileCompilationInfo::SaveFallback relies on
ProfileSaver::ProcessProfilingInfo to create an empty file beforehand if
the file didn't exist. This doesn't work if SaveFallback is called by
other callers (e.g., CopyAndUpdateProfileKey), and there can be a race.

After this change, ProcessProfilingInfo no longer creates an empty file,
and SaveFallback creates the file itself.

This change also avoids unnecessary file creation when saving is not
needed.

Bug: 275378665
Bug: 282191456
Test: art/test.py -b --host -r -t 595-profile-saving
Test: art/test.py --target -r -t 595-profile-saving (chroot on U device)
Test: art/test.py --target -r -t 595-profile-saving (chroot on T device)
Test: profman --copy-and-update-profile-key --profile-file=YouTube.dm --output-profile-type=app --apk=YouTube.apk --dex-location=base.apk --reference-profile-file=/tmp/1.prof
(cherry picked from https://android-review.googlesource.com/q/commit:a476f1aed79b4d714bb4d71a777bcb639c8a23eb)
Merged-In: I01ef27791c2625f0cbd242fcaa8056df4b7d24d0
Change-Id: I01ef27791c2625f0cbd242fcaa8056df4b7d24d0

3 files changed, 22 insertions, 21 deletions

diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc
index 1ef29622e1..4341a1d49a 100644
--- a/libprofile/profile/profile_compilation_info.cc
+++ b/libprofile/profile/profile_compilation_info.cc
@@ -16,6 +16,7 @@
 
 #include "profile_compilation_info.h"
 
+#include <fcntl.h>
 #include <sys/file.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -783,6 +784,9 @@ bool ProfileCompilationInfo::Load(const std::string& filename, bool clear_if_inv
       LockedFile::Open(filename.c_str(), flags, /*block=*/false, &error);
 
   if (profile_file.get() == nullptr) {
+    if (clear_if_invalid && errno == ENOENT) {
+      return true;
+    }
     LOG(WARNING) << "Couldn't lock the profile file " << filename << ": " << error;
     return false;
   }
@@ -800,6 +804,8 @@ bool ProfileCompilationInfo::Load(const std::string& filename, bool clear_if_inv
        (status == ProfileLoadStatus::kBadData))) {
     LOG(WARNING) << "Clearing bad or obsolete profile data from file "
                  << filename << ": " << error;
+    // When ART Service is enabled, this is the only place where we mutate a profile in place.
+    // TODO(jiakaiz): Get rid of this.
     if (profile_file->ClearContent()) {
       return true;
     } else {
@@ -878,9 +884,9 @@ bool ProfileCompilationInfo::Save(const std::string& filename, uint64_t* bytes_w
 bool ProfileCompilationInfo::SaveFallback(const std::string& filename, uint64_t* bytes_written) {
   std::string error;
 #ifdef _WIN32
-  int flags = O_WRONLY;
+  int flags = O_WRONLY | O_CREAT;
 #else
-  int flags = O_WRONLY | O_NOFOLLOW | O_CLOEXEC;
+  int flags = O_WRONLY | O_NOFOLLOW | O_CLOEXEC | O_CREAT;
 #endif
   // There's no need to fsync profile data right away. We get many chances
   // to write it again in case something goes wrong. We can rely on a simple
diff --git a/libprofile/profile/profile_compilation_info.h b/libprofile/profile/profile_compilation_info.h
index 4e15a4777a..68177629b0 100644
--- a/libprofile/profile/profile_compilation_info.h
+++ b/libprofile/profile/profile_compilation_info.h
@@ -463,10 +463,12 @@ class ProfileCompilationInfo {
   //   the dex_file they are in.
   bool VerifyProfileData(const std::vector<const DexFile*>& dex_files);
 
-  // Load profile information from the given file
+  // Loads profile information from the given file.
+  // Returns true on success, false otherwise.
   // If the current profile is non-empty the load will fail.
-  // If clear_if_invalid is true and the file is invalid the method clears the
-  // the file and returns true.
+  // If clear_if_invalid is true:
+  // - If the file is invalid, the method clears the file and returns true.
+  // - If the file doesn't exist, the method returns true.
   bool Load(const std::string& filename, bool clear_if_invalid);
 
   // Merge the data from another ProfileCompilationInfo into the current object. Only merges
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 9eaeaa88d0..3321636122 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -868,22 +868,15 @@ bool ProfileSaver::ProcessProfilingInfo(
     }
     {
       ProfileCompilationInfo info(Runtime::Current()->GetArenaPool(),
-                                  /*for_boot_image=*/ options_.GetProfileBootClassPath());
-      if (OS::FileExists(filename.c_str())) {
-        if (!info.Load(filename, /*clear_if_invalid=*/true)) {
-          LOG(WARNING) << "Could not forcefully load profile " << filename;
-          continue;
-        }
-      } else {
-        // Create a file if it doesn't exist.
-        unix_file::FdFile file(filename,
-                               O_WRONLY | O_TRUNC | O_CREAT,
-                               S_IRUSR | S_IWUSR,
-                               /*check_usage=*/false);
-        if (!file.IsValid()) {
-          LOG(WARNING) << "Could not create profile " << filename;
-          continue;
-        }
+                                  /*for_boot_image=*/options_.GetProfileBootClassPath());
+      // Load the existing profile before saving.
+      // If the file is updated between `Load` and `Save`, the update will be lost. This is
+      // acceptable. The main reason is that the lost entries will eventually come back if the user
+      // keeps using the same methods, or they won't be needed if the user doesn't use the same
+      // methods again.
+      if (!info.Load(filename, /*clear_if_invalid=*/true)) {
+        LOG(WARNING) << "Could not forcefully load profile " << filename;
+        continue;
       }
 
       uint64_t last_save_number_of_methods = info.GetNumberOfMethods();