[cfe-dev] Two questions about address_space attribute

[Richard Smith via cfe-dev]

On Thu, 7 Nov 2019 at 10:30, Y Song via cfe-dev <cfe-dev at lists.llvm.org>
wrote:

> Hi,
>
> Currently, I am thinking to add address_space attribute support for
> x86_64 and BPF to compile linux kernel. The main goal is to get
> address_space information into debug_info and then later to dwarf/BTF.
> But address_space enforcement itself can also help with better code
> for the kernel.
>
> The RFC patch is here:
>     https://reviews.llvm.org/D69393
>
> During experiments, we found two issues which I would like to get
> expert opinion:
>
> issue 1: address_space attributes on function pointers
> =========================================
>
> clang does not allow address_space attribute for function pointers,
> specifically, in clang/lib/Sema/SemaType.cpp,
>
> // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not
> be
> // qualified by an address-space qualifier."
> if (Type->isFunctionType()) {
>   S.Diag(Attr.getLoc(), diag::err_attribute_address_function_type);
>   Attr.setInvalid();
>   return;
> }
>
> But linux kernel tries to annotate signal handling function pointer
> with address space to indicate it is accessing user space.
>
> typedef __signalfn_t __user *__sighandler_t;
> typedef __restorefn_t __user *__sigrestore_t;
>
> Such attribute makes sense for linux since indeed the signal handler
> code resides in user space and the kernel pointer
> merely points to user memory here.
>
> What is the rationale for this restriction in standard? How we could
> make clang tolerate such restrictions for X86/BPF/...?
>
> issue 2: typeof(*ptr) where ptr has address_space attribute.
>
> -bash-4.4$ cat t2.c
> int test(int __attribute__((address_space(1))) *p) {
> #ifdef NO_TYPEOF
>   int t = *p;
> #else
>   typeof(*p) t = *p;
> #endif
>   return t;
> }
> -bash-4.4$ clang -c t2.c
> t2.c:5:14: error: automatic variable qualified with an address space
>   typeof(*p) t = *p;
>              ^
> 1 error generated.
> -bash-4.4$ clang -DNO_TYPEOF -c t2.c
> -bash-4.4$
>
> The IR generated without NO_TYPEOF macro:
>   %p.addr = alloca i32 addrspace(1)*, align 8
>   %t = alloca i32, align 4
>   store i32 addrspace(1)* %p, i32 addrspace(1)** %p.addr, align 8
>   %0 = load i32 addrspace(1)*, i32 addrspace(1)** %p.addr, align 8
>   %1 = load i32, i32 addrspace(1)* %0, align 4
>   store i32 %1, i32* %t, align 4
>   %2 = load i32, i32* %t, align 4
>   ret i32 %2
>
> So we can directly load a i32 addrspacee(1)* to an i32. Maybe
> typeof(*p) should just i32 and we should not emit the error at all?
>

I don't think this suggestion has been fully addressed yet. It's true that
we could make typeof(*p) remove the address space qualifier, but that would
be inconsistent with how typeof(expr) generally behaves (it doesn't remove
type qualifiers from the type of its argument), and it can be important to
preserve the address space when the result is used to form another type.
For example:

int test(int __attribute__((address_space(1))) *p) {
  const typeof(*p) *const_p = p; // should preserve address_space here

'typeof' is a GNU extension, and generally we want to follow GCC's behavior
for GNU extensions, but GCC doesn't implement this attribute (at least not
in builds that I've tested), so we have on explicit guidance there.
Additionally, it seems wise to keep open the option of specifying an
address space on a local variable (even if we don't support it today) for
future extensions (such as
https://www.springerprofessional.de/en/scads/6859296) rather than ignoring
an address space attribute on a local variable.

Can you give an example of code suffering from issue 2? It would help to
see how reasonable or unreasonable such code seems to be, and whether this
would be best addressed in the compiler or in the code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20191112/6195d219/attachment.html>