public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: "Li, Pan2" <pan2.li@intel.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>,
	"rdapp.gcc@gmail.com" <rdapp.gcc@gmail.com>,
	"jeffreyalaw@gmail.com" <jeffreyalaw@gmail.com>,
	"Wang, Yanzhang" <yanzhang.wang@intel.com>,
	"kito.cheng@gmail.com" <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix out of range memory access when lto mode init
Date: Mon, 19 Jun 2023 11:10:10 +0200	[thread overview]
Message-ID: <ZJAbcmS94TBH/KQL@tucnak> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2306190836320.4723@jbgna.fhfr.qr>

On Mon, Jun 19, 2023 at 08:40:58AM +0000, Richard Biener wrote:
> You also have to fix bp_pack_machine_mode/bp_unpack_machine_mode which
> streams exactly values in [0, 1<<8 - 1].
> 
> CCing Jakub who invented this code.

For stream-out, all it stores is a bool flag whether the mode is streamed
out, and on stream in it contains a mapping table between host and
offloading modes.
For stream in, it actually isn't used despite the comment maybe suggesting
it is, so I guess using MAX_MACHINE_MODE for it is ok.

As you said,
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);
}

inline machine_mode
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)];
}
needs changing for the case when MAX_MACHINE_MODE > 256, but far more
places make similar assumptions:
E.g. lto_write_mode_table has
          bp_pack_value (&bp, m, 8);
(if MAX_MACHINE_MODE > 256, this can't encode all modes),
          bp_pack_value (&bp, GET_MODE_INNER (m), 8);
(ditto).
lto_input_mode_table has e.g.
  unsigned char *table = ggc_cleared_vec_alloc<unsigned char> (1 << 8);
  file_data->mode_table = table;
Here we need to decide if we keep requiring that offloading architectures
still have MAX_MACHINE_MODE <= 256 or not.  Currently the offloading
arches are nvptx and amdgcn, intelmic support has been removed.
If yes, table can have unsigned char elements, but its size actually depends
on the number of modes on the host side, so lto_write_mode_table would need
to stream out the host MAX_MACHINE_MODE value and lto_input_mode_table
stream it in and use instead of the 1 << 8 size above.
If not, mode_table and unsigned char * would need to change to unsigned
short *, or just conditionally depending on if MAX_MACHINE_MODE <= 256 or
not.
Then
  while ((m = bp_unpack_value (&bp, 8)) != VOIDmode)
    {
...
      machine_mode inner = (machine_mode) bp_unpack_value (&bp, 8);
again hardcode 8 bits, that needs to match how many bits packs the host
compiler in lto_write_mode_table.

	Jakub


  parent reply	other threads:[~2023-06-19  9:10 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19  8:07 pan2.li
2023-06-19  8:16 ` Li, Pan2
2023-06-19  8:40   ` Richard Biener
2023-06-19  9:08     ` Li, Pan2
2023-06-19  9:10     ` Jakub Jelinek [this message]
2023-06-19  9:05 ` [PATCH] RISC-V: Fix out of range memory access of machine mode table pan2.li
2023-06-19  9:15   ` Richard Biener
2023-06-19  9:16   ` Jakub Jelinek
2023-06-19 13:35     ` Li, Pan2
2023-06-20  7:50       ` Li, Pan2
2023-06-20  8:03         ` Jakub Jelinek
2023-06-20 14:08           ` Li, Pan2
2023-06-20 15:25             ` Jakub Jelinek
2023-06-21  6:59               ` Li, Pan2
2023-06-21  7:16                 ` Jakub Jelinek
2023-06-21  7:23                   ` Li, Pan2
2023-06-22  0:19                     ` Li, Pan2
2023-06-28 18:37                       ` Jeff Law
2023-06-21  7:58 ` [PATCH v3] Streamer: Fix out of range memory access of machine mode pan2.li
2023-06-22 15:26   ` Li, Pan2
2023-06-29  9:29   ` Thomas Schwinge
2023-06-29  9:33     ` juzhe.zhong
2023-06-29  9:47       ` Thomas Schwinge
2023-06-29  9:52         ` juzhe.zhong
2023-06-29 20:14     ` Thomas Schwinge
2023-06-30  1:26       ` juzhe.zhong
2023-06-30  1:39         ` Li, Pan2
2023-06-30  8:50           ` [v4] " Thomas Schwinge
2023-06-30 11:44             ` Li, Pan2
2023-07-04 11:26             ` Richard Biener
2023-07-04 12:40               ` Li, Pan2
2023-06-30  8:23       ` LTO: Capture 'lto_file_decl_data *file_data' in 'class lto_input_block' (was: [PATCH v3] Streamer: Fix out of range memory access of machine mode) Thomas Schwinge
2023-06-30  8:39         ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZJAbcmS94TBH/KQL@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=juzhe.zhong@rivai.ai \
    --cc=kito.cheng@gmail.com \
    --cc=pan2.li@intel.com \
    --cc=rdapp.gcc@gmail.com \
    --cc=rguenther@suse.de \
    --cc=yanzhang.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).