Re: [PATH] PR/49139 fix always_inline failures diagnostics

[Christian Bruel <christian.bruel@st.com>]

>>
>> Unfortunately still not satisfactory, I've been testing it against a few
>> packages, and I notice excessive warnings with the use of __typeof (__error)
>> that doesn't propagate the inline keyword.
>>
>> For instance, a reduced use extracted from the glibc
>>
>> extern __inline __attribute__ ((__always_inline__))  void
>> error ()
>> {
>>
>> }
>>
>> extern void
>> __error ()
>> {
>> }
>>
>> extern __typeof (__error) error __attribute__ ((weak, alias ("__error")));
>>
>> emits an annoying warning on the error redefinition.
>>
>> So, a check in addition of the DECL_DECLARED_INLINED_P is needed,
>> TREE_ADDRESSABLE seems appropriate, since in the case of missing inline the
>> function would be emitted. So I'm testing:
>>
>> if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (decl))
>>           &&  !DECL_DECLARED_INLINE_P (decl)
>>           &&  TREE_ADDRESSABLE (decl))
>>
>> other idea ? or should be just drop this warning ?
>
> Hmm.  Honza, any idea on the above?  Christian, I suppose you
> could check if the cgraph node for that decl has the redefined_extern_inline
> flag set (checking TREE_ADDRESSABLE looks bogus to me).
> I'm not sure how the frontend merges those two decls - I suppose
> it will have a weak always-inline function with body :/

redefined_extern_inline is not set here. But DECL_STRUCT_FUNCTION(decl)) 
seems even best since it makes no sense to try the inline if the body is 
not available. So the former works well here.

Now, another annoying case, that I reduced from Xorg packages built from 
the glibc with -fstack-protector-all -D_FORTIFY_SOURCE=2.

to the following code:
---------------------------------
#include <stdlib.h>

char *
realpath(path, resolved)
         const char *path;
         char *resolved;
{
    ...
    return (NULL);
}
--------------------------------
preprocesses as:

extern char *__realpath_alias (__const char *__restrict __name, char 
*__restrict __resolved) __asm__ ("" "realpath") __attribute__ 
((__nothrow__)) __attribute__ ((__warn_unused_result__));

extern __inline __attribute__ ((__always_inline__)) __attribute__ 
((__artificial__)) __attribute__ ((__warn_unused_result__)) char *
__attribute__ ((__nothrow__)) realpath (__const char *__restrict __name, 
char *__restrict __resolved)
{
   return __realpath_alias (__name, __resolved);
}

char *
realpath(path, resolved)
  const char *path;
  char *resolved;
{
  return (((void *)0));
}

---

The problem is that the second redefinition, inherits the attributes of 
from the first extern inline declaration. So we get the warning emitted 
for this one..

It should be fine to redefine an extern inline function. is it ? So it 
would be a frontend bug to not reset the attributes when meeting the 
redefinition. Unless the first definition is an declaration and the 
attribute applies to all.

I also thought to test for the attribute artificial before emitting the 
warning, but that doesn't look correct, since this is only use from 
debugging information.

So the question is : is the redefinition of an extern inline function OK 
(I think yes), and should it inherit the attribute of the first one ?

Any idea ?

Many thanks

Christian


>
> Richard.