Correctly sanitize nonnull pointers. (#834)

Only the special nullability attributes (`_Nonnull`) work correctly
through type aliases like we're using with `Ptr`. But they aren't
strictly UB and so have to be specially enabled in our sanitizer config
in order to usefully catch nullness errors early. Turn on those
sanitizers as well.

Also, now that we are using fully remote build output caching for our
CI and not trying to squeeze under an arbitrary size limit, re-enable
the nice error messages for all the UBSan checks.

Note that this will have a (very) slow CI run as it will have to
recompile ~everything and upload fresh artifacts. But those should then
be effective cache hits going forward.

bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -444,7 +444,7 @@ def _impl(ctx):
         flag_sets = [flag_set(
             actions = all_compile_actions + all_link_actions,
             flag_groups = [flag_group(flags = [
-                "-fsanitize=address,undefined",
+                "-fsanitize=address,undefined,nullability",
                 "-fsanitize-address-use-after-scope",
                 # We don't need the recovery behavior of UBSan as we expect
                 # builds to be clean. Not recoverying is a bit cheaper.
@@ -453,10 +453,6 @@ def _impl(ctx):
                 # and combined with line numbers is unlikely to result in many
                 # ambiguities.
                 "-fsanitize-undefined-strip-path-components=-1",
-                # Force some expensive UBSan checks to the cheaper trap mode.
-                # The dedicated debugging message is unlikely to be critical for
-                # these.
-                "-fsanitize-trap=alignment,bool,null,return,unreachable",
                 # Needed due to clang AST issues, such as in
                 # clang/AST/Redeclarable.h line 199.
                 "-fno-sanitize=vptr",

executable_semantics/common/ptr.h

@@ -11,12 +11,11 @@ namespace Carbon {
 
 // A non-nullable pointer. Written as `Nonnull<T*>` instead of `T*`.
 //
-// Note LLVM primarily enforces the attribute on function calls that can be
-// proven to be called with nullptr; in other places, this is essentially a
-// comment.
+// Sanitizers enforce this dynamically on assignment, return, and when passing
+// as an argument. Static analysis will also track erroneous uses of `nullptr`.
 template <typename T,
           typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
-using Nonnull = T _Nonnull __attribute__((nonnull));
+using Nonnull = T _Nonnull;
 
 }  // namespace Carbon