From: "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>
To: "Thomas Schwinge" <thomas@codesourcery.com>,
pan2.li <pan2.li@intel.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
"Robin Dapp" <rdapp.gcc@gmail.com>,
jeffreyalaw <jeffreyalaw@gmail.com>,
yanzhang.wang <yanzhang.wang@intel.com>,
kito.cheng <kito.cheng@gmail.com>,
rguenther <rguenther@suse.de>, jakub <jakub@redhat.com>,
"Tobias Burnus" <tobias@codesourcery.com>
Subject: Re: Re: [PATCH v3] Streamer: Fix out of range memory access of machine mode
Date: Thu, 29 Jun 2023 17:33:14 +0800 [thread overview]
Message-ID: <DBA326815849F573+2023062917331375019143@rivai.ai> (raw)
In-Reply-To: <874jmqwr8q.fsf@euler.schwinge.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 11162 bytes --]
Not sure what happens you said ICEs all over the place...
Actually, even without this patch, current upstream GCC in RISCV port already ICE all over the place:
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup-3.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup-3.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-1.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-1.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-17.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-17.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-3.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-3.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-2.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-2.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/binop/narrow_run-1.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/binop/narrow_run-1.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-16.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-16.c (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-3.c (internal compiler error: tree check: expected none of vector_type, have vector_type in divmod_candidate_p, at tree-ssa-math-opts.cc:4998)
FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-3.c (test for excess errors)
juzhe.zhong@rivai.ai
From: Thomas Schwinge
Date: 2023-06-29 17:29
To: Pan Li
CC: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; jeffreyalaw@gmail.com; yanzhang.wang@intel.com; kito.cheng@gmail.com; rguenther@suse.de; jakub@redhat.com; Tobias Burnus
Subject: Re: [PATCH v3] Streamer: Fix out of range memory access of machine mode
Hi!
On 2023-06-21T15:58:24+0800, Pan Li via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> We extend the machine mode from 8 to 16 bits already. But there still
> one placing missing from the streamer. It has one hard coded array
> for the machine code like size 256.
>
> In the lto pass, we memset the array by MAX_MACHINE_MODE count but the
> value of the MAX_MACHINE_MODE will grow as more and more modes are
> added. While the machine mode array in tree-streamer still leave 256 as is.
>
> Then, when the MAX_MACHINE_MODE is greater than 256, the memset of
> lto_output_init_mode_table will touch the memory out of range unexpected.
Uh. :-O
> This patch would like to take the MAX_MACHINE_MODE as the size of the
> array in streamer, to make sure there is no potential unexpected
> memory access in future. Meanwhile, this patch also adjust some place
> which has MAX_MACHINE_MODE <= 256 assumption.
Thanks to Jakub and Richard for guidance re the offloading compilation
case, where we've got different 'MAX_MACHINE_MODE's between stream-out
and stream-in, and a modes mapping table.
However, with this patch, there are ICEs all over the place... I'm
having a look.
Grüße
Thomas
> gcc/ChangeLog:
>
> * lto-streamer-in.cc (lto_input_mode_table): Stream in the mode
> bits for machine mode table.
> * lto-streamer-out.cc (lto_write_mode_table): Stream out the
> HOST machine mode bits.
> * lto-streamer.h (struct lto_file_decl_data): New fields mode_bits.
> * tree-streamer.cc (streamer_mode_table): Take MAX_MACHINE_MODE
> as the table size.
> * tree-streamer.h (streamer_mode_table): Ditto.
> (bp_pack_machine_mode): Take 1 << ceil_log2 (MAX_MACHINE_MODE)
> as the packing limit.
> (bp_unpack_machine_mode): Ditto.
> ---
> gcc/lto-streamer-in.cc | 12 ++++++++----
> gcc/lto-streamer-out.cc | 11 ++++++++---
> gcc/lto-streamer.h | 2 ++
> gcc/tree-streamer.cc | 2 +-
> gcc/tree-streamer.h | 14 +++++++++-----
> 5 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc
> index 2cb83406db5..2a0720b4e6f 100644
> --- a/gcc/lto-streamer-in.cc
> +++ b/gcc/lto-streamer-in.cc
> @@ -1985,8 +1985,6 @@ 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);
> - file_data->mode_table = table;
> const struct lto_simple_header_with_strings *header
> = (const struct lto_simple_header_with_strings *) data;
> int string_offset;
> @@ -1998,16 +1996,22 @@ lto_input_mode_table (struct lto_file_decl_data *file_data)
> header->string_size, vNULL);
> bitpack_d bp = streamer_read_bitpack (&ib);
>
> + unsigned mode_bits = bp_unpack_value (&bp, 5);
> + unsigned char *table = ggc_cleared_vec_alloc<unsigned char> (1 << mode_bits);
> +
> + file_data->mode_table = table;
> + file_data->mode_bits = mode_bits;
> +
> table[VOIDmode] = VOIDmode;
> table[BLKmode] = BLKmode;
> unsigned int m;
> - while ((m = bp_unpack_value (&bp, 8)) != VOIDmode)
> + while ((m = bp_unpack_value (&bp, mode_bits)) != VOIDmode)
> {
> enum mode_class mclass
> = bp_unpack_enum (&bp, mode_class, MAX_MODE_CLASS);
> poly_uint16 size = bp_unpack_poly_value (&bp, 16);
> poly_uint16 prec = bp_unpack_poly_value (&bp, 16);
> - machine_mode inner = (machine_mode) bp_unpack_value (&bp, 8);
> + machine_mode inner = (machine_mode) bp_unpack_value (&bp, mode_bits);
> poly_uint16 nunits = bp_unpack_poly_value (&bp, 16);
> unsigned int ibit = 0, fbit = 0;
> unsigned int real_fmt_len = 0;
> diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
> index 5ab2eb4301e..36899283ded 100644
> --- a/gcc/lto-streamer-out.cc
> +++ b/gcc/lto-streamer-out.cc
> @@ -3196,6 +3196,11 @@ lto_write_mode_table (void)
> if (inner_m != m)
> streamer_mode_table[(int) inner_m] = 1;
> }
> +
> + /* Pack the mode_bits value within 5 bits (up to 31) in the beginning. */
> + unsigned mode_bits = ceil_log2 (MAX_MACHINE_MODE);
> + bp_pack_value (&bp, mode_bits, 5);
> +
> /* First stream modes that have GET_MODE_INNER (m) == m,
> so that we can refer to them afterwards. */
> for (int pass = 0; pass < 2; pass++)
> @@ -3205,11 +3210,11 @@ lto_write_mode_table (void)
> machine_mode m = (machine_mode) i;
> if ((GET_MODE_INNER (m) == m) ^ (pass == 0))
> continue;
> - bp_pack_value (&bp, m, 8);
> + bp_pack_value (&bp, m, mode_bits);
> bp_pack_enum (&bp, mode_class, MAX_MODE_CLASS, GET_MODE_CLASS (m));
> bp_pack_poly_value (&bp, GET_MODE_SIZE (m), 16);
> bp_pack_poly_value (&bp, GET_MODE_PRECISION (m), 16);
> - bp_pack_value (&bp, GET_MODE_INNER (m), 8);
> + bp_pack_value (&bp, GET_MODE_INNER (m), mode_bits);
> bp_pack_poly_value (&bp, GET_MODE_NUNITS (m), 16);
> switch (GET_MODE_CLASS (m))
> {
> @@ -3229,7 +3234,7 @@ lto_write_mode_table (void)
> }
> bp_pack_string (ob, &bp, GET_MODE_NAME (m), true);
> }
> - bp_pack_value (&bp, VOIDmode, 8);
> + bp_pack_value (&bp, VOIDmode, mode_bits);
>
> streamer_write_bitpack (&bp);
>
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index fc7133d07ba..443f0cd616e 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -604,6 +604,8 @@ struct GTY(()) lto_file_decl_data
> int order_base;
>
> int unit_base;
> +
> + unsigned mode_bits;
> };
>
> typedef struct lto_file_decl_data *lto_file_decl_data_ptr;
> diff --git a/gcc/tree-streamer.cc b/gcc/tree-streamer.cc
> index ed65a7692e3..a28ef9c7920 100644
> --- a/gcc/tree-streamer.cc
> +++ b/gcc/tree-streamer.cc
> @@ -35,7 +35,7 @@ along with GCC; see the file COPYING3. If not see
> During streaming in, we translate the on the disk mode using this
> table. For normal LTO it is set to identity, for ACCEL_COMPILER
> depending on the mode_table content. */
> -unsigned char streamer_mode_table[1 << 8];
> +unsigned char streamer_mode_table[MAX_MACHINE_MODE];
>
> /* Check that all the TS_* structures handled by the streamer_write_* and
> streamer_read_* routines are exactly ALL the structures defined in
> diff --git a/gcc/tree-streamer.h b/gcc/tree-streamer.h
> index 170d61cf20b..ff8bccf901a 100644
> --- a/gcc/tree-streamer.h
> +++ b/gcc/tree-streamer.h
> @@ -75,7 +75,7 @@ void streamer_write_tree_body (struct output_block *, tree);
> void streamer_write_integer_cst (struct output_block *, tree);
>
> /* In tree-streamer.cc. */
> -extern unsigned char streamer_mode_table[1 << 8];
> +extern unsigned char streamer_mode_table[MAX_MACHINE_MODE];
> void streamer_check_handled_ts_structures (void);
> bool streamer_tree_cache_insert (struct streamer_tree_cache_d *, tree,
> hashval_t, unsigned *);
> @@ -108,15 +108,19 @@ 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);
> + int last = 1 << ceil_log2 (MAX_MACHINE_MODE);
> +
> + bp_pack_enum (bp, machine_mode, last, 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)];
> + int last = 1 << ceil_log2 (MAX_MACHINE_MODE);
> + lto_input_block *input_block = (class lto_input_block *) bp->stream;
> + int index = bp_unpack_enum (bp, machine_mode, last);
> +
> + return (machine_mode) input_block->mode_table[index];
> }
>
> #endif /* GCC_TREE_STREAMER_H */
> --
> 2.34.1
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
next prev parent reply other threads:[~2023-06-29 9:33 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-19 8:07 [PATCH v1] RISC-V: Fix out of range memory access when lto mode init 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
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 [this message]
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=DBA326815849F573+2023062917331375019143@rivai.ai \
--to=juzhe.zhong@rivai.ai \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jeffreyalaw@gmail.com \
--cc=kito.cheng@gmail.com \
--cc=pan2.li@intel.com \
--cc=rdapp.gcc@gmail.com \
--cc=rguenther@suse.de \
--cc=thomas@codesourcery.com \
--cc=tobias@codesourcery.com \
--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).