This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GOOGLE] More strict checking for call args
- From: Martin Jambor <mjambor at suse dot cz>
- To: Dehao Chen <dehao at google dot com>
- Cc: Xinliang David Li <davidxl at google dot com>, Richard Biener <richard dot guenther at gmail dot com>, Duncan Sands <baldrick at free dot fr>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 6 Jun 2013 16:11:24 +0200
- Subject: Re: [GOOGLE] More strict checking for call args
- References: <CAO2gOZWs7maFPDo=EZUe1mPARfNFxxnA5Yg3z0Wo0WS1+2ji2Q at mail dot gmail dot com> <51A85C16 dot 1030505 at free dot fr> <CAAkRFZL7aPp9WSxsj1yaujdBWrs91D=H0d_+KxVcgnh=xnt7Ng at mail dot gmail dot com> <CAO2gOZUZ0xTXoMPmLyhsCRkyFQehu7A7EiuqS1E40hBvd8yvxQ at mail dot gmail dot com> <CAFiYyc2q4nGUWNqDbqcpMunEPr-jQN0f2==h-rmMUCjqTWtTUw at mail dot gmail dot com> <CAO2gOZXqae0ju2nMK=f_XSKZFMOscdmL__P=MYf7x01BBs=jOw at mail dot gmail dot com> <CAAkRFZK4rfGvcaJ2PH34TGXtGaC41Wk8SeJ5hEyiC_eU9LTiTA at mail dot gmail dot com> <CAO2gOZViTTifco=SJ8sFqGjdAzg2h8Dzs87xtLa0ZVGaAJzGLg at mail dot gmail dot com>
Hi,
On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
> attached is a testcase that would cause problem when source has changed:
>
> $ g++ test.cc -O2 -fprofile-generate -DOLD
> $ ./a.out
> $ g++ test.cc -O2 -fprofile-use
> test.cc:34:1: internal compiler error: in operator[], at vec.h:815
> }
> ^
> 0x512740 vec<tree_node*, va_heap, vl_embed>::operator[](unsigned int)
> ../../gcc/vec.h:815
> 0x512740 vec<tree_node*, va_heap, vl_ptr>::operator[](unsigned int)
> ../../gcc/vec.h:1244
> 0xf24464 vec<tree_node*, va_heap, vl_embed>::operator[](unsigned int)
> ../../gcc/vec.h:815
> 0xf24464 vec<tree_node*, va_heap, vl_ptr>::operator[](unsigned int)
> ../../gcc/vec.h:1244
> 0xf24464 ipa_get_indirect_edge_target_1
> ../../gcc/ipa-cp.c:1535
> 0x971b9a estimate_edge_devirt_benefit
> ../../gcc/ipa-inline-analysis.c:2757
Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
Since it is called also from inlining, we can have parameter count
mismatches... and in fact in non-virtual paths of that function we do
check that we don't. Because all callers have to pass known_vals
describing all formal parameters of the inline tree root, we should
apply the fix below (I've only just started running a bootstrap and
testsuite on x86_64, though).
OTOH, while I understand that FDO can change inlining sufficiently so
that this error occurs, IMHO this should not be caused by outdated
profiles but there is somewhere a parameter mismatch in the source.
Dehao, can you please check that this patch helps?
Richi, if it does and the patch passes bootstrap and tests, is it OK
for trunk and 4.8 branch?
Thanks and sorry for the trouble,
Martin
2013-06-06 Martin Jambor <mjambor@suse.cz>
* ipa-cp.c (ipa_get_indirect_edge_target_1): Check that param_index is
within bounds at the beginning of the function.
Index: src/gcc/ipa-cp.c
===================================================================
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
tree otr_type;
tree t;
- if (param_index == -1)
+ if (param_index == -1
+ || known_vals.length () <= (unsigned int) param_index)
return NULL_TREE;
if (!ie->indirect_info->polymorphic)
@@ -1516,8 +1517,7 @@ ipa_get_indirect_edge_target_1 (struct c
t = NULL;
}
else
- t = (known_vals.length () > (unsigned int) param_index
- ? known_vals[param_index] : NULL);
+ t = NULL;
if (t &&
TREE_CODE (t) == ADDR_EXPR