riscv: Use __has_include__ to include <asm/syscalls.h> [BZ #24022]

Message ID 20181221174840.10933-1-hjl.tools@gmail.com
State New
Headers show
Series
  • riscv: Use __has_include__ to include <asm/syscalls.h> [BZ #24022]
Related show

Commit Message

H.J. Lu Dec. 21, 2018, 5:48 p.m.
<asm/syscalls.h> has been removed by

commit 27f8899d6002e11a6e2d995e29b8deab5aa9cc25
Author: David Abdurachmanov <david.abdurachmanov@gmail.com>
Date:   Thu Nov 8 20:02:39 2018 +0100

    riscv: add asm/unistd.h UAPI header

    Marcin Juszkiewicz reported issues while generating syscall table for riscv
    using 4.20-rc1. The patch refactors our unistd.h files to match some other
    architectures.

    - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 64-bit
    - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
    - Adjust kernel asm/unistd.h

    So now asm/unistd.h UAPI header should show all syscalls for riscv.

<asm/syscalls.h> may be restored by

Subject: [PATCH] riscv: restore asm/syscalls.h UAPI header
Date: Tue, 11 Dec 2018 09:09:35 +0100

UAPI header asm/syscalls.h was merged into UAPI asm/unistd.h header,
which did resolve issue with missing syscalls macros resulting in
glibc (2.28) build failure. It also broke glibc in a different way:
asm/syscalls.h is being used by glibc. I noticed this while doing
Fedora 30/Rawhide mass rebuild.

The patch returns asm/syscalls.h header and incl. it into asm/unistd.h.
I plan to send a patch to glibc to use asm/unistd.h instead of
asm/syscalls.h

In the meantime, we use __has_include__, which was added to GCC 5, to
check if <asm/syscalls.h> exists before including it.  Tested with
build-many-glibcs.py for riscv against kernel 4.19.12 and 4.20-rc7.

	[BZ #24022]
	* sysdeps/unix/sysv/linux/riscv/flush-icache.c: Check if
	<asm/syscalls.h> exists with __has_include__ before including it.
---
 sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.19.2

Comments

Joseph Myers Dec. 31, 2018, 5:10 p.m. | #1
On Fri, 21 Dec 2018, H.J. Lu wrote:

> +#if __has_include__ (<asm/syscalls.h>)

>  #include <asm/syscalls.h>

> +#else

> +#include <asm/unistd.h>

> +#endif


Missing preprocessor indentation, "# include", in both halves of the #if.  
OK with that fixed, please commit, given that the asm/syscalls.h header is 
in fact not in 4.20.

-- 
Joseph S. Myers
joseph@codesourcery.com
Florian Weimer Dec. 31, 2018, 5:36 p.m. | #2
* H. J. Lu:

> +#if __has_include__ (<asm/syscalls.h>)


Isn't __has_include (no trailing __) the more canonical name?
I think that's what's in C++17.
H.J. Lu Dec. 31, 2018, 6:17 p.m. | #3
On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>

> * H. J. Lu:

>

> > +#if __has_include__ (<asm/syscalls.h>)

>

> Isn't __has_include (no trailing __) the more canonical name?

> I think that's what's in C++17.

>


I think __has_include__ is OK for glibc internal source.

-- 
H.J.
Florian Weimer Dec. 31, 2018, 6:25 p.m. | #4
* H. J. Lu:

> On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:

>>

>> * H. J. Lu:

>>

>> > +#if __has_include__ (<asm/syscalls.h>)

>>

>> Isn't __has_include (no trailing __) the more canonical name?

>> I think that's what's in C++17.

>

> I think __has_include__ is OK for glibc internal source.


Both are accepted by the compiler.  But I want to reduce cognitive
load for the reader, and avoid use unportable constructs when
prefectly portable ones exist.
H.J. Lu Dec. 31, 2018, 6:32 p.m. | #5
On Mon, Dec 31, 2018 at 10:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>

> * H. J. Lu:

>

> > On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:

> >>

> >> * H. J. Lu:

> >>

> >> > +#if __has_include__ (<asm/syscalls.h>)

> >>

> >> Isn't __has_include (no trailing __) the more canonical name?

> >> I think that's what's in C++17.

> >

> > I think __has_include__ is OK for glibc internal source.

>

> Both are accepted by the compiler.  But I want to reduce cognitive

> load for the reader, and avoid use unportable constructs when

> prefectly portable ones exist.


Fine with me.

-- 
H.J.
Mark Corbin Jan. 3, 2019, 11:59 a.m. | #6
On 21/12/2018 17:48, H.J. Lu wrote:
> ---

>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> index d612ef4c6c..d0ecec5107 100644

> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> @@ -21,7 +21,11 @@

>  #include <stdlib.h>

>  #include <atomic.h>

>  #include <sys/cachectl.h>

> +#if __has_include__ (<asm/syscalls.h>)

>  #include <asm/syscalls.h>

> +#else

> +#include <asm/unistd.h>

> +#endif

>  

>  typedef int (*func_type) (void *, void *, unsigned long int);

>  


Hello

I was wondering whether it would be possible for somebody to backport
this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from
master to the release/2.28/master branch.

The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel
and this is currently causing glibc build failures for the RISC-V
architecture under Buildroot (https://buildroot.org). I have a patch for
glibc in Buildroot, but it would be good to use the upstream release.

Many thanks

Mark

-- 
Mark Corbin
Embecosm Ltd.
https://www.embecosm.com
Florian Weimer Jan. 3, 2019, 1:51 p.m. | #7
* Mark Corbin:

> On 21/12/2018 17:48, H.J. Lu wrote:

>> ---

>>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++

>>  1 file changed, 4 insertions(+)

>>

>> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>> index d612ef4c6c..d0ecec5107 100644

>> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>> @@ -21,7 +21,11 @@

>>  #include <stdlib.h>

>>  #include <atomic.h>

>>  #include <sys/cachectl.h>

>> +#if __has_include__ (<asm/syscalls.h>)

>>  #include <asm/syscalls.h>

>> +#else

>> +#include <asm/unistd.h>

>> +#endif

>>  

>>  typedef int (*func_type) (void *, void *, unsigned long int);

>>  

>

> Hello

>

> I was wondering whether it would be possible for somebody to backport

> this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from

> master to the release/2.28/master branch.

>

> The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel

> and this is currently causing glibc build failures for the RISC-V

> architecture under Buildroot (https://buildroot.org). I have a patch for

> glibc in Buildroot, but it would be good to use the upstream release.


I have now done so.

Thanks,
Florian
Mark Corbin Jan. 3, 2019, 2:14 p.m. | #8
On 03/01/2019 13:51, Florian Weimer wrote:
> * Mark Corbin:

>

>> On 21/12/2018 17:48, H.J. Lu wrote:

>>> ---

>>>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++

>>>  1 file changed, 4 insertions(+)

>>>

>>> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>>> index d612ef4c6c..d0ecec5107 100644

>>> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>>> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

>>> @@ -21,7 +21,11 @@

>>>  #include <stdlib.h>

>>>  #include <atomic.h>

>>>  #include <sys/cachectl.h>

>>> +#if __has_include__ (<asm/syscalls.h>)

>>>  #include <asm/syscalls.h>

>>> +#else

>>> +#include <asm/unistd.h>

>>> +#endif

>>>  

>>>  typedef int (*func_type) (void *, void *, unsigned long int);

>>>  

>> Hello

>>

>> I was wondering whether it would be possible for somebody to backport

>> this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from

>> master to the release/2.28/master branch.

>>

>> The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel

>> and this is currently causing glibc build failures for the RISC-V

>> architecture under Buildroot (https://buildroot.org). I have a patch for

>> glibc in Buildroot, but it would be good to use the upstream release.

> I have now done so.

>

> Thanks,

> Florian


Thanks - appreciate it.

-- 
Mark Corbin
Embecosm Ltd.
https://www.embecosm.com
Palmer Dabbelt Jan. 4, 2019, 10:56 p.m. | #9
On Fri, 21 Dec 2018 09:48:40 PST (-0800), H.J. Lu wrote:
> <asm/syscalls.h> has been removed by

>

> commit 27f8899d6002e11a6e2d995e29b8deab5aa9cc25

> Author: David Abdurachmanov <david.abdurachmanov@gmail.com>

> Date:   Thu Nov 8 20:02:39 2018 +0100

>

>     riscv: add asm/unistd.h UAPI header

>

>     Marcin Juszkiewicz reported issues while generating syscall table for riscv

>     using 4.20-rc1. The patch refactors our unistd.h files to match some other

>     architectures.

>

>     - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 64-bit

>     - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h

>     - Adjust kernel asm/unistd.h

>

>     So now asm/unistd.h UAPI header should show all syscalls for riscv.

>

> <asm/syscalls.h> may be restored by

>

> Subject: [PATCH] riscv: restore asm/syscalls.h UAPI header

> Date: Tue, 11 Dec 2018 09:09:35 +0100

>

> UAPI header asm/syscalls.h was merged into UAPI asm/unistd.h header,

> which did resolve issue with missing syscalls macros resulting in

> glibc (2.28) build failure. It also broke glibc in a different way:

> asm/syscalls.h is being used by glibc. I noticed this while doing

> Fedora 30/Rawhide mass rebuild.

>

> The patch returns asm/syscalls.h header and incl. it into asm/unistd.h.

> I plan to send a patch to glibc to use asm/unistd.h instead of

> asm/syscalls.h

>

> In the meantime, we use __has_include__, which was added to GCC 5, to

> check if <asm/syscalls.h> exists before including it.  Tested with

> build-many-glibcs.py for riscv against kernel 4.19.12 and 4.20-rc7.

>

> 	[BZ #24022]

> 	* sysdeps/unix/sysv/linux/riscv/flush-icache.c: Check if

> 	<asm/syscalls.h> exists with __has_include__ before including it.

> ---

>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> index d612ef4c6c..d0ecec5107 100644

> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> @@ -21,7 +21,11 @@

>  #include <stdlib.h>

>  #include <atomic.h>

>  #include <sys/cachectl.h>

> +#if __has_include__ (<asm/syscalls.h>)

>  #include <asm/syscalls.h>

> +#else

> +#include <asm/unistd.h>

> +#endif

>

>  typedef int (*func_type) (void *, void *, unsigned long int);


Thanks!

Patch

diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
index d612ef4c6c..d0ecec5107 100644
--- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
+++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
@@ -21,7 +21,11 @@ 
 #include <stdlib.h>
 #include <atomic.h>
 #include <sys/cachectl.h>
+#if __has_include__ (<asm/syscalls.h>)
 #include <asm/syscalls.h>
+#else
+#include <asm/unistd.h>
+#endif
 
 typedef int (*func_type) (void *, void *, unsigned long int);