Patchwork Fix up assembly thunks with -gstabs+ (PR debug/54499)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 8, 2012, 7:55 p.m.
Message ID <20121108195506.GF1859@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/197873/
State New
Headers show

Comments

Jakub Jelinek - Nov. 8, 2012, 7:55 p.m.
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
Richard Henderson - Nov. 8, 2012, 8:07 p.m.
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~
Cary Coutant - Nov. 9, 2012, 1:37 a.m.
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

Patch

--- 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 ()
+{
+}