On 11/26/2016 05:52 PM, Martin Sebor wrote:
On 11/25/2016 12:51 PM, Jeff Law wrote:
On 11/23/2016 06:15 PM, Martin Sebor wrote:
gcc_assert works only in some instances (e.g., in c-ada-spec.c:191)
but not in others because some actually do make the alloca(0) call
at runtime: at a minimum, lto.c:3285, reg-stack.c:2008, and
tree-ssa-threadedge.c:344 assert during bootstrap.
You might have the wrong line number of reg-stack.c and lto. You've
pointed to the start of subst_asm_stack_regs and lto_main respectively.
It'd probably be better if you posted the line with a bit of context.
I must have copied the wrong line numbers or had stale sources
in my tree. Sorry about that. In lto.c, there are two calls
to XALLOCAVEC. I believe the first one is the one where the
alloca(0) call takes place:
1580
1581 tree *map = XALLOCAVEC (tree, 2 * len);
1582 for (tree_scc *pscc = *slot; pscc; pscc = pscc->next)
--
1610 {
1611 tree *map2 = XALLOCAVEC (tree, 2 * len);
1612 for (unsigned i = 0; i < len; ++i)
I'm not at all familiar with this code, but something looks real fishy
here. Essentially if pscc->entry_len is >= 1 and len == 0, then we'll
read map[0] and map[1] which were never allocated (see compare_tree_sccs
and compare_tree_sccs_1).
It may be the case that pscc->entry_len and len are related in such a
way that never happens, but I can't easily prove it. I'd really like
Richi to chime in on how this stuff is supposed to work.
In reg-stack.c it's these three:
2052
2053 note_reg = XALLOCAVEC (rtx, i);
2054 note_loc = XALLOCAVEC (rtx *, i);
2055 note_kind = XALLOCAVEC (enum reg_note, i);
2056
So for reg-stack.c I think we move the n_notes initialization before the
XALLOCAVEC, then wrap the XALLOCAVEC calls and the subsequent loop over
the notes inside an if (i > 0) conditional.
Damn you for making me look at reg-stack.c. It's been years and
hopefully it'll be years before I have to do it again :-)
n_notes = 0;
if (i > 0)
{
note_reg =
note_loc =
note_kind =
for (note = REG_NOTES (insn); ...)
{
...
}
}
I'm pretty sure I can twiddle the tree-ssa-threadedge code to avoid the
problem in there.
To find all such calls I modified GCC to emit an inform call for
every XALLOCAVEC invocation with a zero argument, configured the
patched GCC on x86_64 with all languages (including lto),
bootstrapped it, ran the full test suite, and extracted the set
of unique notes from the logs. Attached in the .log file is
the output along with counts of each. Curiously, neither of
the two above shows up, even though adding asserts for them
broke bootstrap. I haven't investigated why.
Thanks. That's interesting data -- every one of those should be deeply
investigated. I suspect we'd probably trip even more if we did a test
with config-list.mk and perhaps even more if we took those cross
compilers and built the target libraries.
What I think this tells us is that we're not at a place where we're
clean. But we can incrementally get there. The warning is only
catching a fairly small subset of the cases AFAICT. That's not unusual
and analyzing why it didn't trigger on those cases might be useful as well.
So where does this leave us for gcc-7? I'm wondering if we drop the
warning in, but not enable it by default anywhere. We fix the cases we
can (such as reg-stack,c tree-ssa-threadedge.c, maybe others) before
stage3 closes, and shoot for the rest in gcc-8, including improvign the
warning (if there's something we can clearly improve), and enabling the
warning in -Wall or -Wextra.
Jeff
