| Message ID | c7f2f1ae-5b16-e648-9b83-6a29883fe09b@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | handle unusual targets in -Wbuiltin-declaration-mismatch (PR 88098) | expand |
On Mon, 19 Nov 2018 at 22:38, Martin Sebor <msebor@redhat.com> wrote: > > The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with > the recent -Wbuiltin-declaration-mismatch enhancement to detect > calls with incompatible arguments to built-ins declared without > a prototype fails on a few targets due to incorrect assumptions > hardcoded into the test. Besides removing those assumptions > (or adding appropriate { target } attributes, the attached patch > also adjusts the implementation of the warning to avoid triggering > for enum promotion to int on short_enums targets. > > Since the fix is trivial I plan to commit it tomorrow if there > are no concerns. > > Tested on x86_64-linux and with an arm-none-eabi cross-compiler. > I also did a little bit of testing with sparc-solaris2.11 cross > compiler but there the test harness fails due to the -m32 option > so the Wbuiltin-declaration-mismatch-4.c still has unexpected > FAILs. I've raised bug 88104 for the outstanding problem on > sparc-solaris2.11. > Hello, I tested your patch on arm* and aarch64*. It does the job on arm, but on aarch64*elf, I'm seeing new failures: gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for warnings, line 121) gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for warnings, line 123) gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for warnings, line 98) > Martin
On 11/20/2018 08:56 AM, Christophe Lyon wrote: > On Mon, 19 Nov 2018 at 22:38, Martin Sebor <msebor@redhat.com> wrote: >> >> The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with >> the recent -Wbuiltin-declaration-mismatch enhancement to detect >> calls with incompatible arguments to built-ins declared without >> a prototype fails on a few targets due to incorrect assumptions >> hardcoded into the test. Besides removing those assumptions >> (or adding appropriate { target } attributes, the attached patch >> also adjusts the implementation of the warning to avoid triggering >> for enum promotion to int on short_enums targets. >> >> Since the fix is trivial I plan to commit it tomorrow if there >> are no concerns. >> >> Tested on x86_64-linux and with an arm-none-eabi cross-compiler. >> I also did a little bit of testing with sparc-solaris2.11 cross >> compiler but there the test harness fails due to the -m32 option >> so the Wbuiltin-declaration-mismatch-4.c still has unexpected >> FAILs. I've raised bug 88104 for the outstanding problem on >> sparc-solaris2.11. >> > > Hello, > > I tested your patch on arm* and aarch64*. It does the job on arm, but > on aarch64*elf, > I'm seeing new failures: > gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for > warnings, line 121) > gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for > warnings, line 123) > gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for > warnings, line 98) The failures are due to the target apparently not supporting the fabsl built-in used by the test. Martin
By calling builtin_decl_explicit rather than builtin_decl_implicit the updated patch in the attachment avoids test failures due to missing warnings on targets with support for long double but whose libc doesn't support C99 functions like fabsl (such as apparently aarch64-linux). Martin On 11/19/2018 02:37 PM, Martin Sebor wrote: > The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with > the recent -Wbuiltin-declaration-mismatch enhancement to detect > calls with incompatible arguments to built-ins declared without > a prototype fails on a few targets due to incorrect assumptions > hardcoded into the test. Besides removing those assumptions > (or adding appropriate { target } attributes, the attached patch > also adjusts the implementation of the warning to avoid triggering > for enum promotion to int on short_enums targets. > > Since the fix is trivial I plan to commit it tomorrow if there > are no concerns. > > Tested on x86_64-linux and with an arm-none-eabi cross-compiler. > I also did a little bit of testing with sparc-solaris2.11 cross > compiler but there the test harness fails due to the -m32 option > so the Wbuiltin-declaration-mismatch-4.c still has unexpected > FAILs. I've raised bug 88104 for the outstanding problem on > sparc-solaris2.11. > > Martin PR testsuite/88098 - FAIL: gcc.dg/Wbuiltin-declaration-mismatch-4.c gcc/c/ChangeLog: PR testsuite/88098 * c-typeck.c (convert_arguments): Call builtin_decl_explicit instead. (maybe_warn_builtin_no_proto_arg): Handle short enum to int promotion. gcc/testsuite/ChangeLog: PR testsuite/88098 * gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust. * gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test. Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 266320) +++ gcc/c/c-typeck.c (working copy) @@ -3422,7 +3422,10 @@ convert_arguments (location_t loc, vec<location_t> built_in_function code = DECL_FUNCTION_CODE (fundecl); if (C_DECL_BUILTIN_PROTOTYPE (fundecl)) { - if (tree bdecl = builtin_decl_implicit (code)) + /* For a call to a built-in function declared without a prototype + use the types of the parameters of the internal built-in to + match those of the arguments to. */ + if (tree bdecl = builtin_decl_explicit (code)) builtin_typelist = TYPE_ARG_TYPES (TREE_TYPE (bdecl)); } @@ -6461,7 +6464,9 @@ maybe_warn_builtin_no_proto_arg (location_t loc, t && TYPE_MODE (parmtype) == TYPE_MODE (argtype)) return; - if (parmcode == argcode + if ((parmcode == argcode + || (parmcode == INTEGER_TYPE + && argcode == ENUMERAL_TYPE)) && TYPE_MAIN_VARIANT (parmtype) == TYPE_MAIN_VARIANT (promoted)) return; Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c =================================================================== --- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (revision 266320) +++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (working copy) @@ -77,9 +77,9 @@ void test_integer_conversion_memset (void *d) /* Passing a ptrdiff_t where size_t is expected may not be unsafe but because GCC may emits suboptimal code for such calls warning for them helps improve efficiency. */ - memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .long int.} where .long unsigned int. is expected" } */ + memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .\(long \)?int.} where .\(long \)?unsigned int. is expected" } */ - memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where 'long unsigned int' is expected" } */ + memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where '\(long \)?unsigned int' is expected" } */ /* Verify that the same call as above but to the built-in doesn't trigger a warning. */ @@ -108,7 +108,8 @@ void test_real_conversion_fabs (void) /* In C, the type of an enumeration constant is int. */ d = fabs (e0); /* { dg-warning ".fabs. argument 1 type is .int. where .double. is expected in a call to built-in function declared without prototype" } */ - d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" } */ + d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" "ordinary enum" { target { ! short_enums } } } */ + /* { dg-warning ".fabs. argument 1 promotes to .int. where .double. is expected in a call to built-in function declared without prototype" "size 1 enum" { target short_enums } .-1 } */ /* No warning here since float is promoted to double. */ d = fabs (f);
Hi Martin, > By calling builtin_decl_explicit rather than builtin_decl_implicit > the updated patch in the attachment avoids test failures due to > missing warnings on targets with support for long double but whose > libc doesn't support C99 functions like fabsl (such as apparently > aarch64-linux). [...] > gcc/testsuite/ChangeLog: > > PR testsuite/88098 > * gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust. > * gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test. is the Wbuiltin-declaration-mismatch-5.c testcase still supposed to be part of the patch? It's in the ChangeLog, but missing from the revised patch. Rainer
On 11/21/18 6:08 AM, Rainer Orth wrote: > Hi Martin, > >> By calling builtin_decl_explicit rather than builtin_decl_implicit >> the updated patch in the attachment avoids test failures due to >> missing warnings on targets with support for long double but whose >> libc doesn't support C99 functions like fabsl (such as apparently >> aarch64-linux). > [...] >> gcc/testsuite/ChangeLog: >> >> PR testsuite/88098 >> * gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust. >> * gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test. > > is the Wbuiltin-declaration-mismatch-5.c testcase still supposed to be > part of the patch? It's in the ChangeLog, but missing from the revised > patch. It should still be there. I must have excluded it by accident. I will make sure to include it in the commit. Thanks for pointing it out! Martin
I have committed this in r266417. Martin On 11/21/18 10:04 AM, Martin Sebor wrote: > On 11/21/18 6:08 AM, Rainer Orth wrote: >> Hi Martin, >> >>> By calling builtin_decl_explicit rather than builtin_decl_implicit >>> the updated patch in the attachment avoids test failures due to >>> missing warnings on targets with support for long double but whose >>> libc doesn't support C99 functions like fabsl (such as apparently >>> aarch64-linux). >> [...] >>> gcc/testsuite/ChangeLog: >>> >>> PR testsuite/88098 >>> * gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust. >>> * gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test. >> >> is the Wbuiltin-declaration-mismatch-5.c testcase still supposed to be >> part of the patch? It's in the ChangeLog, but missing from the revised >> patch. > > It should still be there. I must have excluded it by accident. > I will make sure to include it in the commit. > > Thanks for pointing it out! > Martin
PR testsuite/88098 - FAIL: gcc.dg/Wbuiltin-declaration-mismatch-4.c (test for warnings gcc/c/ChangeLog: PR testsuite/88098 * c-typeck.c (maybe_warn_builtin_no_proto_arg): Handle short enum to int promotion. gcc/testsuite/ChangeLog: PR testsuite/88098 * gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust. * gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test. Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 266284) +++ gcc/c/c-typeck.c (working copy) @@ -6461,7 +6461,9 @@ maybe_warn_builtin_no_proto_arg (location_t loc, t && TYPE_MODE (parmtype) == TYPE_MODE (argtype)) return; - if (parmcode == argcode + if ((parmcode == argcode + || (parmcode == INTEGER_TYPE + && argcode == ENUMERAL_TYPE)) && TYPE_MAIN_VARIANT (parmtype) == TYPE_MAIN_VARIANT (promoted)) return; Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c =================================================================== --- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (revision 266284) +++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (working copy) @@ -77,9 +77,9 @@ void test_integer_conversion_memset (void *d) /* Passing a ptrdiff_t where size_t is expected may not be unsafe but because GCC may emits suboptimal code for such calls warning for them helps improve efficiency. */ - memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .long int.} where .long unsigned int. is expected" } */ + memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .\(long \)?int.} where .\(long \)?unsigned int. is expected" } */ - memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where 'long unsigned int' is expected" } */ + memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where '\(long \)?unsigned int' is expected" } */ /* Verify that the same call as above but to the built-in doesn't trigger a warning. */ @@ -95,7 +95,7 @@ double fabs (); /* { dg-message "built-i /* Expect a warning for fabsf below because even a float argument promotes to double. Unfortunately, invalid calls to fabsf() are not diagnosed. */ float fabsf (); /* { dg-warning "conflicting types for built-in function .fabsf.; expected .float\\\(float\\\)." } */ -long double fabsl (); /* { dg-message "built-in .fabsl. declared here" } */ +long double fabsl (); /* { dg-message "built-in .fabsl. declared here" "large long double" { target large_long_double } } */ void test_real_conversion_fabs (void) { @@ -108,7 +108,8 @@ void test_real_conversion_fabs (void) /* In C, the type of an enumeration constant is int. */ d = fabs (e0); /* { dg-warning ".fabs. argument 1 type is .int. where .double. is expected in a call to built-in function declared without prototype" } */ - d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" } */ + d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" "ordinary enum" { target { ! short_enums } } } */ + /* { dg-warning ".fabs. argument 1 promotes to .int. where .double. is expected in a call to built-in function declared without prototype" "size 1 enum" { target short_enums } .-1 } */ /* No warning here since float is promoted to double. */ d = fabs (f); @@ -117,9 +118,9 @@ void test_real_conversion_fabs (void) d = fabsf (c); /* { dg-warning ".fabsf. argument 1 promotes to .int. where .float. is expected in a call to built-in function declared without prototype" "pr87890" { xfail *-*-* } } */ - d = fabsl (c); /* { dg-warning ".fabsl. argument 1 promotes to .int. where .long double. is expected in a call to built-in function declared without prototype" } */ + d = fabsl (c); /* { dg-warning ".fabsl. argument 1 promotes to .int. where .long double. is expected in a call to built-in function declared without prototype" "large long double" { target large_long_double } } */ - d = fabsl (f); /* { dg-warning ".fabsl. argument 1 promotes to .double. where .long double. is expected in a call to built-in function declared without prototype" } */ + d = fabsl (f); /* { dg-warning ".fabsl. argument 1 promotes to .double. where .long double. is expected in a call to built-in function declared without prototype" "large long double" { target large_long_double } } */ /* Verify that the same call as above but to the built-in doesn't trigger a warning. */ Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c =================================================================== --- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c (working copy) @@ -0,0 +1,19 @@ +/* PR testsuite/88098 - FAIL: gcc.dg/Wbuiltin-declaration-mismatch-4.c + { dg-do compile } + { dg-options "-Wbuiltin-declaration-mismatch -fshort-enums" } */ + +int abs (); +double fabs (); /* { dg-message "built-in .fabs. declared here" } */ + +enum E { e0 } e; + +int i; +double d; + +void test_short_enums (void) +{ + /* enum e promotes to int. */ + i = abs (e); + + d = fabs (e); /* { dg-warning ".fabs. argument 1 promotes to .int. where .double. is expected in a call to built-in function declared without prototype" } */ +}
The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with the recent -Wbuiltin-declaration-mismatch enhancement to detect calls with incompatible arguments to built-ins declared without a prototype fails on a few targets due to incorrect assumptions hardcoded into the test. Besides removing those assumptions (or adding appropriate { target } attributes, the attached patch also adjusts the implementation of the warning to avoid triggering for enum promotion to int on short_enums targets. Since the fix is trivial I plan to commit it tomorrow if there are no concerns. Tested on x86_64-linux and with an arm-none-eabi cross-compiler. I also did a little bit of testing with sparc-solaris2.11 cross compiler but there the test harness fails due to the -m32 option so the Wbuiltin-declaration-mismatch-4.c still has unexpected FAILs. I've raised bug 88104 for the outstanding problem on sparc-solaris2.11. Martin