On Wed, 23 Feb 2022, Qing Zhao wrote: > > > > On Feb 23, 2022, at 11:49 AM, Jakub Jelinek <ja...@redhat.com> wrote: > > > > On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote: > >> From my understanding, __builtin_clear_padding (&object), does not _use_ > >> any variable, > >> therefore, no uninitialized usage warning should be emitted for it. > > > > __builtin_clear_padding (&object) > > sometimes expands to roughly: > > *(int *)((char *)&object + 32) = 0; > > etc., in that case it shouldn't be suppressed in any way, it doesn't read > > anything, only stores. > > Or at other times it is: > > *(int *)((char *)&object + 32) &= 0xfec7dab1; > > etc., in that case it reads bytes from the object which can be > > uninitialized, we mask some bits off and store. > > Okay, I see. > So, only the MEM_REF that will be used to read first should be suppressed > warning. Then there is only one (out of 4) MEM_REF > should be suppressed warning, that’s the following one (line 4371 and then > line 4382): > > 4371 tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base, > 4372 build_int_cst (buf->alias_type, > off)); > 4373 tree src; > 4374 gimple *g; > 4375 if (all_ones > 4376 && nonzero_first == start > 4377 && nonzero_last == start + eltsz) > 4378 src = build_zero_cst (type); > 4379 else > 4380 { > 4381 src = make_ssa_name (type); > 4382 g = gimple_build_assign (src, unshare_expr (dst)); > 4383 gimple_set_location (g, buf->loc); > 4384 gsi_insert_before (buf->gsi, g, GSI_SAME_STMT); > 4385 tree mask = native_interpret_expr (type, > 4386 buf->buf + i + > start, > 4387 eltsz); > 4388 gcc_assert (mask && TREE_CODE (mask) == INTEGER_CST); > 4389 mask = fold_build1 (BIT_NOT_EXPR, type, mask); > 4390 tree src_masked = make_ssa_name (type); > 4391 g = gimple_build_assign (src_masked, BIT_AND_EXPR, > 4392 src, mask); > 4393 gimple_set_location (g, buf->loc); > 4394 gsi_insert_before (buf->gsi, g, GSI_SAME_STMT); > 4395 src = src_masked; > 4396 } > 4397 g = gimple_build_assign (dst, src); > > > All the other 3 MEM_REFs are not read. So, we can just exclude them from > suppressing warning, right? > Another question, for the above MEM_REF, should I suppress warning for line > 4371 “dst”? Or shall I > Suppress warning for line 4382 (for the “unshared_expr(dst)”)? > > I think that we should suppress warning for the latter, i.e > “unshared_expr(dst)” at line 4382, right?
Yes, the one that's put into the GIMPLE stmt. > > > > It is similar to what object.bitfld = 3; expands to, > > but usually only after the uninit pass. Though, we have the > > optimize_bit_field_compare optimization, that is done very early > > and I wonder what uninit does about that. Perhaps it ignores > > BIT_FIELD_REFs, I'd need to check that. > > Yes, I see that uninitialized warning specially handles BIT_INSERT_EXPR as: > (tree-ssa-uninit.cc) > > 573 /* Do not warn if the result of the access is then used for > 574 a BIT_INSERT_EXPR. */ > 575 if (lhs && TREE_CODE (lhs) == SSA_NAME) > 576 FOR_EACH_IMM_USE_FAST (luse_p, liter, lhs) > 577 { > 578 gimple *use_stmt = USE_STMT (luse_p); > 579 /* BIT_INSERT_EXPR first operand should not be considered > 580 a use for the purpose of uninit warnings. */ That follows the COMPLEX_EXPR handling I think. > > > > Anyway, if we want to disable uninit warnings for __builtin_clear_padding, > > we should do that with suppress_warning on the read stmts that load > > a byte (or more adjacent ones) before they are masked off and stored again, > > so that we don't warn about that. > > IN addition to this read stmts, shall we suppress warnings for the following: > > /* Emit a runtime loop: > for (; buf.base != end; buf.base += sz) > __builtin_clear_padding (buf.base); */ > > static void > clear_padding_emit_loop (clear_padding_struct *buf, tree type, > tree end, bool for_auto_init) > { > > i.e, should we suppress warnings for the above “buf.base != end”, “buf.base > += sz”? > > No need to suppress warning for them since they just read the address of the > object, not the object itself? No need to supporess those indeed. Richard.