Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra <amodra at bigpond dot net dot au>]

On Fri, Mar 03, 2006 at 09:46:39AM -0800, H. J. Lu wrote:
> We need to check end_sequence when we compare line addresses.

Much better.

> +/* 2 lines can have the same address. But their end_sequence values
> +   must be different.  The one with end_sequence != 0 will never be
> +   reached.  The one with end_sequence == 0 is real.  */
> +
> +static bfd_boolean
> +compare_line_address (bfd_vma address, int end_sequence,
> +		      struct line_info *line)
> +{
> +  if (address == line->address && end_sequence == line->end_sequence)
> +    /* FIXME: Will we ever get here?  */
> +    abort ();
> +  else if (address > line->address
> +	   || (address == line->address
> +	       && end_sequence < line->end_sequence))
> +    return 1;
> +  else
> +    return -1;
> +}

Other comments in add_line_info indicate that you might need to handle
duplicate line info.  So I believe the compare_line_address function
comment and the abort are incorrect.

I'd rather see something like the following (untested!).  If this works
for you, then this add_line_info patch along with the rest of your patch
is OK to commit.

Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.84
diff -u -p -r1.84 dwarf2.c
--- bfd/dwarf2.c	15 Feb 2006 22:29:42 -0000	1.84
+++ bfd/dwarf2.c	5 Mar 2006 22:37:36 -0000
@@ -744,6 +744,17 @@ struct varinfo
   unsigned int stack: 1;
 };
 
+/* Return TRUE if NEW_LINE should sort after LINE.  */
+
+static inline bfd_boolean
+new_line_sorts_after (struct line_info *new_line, struct line_info *line)
+{
+  return (new_line->address > line->address
+	  || (new_line->address == line->address
+	      && new_line->end_sequence < line->end_sequence));
+}
+
+
 /* Adds a new entry to the line_info list in the line_info_table, ensuring
    that the list is sorted.  Note that the line_info list is sorted from
    highest to lowest VMA (with possible duplicates); that is,
@@ -760,6 +771,21 @@ add_line_info (struct line_info_table *t
   bfd_size_type amt = sizeof (struct line_info);
   struct line_info* info = bfd_alloc (table->abfd, amt);
 
+  /* Set member data of 'info'.  */
+  info->address = address;
+  info->line = line;
+  info->column = column;
+  info->end_sequence = end_sequence;
+
+  if (filename && filename[0])
+    {
+      info->filename = bfd_alloc (table->abfd, strlen (filename) + 1);
+      if (info->filename)
+	strcpy (info->filename, filename);
+    }
+  else
+    info->filename = NULL;
+
   /* Find the correct location for 'info'.  Normally we will receive
      new line_info data 1) in order and 2) with increasing VMAs.
      However some compilers break the rules (cf. decode_line_info) and
@@ -777,7 +803,7 @@ add_line_info (struct line_info_table *t
 
   while (1)
     if (!table->last_line
-	|| address >= table->last_line->address)
+	|| new_line_sorts_after (info, table->last_line))
       {
 	/* Normal case: add 'info' to the beginning of the list */
 	info->prev_line = table->last_line;
@@ -789,7 +815,7 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (!table->lcl_head->prev_line
-	     && table->lcl_head->address > address)
+	     && !new_line_sorts_after (info, table->lcl_head))
       {
 	/* Abnormal but easy: lcl_head is 1) at the *end* of the line
 	   list and 2) the head of 'info'.  */
@@ -798,8 +824,8 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (table->lcl_head->prev_line
-	     && table->lcl_head->address > address
-	     && address >= table->lcl_head->prev_line->address)
+	     && !new_line_sorts_after (info, table->lcl_head)
+	     && new_line_sorts_after (info, table->lcl_head->prev_line))
       {
 	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
 	   list and 2) the head of 'info'.  */
@@ -816,7 +842,8 @@ add_line_info (struct line_info_table *t
 
 	while (li1)
 	  {
-	    if (li2->address > address && address >= li1->address)
+	    if (!new_line_sorts_after (info, li2)
+		&& new_line_sorts_after (info, li1))
 	      break;
 
 	    li2 = li1; /* always non-NULL */
@@ -824,21 +851,6 @@ add_line_info (struct line_info_table *t
 	  }
 	table->lcl_head = li2;
       }
-
-  /* Set member data of 'info'.  */
-  info->address = address;
-  info->line = line;
-  info->column = column;
-  info->end_sequence = end_sequence;
-
-  if (filename && filename[0])
-    {
-      info->filename = bfd_alloc (table->abfd, strlen (filename) + 1);
-      if (info->filename)
-	strcpy (info->filename, filename);
-    }
-  else
-    info->filename = NULL;
 }
 
 /* Extract a fully qualified filename from a line info table.