[Bug sanitizer/62307] New: -fsanitize=undefined doesn't pay attention to __attribute__((returns_nonnull))

[zackw at panix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62307

            Bug ID: 62307
           Summary: -fsanitize=undefined doesn't pay attention to
                    __attribute__((returns_nonnull))
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zackw at panix dot com
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org

Consider

  extern int *__errno_location(void)
    __attribute__ ((__const__))
    __attribute__ ((__returns_nonnull__));

  void set_errno(int x)
  {
    (*__errno_location()) = x;
  }

(This is a close approximation to how GNU libc defines errno -- there currently
isn't a returns_nonnull annotation, but I'm about to file a bug for _that_ ;-)

With 4.9.1, -fsanitize=undefined -O2 -fdump-tree-optimized, I get

  set_errno (int x)
  {
    int * _1;

    <bb 2>:
    _1 = __errno_location ();
    if (_1 == 0B)
      goto <bb 3>;
    else
      goto <bb 4>;

    <bb 3>:
    __builtin___ubsan_handle_type_mismatch (&*.Lubsan_data0, 0);

    <bb 4>:
    *_1 = x_3(D);
    return;
  }

The returns_nonnull annotation should cause the compiler to delete the runtime
check.  (None of the RTL passes delete it either.)

This type of unnecessary check is the single largest source of calls to
__builtin___ubsan_handle_type_mismatch in a real program.  I see a number of
other cases where we could be doing better at eliminating these checks on "this
pointer is already known to be non-NULL" grounds, e.g.

  if (_191 == 0B)
    goto <bb 17>;
  else
    goto <bb 18>;

  <bb 17>:
  __builtin___ubsan_handle_type_mismatch (&*.Lubsan_data26, 0);

  <bb 18>:
  *_191 = _37;
  if (_191 == 0B)
    goto <bb 19>;
  else
    goto <bb 20>;

  <bb 19>:
  __builtin___ubsan_handle_type_mismatch (&*.Lubsan_data27, 0);

... so maybe it's a pass-ordering problem?  Or rather, the fact that
UBSAN_NULL() is an opaque operation until .sanopt, which prevents nearly all of
the tree optimizers from doing anything with it?