On Fri, Dec 15, 2017 at 11:17:37AM -0600, Qing Zhao wrote:
> HOST_WIDE_INT const_string_leni = -1;
>
> if (idx1)
> {
> const_string_leni = compute_string_length (idx1);
> var_string = arg2;
> }
> else if (idx2)
> {
> const_string_leni = compute_string_length (idx2);
> var_string = arg1;
> }
>
> so, the -Wmaybe-uninitialized should NOT issue warning, right?
Well, you had the var_string var uninitialized, so that is what I was
talking about.
> but anyway, I can change the above as following:
>
> HOST_WIDE_INT const_string_leni = -1;
And here you don't need to initialize it.
> if (idx1)
> {
> const_string_leni = compute_string_length (idx1);
> var_string = arg2;
> }
> else
> {
> gcc_assert (idx2);
> const_string_leni = compute_string_length (idx2);
> var_string = arg1;
> }
>
> is this better?
Yes, though the gcc_assert could be just gcc_checking_assert (idx2);
> > so that would be mode 2, though that
> > mode isn't actually used in real-world code and thus might be not fully
> > tested.
>
> so, using this routine with mode 2 should be the right approach to go?
> and we need fully testing on this too?
It has been a while since I wrote it, so it would need careful analysis.
> >> B. use “get_range_strlen” in gimple-fold.h to decide the size of the
> >> object. however,
> >> it cannot provide valid info for simple array, either.
> >
> > get_range_strlen returns you a range, the minval is not what you're looking
> > for, that is the minimum string length, so might be too short for your
> > purposes. And maxval is an upper bound, but you are looking for lower
> > bound, you need guarantees this amount of memory can be accessed, even if
> > there is 0 in the first byte.
>
> my understanding is that: get_range_strlen returns the minimum and maximum
> length of the string pointed by the
> pointer, and the maximum length of the string is determined by the size of
> the allocated memory pointed by the
> pointer, so, it should serve my purpose, did I misunderstand it?
What I'm worried about is:
struct S { int a; char b[64]; };
struct T { struct S c; char d; };
int
foo (struct T *x)
{
return strcmp (x->c.b, "01234567890123456789012345678901234567890123456789")
== 0;
}
int
bar (void)
{
struct S *p = malloc (offsetof (struct S, b) + 8);
p->a = 123;
strcpy (p->b, "0123456");
return foo ((struct T *) p);
}
etc. where if you transform that into memcmp (x->c.b,
"01234567890123456789012345678901234567890123456789", 51) == 0
it will segfault, whereas strcmp would not.
> > But if we find out during
> > expansion we don't want to expand it inline, we should fall back to calling
> > strcmp or strncmp.
>
> under what situation we will NOT expand the memcpy_eq call inline?
target = expand_builtin_memcmp (exp, target, fcode == BUILT_IN_MEMCMP_EQ);
if (target)
return target;
if (fcode == BUILT_IN_MEMCMP_EQ)
{
tree newdecl = builtin_decl_explicit (BUILT_IN_MEMCMP);
TREE_OPERAND (exp, 1) = build_fold_addr_expr (newdecl);
}
is what builtins.c has, so it certainly counts with the possibility.
Now, both expand_builtin_memcmp, and emit_block_cmp_hints has several cases
when it fails. E.g. can_do_by_pieces decides it is too expensive to do it
inline, and emit_block_cmp_via_cmpmem fails because the target doesn't have
cmpmemsi expander. Various other cases.
Also, note that some target might have cmpstr*si expanders implemented, but
not cmpmemsi, in which case trying to optimize strcmp as memcmp_eq might be a
severe pessimization.
Jakub
