[ubsan] support print_module_map flag in standalone mode

[aralisza]

Summary

Currently, print_module_map is only respected for ubsan if it is ran in tandem with asan. This patch adds support for this flag in standalone mode. I copied the pattern used to implement this for asan.

Also added a common print_module_map lit test for Darwin only. Since the print messages are different per platform, we need to write a regex test to cover them. This test is coming in a separate patch

rdar://56135732

[vsk]

Thanks, this looks good to me with a nit. I haven't worked on sanitizer_common very much, so please wait for another +1.

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-module-map.cpp

28

The '{{ . }}' syntax should only be necessary when matching a regex. You should be able to simply write:

// CHECK: SUMMARY:

here and below. The check on line 32 for {{.*ABORTING}} can also just be ABORTING.
(FileCheck does have a mode that's strict about matching full lines, and another separate mode for matching whitespace exactly, but unless something has changed recently those are not on by default.)

[aralisza]

remove unecessary braces in lit test

[delcypher]

Since the print messages are different per platform, it would be difficult to write a platform-agnostic test.

So the mac does

Printf("Process module map:\n");

Other POSIX platforms do

Report("Process memory map follows:\n");

and the Windows implementation does

Report("Dumping process modules:\n");

It's unfortunate that these outputs are not consistent. We could write a regex here to try to cover all 3 which means we could re-use this test elsewhere. However, that seems like a pain. Instead I think we should land this patch as it is and then as a separate patch unify what we print when we print the module map and then make this test non Darwin specific.

[delcypher]

LGTM but let's give @vitalybuka time to take a look

[vitalybuka]

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-module-map.cpp

11–13

Lets make it xfail, so if it's somehow fixed it, they don't forget to update the test.

17–20

my clang reports warning here
let's make something less obvious

29–36

With pattern like this we can move it into Posix/

[vsk]

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-module-map.cpp

17–20

Just curious - what motivates this? Clang is required to emit the ubsan diagnostic even if the UB is detectable statically. And passing -w would hide the warning, if that's important.

[vitalybuka]

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-module-map.cpp

17–20

unnecessary noise when looking at text failure.
"-w" is fine as well

[aralisza]

change unsupported platforms to fail the test and pass -w to suppress compiler warning noise

[vitalybuka]

In D97746#2607394, @aralisza wrote:

change unsupported platforms to fail the test and pass -w to suppress compiler warning noise

So what's about the rest?
The patch is already LGTM, but it would be nice to have the test coverage for more platforms. Doing that in followup patch is fine as well.

[aralisza]

It's slightly complicated to write a platform-agnostic test because the strings printed are different for each platform. Maybe the right course of action is to make at least the heading of the module map section be uniform? What do you think?

[vitalybuka]

In D97746#2607524, @aralisza wrote:

It's slightly complicated to write a platform-agnostic test because the strings printed are different for each platform. Maybe the right course of action is to make at least the heading of the module map section be uniform? What do you think?

There is proposed regexp version which works for linux and should work for OSX. I don't find it complicated at all.

I don't mind to unify, in a separate patch. However "Process module map" does not match Linux output. I didn't check content of darwin output if it's reasonable to rename it into "memory map".

[aralisza]

ah sorry I missed that comment. I'll go ahead and move the test in a separate patch

[delcypher]

In D97746#2607796, @vitalybuka wrote:

In D97746#2607524, @aralisza wrote:

It's slightly complicated to write a platform-agnostic test because the strings printed are different for each platform. Maybe the right course of action is to make at least the heading of the module map section be uniform? What do you think?

There is proposed regexp version which works for linux and should work for OSX. I don't find it complicated at all.

I don't mind to unify, in a separate patch. However "Process module map" does not match Linux output. I didn't check content of darwin output if it's reasonable to rename it into "memory map".

@vitalybuka @aralisza I would prefer if we unified the output to be (Process module map) because I have some private code that depends on it but it isn't a dealbreaker for me if we change it. However, if we do change it
we will need to update lib/asan/scripts/asan_symbolize.py because it has code designed to parse the module map as it is currently printed on Darwin.

[vitalybuka]

In D97746#2608101, @delcypher wrote:

In D97746#2607796, @vitalybuka wrote:

In D97746#2607524, @aralisza wrote:

It's slightly complicated to write a platform-agnostic test because the strings printed are different for each platform. Maybe the right course of action is to make at least the heading of the module map section be uniform? What do you think?

There is proposed regexp version which works for linux and should work for OSX. I don't find it complicated at all.

I don't mind to unify, in a separate patch. However "Process module map" does not match Linux output. I didn't check content of darwin output if it's reasonable to rename it into "memory map".

@vitalybuka @aralisza I would prefer if we unified the output to be (Process module map) because I have some private code that depends on it but it isn't a dealbreaker for me if we change it. However, if we do change it
we will need to update lib/asan/scripts/asan_symbolize.py because it has code designed to parse the module map as it is currently printed on Darwin.

So I don't think this regexp is worth of hassle and I'd keep it as-is.

compiler-rt/lib/ubsan/ubsan_diag.cpp

 
 ScopedReport::ScopedReport(ReportOptions Opts, Location SummaryLoc,
                            ErrorType Type)
     : Opts(Opts), SummaryLoc(SummaryLoc), Type(Type) {}
 
 ScopedReport::~ScopedReport() {
   MaybePrintStackTrace(Opts.pc, Opts.bp);
   MaybeReportErrorSummary(SummaryLoc, Type);
+
+  if (common_flags()->print_module_map >= 2)
+    DumpProcessMap();
+
   if (flags()->halt_on_error)
     Die();
 }
 
 ALIGNED(64) static char suppression_placeholder[sizeof(SuppressionContext)];
 static SuppressionContext *suppression_ctx = nullptr;
 static const char kVptrCheck[] = "vptr_check";
 static const char *kSuppressionTypes[] = {

compiler-rt/lib/ubsan/ubsan_init.cpp

 
 static bool ubsan_initialized;
 static StaticSpinMutex ubsan_init_mu;
 
 static void CommonInit() {
   InitializeSuppressions();
 }
 
+static void UbsanDie() {
+  if (common_flags()->print_module_map >= 1)
+    DumpProcessMap();
+}
+
 static void CommonStandaloneInit() {
   SanitizerToolName = GetSanititizerToolName();
   CacheBinaryName();
   InitializeFlags();
   __sanitizer::InitializePlatformEarly();
   __sanitizer_set_report_path(common_flags()->log_path);
   AndroidLogInit();
   InitializeCoverage(common_flags()->coverage, common_flags()->coverage_dir);
   CommonInit();
+
+  // Only add die callback when running in standalone mode to avoid printing
+  // the same information from multiple sanitizers' output
+  AddDieCallback(UbsanDie);
   Symbolizer::LateInitialize();
 }
 
 void __ubsan::InitAsStandalone() {
   SpinMutexLock l(&ubsan_init_mu);
   if (!ubsan_initialized) {
     CommonStandaloneInit();
     ubsan_initialized = true;

compiler-rt/test/sanitizer_common/TestCases/Darwin/print-module-map.cpp

+// Checks that module map does not print at 0, prints once after aborting with 1,
+// and prints once before and after aborting with 2
+
+// RUN: %clangxx -DUSING_%tool_name %s -o %t
+
+// RUN: %env_tool_opts="print_module_map=0" not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-MM0
+// RUN: %env_tool_opts="print_module_map=1" not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-MM1
+// RUN: %env_tool_opts="print_module_map=2" not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-MM2
+
+// UNSUPPORTED: lsan
+// tsan support pending rdar://67747473
+// UNSUPPORTED: tsan
+
+int global;
+
+int main() {
+#if defined(USING_ubsan)
+  int value = 5;
+  int computation = value / 0; // Division by zero.
+#else
+  volatile int *a = new int[100];
+  delete[] a;
+  global = a[0]; // use-after-free: triggers ASan/TSan report.
+#endif
+  return 0;
+}
+
+// CHECK: {{SUMMARY:}}
+// CHECK-MM0-NOT: {{Process module map:}}
+// CHECK-MM1-NOT: {{Process module map:}}
+// CHECK-MM2: {{Process module map:}}
+// CHECK: {{.*ABORTING}}
+// CHECK-MM0-NOT: {{Process module map:}}
+// CHECK-MM1-NEXT: {{Process module map:}}
+// CHECK-MM2-NEXT: {{Process module map:}}