From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 307353858D35 for ; Thu, 29 Jun 2023 09:47:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 307353858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.01,168,1684828800"; d="scan'208";a="11510563" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 29 Jun 2023 01:47:25 -0800 IronPort-SDR: UsWHq84Wu/UUKsjE1miUdOf0PHcpSDhaKNAqsdSD0P1yguJkHwzwFLawuFaN+4mxcxT0WzNXYJ m4yS0tOgucM5T2eASRzfJBG0e2GfTTTNvIHxudLpN2ybJjFLX3638FuykXv5bxHwIPgbn4z+6Y lRNCtnJzgx/VvU0fGenu6FQngd/STNZJNKlndl1VrursLPFjong6WzZgRhjBZcgXvSha0wRPhH XmRpSfjS3D/++FBipXWPM9MImhGWJx+fjKdzBQyabdqm7esdsqyKmA7i/OLCY5hPjVoevua7EB 3Pg= From: Thomas Schwinge To: CC: , , Robin Dapp , , , , , , "Tobias Burnus" Subject: Re: Re: [PATCH v3] Streamer: Fix out of range memory access of machine mode In-Reply-To: References: <20230619080710.1536456-1-pan2.li@intel.com> <20230621075824.1990571-1-pan2.li@intel.com> <874jmqwr8q.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Thu, 29 Jun 2023 11:47:18 +0200 Message-ID: <87pm5ed2hl.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi! On 2023-06-29T17:33:14+0800, "juzhe.zhong@rivai.ai" = wrote: > Not sure what happens you said ICEs all over the place... Ah, sorry for not providing proper context here. My comment was about heterogeneous GCC configurations involving code offloading, like: x86_64 host with GCN, nvptx offload targets, which I'd been asked to test this "Streamer: Fix out of range memory access of machine mode" for (in private email) -- for good reasons, as we've now found. ;-| > Actually, even without this patch, current upstream GCC in RISCV port alr= eady 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 divmo= d_candidate_p, at tree-ssa-math-opts.cc:4998) > FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-1.c (test for excess e= rrors) > FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-17.c (internal compile= r error: tree check: expected none of vector_type, have vector_type in divm= od_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 (inter= nal compiler error: tree check: expected none of vector_type, have vector_t= ype 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 divmo= d_candidate_p, at tree-ssa-math-opts.cc:4998) > FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-2.c (test for excess e= rrors) > FAIL: gcc.target/riscv/rvv/autovec/binop/narrow_run-1.c (internal compile= r error: tree check: expected none of vector_type, have vector_type in divm= od_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 compile= r error: tree check: expected none of vector_type, have vector_type in divm= od_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 divmo= d_candidate_p, at tree-ssa-math-opts.cc:4998) > FAIL: gcc.target/riscv/rvv/autovec/partial/slp_run-3.c (test for excess e= rrors) That looks like a different issue, though? Gr=C3=BC=C3=9Fe Thomas > 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; j= effreyalaw@gmail.com; yanzhang.wang@intel.com; kito.cheng@gmail.com; rguent= her@suse.de; jakub@redhat.com; Tobias Burnus > Subject: Re: [PATCH v3] Streamer: Fix out of range memory access of machi= ne mode > Hi! > > On 2023-06-21T15:58:24+0800, Pan Li via Gcc-patches 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 <=3D 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=C3=BC=C3=9Fe > 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 *f= ile_data) >> internal_error ("cannot read LTO mode table from %s", >> file_data->file_name); >> >> - unsigned char *table =3D ggc_cleared_vec_alloc (1 << 8= ); >> - file_data->mode_table =3D table; >> const struct lto_simple_header_with_strings *header >> =3D (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 =3D streamer_read_bitpack (&ib); >> >> + unsigned mode_bits =3D bp_unpack_value (&bp, 5); >> + unsigned char *table =3D ggc_cleared_vec_alloc (1 << m= ode_bits); >> + >> + file_data->mode_table =3D table; >> + file_data->mode_bits =3D mode_bits; >> + >> table[VOIDmode] =3D VOIDmode; >> table[BLKmode] =3D BLKmode; >> unsigned int m; >> - while ((m =3D bp_unpack_value (&bp, 8)) !=3D VOIDmode) >> + while ((m =3D bp_unpack_value (&bp, mode_bits)) !=3D VOIDmode) >> { >> enum mode_class mclass >> =3D bp_unpack_enum (&bp, mode_class, MAX_MODE_CLASS); >> poly_uint16 size =3D bp_unpack_poly_value (&bp, 16); >> poly_uint16 prec =3D bp_unpack_poly_value (&bp, 16); >> - machine_mode inner =3D (machine_mode) bp_unpack_value (&bp, 8); >> + machine_mode inner =3D (machine_mode) bp_unpack_value (&bp, mode_= bits); >> poly_uint16 nunits =3D bp_unpack_poly_value (&bp, 16); >> unsigned int ibit =3D 0, fbit =3D 0; >> unsigned int real_fmt_len =3D 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 !=3D m) >> streamer_mode_table[(int) inner_m] =3D 1; >> } >> + >> + /* Pack the mode_bits value within 5 bits (up to 31) in the beginning= . */ >> + unsigned mode_bits =3D ceil_log2 (MAX_MACHINE_MODE); >> + bp_pack_value (&bp, mode_bits, 5); >> + >> /* First stream modes that have GET_MODE_INNER (m) =3D=3D m, >> so that we can refer to them afterwards. */ >> for (int pass =3D 0; pass < 2; pass++) >> @@ -3205,11 +3210,11 @@ lto_write_mode_table (void) >> machine_mode m =3D (machine_mode) i; >> if ((GET_MODE_INNER (m) =3D=3D m) ^ (pass =3D=3D 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_* a= nd >> 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] =3D 1; >> - bp_pack_enum (bp, machine_mode, 1 << 8, mode); >> + int last =3D 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 =3D 1 << ceil_log2 (MAX_MACHINE_MODE); >> + lto_input_block *input_block =3D (class lto_input_block *) bp->stream= ; >> + int index =3D 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=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955