| Message ID | 20121108195506.GF1859@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2012-11-08 11:55, Jakub Jelinek wrote: > 2012-11-08 Jakub Jelinek <jakub@redhat.com> > > PR debug/54499 > * cgraphunit.c (assemble_thunk): Don't call source_line debug hook > here, instead call insn_locations_{init,finalize} and initialize > prologue_location. > > * g++.dg/debug/pr54499.C: New test. I recall mentioning at the time that I thought Cary's approach was error-prone. Ok. r~
jakub> 2012-11-08 Jakub Jelinek <jakub@redhat.com>
jakub>
jakub> PR debug/54499
jakub> * cgraphunit.c (assemble_thunk): Don't call source_line debug hook
jakub> here, instead call insn_locations_{init,finalize} and initialize
jakub> prologue_location.
jakub>
jakub> * g++.dg/debug/pr54499.C: New test.
Thanks, Jakub, this does look like a better fix. I had tried something
like this, but it didn't work -- I was missing the _init and _finalize
calls, and I didn't set prologue_location either.
rth> I recall mentioning at the time that I thought Cary's approach
was error-prone.
Well, yes, you did, but then you backed off after I tested an ARM compiler:
On Mon, Aug 6, 2012 at 2:53 PM, Richard Henderson <rth@redhat.com> wrote:
> On 08/06/2012 02:45 PM, Cary Coutant wrote:
>> Do you still have concerns about the patch?
>
> Nope. I'd mis-remembered from whence things get finalized.
>
> Revised patch is ok.
I do agree with you, though, that it would still be better to treat
thunks as first-class objects.
-cary
--- gcc/cgraphunit.c.jj 2012-11-06 09:03:53.000000000 +0100 +++ gcc/cgraphunit.c 2012-11-08 13:16:48.010938659 +0100 @@ -1413,16 +1413,16 @@ assemble_thunk (struct cgraph_node *node DECL_INITIAL (thunk_fndecl) = fn_block; init_function_start (thunk_fndecl); cfun->is_thunk = 1; + insn_locations_init (); + set_curr_insn_location (DECL_SOURCE_LOCATION (thunk_fndecl)); + prologue_location = curr_insn_location (); assemble_start_function (thunk_fndecl, fnname); - (*debug_hooks->source_line) (DECL_SOURCE_LINE (thunk_fndecl), - DECL_SOURCE_FILE (thunk_fndecl), - /* discriminator */ 0, - /* is_stmt */ 1); targetm.asm_out.output_mi_thunk (asm_out_file, thunk_fndecl, fixed_offset, virtual_value, alias); assemble_end_function (thunk_fndecl, fnname); + insn_locations_finalize (); init_insn_lengths (); free_after_compilation (cfun); set_cfun (NULL); --- gcc/testsuite/g++.dg/debug/pr54499.C.jj 2012-11-08 13:23:19.906740003 +0100 +++ gcc/testsuite/g++.dg/debug/pr54499.C 2012-11-08 13:23:55.189531409 +0100 @@ -0,0 +1,22 @@ +// PR debug/54499 +// { dg-do assemble } + +struct S1 +{ + virtual void f () = 0; +}; + +struct S2 +{ + virtual ~S2 () { } +}; + +struct S3 : public S1, public S2 +{ + void f (); +}; + +void +S3::f () +{ +}
Hi! Cary added recently a debug_hooks->source_line call for asm thunks, but that breaks e.g. for -gstabs+, which emits a relocation relative to whatever label is emitted in the begin_prologue debug hook, so obviously doesn't work well if called before the begin_prologue debug hook. Fixed by reverting that change and instead setting up proper prologue_location, so that final_start_function called by output_mi_thunk when calling the debug_hooks->begin_prologue hook passes the right line/file. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2012-11-08 Jakub Jelinek <jakub@redhat.com> PR debug/54499 * cgraphunit.c (assemble_thunk): Don't call source_line debug hook here, instead call insn_locations_{init,finalize} and initialize prologue_location. * g++.dg/debug/pr54499.C: New test. Jakub