| Message ID | 20240103171502.1358371-2-bugaevc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64-gnu port | expand |
Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit: > diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h > index 568092d6..189fd44e 100644 > --- a/sysdeps/hurd/include/hurd.h > +++ b/sysdeps/hurd/include/hurd.h > @@ -1,4 +1,5 @@ > #ifndef _HURD_H > +#include <tls.h> > #include_next <hurd.h> > > void _hurd_libc_proc_init (char **argv); > diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h > index 1dc8a1f3..9b1bf3df 100644 > --- a/sysdeps/hurd/include/hurd/signal.h > +++ b/sysdeps/hurd/include/hurd/signal.h > @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_ > libc_hidden_proto (_hurd_self_sigstate) > #endif > > +#include <tls.h> > #include_next <hurd/signal.h> > > #ifndef _ISOMAC Why? These are breaking hurd/check-installed-headers-c Samuel
Applied the rest, thanks! Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit: > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > hurd/hurdsig.c | 2 +- > mach/lowlevellock.h | 1 + > sysdeps/hurd/include/hurd.h | 1 + > sysdeps/hurd/include/hurd/signal.h | 1 + > sysdeps/mach/hurd/mig-reply.c | 1 + > 5 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c > index 3deb94f9..fe788193 100644 > --- a/hurd/hurdsig.c > +++ b/hurd/hurdsig.c > @@ -15,6 +15,7 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <assert.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > @@ -260,7 +261,6 @@ _hurd_sigstate_actions (struct hurd_sigstate *ss) > #include <hurd/msg_server.h> > #include <hurd/msg_reply.h> /* For __msg_sig_post_reply. */ > #include <hurd/interrupt.h> > -#include <assert.h> > #include <unistd.h> > > > diff --git a/mach/lowlevellock.h b/mach/lowlevellock.h > index 129a7bd9..c5bb0879 100644 > --- a/mach/lowlevellock.h > +++ b/mach/lowlevellock.h > @@ -19,6 +19,7 @@ > #ifndef _MACH_LOWLEVELLOCK_H > #define _MACH_LOWLEVELLOCK_H 1 > > +#include <mach.h> > #include <mach/gnumach.h> > #include <atomic.h> > > diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h > index 568092d6..189fd44e 100644 > --- a/sysdeps/hurd/include/hurd.h > +++ b/sysdeps/hurd/include/hurd.h > @@ -1,4 +1,5 @@ > #ifndef _HURD_H > +#include <tls.h> > #include_next <hurd.h> > > void _hurd_libc_proc_init (char **argv); > diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h > index 1dc8a1f3..9b1bf3df 100644 > --- a/sysdeps/hurd/include/hurd/signal.h > +++ b/sysdeps/hurd/include/hurd/signal.h > @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_ > libc_hidden_proto (_hurd_self_sigstate) > #endif > > +#include <tls.h> > #include_next <hurd/signal.h> > > #ifndef _ISOMAC > diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c > index 57bcd808..85c3af13 100644 > --- a/sysdeps/mach/hurd/mig-reply.c > +++ b/sysdeps/mach/hurd/mig-reply.c > @@ -15,6 +15,7 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <assert.h> > #include <mach.h> > #include <mach/mig_support.h> > #include <tls.h> > -- > 2.43.0 > >
On Wed, Jan 3, 2024 at 11:43 PM Samuel Thibault <samuel.thibault@gnu.org> wrote: > Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit: > > diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h > > index 568092d6..189fd44e 100644 > > --- a/sysdeps/hurd/include/hurd.h > > +++ b/sysdeps/hurd/include/hurd.h > > @@ -1,4 +1,5 @@ > > #ifndef _HURD_H > > +#include <tls.h> > > #include_next <hurd.h> > > > > void _hurd_libc_proc_init (char **argv); > > diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h > > index 1dc8a1f3..9b1bf3df 100644 > > --- a/sysdeps/hurd/include/hurd/signal.h > > +++ b/sysdeps/hurd/include/hurd/signal.h > > @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_ > > libc_hidden_proto (_hurd_self_sigstate) > > #endif > > > > +#include <tls.h> > > #include_next <hurd/signal.h> > > > > #ifndef _ISOMAC > > Why? Because hurd/hurd/signal.h is using tls.h macros (THREAD_GETMEM / THREAD_SETMEM / THREAD_SELF), guarded under "defined _LIBC", and sysdeps/hurd/include/hurd/signal.h being the internal version of that header seemed to be the appropriate place to add the missing #include <tls.h>. Otherwise, I get this: In file included from ../sysdeps/hurd/include/hurd/signal.h:9, from siginfo.c:18: ../hurd/hurd/signal.h: In function ‘_hurd_self_sigstate’: ../hurd/hurd/signal.h:169:30: error: implicit declaration of function ‘THREAD_GETMEM’ [-Wimplicit-function-declaration] 169 | struct hurd_sigstate *ss = THREAD_GETMEM (THREAD_SELF, _hurd_sigstate); and so on. This must have happened to work on both x86 architectures due to some other header implicitly pulling <tls.h> in, but we should not rely on that. Perhaps a better solution would be to move the inline versions of _hurd_self_sigstate and _hurd_critical_section_lock/unlock to the internal header. Is there any reason why they have to be in the public one? > These are breaking hurd/check-installed-headers-c Indeed, thanks for pointing that out. But the error I seem to get: :: hurd.h :::: In file included from ../sysdeps/unix/i386/sysdep.h:18, from ../sysdeps/mach/x86/sysdep.h:47, from ../sysdeps/mach/hurd/tls.h:27, from ../sysdeps/mach/hurd/i386/tls.h:24, from ../sysdeps/hurd/include/hurd.h:2, from /tmp/cih_test_z99fCI.c:10: ../sysdeps/unix/sysdep.h:111:5: error: "IS_IN" is not defined, evaluates to 0 [-Werror=undef] 111 | #if IS_IN (rtld) | ^~~~~ ../sysdeps/unix/sysdep.h:111:11: error: missing binary operator before token "(" 111 | #if IS_IN (rtld) | ^ ../sysdeps/mach/hurd/i386/tls.h:123:32: error: missing binary operator before token "(" 123 | #if !defined (SHARED) || IS_IN (rtld) | ^ ../sysdeps/mach/hurd/i386/tls.h: In function ‘_hurd_tls_fork’: ../sysdeps/mach/hurd/i386/tls.h:379:3: error: unknown type name ‘error_t’ 379 | error_t err; | ^~~~~~~ ...makes no sense. This is testing installed headers, isn't it? — then how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>? I'm rather sure the installed <hurd.h> is a different file. So it would look like the test setup is broken, and this patch just exposes that. Sergey
On Thu, Jan 4, 2024 at 12:08 AM Sergey Bugaev <bugaevc@gmail.com> wrote: > This is testing installed headers, isn't it? — then > how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>? > I'm rather sure the installed <hurd.h> is a different file. So it > would look like the test setup is broken, and this patch just exposes > that. Should the include <tls.h> be guarded by ifndef _ISOMAC perhaps? And if we indeed move the inline functions to the internal hurd.h, they'd also be guarded by ifndef _ISOMAC? But really, why doesn't the test filter out the internal include directories? Sergey
Sergey Bugaev, le jeu. 04 janv. 2024 00:24:30 +0300, a ecrit: > On Thu, Jan 4, 2024 at 12:08 AM Sergey Bugaev <bugaevc@gmail.com> wrote: > > This is testing installed headers, isn't it? — then > > how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>? > > I'm rather sure the installed <hurd.h> is a different file. So it > > would look like the test setup is broken, and this patch just exposes > > that. > > Should the include <tls.h> be guarded by ifndef _ISOMAC perhaps? And > if we indeed move the inline functions to the internal hurd.h, they'd > also be guarded by ifndef _ISOMAC? Yes. > But really, why doesn't the test filter out the internal include directories? I'll leave that to the people who would know examples of needing internal definitions. Samuel
diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c index 3deb94f9..fe788193 100644 --- a/hurd/hurdsig.c +++ b/hurd/hurdsig.c @@ -15,6 +15,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <assert.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -260,7 +261,6 @@ _hurd_sigstate_actions (struct hurd_sigstate *ss) #include <hurd/msg_server.h> #include <hurd/msg_reply.h> /* For __msg_sig_post_reply. */ #include <hurd/interrupt.h> -#include <assert.h> #include <unistd.h> diff --git a/mach/lowlevellock.h b/mach/lowlevellock.h index 129a7bd9..c5bb0879 100644 --- a/mach/lowlevellock.h +++ b/mach/lowlevellock.h @@ -19,6 +19,7 @@ #ifndef _MACH_LOWLEVELLOCK_H #define _MACH_LOWLEVELLOCK_H 1 +#include <mach.h> #include <mach/gnumach.h> #include <atomic.h> diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h index 568092d6..189fd44e 100644 --- a/sysdeps/hurd/include/hurd.h +++ b/sysdeps/hurd/include/hurd.h @@ -1,4 +1,5 @@ #ifndef _HURD_H +#include <tls.h> #include_next <hurd.h> void _hurd_libc_proc_init (char **argv); diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h index 1dc8a1f3..9b1bf3df 100644 --- a/sysdeps/hurd/include/hurd/signal.h +++ b/sysdeps/hurd/include/hurd/signal.h @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_ libc_hidden_proto (_hurd_self_sigstate) #endif +#include <tls.h> #include_next <hurd/signal.h> #ifndef _ISOMAC diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c index 57bcd808..85c3af13 100644 --- a/sysdeps/mach/hurd/mig-reply.c +++ b/sysdeps/mach/hurd/mig-reply.c @@ -15,6 +15,7 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <assert.h> #include <mach.h> #include <mach/mig_support.h> #include <tls.h>
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- hurd/hurdsig.c | 2 +- mach/lowlevellock.h | 1 + sysdeps/hurd/include/hurd.h | 1 + sysdeps/hurd/include/hurd/signal.h | 1 + sysdeps/mach/hurd/mig-reply.c | 1 + 5 files changed, 5 insertions(+), 1 deletion(-)