[RFA] Dwarf2CFI fix

[RFA] Dwarf2CFI fix

[Michal Ludvig]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

Hi all,
my code in dwarf2cfi.c assumed that CIE and FDE entries in .eh_frame are 
always in the chain CIE, FDE, FDE, ..., CIE, FDE, ...
where every FDE refers to the precedent CIE. Unfortunately this is not 
alway the case. Linker (?) sometimes optimizes things in a way that the 
assumption isn't true anymore. This patch addresses the problem.

OK to commit? Head and 5.3?

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz

[-- Attachment #2: cfi-ciefdeorder-2-export.diff --]
[-- Type: text/plain, Size: 2587 bytes --]

2003-02-26  Michal Ludvig  <mludvig@suse.cz>

	* dwarf2cfi.c (parse_frame_info): It's not true that FDEs 
	always refer to the last CIE. We must deal with such a
	situation.
	* dwarf2cfi.h (

diff -upr gdb-upto-08/dwarf2cfi.c gdb/dwarf2cfi.c
--- gdb-upto-08/dwarf2cfi.c	2003-02-26 10:20:02.000000000 +0100
+++ gdb/dwarf2cfi.c	2003-02-26 17:17:30.000000000 +0100
@@ -50,6 +50,10 @@ struct cie_unit
   char *data;
   unsigned int data_length;
 
+  /* This field is 1 for CIE from .eh_frame
+     and 0 for one from .debug_frame. */
+  int eh_frame;
+
   struct objfile *objfile;
 
   /* Next in chain.  */
@@ -1502,6 +1502,7 @@ parse_frame_info (struct objfile *objfil
 	      char *aug;
 
 	      cie->objfile = objfile;
+	      cie->eh_frame = (eh_frame > 0);
 	      cie->next = cie_chunks;
 	      cie_chunks = cie;
 
@@ -1586,26 +1587,31 @@ parse_frame_info (struct objfile *objfil
 
 	      /* We assume that debug_frame is in order 
 	         CIE,FDE,CIE,FDE,FDE,...  and thus the CIE for this FDE
-	         should be stored in last_cie pointer. If not, we'll 
-	         try to find it by the older way.  */
-	      if (last_cie)
+	         should be stored in last_cie pointer. 
+		 However this assumption is not always true, because
+		 the linker does some magic and adds ghost CIEs under
+		 some circumstances. Thus we need to do some safety
+		 checks.  If last_cie isn't valid, we'll try to find
+		 the correct one using the older way.  */
+	      if (last_cie && last_cie->objfile == objfile &&
+		  last_cie->eh_frame == (eh_frame > 0) &&
+		  ((eh_frame &&
+		    last_cie->offset == (unit_offset + bytes_read - cie_id)) ||
+		   (!eh_frame && (cie->offset == cie_id))))
 		cie = last_cie;
 	      else
 		{
-		  warning ("CFI: last_cie == NULL. "
-			   "Perhaps a malformed %s section in '%s'...?\n",
-			   curr_section_name, objfile->name);
-
 		  cie = cie_chunks;
 		  while (cie)
 		    {
 		      if (cie->objfile == objfile)
 			{
-			  if (eh_frame &&
-			      (cie->offset ==
-			       (unit_offset + bytes_read - cie_id)))
+			  if (eh_frame && cie->eh_frame &&
+			      cie->offset ==
+			       (unit_offset + bytes_read - cie_id))
 			    break;
-			  if (!eh_frame && (cie->offset == cie_id))
+			  if (!eh_frame && !cie->eh_frame && 
+			      cie->offset == cie_id)
 			    break;
 			}
 
@@ -1614,6 +1620,7 @@ parse_frame_info (struct objfile *objfil
 		  if (!cie)
 		    error ("CFI: can't find CIE pointer [in module %s]", 
 				    bfd_get_filename (abfd));
+		  last_cie = cie;
 		}
 
 	      init_loc = read_encoded_pointer (abfd, &start,

Re: [RFA] Dwarf2CFI fix

[Daniel Jacobowitz]

On Wed, Feb 26, 2003 at 06:14:29PM +0100, Michal Ludvig wrote:
> Hi all,
> my code in dwarf2cfi.c assumed that CIE and FDE entries in .eh_frame are 
> always in the chain CIE, FDE, FDE, ..., CIE, FDE, ...
> where every FDE refers to the precedent CIE. Unfortunately this is not 
> alway the case. Linker (?) sometimes optimizes things in a way that the 
> assumption isn't true anymore. This patch addresses the problem.
> 
> OK to commit? Head and 5.3?

> 2003-02-26  Michal Ludvig  <mludvig@suse.cz>
> 
> 	* dwarf2cfi.c (parse_frame_info): It's not true that FDEs 
> 	always refer to the last CIE. We must deal with such a
> 	situation.
> 	* dwarf2cfi.h (

Either you're missing a bit of a patch or you're missing a bit of a
changelog.  And there's no changelog entry for struct eh_frame either.

> diff -upr gdb-upto-08/dwarf2cfi.c gdb/dwarf2cfi.c
> --- gdb-upto-08/dwarf2cfi.c	2003-02-26 10:20:02.000000000 +0100
> +++ gdb/dwarf2cfi.c	2003-02-26 17:17:30.000000000 +0100
> @@ -50,6 +50,10 @@ struct cie_unit
>    char *data;
>    unsigned int data_length;
>  
> +  /* This field is 1 for CIE from .eh_frame
> +     and 0 for one from .debug_frame. */

Formatting: don't wrap that early, and two spaces after a period.

The patch itself looks OK.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Dwarf2CFI fix

[Michal Ludvig]

Daniel Jacobowitz wrote:

> Either you're missing a bit of a patch or you're missing a bit of a
> changelog.  And there's no changelog entry for struct eh_frame either.

Oops, sorry. The changelog wasn't yet saved while I was sending the mail...

Here it goes:

2003-02-26  Michal Ludvig  <mludvig@suse.cz>

         * dwarf2cfi.c (struct cie_unit): Added eh_frame field.
         (parse_frame_info): It's not true that FDEs always refer to
         the last CIE. We must deal with such a situation.

>>diff -upr gdb-upto-08/dwarf2cfi.c gdb/dwarf2cfi.c
>>--- gdb-upto-08/dwarf2cfi.c	2003-02-26 10:20:02.000000000 +0100
>>+++ gdb/dwarf2cfi.c	2003-02-26 17:17:30.000000000 +0100
>>@@ -50,6 +50,10 @@ struct cie_unit
>>   char *data;
>>   unsigned int data_length;
>> 
>>+  /* This field is 1 for CIE from .eh_frame
>>+     and 0 for one from .debug_frame. */
> 
> 
> Formatting: don't wrap that early, and two spaces after a period.

OK, I'll reindent it with gdb_indent.sh before committing.


> The patch itself looks OK.

Is this an approval? :-)

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz

Re: [RFA] Dwarf2CFI fix

[Daniel Jacobowitz]

On Wed, Feb 26, 2003 at 11:07:38PM +0100, Michal Ludvig wrote:
> Daniel Jacobowitz wrote:
> 
> >Either you're missing a bit of a patch or you're missing a bit of a
> >changelog.  And there's no changelog entry for struct eh_frame either.
> 
> Oops, sorry. The changelog wasn't yet saved while I was sending the mail...
> 
> Here it goes:
> 
> 2003-02-26  Michal Ludvig  <mludvig@suse.cz>
> 
>         * dwarf2cfi.c (struct cie_unit): Added eh_frame field.
>         (parse_frame_info): It's not true that FDEs always refer to
>         the last CIE. We must deal with such a situation.
> 
> >>diff -upr gdb-upto-08/dwarf2cfi.c gdb/dwarf2cfi.c
> >>--- gdb-upto-08/dwarf2cfi.c	2003-02-26 10:20:02.000000000 +0100
> >>+++ gdb/dwarf2cfi.c	2003-02-26 17:17:30.000000000 +0100
> >>@@ -50,6 +50,10 @@ struct cie_unit
> >>  char *data;
> >>  unsigned int data_length;
> >>
> >>+  /* This field is 1 for CIE from .eh_frame
> >>+     and 0 for one from .debug_frame. */
> >
> >
> >Formatting: don't wrap that early, and two spaces after a period.
> 
> OK, I'll reindent it with gdb_indent.sh before committing.

I don't think that will fix the two-spaces thing; you have to do it by
hand.

> >The patch itself looks OK.
> 
> Is this an approval? :-)

No.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Dwarf2CFI fix

[Michal Ludvig]

Daniel Jacobowitz wrote:
> On Wed, Feb 26, 2003 at 11:07:38PM +0100, Michal Ludvig wrote:
>>Daniel Jacobowitz wrote:
>>>The patch itself looks OK.
>>
>>Is this an approval? :-)
> 
> No.

Whew. Seems like I'll have to wait for two months for Elena :-(((
Thanks anyway.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz

Re: [RFA] Dwarf2CFI fix

[Elena Zannoni]

Michal Ludvig writes:
 > Daniel Jacobowitz wrote:
 > > On Wed, Feb 26, 2003 at 11:07:38PM +0100, Michal Ludvig wrote:
 > >>Daniel Jacobowitz wrote:
 > >>>The patch itself looks OK.
 > >>
 > >>Is this an approval? :-)
 > > 
 > > No.
 > 
 > Whew. Seems like I'll have to wait for two months for Elena :-(((
 > Thanks anyway.
 > 

I don't appreciate being singled out like this.  First of all, I am
not the primary maintainer for dwarf stuff. Mr Blandy is.  Second, as
you can verify looking at the mailing list archives, *I* have not
disappeared, while others have.  Third, I don't have the chance to
reply in real time, after 3 milliseconds, to every single message that
appears on this mailing list. I think this is an unreasonable request
to ask of any maintainer. Even if I had the time, I wouldn't do it,
because it generally leads to hurried decisions which have not been
completely thought through.

As far as x86-64 goes, I have just got a machine set up where I can
test stuff. I am getting to it. There is a lot of stuff to clean up in
this port.



 > Michal Ludvig
 > -- 
 > * SuSE CR, s.r.o     * mludvig@suse.cz
 > * (+420) 296.545.373 * http://www.suse.cz

Re: [RFA] Dwarf2CFI fix

[Andreas Jaeger]

Elena Zannoni <ezannoni@redhat.com> writes:

> As far as x86-64 goes, I have just got a machine set up where I can
> test stuff. I am getting to it. There is a lot of stuff to clean up in

Glad to hear!

> this port.

If there's anything where Michal can help, feel free to contact us and
let's work together on this,

Thanks,
Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [RFA] Dwarf2CFI fix

[Michal Ludvig]

Elena Zannoni wrote:
> Michal Ludvig writes:
>  > Daniel Jacobowitz wrote:
>  > > On Wed, Feb 26, 2003 at 11:07:38PM +0100, Michal Ludvig wrote:
>  > >>Daniel Jacobowitz wrote:
>  > >>>The patch itself looks OK.
>  > >>
>  > >>Is this an approval? :-)
>  > > 
>  > > No.
>  > 
>  > Whew. Seems like I'll have to wait for two months for Elena :-(((
>  > Thanks anyway.
> 
> I don't appreciate being singled out like this.  First of all, I am
> not the primary maintainer for dwarf stuff. Mr Blandy is.  Second, as
> you can verify looking at the mailing list archives, *I* have not
> disappeared, while others have.  Third, I don't have the chance to
> reply in real time, after 3 milliseconds, to every single message that
> appears on this mailing list. I think this is an unreasonable request
> to ask of any maintainer. Even if I had the time, I wouldn't do it,
> because it generally leads to hurried decisions which have not been
> completely thought through.

I'm sorry Elena. Seems like I've had a bad day yesterday and replied to 
your mail while still having a "hot head". Please don't take it 
personaly and accept my apologise. Thanks in advance and sorry once again.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz