Patchwork Request to merge Undefined Behavior Sanitizer in (take 2)

login
register
mail settings
Submitter Marek Polacek
Date Aug. 7, 2013, 4:12 p.m.
Message ID <20130807161258.GF17022@redhat.com>
Download mbox | patch
Permalink /patch/265559/
State New
Headers show

Comments

Marek Polacek - Aug. 7, 2013, 4:12 p.m.
On Wed, Aug 07, 2013 at 10:24:59AM -0400, Jason Merrill wrote:
> On 08/07/2013 06:06 AM, Marek Polacek wrote:
> >I might've misunderstood what you mean.  If we drop the hunk above,
> >then we'll call
> >     error ("%q+E is not a constant expression", t);
> >so, we'll print "is not a constant expression" no matter what
> 
> Yes, that's fine; 1/0 is not a constant expression, because it has
> undefined behavior.

Indeed.

> Print something more meaningful to the user.

Something along these lines?  (Not including CL/testcase yet on purpose.)
In beforementioned situation it'd print:

w.C:7:22: error: ‘<ubsan routine call>’ is not a constant expression
       case 0 * (1 / 0):
                      ^
I'm not entirely happy about it, on the other hand, this situation
should be very rare...

	Marek
Marek Polacek - Aug. 15, 2013, 8 a.m.
Ping.

On Wed, Aug 07, 2013 at 06:12:58PM +0200, Marek Polacek wrote:
> On Wed, Aug 07, 2013 at 10:24:59AM -0400, Jason Merrill wrote:
> > On 08/07/2013 06:06 AM, Marek Polacek wrote:
> > >I might've misunderstood what you mean.  If we drop the hunk above,
> > >then we'll call
> > >     error ("%q+E is not a constant expression", t);
> > >so, we'll print "is not a constant expression" no matter what
> > 
> > Yes, that's fine; 1/0 is not a constant expression, because it has
> > undefined behavior.
> 
> Indeed.
> 
> > Print something more meaningful to the user.
> 
> Something along these lines?  (Not including CL/testcase yet on purpose.)
> In beforementioned situation it'd print:
> 
> w.C:7:22: error: ‘<ubsan routine call>’ is not a constant expression
>        case 0 * (1 / 0):
>                       ^
> I'm not entirely happy about it, on the other hand, this situation
> should be very rare...
> 
> diff --git a/gcc/cp/error.c b/gcc/cp/error.c
> index 440169a..db50b5f 100644
> --- a/gcc/cp/error.c
> +++ b/gcc/cp/error.c
> @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-pretty-print.h"
>  #include "pointer-set.h"
>  #include "c-family/c-objc.h"
> +#include "ubsan.h"
>  
>  #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',')
>  #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';')
> @@ -1972,6 +1973,12 @@ dump_expr (tree t, int flags)
>  	      }
>  	    skipfirst = true;
>  	  }
> +	if (flag_sanitize & SANITIZE_UNDEFINED
> +	    && is_ubsan_builtin_p (fn))
> +	  {
> +	    pp_string (cxx_pp, M_("<ubsan routine call>"));
> +	    break;
> +	  }
>  	dump_expr (fn, flags | TFF_EXPR_IN_PARENS);
>  	dump_call_expr_args (t, flags, skipfirst);
>        }
> diff --git a/gcc/ubsan.c b/gcc/ubsan.c
> index 8135cc9..565758d 100644
> --- a/gcc/ubsan.c
> +++ b/gcc/ubsan.c
> @@ -456,3 +456,13 @@ ubsan_instrument_unreachable (location_t loc)
>    tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE);
>    return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
>  }
> +
> +/* Return true if T is a call to a libubsan routine.  */
> +
> +bool
> +is_ubsan_builtin_p (tree t)
> +{
> +  gcc_checking_assert (TREE_CODE (t) == FUNCTION_DECL);
> +  return strncmp (IDENTIFIER_POINTER (DECL_NAME (t)),
> +		  "__builtin___ubsan_", 18) == 0;
> +}
> diff --git a/gcc/ubsan.h b/gcc/ubsan.h
> index abf4f5d..3553a6c 100644
> --- a/gcc/ubsan.h
> +++ b/gcc/ubsan.h
> @@ -25,6 +25,7 @@ extern tree ubsan_instrument_unreachable (location_t);
>  extern tree ubsan_create_data (const char *, location_t, ...);
>  extern tree ubsan_type_descriptor (tree);
>  extern tree ubsan_encode_value (tree);
> +extern bool is_ubsan_builtin_p (tree);
>  
>  #endif  /* GCC_UBSAN_H  */

	Marek
Jason Merrill - Aug. 15, 2013, 3:29 p.m.
OK.

Jason

Patch

diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 440169a..db50b5f 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -32,6 +32,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-pretty-print.h"
 #include "pointer-set.h"
 #include "c-family/c-objc.h"
+#include "ubsan.h"
 
 #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',')
 #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';')
@@ -1972,6 +1973,12 @@  dump_expr (tree t, int flags)
 	      }
 	    skipfirst = true;
 	  }
+	if (flag_sanitize & SANITIZE_UNDEFINED
+	    && is_ubsan_builtin_p (fn))
+	  {
+	    pp_string (cxx_pp, M_("<ubsan routine call>"));
+	    break;
+	  }
 	dump_expr (fn, flags | TFF_EXPR_IN_PARENS);
 	dump_call_expr_args (t, flags, skipfirst);
       }
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 8135cc9..565758d 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -456,3 +456,13 @@  ubsan_instrument_unreachable (location_t loc)
   tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE);
   return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
 }
+
+/* Return true if T is a call to a libubsan routine.  */
+
+bool
+is_ubsan_builtin_p (tree t)
+{
+  gcc_checking_assert (TREE_CODE (t) == FUNCTION_DECL);
+  return strncmp (IDENTIFIER_POINTER (DECL_NAME (t)),
+		  "__builtin___ubsan_", 18) == 0;
+}
diff --git a/gcc/ubsan.h b/gcc/ubsan.h
index abf4f5d..3553a6c 100644
--- a/gcc/ubsan.h
+++ b/gcc/ubsan.h
@@ -25,6 +25,7 @@  extern tree ubsan_instrument_unreachable (location_t);
 extern tree ubsan_create_data (const char *, location_t, ...);
 extern tree ubsan_type_descriptor (tree);
 extern tree ubsan_encode_value (tree);
+extern bool is_ubsan_builtin_p (tree);
 
 #endif  /* GCC_UBSAN_H  */