On Wed, 2023-08-23 at 15:55 +0200, Benjamin Priour wrote: > Hi David, > > A quick update on transitioning the analyzer tests from gcc.dg to > c-c++-common. > As discussed previously, C builtins are not C++ builtins, therefore I > had > to add > recognition of the builtins by their name rather than with the > predicate > fndecl_built_in_p. > > Do you know why this happens? I see both of those tests have their > own > > prototypes for the functions in question, rather than using system > > headers; maybe those prototypes don't match some property that the > > C++ > > FE is checking for? > > > > The C FE relies on c-decl.cc:c_builtin_function, the macro > C_DECL_BUILTIN_PROTOTYPE, > as well as c-decl.cc:match_builtin_function_types to recognize a > builtin > function decl > and provide a unified type between what the user wrote, and the known > builtin fndecl. > > I'd like however to correct something I've said previously > > My question is then : Should G++ behave as GCC and ignore the user's > > signatures of built-ins, instead using the attributes specified by > > GCC ? > > At the moment I went with a "yes". If the function is a builtin, > > I'm > > operating on its builtin_known_function::builtin_decl () (see > > above) rather > > than the callee_fndecl > > deduced from a gimple call, therefore overriding the user's > > signature. > > > > Actually, GCC does not ignore the user's signatures of built-ins, but > if > match_builtin_function_types finds out that the user's signature > is close enough to the one described in builtins.def, then it will > complete > the user's add with attributes and the likes so that we get an > unified type. > > A case where the user's signature *isn't* close enough to the built- > in's is > gcc.dg/analyzer/pr96841.c's malloc's signature. > > __SIZE_TYPE__ > malloc (__SIZE_TYPE__); > > To simplify, whenever a Wbuiltin-declaration-mismatch is emitted, > then the > user's decl isn't unified with the built-ins, > and the extra GCC's attributes defined in builtins.def are not added > to the > user's decl. > In such cases, the function decl is not recognized as a builtin by > fndecl_builtin_p, hence the analyzer's (before my patch) > known_function > is never found out. > What this mean is that pr96841.c's malloc uses don't go within > kf_malloc:impl_call_pre nor kf_malloc:impl_call_post with the current > analyzer's handling of built-ins - do note however that sm-malloc > goes as > expected, as it recognizes malloc calls both by name > and builtin decl (sm-malloc.cc::known_allocator_p). > > With my patch, since the recognition is by name and no longer > dependent on > the fndecl_builtin_p predicate, pr96841's malloc *is* > recognized as a builtin, and its kf is processed. > This led to pr96841.c no longer passing, as the while loop makes the > analysis too complex. (-Wanalyzer-too-complex)
Are you able to post the part of that patch that affects recognition of builtins? (even if it's just a work-in-progress with no ChangeLog) > > > > My question is then : Should G++ behave as GCC and ignore the > > > user's > > > signatures of built-ins, instead using the attributes specified > > > by > > > GCC ? > > > At the moment I went with a "yes". If the function is a builtin, > > > I'm > > > operating on its builtin_known_function::builtin_decl () (see > > > above) > > > rather > > > than the callee_fndecl > > > deduced from a gimple call, therefore overriding the user's > > > signature. > > > > > > Doing so led to tests sprintf-2.c and realloc-1.c to pass both in > > > GCC > > > and > > > G++. > > > > I think I agree with "yes" here. > > I would like to proceed with what we said here, so that the analyzer > proceeds > with the analysis of malloc or any other builtins even when their > fndecl is > not recognized > as such. Then I would update pr96841.c to expect a -Wanalyzer-too- > complex. > Besides, sm-malloc already makes the assumption that a call named > malloc > should be treated > as a malloc, whether it is matching a builtin or not. Sounds good. Dave
