This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 1/1] Don't rewind PC for GHC generated frames
Hi Bartosz,
Thanks for the additional info in the commit message, it's very useful.
I hoped there would be some comments from others. In particular, is
anybody able to tell if adding a call to find_pc_compunit_symtab in
get_frame_address_in_block is a performance concern? How frequently is
get_frame_address_in_block called, and how costly is
find_pc_compunit_symtab to call?
I noted two small comments, no need to submit another version just for
that, we can fix it before pushing. However, please tell me if you are
fine with my suggestion below.
diff --git a/gdb/frame.c b/gdb/frame.c
index 1384ecca4f..9ff0dcb130 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2458,7 +2458,14 @@ get_frame_address_in_block (struct frame_info
*this_frame)
&& (get_frame_type (this_frame) == NORMAL_FRAME
|| get_frame_type (this_frame) == TAILCALL_FRAME
|| get_frame_type (this_frame) == INLINE_FRAME))
- return pc - 1;
+ {
+ /* GHC intermixes metadata (info tables) with code, going back
is
+ guaranteed to land us in the metadata. */
I think the main reason why we don't want to subtract one is that the
compiler generates return addresses that refer to the jump instruction
that made the "call", and not to the instruction after, as with the call
instruction. I think the comment should reflect that. What about:
/* GHC (Glasgow Haskell Compiler) uses jump and manages its own stack
rather than
using the call instruction. The return address it generates is that
of the
jump instruction, not the following instruction. It is therefore not
necessary to go back one byte. It would not be desirable anyway,
because it
intermixes metadat (info tables) with code, so going back would land
us in the
metadata. */
+ struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
+ if (cust != NULL && cust->producer_is_ghc)
+ return pc;
+ return pc - 1;
+ }
return pc;
}
diff --git a/gdb/symtab.h b/gdb/symtab.h
index f9d52e7697..c164e5ba5f 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1432,6 +1432,9 @@ struct compunit_symtab
instruction). This is supported by GCC since 4.5.0. */
unsigned int epilogue_unwind_valid : 1;
+ /* This CU was produced by Glasgow Haskell Compiler */
The comment should end with a period and two spaces.
Thanks,
Simon