RE: [PATCH] RISC-V: Fix out of range memory access of machine mode table

["Li, Pan2" <pan2.li@intel.com>]

Hi Jakub,

Thanks for reviewing but I am not quite sure if I fully understand how to fix this issue. Could you please help to enlighten me more about this ?

Currently for RISC-V, the memset has touched out of range memory already due to MAX_MACHINE_MODE > 256. And we may have below parts require adjusting.

1. streamer_mode_table.
2.  bp_unpack_machine_mode/bp_pack_machine_mode 
3.  bp_pack_value/bp_unpack_value in lto_write_mode_table.
4. unsigned char *table = ggc_cleared_vec_alloc<unsigned char> (1 << 8) in lto_input_mode_table.

For 1. is safe to extend the size to MAX_MACHINE_MODE as the array only used as Boolean, aka streamer_mode_table[XXXmode] = 1.
For 2 & 3. Keep 1 << 8 as is, or stream out the host MAX_MACHINE_MODE value somewhere for underlying consuming?
For 4, one possible approach is that extend unsigned char to unsigned short, as well as 256 to MAX_MACHINE_MODE. Because it stores the actually machine mode in array.

Pan

-----Original Message-----
From: Li, Pan2 
Sent: Monday, June 19, 2023 9:36 PM
To: Jakub Jelinek <jakub@redhat.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; jeffreyalaw@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; rguenther@suse.de
Subject: RE: [PATCH] RISC-V: Fix out of range memory access of machine mode table

Thanks Jakub for reviewing, sorry for misleading and will have a try for PATCH v3.

Pan

-----Original Message-----
From: Jakub Jelinek <jakub@redhat.com> 
Sent: Monday, June 19, 2023 5:17 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; jeffreyalaw@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; rguenther@suse.de
Subject: Re: [PATCH] RISC-V: Fix out of range memory access of machine mode table

On Mon, Jun 19, 2023 at 05:05:48PM +0800, pan2.li@intel.com wrote:
> --- a/gcc/lto-streamer-in.cc
> +++ b/gcc/lto-streamer-in.cc
> @@ -1985,7 +1985,8 @@ lto_input_mode_table (struct lto_file_decl_data *file_data)
>      internal_error ("cannot read LTO mode table from %s",
>  		    file_data->file_name);
>  
> -  unsigned char *table = ggc_cleared_vec_alloc<unsigned char> (1 << 8);
> +  unsigned char *table = ggc_cleared_vec_alloc<unsigned char> (
> +    MAX_MACHINE_MODE);

Incorrect formatting.  And, see my other mail, this is wrong.

> @@ -108,7 +108,7 @@ inline void
>  bp_pack_machine_mode (struct bitpack_d *bp, machine_mode mode)
>  {
>    streamer_mode_table[mode] = 1;
> -  bp_pack_enum (bp, machine_mode, 1 << 8, mode);
> +  bp_pack_enum (bp, machine_mode, MAX_MACHINE_MODE, mode);
>  }
>  
>  inline machine_mode
> @@ -116,7 +116,8 @@ bp_unpack_machine_mode (struct bitpack_d *bp)
>  {
>    return (machine_mode)
>  	   ((class lto_input_block *)
> -	    bp->stream)->mode_table[bp_unpack_enum (bp, machine_mode, 1 << 8)];
> +	    bp->stream)->mode_table[bp_unpack_enum (bp, machine_mode,
> +						    MAX_MACHINE_MODE)];
>  }

And these two are wrong as well.  The value passed to bp_pack_enum
has to match the one used on bp_unpack_enum.  But that is not the case
after your changes.  You stream out with the host MAX_MACHINE_MODE, and
stream in for normal LTO with the same value (ok), but for offloading
targets (nvptx, amdgcn) with a different MAX_MACHINE_MODE.  That will
immediate result in LTO streaming being out of sync and ICEs all around.
The reason for using 1 << 8 there was exactly to make it interoperable for
offloading.  What could be perhaps done is that you stream out the
host MAX_MACHINE_MODE value somewhere and stream it in inside of
lto_input_mode_table before you allocate the table.  But, that streamed
in host max_machine_mdoe has to be remembered somewhere and used e.g. in
bp_unpack_machine_mode instead of MAX_MACHINE_MODE.

	Jakub