Revert " [profile] Implement a non-mmap path when reading profile files from a non-local filesystem (#131177)"

This reverts commit 14c95e0.

Test fails on mac, e.g. https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/3899/testReport/junit/Profile-x86_64/Profile-x86_64/instrprof_no_mmap_during_merging_c/

Author: aeubanks

compiler-rt/lib/profile/InstrProfilingFile.c

@@ -419,17 +419,17 @@ static int getProfileFileSizeForMerging(FILE *ProfileFile,
  * \p ProfileBuffer. Returns -1 on failure. On success, the caller is
  * responsible for unmapping the mmap'd buffer in \p ProfileBuffer. */
 static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
-                                 ManagedMemory *ProfileBuffer) {
-  lprofGetFileContentBuffer(ProfileFile, ProfileFileSize, ProfileBuffer);
-
-  if (ProfileBuffer->Status == MS_INVALID) {
-    PROF_ERR("Unable to merge profile data: %s\n", "reading file failed");
+                                 char **ProfileBuffer) {
+  *ProfileBuffer = mmap(NULL, ProfileFileSize, PROT_READ, MAP_SHARED | MAP_FILE,
+                        fileno(ProfileFile), 0);
+  if (*ProfileBuffer == MAP_FAILED) {
+    PROF_ERR("Unable to merge profile data, mmap failed: %s\n",
+             strerror(errno));
     return -1;
   }
 
-  if (__llvm_profile_check_compatibility(ProfileBuffer->Addr,
-                                         ProfileFileSize)) {
-    (void)lprofReleaseBuffer(ProfileBuffer, ProfileFileSize);
+  if (__llvm_profile_check_compatibility(*ProfileBuffer, ProfileFileSize)) {
+    (void)munmap(*ProfileBuffer, ProfileFileSize);
     PROF_WARN("Unable to merge profile data: %s\n",
               "source profile file is not compatible.");
     return -1;
@@ -444,7 +444,7 @@ static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
 */
 static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
   uint64_t ProfileFileSize;
-  ManagedMemory ProfileBuffer;
+  char *ProfileBuffer;
 
   /* Get the size of the profile on disk. */
   if (getProfileFileSizeForMerging(ProfileFile, &ProfileFileSize) == -1)
@@ -460,9 +460,9 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
     return -1;
 
   /* Now start merging */
-  if (__llvm_profile_merge_from_buffer(ProfileBuffer.Addr, ProfileFileSize)) {
+  if (__llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize)) {
     PROF_ERR("%s\n", "Invalid profile data to merge");
-    (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+    (void)munmap(ProfileBuffer, ProfileFileSize);
     return -1;
   }
 
@@ -471,7 +471,7 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
   (void)COMPILER_RT_FTRUNCATE(ProfileFile,
                               __llvm_profile_get_size_for_buffer());
 
-  (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+  (void)munmap(ProfileBuffer, ProfileFileSize);
   *MergeDone = 1;
 
   return 0;
@@ -672,13 +672,13 @@ static void initializeProfileForContinuousMode(void) {
     } else {
       /* The merged profile has a non-zero length. Check that it is compatible
        * with the data in this process. */
-      ManagedMemory ProfileBuffer;
+      char *ProfileBuffer;
       if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
         lprofUnlockFileHandle(File);
         fclose(File);
         return;
       }
-      (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+      (void)munmap(ProfileBuffer, ProfileFileSize);
     }
   } else {
     File = fopen(Filename, FileOpenMode);
@@ -1257,12 +1257,12 @@ COMPILER_RT_VISIBILITY int __llvm_profile_set_file_object(FILE *File,
     } else {
       /* The merged profile has a non-zero length. Check that it is compatible
        * with the data in this process. */
-      ManagedMemory ProfileBuffer;
+      char *ProfileBuffer;
       if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
         lprofUnlockFileHandle(File);
         return 1;
       }
-      (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+      (void)munmap(ProfileBuffer, ProfileFileSize);
     }
     mmapForContinuousMode(0, File);
     lprofUnlockFileHandle(File);

compiler-rt/lib/profile/InstrProfilingPort.h

@@ -23,7 +23,6 @@
 #define COMPILER_RT_FTRUNCATE(f,l) _chsize(_fileno(f),l)
 #define COMPILER_RT_ALWAYS_INLINE __forceinline
 #define COMPILER_RT_CLEANUP(x)
-#define COMPILER_RT_UNUSED
 #define COMPILER_RT_USED
 #elif __GNUC__
 #ifdef _WIN32
@@ -39,7 +38,6 @@
 #define COMPILER_RT_ALLOCA __builtin_alloca
 #define COMPILER_RT_ALWAYS_INLINE inline __attribute((always_inline))
 #define COMPILER_RT_CLEANUP(x) __attribute__((cleanup(x)))
-#define COMPILER_RT_UNUSED __attribute__((unused))
 #define COMPILER_RT_USED __attribute__((used))
 #endif
 

compiler-rt/lib/profile/InstrProfilingUtil.c

@@ -21,15 +21,6 @@
 #include <unistd.h>
 #endif
 
-#ifdef _AIX
-#include <sys/statfs.h>
-// <sys/vmount.h> depends on `uint` to be a typedef from <sys/types.h> to
-// `uint_t`; however, <sys/types.h> does not always declare `uint`. We provide
-// the typedef prior to including <sys/vmount.h> to work around this issue.
-typedef uint_t uint;
-#include <sys/vmount.h>
-#endif
-
 #ifdef COMPILER_RT_HAS_UNAME
 #include <sys/utsname.h>
 #endif
@@ -267,121 +258,6 @@ COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
   return f;
 }
 
-#if defined(_AIX)
-// Return 1 (true) if the file descriptor Fd represents a file that is on a
-// local filesystem, otherwise return 0.
-static int isLocalFilesystem(int Fd) {
-  struct statfs Vfs;
-  if (fstatfs(Fd, &Vfs) != 0) {
-    PROF_ERR("%s: fstatfs(%d) failed: %s\n", __func__, Fd, strerror(errno));
-    return 0;
-  }
-
-  int Ret;
-  size_t BufSize = 2048u;
-  char *Buf;
-  int Tries = 3;
-  while (Tries--) {
-    Buf = malloc(BufSize);
-    // mntctl returns -1 if `Buf` is `NULL`.
-    Ret = mntctl(MCTL_QUERY, BufSize, Buf);
-    if (Ret != 0)
-      break;
-    BufSize = *(unsigned int *)Buf;
-    free(Buf);
-  }
-
-  if (Ret != -1) {
-    // Look for the correct vmount entry.
-    char *CurObjPtr = Buf;
-    while (Ret--) {
-      struct vmount *Vp = (struct vmount *)CurObjPtr;
-      _Static_assert(sizeof(Vfs.f_fsid) == sizeof(Vp->vmt_fsid),
-                     "fsid length mismatch");
-      if (memcmp(&Vfs.f_fsid, &Vp->vmt_fsid, sizeof Vfs.f_fsid) == 0) {
-        int Answer = (Vp->vmt_flags & MNT_REMOTE) == 0;
-        free(Buf);
-        return Answer;
-      }
-      CurObjPtr += Vp->vmt_length;
-    }
-  }
-
-  free(Buf);
-  // There was an error in mntctl or vmount entry not found; "remote" is the
-  // conservative answer.
-  return 0;
-}
-#endif
-
-static int isMmapSafe(int Fd) {
-  if (getenv("LLVM_PROFILE_NO_MMAP")) // For testing purposes.
-    return 0;
-#ifdef _AIX
-  return isLocalFilesystem(Fd);
-#else
-  return 1;
-#endif
-}
-
-COMPILER_RT_VISIBILITY void lprofGetFileContentBuffer(FILE *F, uint64_t Length,
-                                                      ManagedMemory *Buf) {
-  Buf->Status = MS_INVALID;
-  if (isMmapSafe(fileno(F))) {
-    Buf->Addr =
-        mmap(NULL, Length, PROT_READ, MAP_SHARED | MAP_FILE, fileno(F), 0);
-    if (Buf->Addr == MAP_FAILED)
-      PROF_ERR("%s: mmap failed: %s\n", __func__, strerror(errno))
-    else
-      Buf->Status = MS_MMAP;
-    return;
-  }
-
-  if (getenv("LLVM_PROFILE_VERBOSE"))
-    PROF_NOTE("%s\n", "could not use mmap; using fread instead");
-
-  void *Buffer = malloc(Length);
-  if (!Buffer) {
-    PROF_ERR("%s: malloc failed: %s\n", __func__, strerror(errno));
-    return;
-  }
-  if (ftell(F) != 0) {
-    PROF_ERR("%s: expecting ftell to return zero\n", __func__);
-    free(Buffer);
-    return;
-  }
-
-  // Read the entire file into memory.
-  size_t BytesRead = fread(Buffer, 1, Length, F);
-  if (BytesRead != (size_t)Length) {
-    PROF_ERR("%s: fread failed%s\n", __func__,
-             feof(F) ? ": end of file reached" : "");
-    free(Buffer);
-    return;
-  }
-
-  // Reading was successful, record the result in the Buf parameter.
-  Buf->Addr = Buffer;
-  Buf->Status = MS_MALLOC;
-}
-
-COMPILER_RT_VISIBILITY
-void lprofReleaseBuffer(ManagedMemory *Buf, size_t Length) {
-  switch (Buf->Status) {
-  case MS_MALLOC:
-    free(Buf->Addr);
-    break;
-  case MS_MMAP:
-    (void)munmap(Buf->Addr, Length);
-    break;
-  default:
-    PROF_ERR("%s: Buffer has invalid state: %d\n", __func__, Buf->Status);
-    break;
-  }
-  Buf->Addr = NULL;
-  Buf->Status = MS_INVALID;
-}
-
 COMPILER_RT_VISIBILITY const char *lprofGetPathPrefix(int *PrefixStrip,
                                                       size_t *PrefixLen) {
   const char *Prefix = getenv("GCOV_PREFIX");

compiler-rt/lib/profile/InstrProfilingUtil.h

@@ -31,25 +31,6 @@ int lprofUnlockFileHandle(FILE *F);
  * lock for exclusive access. The caller will block
  * if the lock is already held by another process. */
 FILE *lprofOpenFileEx(const char *Filename);
-
-enum MemoryStatus {
-  MS_INVALID, // Addr is not a valid address
-  MS_MMAP,    // Addr was mmap'ed
-  MS_MALLOC   // Addr was malloc'ed
-};
-typedef struct {
-  void *Addr;
-  enum MemoryStatus Status;
-} ManagedMemory;
-
-/* Read the content of a file using mmap or fread into a buffer.
- * Certain files (e.g. NFS mounted) cannot be opened reliably with mmap,
- * so we use fread in those cases. The corresponding lprofReleaseBuffer
- * will free/munmap the buffer.
- */
-void lprofGetFileContentBuffer(FILE *F, uint64_t FileSize, ManagedMemory *Buf);
-void lprofReleaseBuffer(ManagedMemory *FileBuffer, size_t Length);
-
 /* PS4 doesn't have setenv/getenv/fork. Define a shim. */
 #if __ORBIS__
 #include <sys/types.h>

compiler-rt/test/profile/Posix/instrprof-fork.c

@@ -3,15 +3,11 @@
 // RUN: %clang_pgogen=%t.profdir -o %t -O2 %s
 // RUN: %run %t
 // RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw  | FileCheck %s
-// RUN: rm -fr %t.profdir
-// RUN: env LLVM_PROFILE_NO_MMAP=1 %run %t
-// RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw  | FileCheck %s
-
 //
 // CHECK: func1:
-// CHECK: Block counts: [21]
+// CHECK: Block counts: [4]
 // CHECK:  func2:
-// CHECK: Block counts: [10]
+// CHECK: Block counts: [1]
 
 #include <sys/wait.h>
 #include <unistd.h>
@@ -20,23 +16,17 @@ __attribute__((noinline)) void func1() {}
 __attribute__((noinline)) void func2() {}
 
 int main(void) {
-  //                           child     | parent
-  //                         func1 func2 | func1 func2
-  func1();              //   +10       |   +1        (*)
-  int i = 10;           //             |
-  while (i-- > 0) {     //             |
-    pid_t pid = fork(); //             |
-    if (pid == -1)      //             |
-      return 1;         //             |
-    if (pid == 0) {     //             |
-      func2();          //         +10 |
-      func1();          //   +10       |
-      return 0;         //             |
-    }                   //             |
-  }                     // ------------+------------
-  int status;           //   20     10 |   1     0
-  i = 10;               // (*)  the child inherits counter values prior to fork
-  while (i-- > 0)       //      from the parent in non-continuous mode.
-    wait(&status);
-  return 0;
+  //                       child     | parent
+  int status;         // func1 func2 | func1 func2
+  func1();            //   +1        |   +1        (*)
+  pid_t pid = fork(); //             |
+  if (pid == -1)      //             |
+    return 1;         //             |
+  if (pid == 0)       //             |
+    func2();          //         +1  |
+  func1();            //   +1        |   +1
+  if (pid)            // ------------+------------
+    wait(&status);    //    2     1  |    2    0
+  return 0;           // (*)  the child inherits counter values prior to fork
+                      //      from the parent in non-continuous mode.
 }