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.
