* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-11-19 15:19 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-11-19 15:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: gdb-patches
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 2648 bytes --]
Hi,
Yes the below code (uses symbol table to determine the end of prologue) may not be target dependent in this case.
But the same method in ARM for e.g.(arm-tdep.c /arm_skip_prologue) uses some target specific call such as -
(post_prologue_pc)
post_prologue_pc = arm_skip_stack_protector (post_prologue_pc, gdbarch);
so moving the function to a target-independent function may not be that useful. Hence added the same in i386-tdep.c.
Regards
Karthik
------- Original Message -------
Sender : H.J. Lu<hjl.tools@gmail.com>
Date : Nov 19, 2012 23:42 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
On Mon, Nov 19, 2012 at 12:06 AM, KARTHIKVENKATESH BHAT
wrote:
> Dear All,
> I wanted to add a patch in i386-tdep.c . Similar to what is done in other architectures such as ARM,
> instead of actually going through the complete prologue if we can use the symbol table information to resolve prologue end.
>
>
> Index: gdb/i386-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-tdep.c,v
> retrieving revision 1.362
> diff -u -p -r1.362 i386-tdep.c
> --- gdb/i386-tdep.c 12 Nov 2012 21:59:06 -0000 1.362
> +++ gdb/i386-tdep.c 19 Nov 2012 07:56:45 -0000
> @@ -1582,8 +1582,30 @@ i386_skip_prologue (struct gdbarch *gdba
> CORE_ADDR pc;
> gdb_byte op;
> int i;
> + cache.locals = -1;
> + CORE_ADDR func_addr;
> + struct symtab *s = find_pc_symtab (func_addr);
> +
> + if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
> + {
> + CORE_ADDR post_prologue_pc
> + = skip_prologue_using_sal (gdbarch, func_addr);
> +
> + /* GCC always emits a line note before the prologue and another
> + one after, even if the two are at the same address or on the
> + same line. Take advantage of this so that we do not need to
> + know every instruction that might appear in the prologue. We
> + will have producer information for most binaries; if it is
> + missing (e.g. for -gstabs), assuming the GNU tools. */
> + if (post_prologue_pc
> + && (s == NULL
> + || s->producer == NULL
> + || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
> + || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
> + return max (start_pc, post_prologue_pc);
> + }
> +
>
>
It doesn't look like target-dependent. If we do this, why not
make it a target-independent function make all targets call it?
--
H.J.\x16º&Öéj×!zÊÞ¶êç÷:âX¬µªÜ†\a[¥«\…ë
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-11-20 4:58 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-11-20 4:58 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 3377 bytes --]
Hi Tom
I had missed out to paste the changeLog in the mail. Please find the patch with changelog -
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14836
diff -u -p -r1.14836 ChangeLog
--- ChangeLog 15 Nov 2012 16:12:11 -0000 1.14836
+++ ChangeLog 20 Nov 2012 04:39:46 -0000
@@ -1,3 +1,9 @@
+2012-11-20 Karthik Bhat <kv.bhat@samsung.com>
+
+ * i386-tdep.c (i386_skip_prologue): See if we
+ can determine the end of the prologue via the symbol table.
+ If so use the same instead of going through prologue instructions.
+
2012-11-15 Pierre Muller <muller@sourceware.org>
ARI fixes: move gdb_wait and gdb_stat headers to common subdirectory.
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.362
diff -u -p -r1.362 i386-tdep.c
--- i386-tdep.c 12 Nov 2012 21:59:06 -0000 1.362
+++ i386-tdep.c 20 Nov 2012 04:39:47 -0000
@@ -1582,8 +1582,30 @@ i386_skip_prologue (struct gdbarch *gdba
CORE_ADDR pc;
gdb_byte op;
int i;
+ cache.locals = -1;
+ CORE_ADDR func_addr;
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+
+ /* GCC always emits a line note before the prologue and another
+ one after, even if the two are at the same address or on the
+ same line. Take advantage of this so that we do not need to
+ know every instruction that might appear in the prologue. We
+ will have producer information for most binaries; if it is
+ missing (e.g. for -gstabs), assuming the GNU tools. */
+ if (post_prologue_pc
+ && (s == NULL
+ || s->producer == NULL
+ || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
+ || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max (start_pc, post_prologue_pc);
+ }
+
Many of the current gcc test cases test this feature. We can reproduce this failure in clang with existing testcases such as break-always.exp,step-test.exp etc when tested with clang compiled binary.
Hence i'm using the existing test cases.
Tom> + Is there any other way?
Currently i could come up with this fix. Similar fix is used in case of ARM architecture.
Regards
Karthik
------- Original Message -------
Sender : Tom Tromey<tromey@redhat.com>
Date : Nov 20, 2012 00:49 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
>>>>> "Karthik" == KARTHIKVENKATESH BHAT writes:
Karthik> I wanted to add a patch in i386-tdep.c . Similar to what is done in
Karthik> other architectures such as ARM,
Karthik> instead of actually going through the complete prologue if we can use
Karthik> the symbol table information to resolve prologue end.
I'm not the person to review this patch, but I did notice that it is
missing a ChangeLog entry and also a test case.
Karthik> + || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
Karthik> + || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
Is there any other way?
Tom\x16º&Öéj×!zÊÞ¶êç÷<öX¬µªÜ†\a[¥«\…ë
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-11-21 15:39 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-11-21 15:39 UTC (permalink / raw)
To: Pedro Alves, Mark Kettenis; +Cc: gdb-patches
Thanks Pedro/Mark. Appologies for the build break. I'm a bit new to GDB community will take care of it from next time.
I have fixed the warning resulting in error and modified the indentation -
Let me also try to explain the fix a bit more.
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14849
diff -u -p -r1.14849 ChangeLog
--- ChangeLog 21 Nov 2012 14:09:03 -0000 1.14849
+++ ChangeLog 21 Nov 2012 15:11:47 -0000
@@ -1,3 +1,9 @@
+2012-11-20 Karthik Bhat <kv.bhat@samsung.com>
+
+ * i386-tdep.c (i386_skip_prologue): See if we
+ can determine the end of the prologue via the symbol table.
+ If so use the same instead of going through prologue instructions.
+
2012-11-21 Yao Qi <yao@codesourcery.com>
PR tdep/7438
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.364
diff -u -p -r1.364 i386-tdep.c
--- i386-tdep.c 21 Nov 2012 14:09:10 -0000 1.364
+++ i386-tdep.c 21 Nov 2012 15:11:48 -0000
@@ -1582,6 +1582,27 @@ i386_skip_prologue (struct gdbarch *gdba
CORE_ADDR pc;
gdb_byte op;
int i;
+ CORE_ADDR func_addr;
+
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ /* GCC and clang always emits a line note before the prologue and another
+ one after, even if the two are at the same address or on the
+ same line. Take advantage of this so that we do not need to
+ know every instruction that might appear in the prologue. We
+ will have producer information for most binaries; if it is
+ missing (e.g. for -gstabs), assuming the GNU tools. */
+ if (post_prologue_pc
+ && (s == NULL
+ || s->producer == NULL
+ || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
+ || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max (start_pc, post_prologue_pc);
+ }
cache.locals = -1;
pc = i386_analyze_prologue (gdbarch, start_pc, 0xffffffff, &cache);
I wanted to add this patch in GDB to fix a problem which we are currently facing when we use gdb with binary compiled with clang.
The problem faced is clang generates function prologue which is a bit different from that of GCC as a result when we try to skip prologue instruction by instruction it results in incorrect prologue_end.
There is one more method to skip prologue which is used in other architectures such as ARM(arm-tdep.c), MIPS(mips-tdep.c) etc. In this method we try to determine prologue end via symbol table.
If we are unable to do this we then we examine instruction to determine prologue end.
Added the same for i386. Here we are trying to see if we can resolve prologue end from symbol table.
This will avoid instruction by instruction examining to determine prologue end if we are able to determine it through symbol table.
Thanks
------- Original Message -------
Sender : Pedro Alves<palves@redhat.com>
Date : Nov 21, 2012 23:10 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
On 11/21/2012 01:20 PM, Mark Kettenis wrote:
> Please back it out.
Since this breaking the build I went ahead and reverted it.
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-11-27 13:47 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-11-27 13:47 UTC (permalink / raw)
To: Mark Kettenis; +Cc: palves, gdb-patches
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=windows-1252, Size: 8618 bytes --]
Hi Mark,
Thanks for the review.
I have tested most of the gdb testuites which require proper line info to be set (with clang compiled binary on x86) and they are working fine after this fix.
> If your immediate goal is to fix things for clang, I recommend you
> resubmit your change addressing only clang and worry about GCC later
Yes currently I'm intrested in fixing this for clang binaries. I have updated the patch to handle only clang compiled binary. For other producers the previous code flow will apply.
> You probably want to add similar code to the prologue skipping code in
> amd64-tdep.c.
Yes. I have updated the file as well.
Please find the Changes after comments -
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14860
diff -u -p -r1.14860 ChangeLog
--- ChangeLog 27 Nov 2012 08:11:58 -0000 1.14860
+++ ChangeLog 27 Nov 2012 13:05:10 -0000
@@ -1,3 +1,10 @@
+2012-11-27 Karthik Bhat <kv.bhat@samsung.com>
+
+ * i386-tdep.c (i386_skip_prologue): Using symbol table
+ to find the end of prologue for clang compiled binaries.
+ * amd64-tdep.c (amd64_skip_prologue):Using symbol table
+ to find the end of prologue for clang compiled binaries.
+
2012-11-27 Daniel Jacobowitz <dan@codesourcery.com>
Kazu Hirata <kazu@codesourcery.com>
Yao Qi <yao@codesourcery.com>
Index: amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.115
diff -u -p -r1.115 amd64-tdep.c
--- amd64-tdep.c 26 Oct 2012 19:34:09 -0000 1.115
+++ amd64-tdep.c 27 Nov 2012 13:05:11 -0000
@@ -2252,6 +2252,22 @@ amd64_skip_prologue (struct gdbarch *gdb
{
struct amd64_frame_cache cache;
CORE_ADDR pc;
+ CORE_ADDR func_addr;
+
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ /* Clang always emits a line note before the prologue and another
+ one after.We trust clang to emit usable line notes */
+ if (post_prologue_pc
+ && (s != NULL
+ && s->producer != NULL
+ && strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max(start_pc,post_prologue_pc);
+ }
amd64_init_frame_cache (&cache);
pc = amd64_analyze_prologue (gdbarch, start_pc, 0xffffffffffffffffLL,
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.364
diff -u -p -r1.364 i386-tdep.c
--- i386-tdep.c 21 Nov 2012 14:09:10 -0000 1.364
+++ i386-tdep.c 27 Nov 2012 13:05:12 -0000
@@ -1582,7 +1582,23 @@ i386_skip_prologue (struct gdbarch *gdba
CORE_ADDR pc;
gdb_byte op;
int i;
+ CORE_ADDR func_addr;
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ /* Clang always emits a line note before the prologue and another
+ one after.We trust clang to emit usable line notes */
+ if (post_prologue_pc
+ && (s != NULL
+ && s->producer != NULL
+ && strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max(start_pc,post_prologue_pc);
+ }
+
cache.locals = -1;
pc = i386_analyze_prologue (gdbarch, start_pc, 0xffffffff, &cache);
if (cache.locals < 0)
Please let me know if it is ok.
Thanks
------- Original Message -------
Sender : Mark Kettenis<mark.kettenis@xs4all.nl>
Date : Nov 27, 2012 20:14 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
> Date: Wed, 21 Nov 2012 15:39:22 +0000 (GMT)
> From: KARTHIKVENKATESH BHAT
>
> Thanks Pedro/Mark. Appologies for the build break. I'm a bit new to GDB community will take care of it from next time.
> I have fixed the warning resulting in error and modified the indentation -
> Let me also try to explain the fix a bit more.
>
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.14849
> diff -u -p -r1.14849 ChangeLog
> --- ChangeLog 21 Nov 2012 14:09:03 -0000 1.14849
> +++ ChangeLog 21 Nov 2012 15:11:47 -0000
> @@ -1,3 +1,9 @@
> +2012-11-20 Karthik Bhat
> +
> + * i386-tdep.c (i386_skip_prologue): See if we
> + can determine the end of the prologue via the symbol table.
> + If so use the same instead of going through prologue instructions.
> +
> 2012-11-21 Yao Qi
>
> PR tdep/7438
> Index: i386-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-tdep.c,v
> retrieving revision 1.364
> diff -u -p -r1.364 i386-tdep.c
> --- i386-tdep.c 21 Nov 2012 14:09:10 -0000 1.364
> +++ i386-tdep.c 21 Nov 2012 15:11:48 -0000
> @@ -1582,6 +1582,27 @@ i386_skip_prologue (struct gdbarch *gdba
> CORE_ADDR pc;
> gdb_byte op;
> int i;
> + CORE_ADDR func_addr;
> +
> + if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
> + {
> + CORE_ADDR post_prologue_pc
> + = skip_prologue_using_sal (gdbarch, func_addr);
> + struct symtab *s = find_pc_symtab (func_addr);
> +
> + /* GCC and clang always emits a line note before the prologue and another
> + one after, even if the two are at the same address or on the
> + same line. Take advantage of this so that we do not need to
> + know every instruction that might appear in the prologue. We
> + will have producer information for most binaries; if it is
> + missing (e.g. for -gstabs), assuming the GNU tools. */
> + if (post_prologue_pc
> + && (s == NULL
> + || s->producer == NULL
> + || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
> + || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
> + return max (start_pc, post_prologue_pc);
> + }
>
> cache.locals = -1;
> pc = i386_analyze_prologue (gdbarch, start_pc, 0xffffffff, &cache);
>
>
> I wanted to add this patch in GDB to fix a problem which we are
> currently facing when we use gdb with binary compiled with clang.
> The problem faced is clang generates function prologue which is a
> bit different from that of GCC as a result when we try to skip
> prologue instruction by instruction it results in incorrect
> prologue_end.
It should only ever result in a prologue_end that's pointing to an
instruction before the "real" end of the prologue. That shouldn't be
a big issue if your compiler emits proper debug information (in
particular unwind information adn location information) for the
prologue. With today's optimizing compilers the concept of function
prologue is fuzzy anyway.
> There is one more method to skip prologue which is used in other
> architectures such as ARM(arm-tdep.c), MIPS(mips-tdep.c) etc. In
> this method we try to determine prologue end via symbol table. If
> we are unable to do this we then we examine instruction to determine
> prologue end.
The problem with that approach is that compilers can not always be
trusted to emit the right information for this to work. In the past
GCC has been particularly flaky in this respect, with the unfortunate
outcome that there were branch instructions before the the prologue
end as determined via the symbol table. That makes debugging really,
really painful.
If you can vouch for clang always getting this right, I have no
objection doing this when clang is the producer. Perhaps these days
GCC can be trusted as well. But we'd need a version check to make
sure we don't use the symbol table approach on known to be broken
versions of GCC. Probably the best thing would be to establish a
known-to-be-good version of GCC and only use the symbol table approach
for GCC starting with that version number.
If your immediate goal is to fix things for clang, I recommend you
resubmit your change addressing only clang and worry about GCC later
(or let somebody else worry about it). Be sure to update the comment.
I'd simple replace it with something like "We trust clang to emit
usable line notes".
You probably want to add similar code to the prologue skipping code in
amd64-tdep.c.
Cheers,
Mark\x16º&Öéj×!zÊÞ¶êç÷¼öX¬µªÜ†\a[¥«\…ë
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-11-28 5:14 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-11-28 5:14 UTC (permalink / raw)
To: Tom Tromey; +Cc: Mark Kettenis, palves, gdb-patches
Hi Tom/Mark,
Done. Took care of formatting comments. Any other inputs?
Thanks
------- Original Message -------
Sender : Tom Tromey<tromey@redhat.com>
Date : Nov 28, 2012 00:58 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
>>>>> "Karthik" == KARTHIKVENKATESH BHAT writes:
Karthik> Please find the Changes after comments -
There are some formatting issues in the patch.
Karthik> + /* Clang always emits a line note before the prologue and another
Karthik> + one after.We trust clang to emit usable line notes */
Two spaces after the ".".
A "." followed by 2 spaces at the end of the second sentence.
Karthik> + return max(start_pc,post_prologue_pc);
Space before open paren and after the comma.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
@ 2012-12-04 6:09 KARTHIKVENKATESH BHAT
0 siblings, 0 replies; 6+ messages in thread
From: KARTHIKVENKATESH BHAT @ 2012-12-04 6:09 UTC (permalink / raw)
To: Mark Kettenis, KARTHIKVENKATESH BHAT; +Cc: palves, gdb-patches, tromey
Hi Mark/Tom/Palves,
Thanks for taking your time out for review.
I have implemented the review comments. Please let me know if i can commit the same.
cvs diff: Diffing .
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14880
diff -u -p -r1.14880 ChangeLog
--- ChangeLog 3 Dec 2012 22:31:02 -0000 1.14880
+++ ChangeLog 4 Dec 2012 05:06:29 -0000
@@ -1,3 +1,10 @@
+2012-12-04 Karthik Bhat <kv.bhat@samsung.com>
+
+ * i386-tdep.c (i386_skip_prologue): Using symbol table
+ to find the end of prologue for clang compiled binaries.
+ * amd64-tdep.c (amd64_skip_prologue):Using symbol table
+ to find the end of prologue for clang compiled binaries.
+
2012-12-03 Doug Evans <dje@google.com>
* dwarf2read.c (struct dwarf2_per_objfile): Clarify comment.
Index: amd64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
retrieving revision 1.115
diff -u -p -r1.115 amd64-tdep.c
--- amd64-tdep.c 26 Oct 2012 19:34:09 -0000 1.115
+++ amd64-tdep.c 4 Dec 2012 05:06:30 -0000
@@ -2252,6 +2252,22 @@ amd64_skip_prologue (struct gdbarch *gdb
{
struct amd64_frame_cache cache;
CORE_ADDR pc;
+ CORE_ADDR func_addr;
+
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ /* Clang always emits a line note before the prologue and another
+ one after. We trust clang to emit usable line notes. */
+ if (post_prologue_pc
+ && (s != NULL
+ && s->producer != NULL
+ && strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max (start_pc, post_prologue_pc);
+ }
amd64_init_frame_cache (&cache);
pc = amd64_analyze_prologue (gdbarch, start_pc, 0xffffffffffffffffLL,
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.364
diff -u -p -r1.364 i386-tdep.c
--- i386-tdep.c 21 Nov 2012 14:09:10 -0000 1.364
+++ i386-tdep.c 4 Dec 2012 05:06:30 -0000
@@ -1582,7 +1582,23 @@ i386_skip_prologue (struct gdbarch *gdba
CORE_ADDR pc;
gdb_byte op;
int i;
+ CORE_ADDR func_addr;
+ if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
+ {
+ CORE_ADDR post_prologue_pc
+ = skip_prologue_using_sal (gdbarch, func_addr);
+ struct symtab *s = find_pc_symtab (func_addr);
+
+ /* Clang always emits a line note before the prologue and another
+ one after. We trust clang to emit usable line notes. */
+ if (post_prologue_pc
+ && (s != NULL
+ && s->producer != NULL
+ && strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
+ return max (start_pc, post_prologue_pc);
+ }
+
cache.locals = -1;
pc = i386_analyze_prologue (gdbarch, start_pc, 0xffffffff, &cache);
if (cache.locals < 0)
Thanks
Karthik
------- Original Message -------
Sender : Mark Kettenis<mark.kettenis@xs4all.nl>
Date : Nov 27, 2012 20:14 (GMT+09:00)
Title : Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary
> Date: Wed, 21 Nov 2012 15:39:22 +0000 (GMT)
> From: KARTHIKVENKATESH BHAT
>
> Thanks Pedro/Mark. Appologies for the build break. I'm a bit new to GDB community will take care of it from next time.
> I have fixed the warning resulting in error and modified the indentation -
> Let me also try to explain the fix a bit more.
>
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.14849
> diff -u -p -r1.14849 ChangeLog
> --- ChangeLog 21 Nov 2012 14:09:03 -0000 1.14849
> +++ ChangeLog 21 Nov 2012 15:11:47 -0000
> @@ -1,3 +1,9 @@
> +2012-11-20 Karthik Bhat
> +
> + * i386-tdep.c (i386_skip_prologue): See if we
> + can determine the end of the prologue via the symbol table.
> + If so use the same instead of going through prologue instructions.
> +
> 2012-11-21 Yao Qi
>
> PR tdep/7438
> Index: i386-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-tdep.c,v
> retrieving revision 1.364
> diff -u -p -r1.364 i386-tdep.c
> --- i386-tdep.c 21 Nov 2012 14:09:10 -0000 1.364
> +++ i386-tdep.c 21 Nov 2012 15:11:48 -0000
> @@ -1582,6 +1582,27 @@ i386_skip_prologue (struct gdbarch *gdba
> CORE_ADDR pc;
> gdb_byte op;
> int i;
> + CORE_ADDR func_addr;
> +
> + if (find_pc_partial_function (start_pc, NULL, &func_addr, NULL))
> + {
> + CORE_ADDR post_prologue_pc
> + = skip_prologue_using_sal (gdbarch, func_addr);
> + struct symtab *s = find_pc_symtab (func_addr);
> +
> + /* GCC and clang always emits a line note before the prologue and another
> + one after, even if the two are at the same address or on the
> + same line. Take advantage of this so that we do not need to
> + know every instruction that might appear in the prologue. We
> + will have producer information for most binaries; if it is
> + missing (e.g. for -gstabs), assuming the GNU tools. */
> + if (post_prologue_pc
> + && (s == NULL
> + || s->producer == NULL
> + || strncmp (s->producer, "GNU ", sizeof ("GNU ") - 1) == 0
> + || strncmp (s->producer, "clang ", sizeof ("clang ") - 1) == 0))
> + return max (start_pc, post_prologue_pc);
> + }
>
> cache.locals = -1;
> pc = i386_analyze_prologue (gdbarch, start_pc, 0xffffffff, &cache);
>
>
> I wanted to add this patch in GDB to fix a problem which we are
> currently facing when we use gdb with binary compiled with clang.
> The problem faced is clang generates function prologue which is a
> bit different from that of GCC as a result when we try to skip
> prologue instruction by instruction it results in incorrect
> prologue_end.
It should only ever result in a prologue_end that's pointing to an
instruction before the "real" end of the prologue. That shouldn't be
a big issue if your compiler emits proper debug information (in
particular unwind information adn location information) for the
prologue. With today's optimizing compilers the concept of function
prologue is fuzzy anyway.
> There is one more method to skip prologue which is used in other
> architectures such as ARM(arm-tdep.c), MIPS(mips-tdep.c) etc. In
> this method we try to determine prologue end via symbol table. If
> we are unable to do this we then we examine instruction to determine
> prologue end.
The problem with that approach is that compilers can not always be
trusted to emit the right information for this to work. In the past
GCC has been particularly flaky in this respect, with the unfortunate
outcome that there were branch instructions before the the prologue
end as determined via the symbol table. That makes debugging really,
really painful.
If you can vouch for clang always getting this right, I have no
objection doing this when clang is the producer. Perhaps these days
GCC can be trusted as well. But we'd need a version check to make
sure we don't use the symbol table approach on known to be broken
versions of GCC. Probably the best thing would be to establish a
known-to-be-good version of GCC and only use the symbol table approach
for GCC starting with that version number.
If your immediate goal is to fix things for clang, I recommend you
resubmit your change addressing only clang and worry about GCC later
(or let somebody else worry about it). Be sure to update the comment.
I'd simple replace it with something like "We trust clang to emit
usable line notes".
You probably want to add similar code to the prologue skipping code in
amd64-tdep.c.
Cheers,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-04 6:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-19 15:19 Re: [PATCH] Fix for incorect breakpoint set in case of clang compiled binary KARTHIKVENKATESH BHAT
2012-11-20 4:58 KARTHIKVENKATESH BHAT
2012-11-21 15:39 KARTHIKVENKATESH BHAT
2012-11-27 13:47 KARTHIKVENKATESH BHAT
2012-11-28 5:14 KARTHIKVENKATESH BHAT
2012-12-04 6:09 KARTHIKVENKATESH BHAT
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox