| Message ID | 20131018101053.GA28991@x4 |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 18, 2013 at 12:10 PM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > On 2013.10.15 at 12:49 +0200, Richard Biener wrote: >> On Tue, Oct 15, 2013 at 12:31 PM, Markus Trippelsdorf >> <markus@trippelsdorf.de> wrote: >> > Valgrind complains: >> > ==27870== Conditional jump or move depends on uninitialised value(s) >> > ==27870== at 0x557CDC: cgraph_create_edge_1(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:695) >> > ==27870== by 0x55882E: cgraph_create_edge(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:890) >> > ==27870== by 0x560891: cgraph_clone_edge(cgraph_edge*, cgraph_node*, gimple_statement_d*, unsigned int, long, int, bool) (cgraphclones.c:135) >> > ==27870== by 0x7F1F14: copy_body(copy_body_data*, long, int, basic_block_def*, basic_block_def*, basic_block_def*) (tree-inline.c:1741) >> > ... >> > ==27870== Uninitialised value was created by a client request >> > ==27870== at 0x50BBEE: ggc_internal_alloc_stat(unsigned long) (ggc-page.c:1339) >> > ==27870== by 0x557D92: cgraph_create_edge_1(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:842) >> > ==27870== by 0x55882E: cgraph_create_edge(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:890) >> > ==27870== by 0x560891: cgraph_clone_edge(cgraph_edge*, cgraph_node*, gimple_statement_d*, unsigned int, long, int, bool) (cgraphclones.c:135) >> > ,,, >> > >> > This happens because e->indirect_unknown_callee may be uninitialized in >> > cgraph_add_edge_to_call_site_hash. Fixed by initializing it earlier. >> > >> > LTO bootstrapped and tested on x86_64-unknown-linux-gnu. >> > >> > Please apply, if this looks reasonable. >> >> As we also have >> >> struct cgraph_edge * >> cgraph_create_indirect_edge (struct cgraph_node *caller, gimple call_stmt, >> int ecf_flags, >> gcov_type count, int freq) >> { >> struct cgraph_edge *edge = cgraph_create_edge_1 (caller, NULL, call_stmt, >> count, freq); >> tree target; >> >> edge->indirect_unknown_callee = 1; >> >> I'd rather change the cgraph_create_edge_1 interface to get an additional >> argument (the value to use for indirect_unknown_callee). Or maybe >> we can statically compute it from the current arguments already? > > IOW something like this: Looks good to me. Honza? Thanks, Richard. > diff --git a/gcc/cgraph.c b/gcc/cgraph.c > index 124ee0adf855..ccd73fa21b80 100644 > --- a/gcc/cgraph.c > +++ b/gcc/cgraph.c > @@ -814,7 +814,8 @@ cgraph_set_call_stmt (struct cgraph_edge *e, gimple new_stmt, > > static struct cgraph_edge * > cgraph_create_edge_1 (struct cgraph_node *caller, struct cgraph_node *callee, > - gimple call_stmt, gcov_type count, int freq) > + gimple call_stmt, gcov_type count, int freq, > + bool indir_unknown_callee) > { > struct cgraph_edge *edge; > > @@ -874,6 +875,7 @@ cgraph_create_edge_1 (struct cgraph_node *caller, struct cgraph_node *callee, > edge->indirect_info = NULL; > edge->indirect_inlining_edge = 0; > edge->speculative = false; > + edge->indirect_unknown_callee = indir_unknown_callee; > if (call_stmt && caller->call_site_hash) > cgraph_add_edge_to_call_site_hash (edge); > > @@ -887,9 +889,8 @@ cgraph_create_edge (struct cgraph_node *caller, struct cgraph_node *callee, > gimple call_stmt, gcov_type count, int freq) > { > struct cgraph_edge *edge = cgraph_create_edge_1 (caller, callee, call_stmt, > - count, freq); > + count, freq, false); > > - edge->indirect_unknown_callee = 0; > initialize_inline_failed (edge); > > edge->next_caller = callee->callers; > @@ -926,10 +927,9 @@ cgraph_create_indirect_edge (struct cgraph_node *caller, gimple call_stmt, > gcov_type count, int freq) > { > struct cgraph_edge *edge = cgraph_create_edge_1 (caller, NULL, call_stmt, > - count, freq); > + count, freq, true); > tree target; > > - edge->indirect_unknown_callee = 1; > initialize_inline_failed (edge); > > edge->indirect_info = cgraph_allocate_init_indirect_info (); > > -- > Markus
> On Fri, Oct 18, 2013 at 12:10 PM, Markus Trippelsdorf > <markus@trippelsdorf.de> wrote: > > On 2013.10.15 at 12:49 +0200, Richard Biener wrote: > >> On Tue, Oct 15, 2013 at 12:31 PM, Markus Trippelsdorf > >> <markus@trippelsdorf.de> wrote: > >> > Valgrind complains: > >> > ==27870== Conditional jump or move depends on uninitialised value(s) > >> > ==27870== at 0x557CDC: cgraph_create_edge_1(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:695) > >> > ==27870== by 0x55882E: cgraph_create_edge(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:890) > >> > ==27870== by 0x560891: cgraph_clone_edge(cgraph_edge*, cgraph_node*, gimple_statement_d*, unsigned int, long, int, bool) (cgraphclones.c:135) > >> > ==27870== by 0x7F1F14: copy_body(copy_body_data*, long, int, basic_block_def*, basic_block_def*, basic_block_def*) (tree-inline.c:1741) > >> > ... > >> > ==27870== Uninitialised value was created by a client request > >> > ==27870== at 0x50BBEE: ggc_internal_alloc_stat(unsigned long) (ggc-page.c:1339) > >> > ==27870== by 0x557D92: cgraph_create_edge_1(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:842) > >> > ==27870== by 0x55882E: cgraph_create_edge(cgraph_node*, cgraph_node*, gimple_statement_d*, long, int) (cgraph.c:890) > >> > ==27870== by 0x560891: cgraph_clone_edge(cgraph_edge*, cgraph_node*, gimple_statement_d*, unsigned int, long, int, bool) (cgraphclones.c:135) > >> > ,,, > >> > > >> > This happens because e->indirect_unknown_callee may be uninitialized in > >> > cgraph_add_edge_to_call_site_hash. Fixed by initializing it earlier. > >> > > >> > LTO bootstrapped and tested on x86_64-unknown-linux-gnu. > >> > > >> > Please apply, if this looks reasonable. > >> > >> As we also have > >> > >> struct cgraph_edge * > >> cgraph_create_indirect_edge (struct cgraph_node *caller, gimple call_stmt, > >> int ecf_flags, > >> gcov_type count, int freq) > >> { > >> struct cgraph_edge *edge = cgraph_create_edge_1 (caller, NULL, call_stmt, > >> count, freq); > >> tree target; > >> > >> edge->indirect_unknown_callee = 1; > >> > >> I'd rather change the cgraph_create_edge_1 interface to get an additional > >> argument (the value to use for indirect_unknown_callee). Or maybe > >> we can statically compute it from the current arguments already? > > > > IOW something like this: > > Looks good to me. > > Honza? Yes, this is OK. Thanks! Honza
diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 124ee0adf855..ccd73fa21b80 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -814,7 +814,8 @@ cgraph_set_call_stmt (struct cgraph_edge *e, gimple new_stmt, static struct cgraph_edge * cgraph_create_edge_1 (struct cgraph_node *caller, struct cgraph_node *callee, - gimple call_stmt, gcov_type count, int freq) + gimple call_stmt, gcov_type count, int freq, + bool indir_unknown_callee) { struct cgraph_edge *edge; @@ -874,6 +875,7 @@ cgraph_create_edge_1 (struct cgraph_node *caller, struct cgraph_node *callee, edge->indirect_info = NULL; edge->indirect_inlining_edge = 0; edge->speculative = false; + edge->indirect_unknown_callee = indir_unknown_callee; if (call_stmt && caller->call_site_hash) cgraph_add_edge_to_call_site_hash (edge); @@ -887,9 +889,8 @@ cgraph_create_edge (struct cgraph_node *caller, struct cgraph_node *callee, gimple call_stmt, gcov_type count, int freq) { struct cgraph_edge *edge = cgraph_create_edge_1 (caller, callee, call_stmt, - count, freq); + count, freq, false); - edge->indirect_unknown_callee = 0; initialize_inline_failed (edge); edge->next_caller = callee->callers; @@ -926,10 +927,9 @@ cgraph_create_indirect_edge (struct cgraph_node *caller, gimple call_stmt, gcov_type count, int freq) { struct cgraph_edge *edge = cgraph_create_edge_1 (caller, NULL, call_stmt, - count, freq); + count, freq, true); tree target; - edge->indirect_unknown_callee = 1; initialize_inline_failed (edge); edge->indirect_info = cgraph_allocate_init_indirect_info ();