[COMMIT] Fix decoding CIE's in DWARF frame info

[COMMIT] Fix decoding CIE's in DWARF frame info

[Mark Kettenis]

The old code didn't take into account that the encoding used to
specify the personality routine in the augmentation could be
DW_PE_EH_aligned.

Fixed by the attach patch.

Committed to mainline.  I'll commit this to the branch later today.

Cheers,

Mark


Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.6577
diff -u -p -r1.6577 ChangeLog
--- ChangeLog	5 Nov 2004 08:53:18 -0000	1.6577
+++ ChangeLog	5 Nov 2004 15:12:42 -0000
@@ -1,3 +1,8 @@
+2004-11-05  Mark Kettenis  <kettenis@jive.nl>
+
+	* dwarf2-frame.c (decode_frame_entry_1): Correctly skip
+	personality routine in a CIE augmentation.
+
 2004-11-05  Felix Lee  <felix+log1@specifixinc.com>
 
 	* stack.c (set_current_sal_from_frame): New function.
Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.41
diff -u -p -r1.41 dwarf2-frame.c
--- dwarf2-frame.c	4 Nov 2004 21:15:15 -0000	1.41
+++ dwarf2-frame.c	5 Nov 2004 15:12:42 -0000
@@ -1381,7 +1381,9 @@ decode_frame_entry_1 (struct comp_unit *
 	  else if (*augmentation == 'P')
 	    {
 	      /* Skip.  */
-	      buf += size_of_encoded_value (*buf++);
+	      unsigned char encoding = *buf++;
+	      read_encoded_value (unit, encoding, buf, &bytes_read);
+	      buf += bytes_read;
 	      augmentation++;
 	    }
 

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Andrew Cagney]

Mark Kettenis wrote:
> The old code didn't take into account that the encoding used to
> specify the personality routine in the augmentation could be
> DW_PE_EH_aligned.
> 
> Fixed by the attach patch.
> 
> Committed to mainline.  I'll commit this to the branch later today.

How are you testing this?  I'm seeing:

dwarf2-frame.c:1083: internal-error: Unsupported encoding: DW_EH_PE_indirect

Andrew


> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.6577
> diff -u -p -r1.6577 ChangeLog
> --- ChangeLog	5 Nov 2004 08:53:18 -0000	1.6577
> +++ ChangeLog	5 Nov 2004 15:12:42 -0000
> @@ -1,3 +1,8 @@
> +2004-11-05  Mark Kettenis  <kettenis@jive.nl>
> +
> +	* dwarf2-frame.c (decode_frame_entry_1): Correctly skip
> +	personality routine in a CIE augmentation.
> +
>  2004-11-05  Felix Lee  <felix+log1@specifixinc.com>
>  
>  	* stack.c (set_current_sal_from_frame): New function.
> Index: dwarf2-frame.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 dwarf2-frame.c
> --- dwarf2-frame.c	4 Nov 2004 21:15:15 -0000	1.41
> +++ dwarf2-frame.c	5 Nov 2004 15:12:42 -0000
> @@ -1381,7 +1381,9 @@ decode_frame_entry_1 (struct comp_unit *
>  	  else if (*augmentation == 'P')
>  	    {
>  	      /* Skip.  */
> -	      buf += size_of_encoded_value (*buf++);
> +	      unsigned char encoding = *buf++;
> +	      read_encoded_value (unit, encoding, buf, &bytes_read);
> +	      buf += bytes_read;
>  	      augmentation++;
>  	    }
>  

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Mark Kettenis]

   Date: Fri, 05 Nov 2004 12:00:12 -0500
   From: Andrew Cagney <cagney@gnu.org>

   Mark Kettenis wrote:
   > The old code didn't take into account that the encoding used to
   > specify the personality routine in the augmentation could be
   > DW_PE_EH_aligned.
   > 
   > Fixed by the attach patch.
   > 
   > Committed to mainline.  I'll commit this to the branch later today.

   How are you testing this?  I'm seeing:

   dwarf2-frame.c:1083: internal-error: Unsupported encoding: DW_EH_PE_indirect

If your toolchain really generates DW_EH_PE_indirect, it's a different
problem; I don't know how to properly support this :-(.

The problem fixed by the patch is different: because we failed to do
proper alignment gdb was essentially interpreting garbage.  Now if
that garbage accidentally matched DW_EH_PE_indirect, it could lead to
the error message you're seeing.

Anyway, I noticed the problem when writing some code at work.
Unfortunately I don't know how to convert it into a test case.

Cheers,

Mark

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Daniel Jacobowitz]

On Fri, Nov 05, 2004 at 07:53:51PM +0100, Mark Kettenis wrote:
>    Date: Fri, 05 Nov 2004 12:00:12 -0500
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    Mark Kettenis wrote:
>    > The old code didn't take into account that the encoding used to
>    > specify the personality routine in the augmentation could be
>    > DW_PE_EH_aligned.
>    > 
>    > Fixed by the attach patch.
>    > 
>    > Committed to mainline.  I'll commit this to the branch later today.
> 
>    How are you testing this?  I'm seeing:
> 
>    dwarf2-frame.c:1083: internal-error: Unsupported encoding: DW_EH_PE_indirect
> 
> If your toolchain really generates DW_EH_PE_indirect, it's a different
> problem; I don't know how to properly support this :-(.

I do know that some GCC toolchains emit DW_EH_PE_indirect - I ran into
it yesterday on PowerPC.  It's exactly what it says; after applying
whatever other encoding (pcrel, etc), dereference the result.  Adding
this to read_encoded_value should be pretty straightforward.

(I encountered it in an SJLJ context rather than .eh_frame, but I think
the same GCC macros control both.)

-- 
Daniel Jacobowitz

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Mark Kettenis]

   Date: Fri, 5 Nov 2004 14:12:17 -0500
   From: Daniel Jacobowitz <drow@false.org>

   On Fri, Nov 05, 2004 at 07:53:51PM +0100, Mark Kettenis wrote:
   >    Date: Fri, 05 Nov 2004 12:00:12 -0500
   >    From: Andrew Cagney <cagney@gnu.org>
   > 
   >    Mark Kettenis wrote:
   >    > The old code didn't take into account that the encoding used to
   >    > specify the personality routine in the augmentation could be
   >    > DW_PE_EH_aligned.
   >    > 
   >    > Fixed by the attach patch.
   >    > 
   >    > Committed to mainline.  I'll commit this to the branch later today.
   > 
   >    How are you testing this?  I'm seeing:
   > 
   >    dwarf2-frame.c:1083: internal-error: Unsupported encoding: DW_EH_PE_indirect
   > 
   > If your toolchain really generates DW_EH_PE_indirect, it's a different
   > problem; I don't know how to properly support this :-(.

   I do know that some GCC toolchains emit DW_EH_PE_indirect - I ran into
   it yesterday on PowerPC.  It's exactly what it says; after applying
   whatever other encoding (pcrel, etc), dereference the result.  Adding
   this to read_encoded_value should be pretty straightforward.

I don't think so.  We read the encoded pointers when we load the
executable in GDB.  But if the value of these encoded pointers is
known at that point, why doesn't the (static) linker resolve things
instead of having us goe through the indirection process?

Mark

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Daniel Jacobowitz]

On Fri, Nov 05, 2004 at 09:36:17PM +0100, Mark Kettenis wrote:
>    Date: Fri, 5 Nov 2004 14:12:17 -0500
>    From: Daniel Jacobowitz <drow@false.org>
> 
>    On Fri, Nov 05, 2004 at 07:53:51PM +0100, Mark Kettenis wrote:
>    >    Date: Fri, 05 Nov 2004 12:00:12 -0500
>    >    From: Andrew Cagney <cagney@gnu.org>
>    > 
>    >    Mark Kettenis wrote:
>    >    > The old code didn't take into account that the encoding used to
>    >    > specify the personality routine in the augmentation could be
>    >    > DW_PE_EH_aligned.
>    >    > 
>    >    > Fixed by the attach patch.
>    >    > 
>    >    > Committed to mainline.  I'll commit this to the branch later today.
>    > 
>    >    How are you testing this?  I'm seeing:
>    > 
>    >    dwarf2-frame.c:1083: internal-error: Unsupported encoding: DW_EH_PE_indirect
>    > 
>    > If your toolchain really generates DW_EH_PE_indirect, it's a different
>    > problem; I don't know how to properly support this :-(.
> 
>    I do know that some GCC toolchains emit DW_EH_PE_indirect - I ran into
>    it yesterday on PowerPC.  It's exactly what it says; after applying
>    whatever other encoding (pcrel, etc), dereference the result.  Adding
>    this to read_encoded_value should be pretty straightforward.
> 
> I don't think so.  We read the encoded pointers when we load the
> executable in GDB.  But if the value of these encoded pointers is
> known at that point, why doesn't the (static) linker resolve things
> instead of having us goe through the indirection process?

Because this is used, as far as I know, only for PIC code in shared
libraries (which we only load after they've been dynamically
relocated).  DW_EH_PE_indirect is used so that symbols which will need
a runtime RELATIVE relocation live in the data segment, instead of the
with the rest of the exception data, which could otherwise be read-only.

It's true that in general we'd have to delay the read until it was
used, but I think it'll work OK to read it right away for all cases I
know of.  But I don't have a test case, so I'll just save this thought
for later.

-- 
Daniel Jacobowitz

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Mark Kettenis]

   Date: Fri, 5 Nov 2004 15:41:02 -0500
   From: Daniel Jacobowitz <drow@false.org>

   > I don't think so.  We read the encoded pointers when we load the
   > executable in GDB.  But if the value of these encoded pointers is
   > known at that point, why doesn't the (static) linker resolve things
   > instead of having us goe through the indirection process?

   Because this is used, as far as I know, only for PIC code in shared
   libraries (which we only load after they've been dynamically
   relocated).  DW_EH_PE_indirect is used so that symbols which will need
   a runtime RELATIVE relocation live in the data segment, instead of the
   with the rest of the exception data, which could otherwise be read-only.

Hmm.  You might be right.  Didn't think about it that way.

Mark

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Andrew Cagney]

> Anyway, I noticed the problem when writing some code at work.
> Unfortunately I don't know how to convert it into a test case.

I was thinking regression testing.  I'm finding that:
	./gdb foo
	(gdb) run
gets a panic on CFI systems - RHEL 3 amd64, FC3 i386 and amd64.

Can we set this patch aside until we know what's going on?  We can then 
think about a backport for 6.3.1.

Andrew

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Daniel Jacobowitz]

On Fri, Nov 05, 2004 at 04:22:01PM -0500, Andrew Cagney wrote:
> 
> >Anyway, I noticed the problem when writing some code at work.
> >Unfortunately I don't know how to convert it into a test case.
> 
> I was thinking regression testing.  I'm finding that:
> 	./gdb foo
> 	(gdb) run
> gets a panic on CFI systems - RHEL 3 amd64, FC3 i386 and amd64.
> 
> Can we set this patch aside until we know what's going on?  We can then 
> think about a backport for 6.3.1.

It does not show any problems here, on Debian i386 (using FCI).

-- 
Daniel Jacobowitz

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Mark Kettenis]

   Date: Fri, 5 Nov 2004 16:25:05 -0500
   From: Daniel Jacobowitz <drow@false.org>

   On Fri, Nov 05, 2004 at 04:22:01PM -0500, Andrew Cagney wrote:
   > 
   > >Anyway, I noticed the problem when writing some code at work.
   > >Unfortunately I don't know how to convert it into a test case.
   > 
   > I was thinking regression testing.  I'm finding that:
   > 	./gdb foo
   > 	(gdb) run
   > gets a panic on CFI systems - RHEL 3 amd64, FC3 i386 and amd64.
   > 
   > Can we set this patch aside until we know what's going on?  We can then 
   > think about a backport for 6.3.1.

   It does not show any problems here, on Debian i386 (using FCI).

But it does for me on OpenBSD/amd64 :-(.

I'll investigate.  Obviously I won't be checking this in on the
branch, util I know what's wrong here.

Mark

Re: [COMMIT] Fix decoding CIE's in DWARF frame info

[Andrew Cagney]

Daniel Jacobowitz wrote:
> On Fri, Nov 05, 2004 at 04:22:01PM -0500, Andrew Cagney wrote:
> 
>>>Anyway, I noticed the problem when writing some code at work.
>>>Unfortunately I don't know how to convert it into a test case.
>>
>>I was thinking regression testing.  I'm finding that:
>>	./gdb foo
>>	(gdb) run
>>gets a panic on CFI systems - RHEL 3 amd64, FC3 i386 and amd64.
>>
>>Can we set this patch aside until we know what's going on?  We can then 
>>think about a backport for 6.3.1.
> 
> 
> It does not show any problems here, on Debian i386 (using FCI).

You'll need to be more specific RHEL 3 / FC3 tie things down to specific 
versions (gcc 3.2.3++ and gcc 3.4.2++ from memory).

Andrew