On Fri, Jan 26, 2018 at 3:16 AM, Martin Sebor <mse...@gmail.com> wrote:
> PR tree-optimization/83776 - [6/7/8 Regression] missing
> -Warray-bounds indexing past the end of a string literal,
> identified a not-so-recent improvement to constant propagation
> as the reason for GCC no longer being able to detect out-of-
> bounds accesses to string literals. The root cause is that
> the change caused accesses to strings to be transformed into
> MEM_REFs that the -Warray-bounds checker isn't prepared to
> handle. A simple example is:
>
> int h (void)
> {
> const char *p = "1234";
> return p[16]; // missing -Warray-bounds
> }
>
> To fix the regression the attached patch extends the array bounds
> checker to handle the small subset of MEM_REF expressions that
> refer to string literals but stops of short of doing more than
> that. There are outstanding gaps in the detection that the patch
> intentionally doesn't handle. They are either caused by other
> regressions (PR 84047) or by other latent bugs/limitations, or
> by limitations in the approach I took to try to keep the patch
> simple. I hope to address some of those in a follow-up patch
> for GCC 9.
+ gimple *def = SSA_NAME_DEF_STMT (arg);
+ if (!is_gimple_assign (def))
+ {
+ if (tree var = SSA_NAME_VAR (arg))
+ arg = var;
this is never correct. Whether sth has an underlying VAR_DECL or not
is irrelevant. It looks like you'll always take the
+ else
+ return;
path then anyways. So why obfuscate the code this way?
+ offset_int ofr[] = {
+ wi::to_offset (fold_convert (ptrdiff_type_node, vr->min)),
+ wi::to_offset (fold_convert (ptrdiff_type_node, vr->max))
+ };
huh. Do you maybe want to use widest_int for ofr[]? What's
wrong with wi::to_offset (vr->min)? Building another intermediate
tree node here looks just bogus.
What are you trying to do in this loop anyways? I suppose
look at
p_1 = &obj[i_2]; // already bounds-checked, but with ignore_off_by_one
... = MEM[p_1 + CST];
? But then
+ if (TREE_CODE (varoff) != SSA_NAME)
+ break;
you should at least handle INTEGER_CSTs here?
+ if (!vr || vr->type == VR_UNDEFINED || !vr->min || !vr->max)
+ break;
please use positive tests here, VR_RANGE || VR_ANTI_RANGE. As usual
the anti-range handling looks odd. Please add comments so we can follow
what you were thinking when writing range merging code. Even better if you
can stick to use existing code rather than always re-inventing the wheel...
I think I commented on earlier variants but this doesn't seem to resemble
any of them.
Richard.
> Martin
