On Thu, Mar 25, 2021 at 03:24:38PM +0100, Richard Biener wrote:
> Well, but in all of those cases the program was invalid (and is diagnosed).
> So it simply means "always inline".  That we abuse the always-inline
> (error) diagnostic to tell users about possible problems (and in HJs case
> and the fortify case reject valid programs) is IMHO a bug.

No, we don't diagnose the cases where we chose to implement the intrinsics
e.g. using generic vectors anymore with H.J.'s patch.
It is fine to use generic vectors directly, but invalid and non-portable
to call e.g. AVX2 intrinsics from SSE2 functions even if they are
implemented using generic vectors.

> We perform always-inline inlining early even with -fno-early-inlining.  But I
> think we don't reliably diagnose indirect calls or address-taking of
> always-inline
> functions and will emit them out-of-line if they end up "used".  That's a bug
> I think.
> 
> static inline __attribute__((always_inline)) void f () {}
> void (*p)() = f;
> 
> is not diagnosed and 'f' is emitted out of line.

I think we can't do such a change, too much code in the wild relies on
always_inline not diagnosing the indirect call.  Including all of glibc
headers.
Doing
int (*p) (const char *, int, ...) = open;
is perfectly valid C and we must not diagnose that, even when for
fortification glibc has an always_inline wrapper around it.

Yes, it is unfortunate that we in the past called the attribute
always_inline when we really didn't mean (or perhaps some of us meant but
didn't implement) always, but at this point we'd need a new attribute to
mean diagnose non-indirect calls/taking address of except in direct calls as
errors.  And IMHO we need a different attribute to mean inline despite
target/optimization option mismatches.
That way users can choose what exactly they want on a case by case basis.

        Jakub

Reply via email to