[compiler-rt] ea1826e - [MemProf] Defer profile file setup until dump time

[Teresa Johnson via llvm-commits]

Author: Teresa Johnson
Date: 2022-11-18T15:09:02-08:00
New Revision: ea1826ee57984d4f44fdb4b35a47169d393618ed

URL: https://github.com/llvm/llvm-project/commit/ea1826ee57984d4f44fdb4b35a47169d393618ed
DIFF: https://github.com/llvm/llvm-project/commit/ea1826ee57984d4f44fdb4b35a47169d393618ed.diff

LOG: [MemProf] Defer profile file setup until dump time

With all of the writing of the memprof profile consolidated into one
place, there is no need to set up the profile file (which creates the
file and also redirects all printing from the runtime to it) until we
are ready to dump the profile.

This allows errors and other messages to be dumped to stderr instead of
the profile file, which by default is in a binary format. Additionally,
reset the output file to stderr after dumping the profile so that any
requested memprof allocator statistics are printed to stderr.

Differential Revision: https://reviews.llvm.org/D138175

Added: 
    

Modified: 
    compiler-rt/lib/memprof/memprof_allocator.cpp
    compiler-rt/lib/memprof/memprof_rtl.cpp
    compiler-rt/test/memprof/TestCases/atexit_stats.cpp
    compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/memprof/memprof_allocator.cpp b/compiler-rt/lib/memprof/memprof_allocator.cpp
index 2733051b2e49b..6f8c61320aac1 100644
--- a/compiler-rt/lib/memprof/memprof_allocator.cpp
+++ b/compiler-rt/lib/memprof/memprof_allocator.cpp
@@ -26,6 +26,7 @@
 #include "sanitizer_common/sanitizer_errno.h"
 #include "sanitizer_common/sanitizer_file.h"
 #include "sanitizer_common/sanitizer_flags.h"
+#include "sanitizer_common/sanitizer_interface_internal.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_list.h"
 #include "sanitizer_common/sanitizer_procmaps.h"
@@ -35,6 +36,9 @@
 #include <sched.h>
 #include <time.h>
 
+// Allow the user to specify a profile output file via the binary.
+SANITIZER_WEAK_ATTRIBUTE char __memprof_profile_filename[1];
+
 namespace __memprof {
 namespace {
 using ::llvm::memprof::MemInfoBlock;
@@ -280,6 +284,12 @@ struct Allocator {
   }
 
   void FinishAndWrite() {
+    // Use profile name specified via the binary itself if it exists, and hasn't
+    // been overrriden by a flag at runtime.
+    if (__memprof_profile_filename[0] != 0 && !common_flags()->log_path)
+      __sanitizer_set_report_path(__memprof_profile_filename);
+    else
+      __sanitizer_set_report_path(common_flags()->log_path);
     if (print_text && common_flags()->print_module_map)
       DumpProcessMap();
 
@@ -304,6 +314,11 @@ struct Allocator {
     }
 
     allocator.ForceUnlock();
+
+    // Set the report back to the default stderr now that we have dumped the
+    // profile, in case there are later errors or stats dumping on exit has been
+    // enabled.
+    __sanitizer_set_report_path("stderr");
   }
 
   // Inserts any blocks which have been allocated but not yet deallocated.

diff  --git a/compiler-rt/lib/memprof/memprof_rtl.cpp b/compiler-rt/lib/memprof/memprof_rtl.cpp
index d30b80304f6bc..c2e0b4e2dc1c0 100644
--- a/compiler-rt/lib/memprof/memprof_rtl.cpp
+++ b/compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -29,9 +29,6 @@
 
 uptr __memprof_shadow_memory_dynamic_address; // Global interface symbol.
 
-// Allow the user to specify a profile output file via the binary.
-SANITIZER_WEAK_ATTRIBUTE char __memprof_profile_filename[1];
-
 namespace __memprof {
 
 static void MemprofDie() {
@@ -169,13 +166,6 @@ static void MemprofInitInternal() {
   AddDieCallback(MemprofDie);
   SetCheckUnwindCallback(CheckUnwind);
 
-  // Use profile name specified via the binary itself if it exists, and hasn't
-  // been overrriden by a flag at runtime.
-  if (__memprof_profile_filename[0] != 0 && !common_flags()->log_path)
-    __sanitizer_set_report_path(__memprof_profile_filename);
-  else
-    __sanitizer_set_report_path(common_flags()->log_path);
-
   __sanitizer::InitializePlatformEarly();
 
   // Setup internal allocator callback.

diff  --git a/compiler-rt/test/memprof/TestCases/atexit_stats.cpp b/compiler-rt/test/memprof/TestCases/atexit_stats.cpp
index 4a5fac9b8d266..29d9a0a56b959 100644
--- a/compiler-rt/test/memprof/TestCases/atexit_stats.cpp
+++ b/compiler-rt/test/memprof/TestCases/atexit_stats.cpp
@@ -2,6 +2,9 @@
 
 // RUN: %clangxx_memprof -O0 %s -o %t
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr:atexit=1 %run %t 2>&1 | FileCheck %s
+// Stats should be dumped to stderr even if the profile log path set to a file.
+// RUN: rm -f %t.log.*
+// RUN: %env_memprof_opts=print_text=true:log_path=%t.log:atexit=1 %run %t 2>&1 | FileCheck %s
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr:atexit=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOATEXIT
 
 // CHECK: MemProfiler exit stats:

diff  --git a/compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp b/compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
index 4eb9e8e1fba58..78825d014ae74 100644
--- a/compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
+++ b/compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
@@ -1,5 +1,8 @@
 // RUN: %clangxx_memprof -O0 %s -o %t
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr:allocator_may_return_null=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-SUMMARY
+// Errors should be printed to stderr even if the log_path set to a file.
+// RUN: rm -f %t.log.*
+// RUN: %env_memprof_opts=print_text=true:log_path=%t.log:allocator_may_return_null=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-SUMMARY
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr:allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NULL
 // Test print_summary
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr:allocator_may_return_null=0:print_summary=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOSUMMARY