[PATCH] ubsan: add 'UndefinedBehaviorSanitizer' to messages

[Ben Boeckel]

>>! In D5629#9, @samsonov wrote:
> I'm opposed to this change. The error messages you suggest are occasionaly confusing
>   UndefinedBehaviorSanitizer function error
> or way too redundant
>   UndefinedBehaviorSanitizer unsigned-integer-overflow error: unsigned integer overflow:
> 
> Also, if UBSan is combined with another sanitizer (most notably, ASan), we treat the latter as the "main" tool, and don't mention UBSan in error reports. For instance,
> look at the SUMMARY: line that would be emiited in this case:
>   SUMMARY: AddressSanitizer: undefined-behavior: file.cc:10

The test cases don't specialize for UBSan and UBSan+ASan together and pass, so I don't know that this is accurate (or you're referring to something else).

> I'd prefer to replace generic "runtime error" with generic "undefined-behavior" (or at least make it a common prefix). I also like the idea of specifying the exact -fsanitize= flag that caused the error (what Richard suggests), if that's easily doable.

The string is the sanitize flag which caused the error, just placed next to "UndefinedBehaviorSanitizer" like it is with AddressSanitizer. I'm not opposed to putting it in parentheses or brackets to distinguish it, but ASan doesn't do this (though ASan is also just ASan and not a group of other sanitizers):

> ERROR: AddressSanitizer heap-use-after-free on address

Error messages could certainly be cleaned up to be have less duplication as well.

================
Comment at: lib/ubsan/ubsan_handlers.cc:19
@@ -18,1 +18,3 @@
 
+#include <string.h>
+
----------------
samsonov wrote:
> It's not allowed to #include system headers into generic sanitizer runtimes.
OK. I'll make the TypeDescriptor aware of bool then which would make this unnecessary.

================
Comment at: lib/ubsan/ubsan_handlers.cc:348
@@ +347,3 @@
+  const char *Name;
+  if (!strcmp(Data->Type.getTypeName(), "'bool'")) // XXX(mathstuf): Is there a better way to do this?
+    Name = "bool";
----------------
samsonov wrote:
> This is wrong, this type of check is also emitted for bool-like types.
Such as? Would 'enum' not be accurate for that? In any case, rsmith did mention (at the WG21 meeting) that TypeDescriptor should probably grow support for 'bool' since its sizeof() is likely 8 and therefore indistinguishable that way.

http://reviews.llvm.org/D5629