[PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[Martin Jambor]

Hi,

the warning for suspicious calls of abs-like functions segfaults if a
user declared their own parameter-less-ish variant of abs like in the
testcase below.  Fixed by looking whether there is any TYPE_ARG_TYPES
before trying to compare the actual argument with it.

Bootstrapped and tested on x86_64-linux and aarch64-linux, the same on
i686-linux is pending.

OK for trunk?

Thanks,

Martin


2018-09-24  Martin Jambor  <mjambor@suse.cz>

	PR c/87347
	c/
	* c-parser.c (warn_for_abs): Bail out if TYPE_ARG_TYPES is NULL.

	testsuite/
	* gcc.dg/pr87347.c: New test.
---
 gcc/c/c-parser.c               | 7 ++++---
 gcc/testsuite/gcc.dg/pr87347.c | 6 ++++++
 2 files changed, 10 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr87347.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1766a256633..a96d15fef1d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9116,9 +9116,10 @@ warn_for_abs (location_t loc, tree fndecl, tree arg)
      -Wint-conversion warnings.  Most other wrong types hopefully lead to type
      mismatch errors.  TODO: Think about what to do with FIXED_POINT_TYPE_P
      types and possibly other exotic types.  */
-  if (!INTEGRAL_TYPE_P (atype)
-      && !SCALAR_FLOAT_TYPE_P (atype)
-      && TREE_CODE (atype) != COMPLEX_TYPE)
+  if ((!INTEGRAL_TYPE_P (atype)
+       && !SCALAR_FLOAT_TYPE_P (atype)
+       && TREE_CODE (atype) != COMPLEX_TYPE)
+      || !TYPE_ARG_TYPES (TREE_TYPE (fndecl)))
     return;
 
   enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
diff --git a/gcc/testsuite/gcc.dg/pr87347.c b/gcc/testsuite/gcc.dg/pr87347.c
new file mode 100644
index 00000000000..d0bdf2a9fec
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87347.c
@@ -0,0 +1,6 @@
+/* {dg-do compile} */
+/* { dg-options "-Wabsolute-value" } */
+
+int a;
+int abs();
+void b() { abs(a); }
-- 
2.18.0

Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[Jakub Jelinek]

On Mon, Sep 24, 2018 at 08:40:13PM +0200, Martin Jambor wrote:
> Hi,
> 
> the warning for suspicious calls of abs-like functions segfaults if a
> user declared their own parameter-less-ish variant of abs like in the
> testcase below.  Fixed by looking whether there is any TYPE_ARG_TYPES
> before trying to compare the actual argument with it.
> 
> Bootstrapped and tested on x86_64-linux and aarch64-linux, the same on
> i686-linux is pending.
> 
> OK for trunk?

Isn't that bail out too early?
I mean most of the warnings that are emitted by the function don't really
need TYPE_ARG_TYPES, only the last one does, so can't you just bail out
before the last warning?

Also, the function comment has "gracely", did you mean "gracefully"?

	Jakub

Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[Martin Jambor]

Hi,

On Tue, Sep 25 2018, Jakub Jelinek wrote:
> On Mon, Sep 24, 2018 at 08:40:13PM +0200, Martin Jambor wrote:
>> Hi,
>> 
>> the warning for suspicious calls of abs-like functions segfaults if a
>> user declared their own parameter-less-ish variant of abs like in the
>> testcase below.  Fixed by looking whether there is any TYPE_ARG_TYPES
>> before trying to compare the actual argument with it.
>> 
>> Bootstrapped and tested on x86_64-linux and aarch64-linux, the same on
>> i686-linux is pending.
>> 
>> OK for trunk?
>
> Isn't that bail out too early?
> I mean most of the warnings that are emitted by the function don't really
> need TYPE_ARG_TYPES, only the last one does, so can't you just bail out
> before the last warning?

my reasoning was that if the function is not what I expect it to be, it
is better not to touch it.  On the other hand, I have no problems moving
this test lower as done in the patch below.

> Also, the function comment has "gracely", did you mean "gracefully"?

Of course I did, thanks for spotting that.

Bootstrapped and tested on x86_64-linux and aarch64-linux.  OK for
trunk?

Thanks,

Martin


2018-09-25  Martin Jambor  <mjambor@suse.cz>

	PR c/87347
	c/
	* c-parser.c (warn_for_abs): Bail out if TYPE_ARG_TYPES is NULL.  Fix
        the function comment.

	testsuite/
	* gcc.dg/pr87347.c: New test.
---
 gcc/c/c-parser.c               | 7 +++++--
 gcc/testsuite/gcc.dg/pr87347.c | 6 ++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr87347.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1766a256633..1f173fc10e2 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9102,8 +9102,8 @@ sizeof_ptr_memacc_comptypes (tree type1, tree type2)
 }
 
 /* Warn for patterns where abs-like function appears to be used incorrectly,
-   gracely ignore any non-abs-like function.  The warning location should be
-   LOC.  FNDECL is the declaration of called function, it must be a
+   gracefully ignore any non-abs-like function.  The warning location should
+   be LOC.  FNDECL is the declaration of called function, it must be a
    BUILT_IN_NORMAL function.  ARG is the first and only argument of the
    call.  */
 
@@ -9223,6 +9223,9 @@ warn_for_abs (location_t loc, tree fndecl, tree arg)
       return;
     }
 
+  if (!TYPE_ARG_TYPES (TREE_TYPE (fndecl)))
+    return;
+
   tree ftype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
   if (TREE_CODE (atype) == COMPLEX_TYPE)
     {
diff --git a/gcc/testsuite/gcc.dg/pr87347.c b/gcc/testsuite/gcc.dg/pr87347.c
new file mode 100644
index 00000000000..d0bdf2a9fec
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87347.c
@@ -0,0 +1,6 @@
+/* {dg-do compile} */
+/* { dg-options "-Wabsolute-value" } */
+
+int a;
+int abs();
+void b() { abs(a); }
-- 
2.18.0

Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[Jakub Jelinek]

On Tue, Sep 25, 2018 at 05:47:49PM +0200, Martin Jambor wrote:
> > Isn't that bail out too early?
> > I mean most of the warnings that are emitted by the function don't really
> > need TYPE_ARG_TYPES, only the last one does, so can't you just bail out
> > before the last warning?
> 
> my reasoning was that if the function is not what I expect it to be, it
> is better not to touch it.  On the other hand, I have no problems moving
> this test lower as done in the patch below.

I guess the question is if we then treat it as a builtin or don't.
Anyway, I'd like to defer that decision to the C FE maintainers.

> > Also, the function comment has "gracely", did you mean "gracefully"?
> 
> Of course I did, thanks for spotting that.
> 
> Bootstrapped and tested on x86_64-linux and aarch64-linux.  OK for
> trunk?

	Jakub

Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL

[Joseph Myers]

On Tue, 25 Sep 2018, Jakub Jelinek wrote:

> On Tue, Sep 25, 2018 at 05:47:49PM +0200, Martin Jambor wrote:
> > > Isn't that bail out too early?
> > > I mean most of the warnings that are emitted by the function don't really
> > > need TYPE_ARG_TYPES, only the last one does, so can't you just bail out
> > > before the last warning?
> > 
> > my reasoning was that if the function is not what I expect it to be, it
> > is better not to touch it.  On the other hand, I have no problems moving
> > this test lower as done in the patch below.
> 
> I guess the question is if we then treat it as a builtin or don't.
> Anyway, I'd like to defer that decision to the C FE maintainers.

This patch is OK.  (If there's something odd about the argument meaning 
it ends up not being handled as a built-in, warn_for_abs will have 
returned early anyway.)

-- 
Joseph S. Myers
joseph@codesourcery.com