[RFC,gnumach,03/34] Make exception subcode a long

  On EXC_BAD_ACCESS, exception subcode is used to pass the faulting memory
address, so it needs to be (at least) pointer-sized. Thus, make it into
a long.

This requires matching changes in glibc and the Hurd.
---

NOTE: Most of this was a pretty mechanical change, but I'm not very sure
I got the exc_subcode_proto right; please do check.

 doc/mach.texi         |  4 ++--
 i386/i386/trap.c      |  2 +-
 i386/i386/trap.h      |  2 +-
 include/mach/exc.defs |  2 +-
 kern/exception.c      | 40 ++++++++++++++++++++++++++++------------
 kern/exception.h      |  6 +++---
 kern/thread.h         |  2 +-
 7 files changed, 37 insertions(+), 21 deletions(-)
  

Applied, thanks!

Sergey Bugaev, le dim. 19 mars 2023 18:09:46 +0300, a ecrit:
> On EXC_BAD_ACCESS, exception subcode is used to pass the faulting memory
> address, so it needs to be (at least) pointer-sized. Thus, make it into
> a long.
> 
> This requires matching changes in glibc and the Hurd.

But the change doesn't affect 32bit glibc and hurd since
rpc_long_integer_t is really like integer_t there, right?

> ---
> 
> NOTE: Most of this was a pretty mechanical change, but I'm not very sure
> I got the exc_subcode_proto right; please do check.
> 
>  doc/mach.texi         |  4 ++--
>  i386/i386/trap.c      |  2 +-
>  i386/i386/trap.h      |  2 +-
>  include/mach/exc.defs |  2 +-
>  kern/exception.c      | 40 ++++++++++++++++++++++++++++------------
>  kern/exception.h      |  6 +++---
>  kern/thread.h         |  2 +-
>  7 files changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/mach.texi b/doc/mach.texi
> index 2c22fa38..fdc36d45 100644
> --- a/doc/mach.texi
> +++ b/doc/mach.texi
> @@ -4830,11 +4830,11 @@ argument set to @code{THREAD_EXCEPTION_PORT}.
>  @node Exceptions
>  @subsection Exceptions
>  
> -@deftypefun kern_return_t catch_exception_raise (@w{mach_port_t @var{exception_port}}, @w{thread_t @var{thread}}, @w{task_t @var{task}}, @w{int @var{exception}}, @w{int @var{code}}, @w{int @var{subcode}})
> +@deftypefun kern_return_t catch_exception_raise (@w{mach_port_t @var{exception_port}}, @w{thread_t @var{thread}}, @w{task_t @var{task}}, @w{int @var{exception}}, @w{int @var{code}}, @w{long @var{subcode}})
>  XXX Fixme
>  @end deftypefun
>  
> -@deftypefun kern_return_t exception_raise (@w{mach_port_t @var{exception_port}}, @w{mach_port_t @var{thread}}, @w{mach_port_t @var{task}}, @w{integer_t @var{exception}}, @w{integer_t @var{code}}, @w{integer_t @var{subcode}})
> +@deftypefun kern_return_t exception_raise (@w{mach_port_t @var{exception_port}}, @w{mach_port_t @var{thread}}, @w{mach_port_t @var{task}}, @w{integer_t @var{exception}}, @w{integer_t @var{code}}, @w{long_integer_t @var{subcode}})
>  XXX Fixme
>  @end deftypefun
>  
> diff --git a/i386/i386/trap.c b/i386/i386/trap.c
> index 34ccb6a5..f7bd8e38 100644
> --- a/i386/i386/trap.c
> +++ b/i386/i386/trap.c
> @@ -628,7 +628,7 @@ void
>  i386_exception(
>  	int	exc,
>  	int	code,
> -	int	subcode)
> +	long	subcode)
>  {
>  	spl_t	s;
>  
> diff --git a/i386/i386/trap.h b/i386/i386/trap.h
> index d9df7afa..e82164d0 100644
> --- a/i386/i386/trap.h
> +++ b/i386/i386/trap.h
> @@ -40,7 +40,7 @@ void
>  i386_exception(
>  	int	exc,
>  	int	code,
> -	int	subcode) __attribute__ ((noreturn));
> +	long	subcode) __attribute__ ((noreturn));
>  
>  extern void
>  thread_kdb_return(void);
> diff --git a/include/mach/exc.defs b/include/mach/exc.defs
> index 94af828c..28638e2f 100644
> --- a/include/mach/exc.defs
> +++ b/include/mach/exc.defs
> @@ -44,4 +44,4 @@ routine		exception_raise(
>  			task		: mach_port_t;
>  			exception	: integer_t;
>  			code		: integer_t;
> -			subcode		: integer_t);
> +			subcode		: rpc_long_integer_t);
> diff --git a/kern/exception.c b/kern/exception.c
> index 1014b3ed..10435b5c 100644
> --- a/kern/exception.c
> +++ b/kern/exception.c
> @@ -85,9 +85,9 @@ boolean_t debug_user_with_kdb = FALSE;
>  
>  void
>  exception(
> -	integer_t _exception, 
> -	integer_t code, 
> -	integer_t subcode)
> +	integer_t _exception,
> +	integer_t code,
> +	long_integer_t subcode)
>  {
>  	ipc_thread_t self = current_thread();
>  	ipc_port_t exc_port;
> @@ -157,9 +157,9 @@ exception(
>  
>  void
>  exception_try_task(
> -	integer_t _exception, 
> -	integer_t code, 
> -	integer_t subcode)
> +	integer_t _exception,
> +	integer_t code,
> +	long_integer_t subcode)
>  {
>  	ipc_thread_t self = current_thread();
>  	task_t task = self->task;
> @@ -277,11 +277,17 @@ struct mach_exception {
>  	mach_msg_type_t		codeType;
>  	integer_t		code;
>  	mach_msg_type_t		subcodeType;
> -	integer_t		subcode;
> +	rpc_long_integer_t	subcode;
>  };
>  
>  #define	INTEGER_T_SIZE_IN_BITS	(8 * sizeof(integer_t))
>  #define	INTEGER_T_TYPE		MACH_MSG_TYPE_INTEGER_T
> +#define RPC_LONG_INTEGER_T_SIZE_IN_BITS	(8 * sizeof(rpc_long_integer_t))
> +#if defined(__x86_64__) && !defined(USER32)
> +#define RPC_LONG_INTEGER_T_TYPE	MACH_MSG_TYPE_INTEGER_64
> +#else
> +#define RPC_LONG_INTEGER_T_TYPE	MACH_MSG_TYPE_INTEGER_32
> +#endif
>  					/* in mach/machine/vm_types.h */
>  
>  mach_msg_type_t exc_port_proto = {
> @@ -304,6 +310,16 @@ mach_msg_type_t exc_code_proto = {
>  	/* msgt_unused = */		0
>  };
>  
> +mach_msg_type_t exc_subcode_proto = {
> +	/* msgt_name = */		RPC_LONG_INTEGER_T_TYPE,
> +	/* msgt_size = */		RPC_LONG_INTEGER_T_SIZE_IN_BITS,
> +	/* msgt_number = */		1,
> +	/* msgt_inline = */		TRUE,
> +	/* msgt_longform = */		FALSE,
> +	/* msgt_deallocate = */		FALSE,
> +	/* msgt_unused = */		0
> +};
> +
>  /*
>   *	Routine:	exception_raise
>   *	Purpose:
> @@ -329,9 +345,9 @@ exception_raise(
>  	ipc_port_t 	dest_port,
>  	ipc_port_t 	thread_port,
>  	ipc_port_t 	task_port,
> -	integer_t 	_exception, 
> -	integer_t 	code, 
> -	integer_t 	subcode)
> +	integer_t 	_exception,
> +	integer_t 	code,
> +	long_integer_t 	subcode)
>  {
>  	ipc_thread_t self = current_thread();
>  	ipc_thread_t receiver;
> @@ -521,7 +537,7 @@ exception_raise(
>  	exc->exception = _exception;
>  	exc->codeType = exc_code_proto;
>  	exc->code = code;
> -	exc->subcodeType = exc_code_proto;
> +	exc->subcodeType = exc_subcode_proto;
>  	exc->subcode = subcode;
>  
>  	/*
> @@ -725,7 +741,7 @@ exception_raise(
>  	exc->exception = _exception;
>  	exc->codeType = exc_code_proto;
>  	exc->code = code;
> -	exc->subcodeType = exc_code_proto;
> +	exc->subcodeType = exc_subcode_proto;
>  	exc->subcode = subcode;
>  
>  	ipc_mqueue_send_always(kmsg);
> diff --git a/kern/exception.h b/kern/exception.h
> index 55902dd1..36138da8 100644
> --- a/kern/exception.h
> +++ b/kern/exception.h
> @@ -26,13 +26,13 @@ extern void
>  exception(
>  	integer_t 	_exception,
>  	integer_t	code,
> -	integer_t	subcode) __attribute__ ((noreturn));
> +	long_integer_t	subcode) __attribute__ ((noreturn));
>  
>  extern void
>  exception_try_task(
>  	integer_t 	_exception,
>  	integer_t	code,
> -	integer_t	subcode) __attribute__ ((noreturn));
> +	long_integer_t	subcode) __attribute__ ((noreturn));
>  
>  extern void
>  exception_no_server(void) __attribute__ ((noreturn));
> @@ -44,7 +44,7 @@ exception_raise(
>  	ipc_port_t task_port,
>  	integer_t  _exception,
>  	integer_t  code,
> -	integer_t  subcode) __attribute__ ((noreturn));
> +	long_integer_t  subcode) __attribute__ ((noreturn));
>  
>  extern kern_return_t
>  exception_parse_reply(ipc_kmsg_t kmsg);
> diff --git a/kern/thread.h b/kern/thread.h
> index f8989f45..3485f6af 100644
> --- a/kern/thread.h
> +++ b/kern/thread.h
> @@ -190,7 +190,7 @@ struct thread {
>  			struct ipc_port *port;
>  			int exc;
>  			int code;
> -			int subcode;
> +			long subcode;
>  		} exception;
>  		void *other;		/* catch-all for other state */
>  	} saved;
> -- 
> 2.39.2
>
  

On Mon, Apr 3, 2023 at 1:45 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Sergey Bugaev, le dim. 19 mars 2023 18:09:46 +0300, a ecrit:
> > On EXC_BAD_ACCESS, exception subcode is used to pass the faulting memory
> > address, so it needs to be (at least) pointer-sized. Thus, make it into
> > a long.
> >
> > This requires matching changes in glibc and the Hurd.
>
> But the change doesn't affect 32bit glibc and hurd since
> rpc_long_integer_t is really like integer_t there, right?

It's supposed to be ABI-compatible on 32-bit, since all these types
are 32-bit integers there, yes. (But maybe I messed up, do check!)

But it may break source-level compatibility (if you only apply the
Mach change but not glibc, or vice versa); as in maybe GCC will
complain about int vs long in function prototype vs definition. I
haven't actually checked, since I have both changes applied locally.

Sergey
  

Hello!

On Mon, Apr 3, 2023 at 5:32 AM Sergey Bugaev via Libc-alpha <
libc-alpha@sourceware.org> wrote:

> On Mon, Apr 3, 2023 at 1:45 AM Samuel Thibault <samuel.thibault@gnu.org>
> wrote:
> > Sergey Bugaev, le dim. 19 mars 2023 18:09:46 +0300, a ecrit:
> > > On EXC_BAD_ACCESS, exception subcode is used to pass the faulting
> memory
> > > address, so it needs to be (at least) pointer-sized. Thus, make it into
> > > a long.
> > >
> > > This requires matching changes in glibc and the Hurd.
> >
> > But the change doesn't affect 32bit glibc and hurd since
> > rpc_long_integer_t is really like integer_t there, right?
>
> It's supposed to be ABI-compatible on 32-bit, since all these types
> are 32-bit integers there, yes. (But maybe I messed up, do check!)
>
> But it may break source-level compatibility (if you only apply the
> Mach change but not glibc, or vice versa); as in maybe GCC will
> complain about int vs long in function prototype vs definition. I
> haven't actually checked, since I have both changes applied locally.
>

It breaks compatibility for Hurd code:

hurd/mach-defpager/default_pager.c:3789:1: error: conflicting types for
'catch_exception_raise'; have 'kern_return_t(mach_port_t, mach_port_t,
mach_port_t, int, int, int)' {aka 'int(unsigned int, unsigned int, unsigned
int, int, int, int)'}
3789 | catch_exception_raise(mach_port_t exception_port,
| ^~~~~~~~~~~~~~~~~~~~~
In file included from
/home/runner/_work/cross-hurd/cross-hurd/src/hurd/mach-defpager/default_pager.c:67:

./exc_S.h:19:15: note: previous declaration of 'catch_exception_raise' with
type 'kern_return_t(mach_port_t, mach_port_t, mach_port_t, integer_t,
integer_t, rpc_long_integer_t)' {aka 'int(unsigned int, unsigned int,
unsigned int, int, int, long int)'}
19 | kern_return_t catch_exception_raise


> Sergey
>
  

Hello,

Flávio Cruz, le mer. 05 avril 2023 22:11:38 -0400, a ecrit:
> On Mon, Apr 3, 2023 at 5:32 AM Sergey Bugaev via Libc-alpha <[1]
> libc-alpha@sourceware.org> wrote:
> 
>     On Mon, Apr 3, 2023 at 1:45 AM Samuel Thibault <[2]samuel.thibault@gnu.org>
>     wrote:
>     > Sergey Bugaev, le dim. 19 mars 2023 18:09:46 +0300, a ecrit:
>     > > On EXC_BAD_ACCESS, exception subcode is used to pass the faulting
>     memory
>     > > address, so it needs to be (at least) pointer-sized. Thus, make it into
>     > > a long.
>     > >
>     > > This requires matching changes in glibc and the Hurd.
>     >
>     > But the change doesn't affect 32bit glibc and hurd since
>     > rpc_long_integer_t is really like integer_t there, right?
> 
>     It's supposed to be ABI-compatible on 32-bit, since all these types
>     are 32-bit integers there, yes. (But maybe I messed up, do check!)
> 
>     But it may break source-level compatibility (if you only apply the
>     Mach change but not glibc, or vice versa); as in maybe GCC will
>     complain about int vs long in function prototype vs definition. I
>     haven't actually checked, since I have both changes applied locally.
> 
> 
> It breaks compatibility for Hurd code:
> 
> hurd/mach-defpager/default_pager.c:3789:1: error: conflicting types for
> 'catch_exception_raise'; have 'kern_return_t(mach_port_t, mach_port_t,
> mach_port_t, int, int, int)' {aka 'int(unsigned int, unsigned int, unsigned
> int, int, int, int)'}
> 3789 | catch_exception_raise(mach_port_t exception_port,
> | ^~~~~~~~~~~~~~~~~~~~~
> In file included from /home/runner/_work/cross-hurd/cross-hurd/src/hurd/
> mach-defpager/default_pager.c:67:
> ./exc_S.h:19:15: note: previous declaration of 'catch_exception_raise' with
> type 'kern_return_t(mach_port_t, mach_port_t, mach_port_t, integer_t,
> integer_t, rpc_long_integer_t)' {aka 'int(unsigned int, unsigned int, unsigned
> int, int, int, long int)'}
> 19 | kern_return_t catch_exception_raise

I have uploaded the fixes as debian packages, in addition to the commits
upstream.

Samuel