[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

37

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

37

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/lit.cfg.py

lit_config.fatal(
       "to lit.site.cfg.py " % attr_name)
   return attr_value
 
 # 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)
+# Set the default to print text to simplify tests.
+default_memprof_opts.append('print_text=true')
 
 default_memprof_opts_str = ':'.join(default_memprof_opts)
 if default_memprof_opts_str:
   config.environment['MEMPROF_OPTIONS'] = default_memprof_opts_str
 config.substitutions.append(('%env_memprof_opts=',
-                             'env MEMPROF_OPTIONS=' + default_memprof_opts_str))
+                             'env MEMPROF_OPTIONS=' + default_memprof_opts_str + ':'))
 
 # Setup source root.
 config.test_source_root = os.path.dirname(__file__)
 
 libdl_flag = '-ldl'
 
 # Setup default compiler flags used with -fmemory-profile option.
 # FIXME: Review the set of required flags and check if it can be reduced.