[Bug sanitizer/60275] New: [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[Bug sanitizer/60275] New: [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

            Bug ID: 60275
           Summary: [UBSAN] Add
                    -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-e
                    rror to make UBSAN's runtime errors fatal
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                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,
                    mpolacek at gcc dot gnu.org

While I personally would like to see more fine tuning via UBSAN_FLAGS - similar
to ASAN, LSAN and TSAN, adding CLANG's -fsanitize-recover/-fno-sanitize-recover
and  -fsanitize-undefined-trap-on-error would be useful as additional feature.

>From CLANG:

   Extra features of UndefinedBehaviorSanitizer:

   -  ``-fno-sanitize-recover``: By default, after a sanitizer diagnoses
      an issue, it will attempt to continue executing the program if there
      is a reasonable behavior it can give to the faulting operation. This
      option causes the program to abort instead.
   -  ``-fsanitize-undefined-trap-on-error``: Causes traps to be emitted
      rather than calls to runtime libraries when a problem is detected.
      This option is intended for use in cases where the sanitizer runtime
      cannot be used (for instance, when building libc or a kernel module).
      This is only compatible with the sanitizers in the ``undefined-trap``
      group.

That would be BUILT_IN_UNREACHABLE and BUILT_IN_TRAP. (But unreachable
shouldn't be dressed by SANITIZE_UNREACHABLE ;-)

See also LLVM's
* tools/clang/docs/UsersManual.rst
* tools/clang/lib/CodeGen/CGExpr.cpp (search for SanitizeUndefinedTrapOnError
and SanitizeRecover)

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-02-19
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Mine.  I think this is 5.0 material.

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Post script: CLANG has:
  '-fsanitize=undefined' not allowed with '-fsanitize-undefined-trap-on-error'

And regarding the function call: With -fno-sanitize-recover one simply appends
an "_abort" to the function call, i.e. __ubsan_handle_add_overflow becomes
__ubsan_handle_add_overflow_abort. [For all functions but
__ubsan_handle_builtin_unreachable and __ubsan_handle_missing_return, which
always abort / Die() themselves.]

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess we shouldn't copy the design mistakes clang makes.  What action to take
on detected undefined behavior should be orthogonal to how to report it
(runtime error message with recovery, fatal runtime error message or abort
without error message).

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> I guess we shouldn't copy the design mistakes clang makes.  What action to
> take on detected undefined behavior should be orthogonal to how to report it
> (runtime error message with recovery, fatal runtime error message or abort
> without error message).

The question is how to implement this properly. One way is using environment
variables such as ASAN and TSAN do, which have environment variables, e.g. ASAN
has https://code.google.com/p/address-sanitizer/wiki/Flags#Run-time_flags ,
which permits:

ASAN_OPTIONS=
  abort_on_error (default 0)
  exitcode (default 1)
etc.

It would be also nice to have a back trace. (I had an always inlined add
function, which overflows and there pointing to the header file does not help
much.)


Still, it is also nice to be able to tell at compile time that failures should
be fatal as one can easily forget to set the environment variable. If one does
not want to go the route of Clang, I wonder how to handle it instead as one
does not have one initializing call to the library.

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 32597
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32597&action=edit
gcc410-pr60275.patch

Untested fix.  Marek, sorry for hacking this up, but I'd like to backport it to
4.9-RH soon, some people complained about the lack of these features.  I'll
leave all other ubsan hacking to you.

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed Apr 23 08:20:12 2014
New Revision: 209672

URL: http://gcc.gnu.org/viewcvs?rev=209672&root=gcc&view=rev
Log:
    PR sanitizer/60275
    * common.opt (fsanitize-recover, fsanitize-undefined-trap-on-error):
    New options.
    * gcc.c (sanitize_spec_function): Don't return "" for "undefined"
    if flag_sanitize_undefined_trap_on_error.
    * sanitizer.def (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT,
    BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT,
    BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE_ABORT,
    BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH_ABORT,
    BUILT_IN_UBSAN_HANDLE_ADD_OVERFLOW_ABORT,
    BUILT_IN_UBSAN_HANDLE_SUB_OVERFLOW_ABORT,
    BUILT_IN_UBSAN_HANDLE_MUL_OVERFLOW_ABORT,
    BUILT_IN_UBSAN_HANDLE_NEGATE_OVERFLOW_ABORT,
    BUILT_IN_UBSAN_HANDLE_LOAD_INVALID_VALUE_ABORT): New builtins.
    * ubsan.c (ubsan_instrument_unreachable): Return
    __builtin_trap () if flag_sanitize_undefined_trap_on_error.
    (ubsan_expand_null_ifn): Emit __builtin_trap ()
    if flag_sanitize_undefined_trap_on_error and
    __ubsan_handle_type_mismatch_abort if !flag_sanitize_recover.
    (ubsan_expand_null_ifn, ubsan_build_overflow_builtin,
    instrument_bool_enum_load): Emit __builtin_trap () if
    flag_sanitize_undefined_trap_on_error and
    __builtin_handle_*_abort () if !flag_sanitize_recover.
    * doc/invoke.texi (-fsanitize-recover,
    -fsanitize-undefined-trap-on-error): Document.
c-family/
    * c-ubsan.c (ubsan_instrument_return): Return __builtin_trap ()
    if flag_sanitize_undefined_trap_on_error.
    (ubsan_instrument_division, ubsan_instrument_shift,
    ubsan_instrument_vla): Likewise.  Use __ubsan_handle_*_abort ()
    if !flag_sanitize_recover.
testsuite/
    * g++.dg/ubsan/return-2.C: Revert 2014-03-24 changes, add
    -fno-sanitize-recover to dg-options.
    * g++.dg/ubsan/cxx11-shift-1.C: Remove c++11 target restriction,
    add -std=c++11 to dg-options.
    * g++.dg/ubsan/cxx11-shift-2.C: Likewise.
    * g++.dg/ubsan/cxx1y-vla.C: Remove c++1y target restriction,
    add -std=c++1y to dg-options.
    * c-c++-common/ubsan/undefined-1.c: Revert 2014-03-24 changes, add
    -fno-sanitize-recover to dg-options.
    * c-c++-common/ubsan/overflow-sub-1.c: Likewise.
    * c-c++-common/ubsan/vla-4.c: Likewise.
    * c-c++-common/ubsan/pr59503.c: Likewise.
    * c-c++-common/ubsan/vla-3.c: Likewise.
    * c-c++-common/ubsan/save-expr-1.c: Likewise.
    * c-c++-common/ubsan/overflow-add-1.c: Likewise.
    * c-c++-common/ubsan/shift-3.c: Likewise.
    * c-c++-common/ubsan/overflow-1.c: Likewise.
    * c-c++-common/ubsan/overflow-negate-2.c: Likewise.
    * c-c++-common/ubsan/vla-2.c: Likewise.
    * c-c++-common/ubsan/overflow-mul-1.c: Likewise.
    * c-c++-common/ubsan/pr60613-1.c: Likewise.
    * c-c++-common/ubsan/shift-6.c: Likewise.
    * c-c++-common/ubsan/overflow-mul-3.c: Likewise.
    * c-c++-common/ubsan/overflow-add-3.c: New test.
    * c-c++-common/ubsan/overflow-add-4.c: New test.
    * c-c++-common/ubsan/div-by-zero-6.c: New test.
    * c-c++-common/ubsan/div-by-zero-7.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/ubsan/div-by-zero-6.c
    trunk/gcc/testsuite/c-c++-common/ubsan/div-by-zero-7.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-add-3.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-add-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-ubsan.c
    trunk/gcc/common.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/gcc.c
    trunk/gcc/sanitizer.def
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-add-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-mul-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-mul-3.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-negate-2.c
    trunk/gcc/testsuite/c-c++-common/ubsan/overflow-sub-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/pr59503.c
    trunk/gcc/testsuite/c-c++-common/ubsan/pr60613-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/save-expr-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/shift-3.c
    trunk/gcc/testsuite/c-c++-common/ubsan/shift-6.c
    trunk/gcc/testsuite/c-c++-common/ubsan/undefined-1.c
    trunk/gcc/testsuite/c-c++-common/ubsan/vla-2.c
    trunk/gcc/testsuite/c-c++-common/ubsan/vla-3.c
    trunk/gcc/testsuite/c-c++-common/ubsan/vla-4.c
    trunk/gcc/testsuite/g++.dg/ubsan/cxx11-shift-1.C
    trunk/gcc/testsuite/g++.dg/ubsan/cxx11-shift-2.C
    trunk/gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C
    trunk/gcc/testsuite/g++.dg/ubsan/return-2.C
    trunk/gcc/ubsan.c

[Bug sanitizer/60275] [UBSAN] Add -f[no-]sanitize-recover/-fsanitize-undefined-trap-on-error to make UBSAN's runtime errors fatal

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60275

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Implemented now.