[COMMITTED] PR tree-optimization/101741 - Ensure toupper and tolower follow the expected pattern.

Andrew MacLeod amacleod@redhat.com
Tue Aug 10 13:21:36 GMT 2021


On 8/10/21 3:45 AM, Richard Biener wrote:
> On Mon, Aug 9, 2021 at 10:31 PM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> The user has overridden the function name "toupper" . Its marked as a
>> builtin function, presumably because it matches the name.  In range
>> folding, we were assuming the LHS and the parameter were compatible
>> types...  but they are not in this case..
>>
>> I don't know if we should be marking such a thing as a builtin function,
>> but regardless.. we shouldn't be trapping.  If the type of the argument
>> is not compatible with  the LHS, then we'll simply assume nothing about
>> the function.
>>
>> Bootstraps with no regression on x86_64-pc-linux-gnu.  pushed.
> I wonder why the gimple_call_combined_fn verification
> using gimple_builtin_call_types_compatible_p isn't enough here?
> Yes, it's matching is a bit lazy, but only with respect to promotion
> of arguments to 'int'.
>
> Richard.
>
I set a breakpoint on the return type field for the builtin... A quick 
check shows the return type of the builtin is being changed to "unsigned 
int" here:

#0  merge_decls (newdecl=0x7fffe9f67100, olddecl=0x7fffe9ed2100, 
newtype=0x7fffe9e3ae70, oldtype=0x7fffe9e3ae70) at 
/gcc/master/gcc/gcc/c/c-decl.c:2598
#1  0x0000000000a0628d in duplicate_decls (newdecl=0x7fffe9f67100, 
olddecl=0x7fffe9ed2100) at /gcc/master/gcc/gcc/c/c-decl.c:2963
#2  0x0000000000a07464 in pushdecl (x=0x7fffe9f67100) at 
/gcc/master/gcc/gcc/c/c-decl.c:3256
#3  0x0000000000a1d113 in start_function (declspecs=0x37728b0, 
declarator=0x3772ae0, attributes=0x0) at /gcc/master/gcc/gcc/c/c-decl.c:9644
#4  0x0000000000a8667c in c_parser_declaration_or_fndef 
(parser=0x7ffff7ff7ab0, fndef_ok=true, static_assert_ok=true, 
empty_ok=true, nested=false, start_attr_ok=true, 
objc_foreach_object_declaration=0x0,
     omp_declare_simd_clauses=0x0, have_attrs=false, attrs=0x0, 
oacc_routine_data=0x0, fallthru_attr_p=0x0) at 
/gcc/master/gcc/gcc/c/c-parser.c:2444


It would appear that merge_decls is merging the olddecl (the bultin) 
with newdecl  and results in changing the LHS to unsigned int, so now it 
thinks the builtin matches.   eeeks.

I don't know what the correct thing to do is, but a quick hack to check 
if old_decl is a builtin and return false from duplicate_decl also seems 
to resolve the problem:

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 221a67fe57b..27ab6ac9f1a 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2960,6 +2960,9 @@ duplicate_decls (tree newdecl, tree olddecl)
        return false;
      }

+  if (DECL_BUILT_IN_CLASS (olddecl) != NOT_BUILT_IN)
+    return false;
+
    merge_decls (newdecl, olddecl, newtype, oldtype);

    /* The NEWDECL will no longer be needed.




More information about the Gcc-patches mailing list