[memprof] Make the raw binary format the default.

[snehasish]

Summary

Set the default memprof serialization format as binary. For the tests,
we update the configuration to include 'print_text=true'.

[tejohnson]

How many tests would be affected by switching the default?

[snehasish]

Half of the tests (9/18) fail if the default is set to the binary format.

Failed Tests (9):

MemProfiler-x86_64-linux :: TestCases/dump_process_map.cpp
MemProfiler-x86_64-linux :: TestCases/log_path_test.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_merge_mib.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_profile_dump.cpp
MemProfiler-x86_64-linux :: TestCases/test_malloc_load_store.c
MemProfiler-x86_64-linux :: TestCases/test_memintrin.cpp
MemProfiler-x86_64-linux :: TestCases/test_new_load_store.cpp
MemProfiler-x86_64-linux :: TestCases/test_terse.cpp
MemProfiler-x86_64-linux :: TestCases/unaligned_loads_and_stores.cpp

[tejohnson]

In D113617#3124729, @snehasish wrote:

Half of the tests (9/18) fail if the default is set to the binary format.

Failed Tests (9):

MemProfiler-x86_64-linux :: TestCases/dump_process_map.cpp
MemProfiler-x86_64-linux :: TestCases/log_path_test.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_merge_mib.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_profile_dump.cpp
MemProfiler-x86_64-linux :: TestCases/test_malloc_load_store.c
MemProfiler-x86_64-linux :: TestCases/test_memintrin.cpp
MemProfiler-x86_64-linux :: TestCases/test_new_load_store.cpp
MemProfiler-x86_64-linux :: TestCases/test_terse.cpp
MemProfiler-x86_64-linux :: TestCases/unaligned_loads_and_stores.cpp

IMO it's probably better to be explicit in the test - e.g. we are already setting log_path=stderr for all of these I believe.

[snehasish]

Update tests to specify print_text=true instead of setting as default.

[snehasish]

Update comment.

[snehasish]

In D113617#3124915, @tejohnson wrote:

In D113617#3124729, @snehasish wrote:

Half of the tests (9/18) fail if the default is set to the binary format.

Failed Tests (9):

MemProfiler-x86_64-linux :: TestCases/dump_process_map.cpp
MemProfiler-x86_64-linux :: TestCases/log_path_test.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_merge_mib.cpp
MemProfiler-x86_64-linux :: TestCases/memprof_profile_dump.cpp
MemProfiler-x86_64-linux :: TestCases/test_malloc_load_store.c
MemProfiler-x86_64-linux :: TestCases/test_memintrin.cpp
MemProfiler-x86_64-linux :: TestCases/test_new_load_store.cpp
MemProfiler-x86_64-linux :: TestCases/test_terse.cpp
MemProfiler-x86_64-linux :: TestCases/unaligned_loads_and_stores.cpp

IMO it's probably better to be explicit in the test - e.g. we are already setting log_path=stderr for all of these I believe.

Done. PTAL, thanks!

[tejohnson]

compiler-rt/test/memprof/lit.cfg.py

35

I noticed that the other sanitizers (e.g. llvm/llvm-project/compiler-rt/test/asan/lit.cfg.py) add the trailing ":" after setting e.g. ASAN_OPTIONS. Will we end up with an extra trailing ":" for that env var?

[snehasish]

Add the trailing separator only if the var is set.

[tejohnson]

lgtm

[snehasish]

compiler-rt/test/memprof/lit.cfg.py

35

Yes, thats a little bit cleaner though we could still end up with a trailing ':' if the test does not provide any options. Since these do not affect option parsing, I'm happy to update this to be consistent with the ASAN config.

[snehasish]

Rebase.

[snehasish]

I can't reproduce the patch application failure on the buildbot. I applied the patches in series to a clean llvm repo and it worked.

compiler-rt/lib/memprof/memprof_flags.inc

 
 MEMPROF_FLAG(bool, halt_on_error, true,
              "Crash the program after printing the first error report "
              "(WARNING: USE AT YOUR OWN RISK!)")
 MEMPROF_FLAG(bool, allocator_frees_and_returns_null_on_realloc_zero, true,
              "realloc(p, 0) is equivalent to free(p) by default (Same as the "
              "POSIX standard). If set to false, realloc(p, 0) will return a "
              "pointer to an allocated space which can not be used.")
-MEMPROF_FLAG(bool, print_text, true,
+MEMPROF_FLAG(bool, print_text, false,
   "If set, prints the heap profile in text format. Else use the raw binary serialization format.")
 MEMPROF_FLAG(bool, print_terse, false,
              "If set, prints memory profile in a terse format. Only applicable if print_text = true.")

compiler-rt/test/memprof/TestCases/atexit_stats.cpp

 // Check atexit option.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr:atexit=1 %run %t 2>&1 | FileCheck %s
-// RUN: %env_memprof_opts=log_path=stderr:atexit=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOATEXIT
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr: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:
 // CHECK: Stats: {{[0-9]+}}M malloced ({{[0-9]+}}M for overhead) by {{[0-9]+}} calls
 // CHECK: Stats: {{[0-9]+}}M realloced by {{[0-9]+}} calls
 // CHECK: Stats: {{[0-9]+}}M freed by {{[0-9]+}} calls
 // CHECK: Stats: {{[0-9]+}}M really freed by {{[0-9]+}} calls
 // CHECK: Stats: {{[0-9]+}}M ({{[0-9]+}}M-{{[0-9]+}}M) mmaped; {{[0-9]+}} maps, {{[0-9]+}} unmaps
 // CHECK:   mallocs by size class:
 // CHECK: Stats: malloc large: {{[0-9]+}}
 
 // NOATEXIT-NOT: MemProfiler exit stats
 
 int main() {
   return 0;
 }

compiler-rt/test/memprof/TestCases/dump_process_map.cpp

 // Check print_module_map option.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr:print_module_map=1 %run %t 2>&1 | FileCheck %s
-// RUN: %env_memprof_opts=log_path=stderr:print_module_map=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOMAP
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:print_module_map=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:print_module_map=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOMAP
 
 // CHECK: Process memory map follows:
 // CHECK: dump_process_map.cpp.tmp
 // CHECK: End of process memory map.
 // NOMAP-NOT: memory map
 
 int main() {
   return 0;
 }

compiler-rt/test/memprof/TestCases/log_path_test.cpp

 // The for loop in the backticks below requires bash.
 // REQUIRES: shell
 //
 // RUN: %clangxx_memprof  %s -o %t
 
-// stderr log_path
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always
+// stderr print_text=true:log_path
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always
 
-// Good log_path.
+// Good print_text=true:log_path.
 // RUN: rm -f %t.log.*
-// RUN: %env_memprof_opts=log_path=%t.log %run %t
+// RUN: %env_memprof_opts=print_text=true:log_path=%t.log %run %t
 // RUN: FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always < %t.log.*
 
-// Invalid log_path.
-// RUN: %env_memprof_opts=log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
+// Invalid print_text=true:log_path.
+// RUN: %env_memprof_opts=print_text=true:log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
 
-// Too long log_path.
-// RUN: %env_memprof_opts=log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
+// Too long print_text=true:log_path.
+// RUN: %env_memprof_opts=print_text=true:log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
 // RUN:   not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-LONG --dump-input=always
 
 // Specifying the log name via the __memprof_profile_filename variable.
 // Note we use an invalid path since that is sufficient for checking that the
 // specified __memprof_profile_filename value is passed through to the runtime.
 // Using an automatically generated name via %t can cause weird issues with
 // unexpected macro expansion if the path includes tokens that match a build
 // system macro (e.g. "linux").

compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp

 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr:allocator_may_return_null=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-SUMMARY
-// RUN: %env_memprof_opts=log_path=stderr:allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NULL
+// 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
+// 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=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
+// 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
 
 #include <stdio.h>
 #include <stdlib.h>
 
 static const size_t kMaxAllowedMallocSizePlusOne = (1ULL << 40) + 1;
 int main() {
   void *p = malloc(kMaxAllowedMallocSizePlusOne);
   // CHECK: {{ERROR: MemProfiler: requested allocation size .* exceeds maximum supported size}}

compiler-rt/test/memprof/TestCases/memprof_merge_mib.cpp

 // RUN: %clangxx_memprof  %s -o %t
 
-// RUN: %env_memprof_opts=log_path=stdout %run %t | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stdout %run %t | FileCheck %s
 
 #include <sanitizer/memprof_interface.h>
 #include <stdlib.h>
 #include <string.h>
 int main(int argc, char **argv) {
   for (int i = 0; i < 3; i++) {
     char *x = (char *)malloc(10);
     if (i % 2)

compiler-rt/test/memprof/TestCases/memprof_profile_dump.cpp

 // RUN: %clangxx_memprof  %s -o %t
 
-// RUN: %env_memprof_opts=log_path=stdout %run %t | FileCheck --check-prefix=CHECK-TEXT %s
-// RUN: %env_memprof_opts=log_path=stdout,print_text=false %run %t > %t.memprofraw
+// RUN: %env_memprof_opts=print_text=true:log_path=stdout %run %t | FileCheck --check-prefix=CHECK-TEXT %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stdout,print_text=false %run %t > %t.memprofraw
 // RUN: od -c -N 8 %t.memprofraw | FileCheck --check-prefix=CHECK-RAW %s
 
 #include <sanitizer/memprof_interface.h>
 #include <stdlib.h>
 #include <string.h>
 int main(int argc, char **argv) {
   char *x = (char *)malloc(10);
   memset(x, 0, 10);

compiler-rt/test/memprof/TestCases/stress_dtls.c

 // REQUIRES: memprof-64-bits
 // Stress test dynamic TLS + dlopen + threads.
 //
 // RUN: %clang_memprof -x c -DSO_NAME=f0 %s -shared -o %t-f0.so -fPIC
 // RUN: %clang_memprof -x c -DSO_NAME=f1 %s -shared -o %t-f1.so -fPIC
 // RUN: %clang_memprof -x c -DSO_NAME=f2 %s -shared -o %t-f2.so -fPIC
 // RUN: %clang_memprof %s -ldl -pthread -o %t
 // RUN: %run %t 0 3
 // RUN: %run %t 2 3
-// RUN: %env_memprof_opts=log_path=stderr:verbosity=2 %run %t 10 2 2>&1 | FileCheck %s
-// RUN: %env_memprof_opts=log_path=stderr:verbosity=2:intercept_tls_get_addr=1 %run %t 10 2 2>&1 | FileCheck %s
-// RUN: %env_memprof_opts=log_path=stderr:verbosity=2:intercept_tls_get_addr=0 %run %t 10 2 2>&1 | FileCheck %s --check-prefix=CHECK0
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:verbosity=2 %run %t 10 2 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:verbosity=2:intercept_tls_get_addr=1 %run %t 10 2 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:verbosity=2:intercept_tls_get_addr=0 %run %t 10 2 2>&1 | FileCheck %s --check-prefix=CHECK0
 // CHECK: ==__tls_get_addr:
 // CHECK: Creating thread 0
 // CHECK: ==__tls_get_addr:
 // CHECK: Creating thread 1
 // CHECK: ==__tls_get_addr:
 // CHECK: Creating thread 2
 // CHECK: ==__tls_get_addr:
 // CHECK: Creating thread 3

compiler-rt/test/memprof/TestCases/test_malloc_load_store.c

 // Check profile with a single malloc call and set of loads and stores. Ensures
 // we get the same profile regardless of whether the memory is deallocated
 // before exit.
 
 // RUN: %clang_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // RUN: %clang_memprof -DFREE -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
 //   alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 // but we need to look for them in the same CHECK to get the correct STACKID.
 // CHECK:  Memory allocation stack id = [[STACKID:[0-9]+]]{{[[:space:]].*}}alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 // CHECK-NEXT:  access_count (ave/min/max): 20.00 / 20 / 20
 // CHECK-NEXT:  lifetime (ave/min/max): [[AVELIFETIME:[0-9]+]].00 / [[AVELIFETIME]] / [[AVELIFETIME]]

compiler-rt/test/memprof/TestCases/test_memintrin.cpp

 // Check profile with calls to memory intrinsics.
 
-// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKIDP
 //   alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 //   access_count (ave/min/max): 11.00 / 11 / 11
 // but we need to look for them in the same CHECK to get the correct STACKIDP.
 // CHECK-DAG:  Memory allocation stack id = [[STACKIDP:[0-9]+]]{{[[:space:]].*}} alloc_count 1, size (ave/min/max) 40.00 / 40 / 40{{[[:space:]].*}} access_count (ave/min/max): 11.00 / 11 / 11
 //

compiler-rt/test/memprof/TestCases/test_new_load_store.cpp

 // Check profile with a single new call and set of loads and stores. Ensures
 // we get the same profile regardless of whether the memory is deallocated
 // before exit.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // RUN: %clangxx_memprof -DFREE -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // Try again with callbacks instead of inline sequences
 // RUN: %clangxx_memprof -mllvm -memprof-use-callbacks -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
 //   alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 // but we need to look for them in the same CHECK to get the correct STACKID.
 // CHECK:  Memory allocation stack id = [[STACKID:[0-9]+]]{{[[:space:]].*}}alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 // CHECK-NEXT:  access_count (ave/min/max): 20.00 / 20 / 20
 // CHECK-NEXT:  lifetime (ave/min/max): [[AVELIFETIME:[0-9]+]].00 / [[AVELIFETIME]] / [[AVELIFETIME]]

compiler-rt/test/memprof/TestCases/test_terse.cpp

 // Check terse format profile with a single malloc call and set of loads and
 // stores. Ensures we get the same profile regardless of whether the memory is
 // deallocated before exit.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr:print_terse=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:print_terse=1 %run %t 2>&1 | FileCheck %s
 
 // RUN: %clangxx_memprof -DFREE -O0 %s -o %t
-// RUN: %env_memprof_opts=log_path=stderr:print_terse=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr:print_terse=1 %run %t 2>&1 | FileCheck %s
 
 // CHECK: MIB:[[STACKID:[0-9]+]]/1/40.00/40/40/20.00/20/20/[[AVELIFETIME:[0-9]+]].00/[[AVELIFETIME]]/[[AVELIFETIME]]/{{[01]}}/0/0/0
 // CHECK: Stack for id [[STACKID]]:
 // CHECK-NEXT: #0 {{.*}} in operator new
 // CHECK-NEXT: #1 {{.*}} in main {{.*}}:[[@LINE+6]]
 
 #include <stdio.h>
 #include <stdlib.h>

compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp

-// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
 //    alloc_count 1, size (ave/min/max) 128.00 / 128 / 128
 // but we need to look for them in the same CHECK to get the correct STACKID.
 // CHECK:      Memory allocation stack id = [[STACKID:[0-9]+]]{{[[:space:]].*}}alloc_count 1, size (ave/min/max) 128.00 / 128 / 128
 // CHECK-NEXT:   access_count (ave/min/max): 22.00 / 22 / 22
 

compiler-rt/test/memprof/lit.cfg.py

 # Setup config name.
 config.name = 'MemProfiler' + config.name_suffix
 
 # Platform-specific default MEMPROF_OPTIONS for lit tests.
 default_memprof_opts = list(config.default_sanitizer_opts)
 
 default_memprof_opts_str = ':'.join(default_memprof_opts)
 if default_memprof_opts_str:
   config.environment['MEMPROF_OPTIONS'] = default_memprof_opts_str
+  default_memprof_opts_str += ':'
 config.substitutions.append(('%env_memprof_opts=',
                              'env MEMPROF_OPTIONS=' + default_memprof_opts_str))
 
 # Setup source root.
 config.test_source_root = os.path.dirname(__file__)
 
 libdl_flag = '-ldl'