Re: [C++ PATCH] Don't clear DECL_BUILT_IN_CLASS/DECL_FUNCTION_CODE when redeclaring a builtin if types match

[Jakub Jelinek <jakub@redhat.com>]

On Sun, Oct 07, 2007 at 10:43:06AM -0700, Mark Mitchell wrote:
> Jakub Jelinek wrote:
> 
> > The C FE always keeps DECL_BUILT_IN_CLASS when types match, even when
> > newdecl defines a function.
> 
> > But the C++ FE doesn't do this:
> >       if (new_defines_function)
> 
> The standard does prohibit redefining these functions.  But, in
> practice, people would be surprised if they did this:
> 
> /* Nobody should use memcpy, I think it's evil!  */
> extern "C" void memcpy (void *, void *, size_t) { abort (); }
> 
> void f() {
>   memcpy (...);
> }

If you do this, then you really want to use -fno-builtin-memcpy.
Clearing DECL_BUILT_IN_CLASS in duplicate_decls will just mean
that the behavior will be different in CUs where you define this
non-conforming memcpy and other CUs linked into your program.

> and the program did not abort because the call to memcpy was inlined.
> Or, for a more practical example, if the implementation of memcpy had
> additional auditing code -- as the GLIBC headers are trying to do.  So,
> I think the C++ front-end behavior is reasonable: if you define memcpy,
> we forget what we think we know about memcpy, and just treat it as we
> would any other function.

To me the C front-end behavior does make much more sense.  That helps
you to optimize conforming programs, even when you define the standard
in the same CU.  If you define memcpy and don't compile with
-fno-builtin-memcpy, the right assumption is that your definition is
standard conforming.  If you have auditing code you want to do always
whenever that fn is called, just make the function always_inline.

> At least in the example you posted, it looks like the GLIBC code is
> checking for something that is known at compile-time: whether the
> arguments to snprintf are inherently wrong.  Why can't we handle that
> with a compiler warning/error so that this check benefits all users on
> all operating systems, regardless of C library?

But we don't want that checking code turned on always, only when the user
asks for it using a special macro.  Furthermore, we need to know that the
particular version of the C library supports the checking function, how
exactly it needs to be checked (which is changing over time) and how other
details are handled (e.g. DFmode vs. TFmode long double related functions
on ppc* etc.).  All this is terribly specific to the exact C library used
(or to libssp if you are using that instead).  The current behavior is
that by calling a __*_chk builtin you are telling GCC that 1) you want to
do the checking 2) the C library supports it 3) among arguments you tell
the user's choice of whether %n should be recognized in writable strings
or not and in the future perhaps other details as well.  Without the
inline wrappers we are talking about here all this would get hardcoded
into GCC, which would terribly tie glibc's hands and would mean users would
need to duplicate the options to request the checking (say
-D_FORTIFY_SOURCE=2 would need to be accompanied by -ffortify-source=2).

>  You can't use that
> approach for functions that GCC doesn't know about -- but, then again,
> you won't have the performance problem that you're concerned about in
> that case either.

As I said in the mail, I'm ATM primarily not concerned about performance
- glibc always uses always_inline gnu_inline functions for this stuff - but
that adding an __asm ("...") redirect to the function will no longer result
in the needed behavior - it will not have any effect on the __builtin_*
functions.

	Jakub