* [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL @ 2018-09-24 18:46 Martin Jambor 2018-09-25 9:25 ` Jakub Jelinek 0 siblings, 1 reply; 5+ messages in thread From: Martin Jambor @ 2018-09-24 18:46 UTC (permalink / raw) To: GCC Patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL 2018-09-24 18:46 [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL Martin Jambor @ 2018-09-25 9:25 ` Jakub Jelinek 2018-09-25 15:50 ` Martin Jambor 0 siblings, 1 reply; 5+ messages in thread From: Jakub Jelinek @ 2018-09-25 9:25 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL 2018-09-25 9:25 ` Jakub Jelinek @ 2018-09-25 15:50 ` Martin Jambor 2018-09-25 16:10 ` Jakub Jelinek 0 siblings, 1 reply; 5+ messages in thread From: Martin Jambor @ 2018-09-25 15:50 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL 2018-09-25 15:50 ` Martin Jambor @ 2018-09-25 16:10 ` Jakub Jelinek 2018-09-25 23:19 ` Joseph Myers 0 siblings, 1 reply; 5+ messages in thread From: Jakub Jelinek @ 2018-09-25 16:10 UTC (permalink / raw) To: Martin Jambor, Joseph S. Myers, Marek Polacek; +Cc: GCC Patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL 2018-09-25 16:10 ` Jakub Jelinek @ 2018-09-25 23:19 ` Joseph Myers 0 siblings, 0 replies; 5+ messages in thread From: Joseph Myers @ 2018-09-25 23:19 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Martin Jambor, Marek Polacek, GCC Patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-25 22:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-24 18:46 [PR 87347] Prevent segfaults if TYPE_ARG_TYPES is NULL Martin Jambor 2018-09-25 9:25 ` Jakub Jelinek 2018-09-25 15:50 ` Martin Jambor 2018-09-25 16:10 ` Jakub Jelinek 2018-09-25 23:19 ` Joseph Myers
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).