[patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Siddhesh Poyarekar]

Use __builtin_dynamic_object_size to get object sizes for ubsan.

gcc/ChangeLog:

	middle-end/70090
	* ubsan.cc (ubsan_expand_objsize_ifn): Allow non-constant SIZE.
	(instrument_object_size): Get dynamic object size expression.

gcc/testsuite/ChangeLog:

	middle-end/70090
	* gcc.dg/ubsan/object-size-dyn.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Proposing for gcc13 since I reckoned this is not feasible for stage 4.

Tested with:
- ubsan bootstrap config on x86_64
- bootstrap build and test on x86_64
- non-bootstrap build and test with i686

 gcc/testsuite/gcc.dg/ubsan/object-size-dyn.c | 45 ++++++++++++++++++++
 gcc/ubsan.cc                                 | 13 +++---
 2 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/object-size-dyn.c

diff --git a/gcc/testsuite/gcc.dg/ubsan/object-size-dyn.c b/gcc/testsuite/gcc.dg/ubsan/object-size-dyn.c
new file mode 100644
index 00000000000..0159f5b9820
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/object-size-dyn.c
@@ -0,0 +1,45 @@
+/* { dg-do run } */
+/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */
+/* { dg-options "-fsanitize=undefined" } */
+#include <stdio.h>
+
+int
+__attribute__ ((noinline))
+dyn (int size, int i)
+{
+  __builtin_printf ("dyn\n");
+  fflush (stdout);
+  int *alloc = __builtin_calloc (size, sizeof (int));
+  int ret = alloc[i];
+  __builtin_free (alloc);
+  return ret;
+}
+
+int
+__attribute__ ((noinline))
+off (int size, int i, int ret)
+{
+  char *mem = __builtin_alloca (size);
+  mem += size - 1;
+
+  return (int) mem[i] & ret;
+}
+
+int
+main (void)
+{
+  int ret = dyn (2, 2);
+
+  ret |= off (4, 4, 0);
+
+  return ret;
+}
+
+/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\\^" } */
diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc
index 5641d3cc3be..11dad4f1095 100644
--- a/gcc/ubsan.cc
+++ b/gcc/ubsan.cc
@@ -942,8 +942,8 @@ ubsan_expand_objsize_ifn (gimple_stmt_iterator *gsi)
   gimple *g;
 
   /* See if we can discard the check.  */
-  if (TREE_CODE (size) != INTEGER_CST
-      || integer_all_onesp (size))
+  if (TREE_CODE (size) == INTEGER_CST
+      && integer_all_onesp (size))
     /* Yes, __builtin_object_size couldn't determine the
        object size.  */;
   else if (TREE_CODE (offset) == INTEGER_CST
@@ -2160,14 +2160,14 @@ instrument_object_size (gimple_stmt_iterator *gsi, tree t, bool is_lhs)
   if (decl_p)
     base_addr = build1 (ADDR_EXPR,
 			build_pointer_type (TREE_TYPE (base)), base);
-  if (compute_builtin_object_size (base_addr, 0, &sizet))
+  if (compute_builtin_object_size (base_addr, OST_DYNAMIC, &sizet))
     ;
   else if (optimize)
     {
       if (LOCATION_LOCUS (loc) == UNKNOWN_LOCATION)
 	loc = input_location;
-      /* Generate __builtin_object_size call.  */
-      sizet = builtin_decl_explicit (BUILT_IN_OBJECT_SIZE);
+      /* Generate __builtin_dynamic_object_size call.  */
+      sizet = builtin_decl_explicit (BUILT_IN_DYNAMIC_OBJECT_SIZE);
       sizet = build_call_expr_loc (loc, sizet, 2, base_addr,
 				   integer_zero_node);
       sizet = force_gimple_operand_gsi (gsi, sizet, false, NULL_TREE, true,
@@ -2219,7 +2219,8 @@ instrument_object_size (gimple_stmt_iterator *gsi, tree t, bool is_lhs)
 	}
     }
 
-  if (bos_stmt && gimple_call_builtin_p (bos_stmt, BUILT_IN_OBJECT_SIZE))
+  if (bos_stmt
+      && gimple_call_builtin_p (bos_stmt, BUILT_IN_DYNAMIC_OBJECT_SIZE))
     ubsan_create_edge (bos_stmt);
 
   /* We have to emit the check.  */
-- 
2.34.1

Re: [patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Jakub Jelinek]

On Mon, Feb 07, 2022 at 05:31:58PM +0530, Siddhesh Poyarekar wrote:
> Use __builtin_dynamic_object_size to get object sizes for ubsan.
> 
> gcc/ChangeLog:
> 
> 	middle-end/70090
> 	* ubsan.cc (ubsan_expand_objsize_ifn): Allow non-constant SIZE.
> 	(instrument_object_size): Get dynamic object size expression.
> 
> gcc/testsuite/ChangeLog:
> 
> 	middle-end/70090
> 	* gcc.dg/ubsan/object-size-dyn.c: New test.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> Proposing for gcc13 since I reckoned this is not feasible for stage 4.

Ok for stage1.

	Jakub

Re: [patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Siddhesh Poyarekar]

On 07/02/2022 17:37, Jakub Jelinek wrote:
> On Mon, Feb 07, 2022 at 05:31:58PM +0530, Siddhesh Poyarekar wrote:
>> Use __builtin_dynamic_object_size to get object sizes for ubsan.
>>
>> gcc/ChangeLog:
>>
>> 	middle-end/70090
>> 	* ubsan.cc (ubsan_expand_objsize_ifn): Allow non-constant SIZE.
>> 	(instrument_object_size): Get dynamic object size expression.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	middle-end/70090
>> 	* gcc.dg/ubsan/object-size-dyn.c: New test.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
>> ---
>> Proposing for gcc13 since I reckoned this is not feasible for stage 4.
> 
> Ok for stage1.
> 
> 	Jakub
> 

Hi Jakub, may I rebase and push this now?

Thanks,
Siddhesh

Re: [patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Jakub Jelinek]

On Mon, May 09, 2022 at 01:02:07PM +0530, Siddhesh Poyarekar wrote:
> On 07/02/2022 17:37, Jakub Jelinek wrote:
> > On Mon, Feb 07, 2022 at 05:31:58PM +0530, Siddhesh Poyarekar wrote:
> > > Use __builtin_dynamic_object_size to get object sizes for ubsan.
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 	middle-end/70090
> > > 	* ubsan.cc (ubsan_expand_objsize_ifn): Allow non-constant SIZE.
> > > 	(instrument_object_size): Get dynamic object size expression.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	middle-end/70090
> > > 	* gcc.dg/ubsan/object-size-dyn.c: New test.
> > > 
> > > Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> > > ---
> > > Proposing for gcc13 since I reckoned this is not feasible for stage 4.
> > 
> > Ok for stage1.
> > 
> > 	Jakub
> > 
> 
> Hi Jakub, may I rebase and push this now?

Yes.

	Jakub

Re: [patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Martin Liška]

On 5/9/22 09:37, Jakub Jelinek via Gcc-patches wrote:
> On Mon, May 09, 2022 at 01:02:07PM +0530, Siddhesh Poyarekar wrote:
>> On 07/02/2022 17:37, Jakub Jelinek wrote:
>>> On Mon, Feb 07, 2022 at 05:31:58PM +0530, Siddhesh Poyarekar wrote:
>>>> Use __builtin_dynamic_object_size to get object sizes for ubsan.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 	middle-end/70090
>>>> 	* ubsan.cc (ubsan_expand_objsize_ifn): Allow non-constant SIZE.
>>>> 	(instrument_object_size): Get dynamic object size expression.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	middle-end/70090
>>>> 	* gcc.dg/ubsan/object-size-dyn.c: New test.
>>>>
>>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
>>>> ---
>>>> Proposing for gcc13 since I reckoned this is not feasible for stage 4.
>>>
>>> Ok for stage1.
>>>
>>> 	Jakub
>>>
>>
>> Hi Jakub, may I rebase and push this now?
> 
> Yes.
> 
> 	Jakub
> 

Hi.

The revision caused:

$ ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/ubsan/bind-c-intent-out-2.f90 -fsanitize=undefined -c -O
during GIMPLE pass: ubsan
/home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/ubsan/bind-c-intent-out-2.f90:39:19:

   39 | end program alloc_p
      |                   ^
internal compiler error: Segmentation fault
0x14b45c7 crash_signal
	/home/marxin/Programming/gcc/gcc/toplev.cc:322
0x7ffff78b83cf ???
	/usr/src/debug/glibc-2.35-2.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0xc2cf10 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
	/home/marxin/Programming/gcc/gcc/tree.h:3570
0x19100d1 build_call_expr_loc_array(unsigned int, tree_node*, int, tree_node**)
	/home/marxin/Programming/gcc/gcc/tree.cc:10629
0x19102ff build_call_expr_loc(unsigned int, tree_node*, int, ...)
	/home/marxin/Programming/gcc/gcc/tree.cc:10662
0x14f59a3 instrument_object_size
	/home/marxin/Programming/gcc/gcc/ubsan.cc:2173
0x14f6770 execute
	/home/marxin/Programming/gcc/gcc/ubsan.cc:2428
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

Martin

Re: [patch gcc13] middle-end/70090: Dynamic sizes for -fsanitize=object-size

[Siddhesh Poyarekar]

On 10/05/2022 16:16, Martin Liška wrote:
> The revision caused:
> 
> $ ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/ubsan/bind-c-intent-out-2.f90 -fsanitize=undefined -c -O
> during GIMPLE pass: ubsan
> /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/ubsan/bind-c-intent-out-2.f90:39:19:
> 
>     39 | end program alloc_p
>        |                   ^
> internal compiler error: Segmentation fault
> 0x14b45c7 crash_signal
> 	/home/marxin/Programming/gcc/gcc/toplev.cc:322
> 0x7ffff78b83cf ???
> 	/usr/src/debug/glibc-2.35-2.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0xc2cf10 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
> 	/home/marxin/Programming/gcc/gcc/tree.h:3570
> 0x19100d1 build_call_expr_loc_array(unsigned int, tree_node*, int, tree_node**)
> 	/home/marxin/Programming/gcc/gcc/tree.cc:10629
> 0x19102ff build_call_expr_loc(unsigned int, tree_node*, int, ...)
> 	/home/marxin/Programming/gcc/gcc/tree.cc:10662
> 0x14f59a3 instrument_object_size
> 	/home/marxin/Programming/gcc/gcc/ubsan.cc:2173
> 0x14f6770 execute
> 	/home/marxin/Programming/gcc/gcc/ubsan.cc:2428
> Please submit a full bug report, with preprocessed source (by using -freport-bug).
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.

Thanks, I just noticed that my non-ubsan bootstrap didn't enable 
sanitizers because of which I didn't see this at all.  I'm testing a fix 
and I'll post it once bootstraps finish.

Siddhesh

[PATCH] middle-end/70090: Register __bdos for sanitizers if necessary

[Siddhesh Poyarekar]

The asan initializer registers __builtin_object_size for languages that
don't have it, e.g. Fortran.  Register __builtin_dynamic_object_size too
(we need both because __builtin_dynamic_object_size computation may
involve generating __builtin_object_size as a fallback) so that
gfortran.dg/ubsan/bind-c-intent-out-2.f90 does not crash anymore.

gcc/ChangeLog:

	PR middle-end/70090
	* asan.cc (initialize_sanitizer_builtins): Register
	__builtin_dynamic_object_size if necessary.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Testing:
- I realized that for some reason I was looking only at gcc.log in the
  testsuite, so expanded my checks to look at failures in the entire check
  output.  Verified that gfortran.dg/ubsan/bind-c-intent-out-2.f90 failed on
  master and passed with this patch.
- Bootstrapped and tested on x86_64
- Bootstrapped --with-build-config=bootstrap-ubsan
- i686 build and test

 gcc/asan.cc | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/gcc/asan.cc b/gcc/asan.cc
index ef59b77ebc2..4b583e54efd 100644
--- a/gcc/asan.cc
+++ b/gcc/asan.cc
@@ -3457,14 +3457,22 @@ initialize_sanitizer_builtins (void)
 
 #include "sanitizer.def"
 
-  /* -fsanitize=object-size uses __builtin_object_size, but that might
-     not be available for e.g. Fortran at this point.  We use
-     DEF_SANITIZER_BUILTIN here only as a convenience macro.  */
-  if ((flag_sanitize & SANITIZE_OBJECT_SIZE)
-      && !builtin_decl_implicit_p (BUILT_IN_OBJECT_SIZE))
-    DEF_SANITIZER_BUILTIN_1 (BUILT_IN_OBJECT_SIZE, "object_size",
-			     BT_FN_SIZE_CONST_PTR_INT,
-			     ATTR_PURE_NOTHROW_LEAF_LIST);
+  /* -fsanitize=object-size uses __builtin_dynamic_object_size and
+     __builtin_object_size, but they might not be available for e.g. Fortran at
+     this point.  We use DEF_SANITIZER_BUILTIN here only as a convenience
+     macro.  */
+  if (flag_sanitize & SANITIZE_OBJECT_SIZE)
+    {
+      if (!builtin_decl_implicit_p (BUILT_IN_OBJECT_SIZE))
+	DEF_SANITIZER_BUILTIN_1 (BUILT_IN_OBJECT_SIZE, "object_size",
+				 BT_FN_SIZE_CONST_PTR_INT,
+				 ATTR_PURE_NOTHROW_LEAF_LIST);
+      if (!builtin_decl_implicit_p (BUILT_IN_DYNAMIC_OBJECT_SIZE))
+	DEF_SANITIZER_BUILTIN_1 (BUILT_IN_DYNAMIC_OBJECT_SIZE,
+				 "dynamic_object_size",
+				 BT_FN_SIZE_CONST_PTR_INT,
+				 ATTR_PURE_NOTHROW_LEAF_LIST);
+    }
 
 #undef DEF_SANITIZER_BUILTIN_1
 #undef DEF_SANITIZER_BUILTIN
-- 
2.35.1

Re: [PATCH] middle-end/70090: Register __bdos for sanitizers if necessary

[Jakub Jelinek]

On Wed, May 11, 2022 at 07:41:31AM +0530, Siddhesh Poyarekar wrote:
> The asan initializer registers __builtin_object_size for languages that
> don't have it, e.g. Fortran.  Register __builtin_dynamic_object_size too
> (we need both because __builtin_dynamic_object_size computation may
> involve generating __builtin_object_size as a fallback) so that
> gfortran.dg/ubsan/bind-c-intent-out-2.f90 does not crash anymore.
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/70090
> 	* asan.cc (initialize_sanitizer_builtins): Register
> 	__builtin_dynamic_object_size if necessary.

Okay.

	Jakub