public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Do not pass NULL to memset in ssa_global_cache.
@ 2021-11-14 13:15 Aldy Hernandez
  2021-11-14 13:53 ` Martin Liška
  0 siblings, 1 reply; 3+ messages in thread
From: Aldy Hernandez @ 2021-11-14 13:15 UTC (permalink / raw)
  To: GCC patches

The code computing ranges in PHIs in the path solver reuses the
temporary ssa_global_cache by calling its clear method.  Calling it on
an empty cache causes us to call memset with NULL.

[The testcase doesn't fail without the patch.  I suppose it needs some
usbsan magic, or to live somewhere else?]

Tested on x86-64 Linux.

gcc/ChangeLog:

	PR tree-optimization/103229
	* gimple-range-cache.cc (ssa_global_cache::clear): Do not pass
	null value to memset.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr103229.c: New test.
---
 gcc/gimple-range-cache.cc       |  3 ++-
 gcc/testsuite/gcc.dg/pr103229.c | 10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103229.c

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index a63e20e7e49..b347edeb474 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -651,7 +651,8 @@ ssa_global_cache::clear_global_range (tree name)
 void
 ssa_global_cache::clear ()
 {
-  memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
+  if (m_tab.address ())
+    memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
 }
 
 // Dump the contents of the global cache to F.
diff --git a/gcc/testsuite/gcc.dg/pr103229.c b/gcc/testsuite/gcc.dg/pr103229.c
new file mode 100644
index 00000000000..96ef9aff67c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103229.c
@@ -0,0 +1,10 @@
+// { dg-do compile }
+// { dg-options "-O -w" }
+
+int main() {
+  int i;
+  for (; i;)
+    ;
+
+  return 0;
+}
-- 
2.31.1


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

* Re: [COMMITTED] Do not pass NULL to memset in ssa_global_cache.
  2021-11-14 13:15 [COMMITTED] Do not pass NULL to memset in ssa_global_cache Aldy Hernandez
@ 2021-11-14 13:53 ` Martin Liška
  2021-11-14 15:18   ` Aldy Hernandez
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Liška @ 2021-11-14 13:53 UTC (permalink / raw)
  To: Aldy Hernandez, GCC patches

On 11/14/21 14:15, Aldy Hernandez wrote:
> The code computing ranges in PHIs in the path solver reuses the
> temporary ssa_global_cache by calling its clear method.  Calling it on
> an empty cache causes us to call memset with NULL.
> 
> [The testcase doesn't fail without the patch.  I suppose it needs some
> usbsan magic, or to live somewhere else?]

Well, the actual test-case is the compiler itself as the source code.
Anyway, the UBSAN error happens for thousands of test-cases when run
during bootstrap-ubsan.mk config file. That said, I would remove
the added test-case.

Cheers,
Martin

> 
> Tested on x86-64 Linux.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/103229
> 	* gimple-range-cache.cc (ssa_global_cache::clear): Do not pass
> 	null value to memset.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pr103229.c: New test.
> ---
>   gcc/gimple-range-cache.cc       |  3 ++-
>   gcc/testsuite/gcc.dg/pr103229.c | 10 ++++++++++
>   2 files changed, 12 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.dg/pr103229.c
> 
> diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> index a63e20e7e49..b347edeb474 100644
> --- a/gcc/gimple-range-cache.cc
> +++ b/gcc/gimple-range-cache.cc
> @@ -651,7 +651,8 @@ ssa_global_cache::clear_global_range (tree name)
>   void
>   ssa_global_cache::clear ()
>   {
> -  memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
> +  if (m_tab.address ())
> +    memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
>   }
>   
>   // Dump the contents of the global cache to F.
> diff --git a/gcc/testsuite/gcc.dg/pr103229.c b/gcc/testsuite/gcc.dg/pr103229.c
> new file mode 100644
> index 00000000000..96ef9aff67c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr103229.c
> @@ -0,0 +1,10 @@
> +// { dg-do compile }
> +// { dg-options "-O -w" }
> +
> +int main() {
> +  int i;
> +  for (; i;)
> +    ;
> +
> +  return 0;
> +}
> 


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

* Re: [COMMITTED] Do not pass NULL to memset in ssa_global_cache.
  2021-11-14 13:53 ` Martin Liška
@ 2021-11-14 15:18   ` Aldy Hernandez
  0 siblings, 0 replies; 3+ messages in thread
From: Aldy Hernandez @ 2021-11-14 15:18 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC patches, Andrew MacLeod

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

Ok, done.

Pushed.

Aldy

On Sun, Nov 14, 2021 at 2:53 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/14/21 14:15, Aldy Hernandez wrote:
> > The code computing ranges in PHIs in the path solver reuses the
> > temporary ssa_global_cache by calling its clear method.  Calling it on
> > an empty cache causes us to call memset with NULL.
> >
> > [The testcase doesn't fail without the patch.  I suppose it needs some
> > usbsan magic, or to live somewhere else?]
>
> Well, the actual test-case is the compiler itself as the source code.
> Anyway, the UBSAN error happens for thousands of test-cases when run
> during bootstrap-ubsan.mk config file. That said, I would remove
> the added test-case.
>
> Cheers,
> Martin
>
> >
> > Tested on x86-64 Linux.
> >
> > gcc/ChangeLog:
> >
> >       PR tree-optimization/103229
> >       * gimple-range-cache.cc (ssa_global_cache::clear): Do not pass
> >       null value to memset.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/pr103229.c: New test.
> > ---
> >   gcc/gimple-range-cache.cc       |  3 ++-
> >   gcc/testsuite/gcc.dg/pr103229.c | 10 ++++++++++
> >   2 files changed, 12 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/pr103229.c
> >
> > diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> > index a63e20e7e49..b347edeb474 100644
> > --- a/gcc/gimple-range-cache.cc
> > +++ b/gcc/gimple-range-cache.cc
> > @@ -651,7 +651,8 @@ ssa_global_cache::clear_global_range (tree name)
> >   void
> >   ssa_global_cache::clear ()
> >   {
> > -  memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
> > +  if (m_tab.address ())
> > +    memset (m_tab.address(), 0, m_tab.length () * sizeof (irange *));
> >   }
> >
> >   // Dump the contents of the global cache to F.
> > diff --git a/gcc/testsuite/gcc.dg/pr103229.c b/gcc/testsuite/gcc.dg/pr103229.c
> > new file mode 100644
> > index 00000000000..96ef9aff67c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr103229.c
> > @@ -0,0 +1,10 @@
> > +// { dg-do compile }
> > +// { dg-options "-O -w" }
> > +
> > +int main() {
> > +  int i;
> > +  for (; i;)
> > +    ;
> > +
> > +  return 0;
> > +}
> >
>

[-- Attachment #2: 0001-Remove-gcc.dg-pr103229.c.patch --]
[-- Type: text/x-patch, Size: 718 bytes --]

From 8a601f9bc45f9faaa91f18d58ba71b141acff701 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Sun, 14 Nov 2021 16:17:36 +0100
Subject: [PATCH] Remove gcc.dg/pr103229.c

gcc/testsuite/ChangeLog:

	* gcc.dg/pr103229.c: Removed.
---
 gcc/testsuite/gcc.dg/pr103229.c | 10 ----------
 1 file changed, 10 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.dg/pr103229.c

diff --git a/gcc/testsuite/gcc.dg/pr103229.c b/gcc/testsuite/gcc.dg/pr103229.c
deleted file mode 100644
index 96ef9aff67c..00000000000
--- a/gcc/testsuite/gcc.dg/pr103229.c
+++ /dev/null
@@ -1,10 +0,0 @@
-// { dg-do compile }
-// { dg-options "-O -w" }
-
-int main() {
-  int i;
-  for (; i;)
-    ;
-
-  return 0;
-}
-- 
2.31.1


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

end of thread, other threads:[~2021-11-14 15:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-14 13:15 [COMMITTED] Do not pass NULL to memset in ssa_global_cache Aldy Hernandez
2021-11-14 13:53 ` Martin Liška
2021-11-14 15:18   ` Aldy Hernandez

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).