[UBSan] Do not overwrite the default print_summary sanitizer option.

[Dor1s]

Summary

This option is true by default in sanitizer common. The default
false value was added a while ago without any reasoning in
https://github.com/llvm-mirror/compiler-rt/commit/524e934112a593ac081bf2b05aa0d60a67987f05

so, presumably it's safe to remove for consistency.

[hctim]

I'm personally okay with this change, but will wait for @kcc to see what he thinks.

The current output for UBSan summaries looks like:

b.cpp:9:4: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior b.cpp:9:4 in

Can we change this to be SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in b.cpp:9:4?

[Dor1s]

Removed @kcc as a "blocking" reviewer, since we've discussed this offline last week. I'll check with @vitalybuka regarding potential breakages and also ping @samsonov. Other than that, should be good to go.

@hctim regarding your question, I think the intention is that the summary line would end with the function name and/or its offset. That seems to work fine with e.g. ASan, but somehow appends an empty name for UBSan. Anyhow, it's better to be fixed in a separate change, as either of those might be reverted and landing two things together would just increase the chances.

See RenderFrame in compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp:

// Function name and offset, if file is unknown.
if (info.function) {
  buffer->append("in %s",
                 DemangleFunctionName(
                   StripFunctionName(info.function, strip_func_prefix)));
  if (!info.file && info.function_offset != AddressInfo::kUnknown)
    buffer->append("+0x%zx", info.function_offset);
}

[samsonov]

IIRC, print_summary=false override was added to UBSan to keep its output more terse, and also to avoid essentially duplicating the information from the "runtime error" line. If the current maintainers agree that it makes sense to print summary for consistency, I have no objections.

[hctim]

SGTM. LGTM.

[vitalybuka]

Oh, can you add some test to check output?

[Dor1s]

Added a test

[Dor1s]

Thanks everyone! Good point regarding the test, added!

lib/ubsan/ubsan_flags.cpp

 #undef UBSAN_FLAG
 }
 
 void InitializeFlags() {
   SetCommonFlagsDefaults();
   {
     CommonFlags cf;
     cf.CopyFrom(*common_flags());
-    cf.print_summary = false;
     cf.external_symbolizer_path = GetFlag("UBSAN_SYMBOLIZER_PATH");
     OverrideCommonFlags(cf);
   }
 
   Flags *f = flags();
   f->SetDefaults();
 
   FlagParser parser;

test/ubsan/TestCases/Misc/print_summary.c

+// RUN: %clang -fsanitize=undefined %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT
+// RUN: %env_ubsan_opts=print_summary=0 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NO_SUMMARY
+
+// CHECK-DEFAULT: SUMMARY: UndefinedBehaviorSanitizer: {{.*}}
+// CHECK-NO_SUMMARY-NOT: SUMMARY: UndefinedBehaviorSanitizer: {{.*}}
+
+int main(int argc, char **argv) {
+  int arr[argc - 2];
+  return 0;
+}