Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C

[Thomas Gleixner]

On Fri, Oct 15 2021 at 11:42, Sami Tolvanen wrote:
> On Fri, Oct 15, 2021 at 10:57 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> Not beautiful, but it gives the information which is needed and it tells
>> me clearly what this is about. While the above lumps everything together
>> whatever it is.
>
> Sure, that makes sense. Ignoring the macro for a moment, how do you
> feel about using incomplete structs for the non-C functions as Andy
> suggested?

I think I agreed with that back then when he suggested it the first
time. That still allows me to do a classification:

struct asm_exception
struct asm_xen_hv_call
....

>> Having __bikeshedme would allow to do:
>>
>>    __hardware_call
>>    __xenhv_call
>>    __inline_asm_call
>>
>> or such, which clearly tells how the function should be used and it can
>> even be validated by tooling.
>
> Previously you suggested adding a built-in function to the compiler:
>
> https://lore.kernel.org/lkml/877dl0sc2m.ffs@xxxxxxxxxxxxxxxxxxxxxxx/
>
> I actually did implement this in Clang, but the feature wasn't
> necessary with opaque types, so I never moved forward with those
> patches. A built-in also won't make the code any cleaner, which was a
> concern last time.
>
> I do agree that a function attribute would look cleaner, but it won't
> stop anyone from mistakenly calling these functions from C code, which
> was something Andy wanted to address at the same time. Do you still
> prefer a function attribute over using an opaque type nevertheless?

For actually callable functions, by some definition of callable,
e.g. the clear_page_*() variants a proper attribute would be definitely
preferred.

That attribute should tell the compiler that the function is using the
register arguments correctly but is not suitable for direct invocation
because it clobbers registers.

So the compiler can just refuse to call such a function if used directly
without an inline asm wrapper which describes the clobbers, right?

But thinking more about clobbers. The only "annotation" of clobbers we
have today are the clobbers in the inline asm, which is fragile too.

Something like

 __attribute__ ((clobbers ("rcx", "rax")))

might be useful by itself because it allows validation of the clobbers
in the inline asm wrappers and also allows a analysis tool to look at
the ASM code and check whether the above list is correct.

Hmm?

Thanks,

        tglx