[patch] Add support for DW_LNE_set_discriminator

[patch] Add support for DW_LNE_set_discriminator

[Cary Coutant]

The discriminator patches that I recently submitted to gcc, gas, and
binutils introduced a bunch of gdb testsuite failures, and I'm
embarrassed to admit that I neglected to run the gdb testsuite before
I checked in the gcc patch. I expected all dwarf consumers to ignore
any unknown opcodes, but it turns out that gdb doesn't do that. This
patch adds support for the new opcode.

One question, though. I've written the patch to maintain the
philosophy that gdb will complain about any unknown extended opcodes,
and will give up on processing the remainder of the line number table.
I could change default case in dwarf_decode_lines() to just skip over
unknown extended opcodes, instead of adding the explicit case for the
one new opcode. Which would be preferable?

-cary


* dwarf2read.c (dwarf_decode_lines): Recognize and ignore
DW_LNE_set_discriminator.


Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.306
diff -u -p -r1.306 dwarf2read.c
--- dwarf2read.c	4 Jun 2009 12:28:39 -0000	1.306
+++ dwarf2read.c	9 Jun 2009 20:40:54 -0000
@@ -7357,6 +7357,11 @@ dwarf_decode_lines (struct line_header *
                     add_file_name (lh, cur_file, dir_index, mod_time, length);
                   }
 		  break;
+		case DW_LNE_set_discriminator:
+		  /* The discriminator is not interesting to the debugger;
+		     just ignore it.  */
+		  line_ptr = extended_end;
+		  break;
 		default:
 		  complaint (&symfile_complaints,
 			     _("mangled .debug_line section"));

Re: [patch] Add support for DW_LNE_set_discriminator

[Tom Tromey]

>>>>> "Cary" == Cary Coutant <ccoutant@google.com> writes:

Cary> One question, though. I've written the patch to maintain the
Cary> philosophy that gdb will complain about any unknown extended opcodes,
Cary> and will give up on processing the remainder of the line number table.
Cary> I could change default case in dwarf_decode_lines() to just skip over
Cary> unknown extended opcodes, instead of adding the explicit case for the
Cary> one new opcode. Which would be preferable?

I think the current approach seems reasonable.  If someone adds a
semantics-changing opcode, it seems preferable to fail than to get the
wrong answer.  But, I'm not very well versed in the ins and outs of
dwarf interpretation...

Right now the complaint does not mention the unrecognized opcode or
anything like that.  It seems like that would be a nice addition.

Cary> * dwarf2read.c (dwarf_decode_lines): Recognize and ignore
Cary> DW_LNE_set_discriminator.

Ok.  For future reference, a link to the standard proposal would have
helped :)

Tom

Re: [patch] Add support for DW_LNE_set_discriminator

[Cary Coutant]

> Cary> * dwarf2read.c (dwarf_decode_lines): Recognize and ignore
> Cary> DW_LNE_set_discriminator.
>
> Ok.  For future reference, a link to the standard proposal would have
> helped :)

Sorry, I should have provided that.

http://wiki.dwarfstd.org/index.php?title=Path_Discriminators

Did you mean I should have put that in the message, or in the ChangeLog entry?

-cary

Re: [patch] Add support for DW_LNE_set_discriminator

[Tom Tromey]

>>>>> "Cary" == Cary Coutant <ccoutant@google.com> writes:

Cary> Sorry, I should have provided that.

Cary> http://wiki.dwarfstd.org/index.php?title=Path_Discriminators

Cary> Did you mean I should have put that in the message, or in the
Cary> ChangeLog entry?

Just the message.
Your patch is good to go, thanks.

Tom