public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ubsan: Fix ICE due to -fsanitize=object-size [PR105093]
@ 2022-03-30  8:41 Jakub Jelinek
  2022-03-30  8:45 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2022-03-30  8:41 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

The following testcase ICEs, because for a volatile X & RESULT_DECL
ubsan wants to take address of that reference.  instrument_object_size
is called with x, so the base is equal to the access and the var
is automatic, so there is no risk of an out of bounds access for it.
Normally we wouldn't instrument those because we fold address of the
t - address of inner to 0, add constant size of the decl and it is
equal to what __builtin_object_size computes.  But the volatile
results in the subtraction not being folded.

The first hunk fixes it by punting if we access the whole automatic
decl, so that even volatile won't cause a problem.
The second hunk (not strictly needed for this testcase) is similar
to what has been added to asan.cc recently, if we actually take
address of a decl and keep it in the IL, we better mark it addressable.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-03-30  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/105093
	* ubsan.cc (instrument_object_size): If t is equal to inner and
	is a decl other than global var, punt.  When emitting call to
	UBSAN_OBJECT_SIZE ifn, make sure base is addressable.

	* g++.dg/ubsan/pr105093.C: New test.

--- gcc/ubsan.cc.jj	2022-02-22 10:38:02.000000000 +0100
+++ gcc/ubsan.cc	2022-03-29 14:02:42.877916597 +0200
@@ -2123,6 +2123,8 @@ instrument_object_size (gimple_stmt_iter
 	   || TREE_CODE (inner) == RESULT_DECL)
 	  && DECL_REGISTER (inner))
 	return;
+      if (t == inner && !is_global_var (t))
+	return;
       base = inner;
     }
   else if (TREE_CODE (inner) == MEM_REF)
@@ -2219,6 +2221,11 @@ instrument_object_size (gimple_stmt_iter
 	}
     }
 
+  if (DECL_P (base)
+      && decl_function_context (base) == current_function_decl
+      && !TREE_ADDRESSABLE (base))
+    mark_addressable (base);
+
   if (bos_stmt && gimple_call_builtin_p (bos_stmt, BUILT_IN_OBJECT_SIZE))
     ubsan_create_edge (bos_stmt);
 
--- gcc/testsuite/g++.dg/ubsan/pr105093.C.jj	2022-03-29 13:34:42.462885890 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr105093.C	2022-03-29 14:22:22.385209238 +0200
@@ -0,0 +1,12 @@
+// PR sanitizer/105093
+// { dg-do compile }
+// { dg-options "-O2 -fsanitize=undefined -Wno-volatile" }
+
+struct X { X (); ~X (); };
+
+volatile X
+foo ()
+{
+  X x;
+  return x;
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ubsan: Fix ICE due to -fsanitize=object-size [PR105093]
  2022-03-30  8:41 [PATCH] ubsan: Fix ICE due to -fsanitize=object-size [PR105093] Jakub Jelinek
@ 2022-03-30  8:45 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2022-03-30  8:45 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Wed, 30 Mar 2022, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because for a volatile X & RESULT_DECL
> ubsan wants to take address of that reference.  instrument_object_size
> is called with x, so the base is equal to the access and the var
> is automatic, so there is no risk of an out of bounds access for it.
> Normally we wouldn't instrument those because we fold address of the
> t - address of inner to 0, add constant size of the decl and it is
> equal to what __builtin_object_size computes.  But the volatile
> results in the subtraction not being folded.
> 
> The first hunk fixes it by punting if we access the whole automatic
> decl, so that even volatile won't cause a problem.
> The second hunk (not strictly needed for this testcase) is similar
> to what has been added to asan.cc recently, if we actually take
> address of a decl and keep it in the IL, we better mark it addressable.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2022-03-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/105093
> 	* ubsan.cc (instrument_object_size): If t is equal to inner and
> 	is a decl other than global var, punt.  When emitting call to
> 	UBSAN_OBJECT_SIZE ifn, make sure base is addressable.
> 
> 	* g++.dg/ubsan/pr105093.C: New test.
> 
> --- gcc/ubsan.cc.jj	2022-02-22 10:38:02.000000000 +0100
> +++ gcc/ubsan.cc	2022-03-29 14:02:42.877916597 +0200
> @@ -2123,6 +2123,8 @@ instrument_object_size (gimple_stmt_iter
>  	   || TREE_CODE (inner) == RESULT_DECL)
>  	  && DECL_REGISTER (inner))
>  	return;
> +      if (t == inner && !is_global_var (t))
> +	return;
>        base = inner;
>      }
>    else if (TREE_CODE (inner) == MEM_REF)
> @@ -2219,6 +2221,11 @@ instrument_object_size (gimple_stmt_iter
>  	}
>      }
>  
> +  if (DECL_P (base)
> +      && decl_function_context (base) == current_function_decl
> +      && !TREE_ADDRESSABLE (base))
> +    mark_addressable (base);
> +
>    if (bos_stmt && gimple_call_builtin_p (bos_stmt, BUILT_IN_OBJECT_SIZE))
>      ubsan_create_edge (bos_stmt);
>  
> --- gcc/testsuite/g++.dg/ubsan/pr105093.C.jj	2022-03-29 13:34:42.462885890 +0200
> +++ gcc/testsuite/g++.dg/ubsan/pr105093.C	2022-03-29 14:22:22.385209238 +0200
> @@ -0,0 +1,12 @@
> +// PR sanitizer/105093
> +// { dg-do compile }
> +// { dg-options "-O2 -fsanitize=undefined -Wno-volatile" }
> +
> +struct X { X (); ~X (); };
> +
> +volatile X
> +foo ()
> +{
> +  X x;
> +  return x;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-03-30  8:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30  8:41 [PATCH] ubsan: Fix ICE due to -fsanitize=object-size [PR105093] Jakub Jelinek
2022-03-30  8:45 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).