Mirror of the gdb mailing list
 help / color / mirror / Atom feed
* Is this "gdb disassembler" code still needed?
@ 2013-06-26 18:35 Richard Sandiford
  2013-06-26 19:08 ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2013-06-26 18:35 UTC (permalink / raw)
  To: gdb; +Cc: macro

Hi,

opcodes/mips-dis.c has some code marked "For gdb disassembler, ...".

(1) For jumps in the standard encoding:

	  /* For gdb disassembler, force odd address on jalx.  */
	  if (info->flavour == bfd_target_unknown_flavour
	      && strcmp (opp->name, "jalx") == 0)
	    info->target |= 1;

(2) For MIPS16 branches:

	    if (pcrel && branch
		&& info->flavour == bfd_target_unknown_flavour)
	      /* For gdb disassembler, maintain odd address.  */
	      info->target |= 1;

(3) For MIPS16 jumps:

	if (!jalx && info->flavour == bfd_target_unknown_flavour)
	  /* For gdb disassembler, maintain odd address.  */
	  l |= 1;

(4) For microMIPS jumps:

		  /* For gdb disassembler, force odd address on jalx.  */
		  if (info->flavour == bfd_target_unknown_flavour
		      && strcmp (op->name, "jalx") == 0)
		    info->target |= 1;

(4) seems like it's doing the opposite of (3), whereas I'd have expected
it to do the same.  It doesn't like microMIPS has the equivalent of (2).
(Hope I'm reading this right.)

Do you know if this special handling is still needed?  If so, is the
current behaviour intentional?

Thanks,
Richard


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Is this "gdb disassembler" code still needed?
  2013-06-26 18:35 Is this "gdb disassembler" code still needed? Richard Sandiford
@ 2013-06-26 19:08 ` Maciej W. Rozycki
  2013-06-26 22:10   ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2013-06-26 19:08 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gdb

On Wed, 26 Jun 2013, Richard Sandiford wrote:

> opcodes/mips-dis.c has some code marked "For gdb disassembler, ...".
> 
> (1) For jumps in the standard encoding:
> 
> 	  /* For gdb disassembler, force odd address on jalx.  */
> 	  if (info->flavour == bfd_target_unknown_flavour
> 	      && strcmp (opp->name, "jalx") == 0)
> 	    info->target |= 1;
> 
> (2) For MIPS16 branches:
> 
> 	    if (pcrel && branch
> 		&& info->flavour == bfd_target_unknown_flavour)
> 	      /* For gdb disassembler, maintain odd address.  */
> 	      info->target |= 1;
> 
> (3) For MIPS16 jumps:
> 
> 	if (!jalx && info->flavour == bfd_target_unknown_flavour)
> 	  /* For gdb disassembler, maintain odd address.  */
> 	  l |= 1;
> 
> (4) For microMIPS jumps:
> 
> 		  /* For gdb disassembler, force odd address on jalx.  */
> 		  if (info->flavour == bfd_target_unknown_flavour
> 		      && strcmp (op->name, "jalx") == 0)
> 		    info->target |= 1;
> 
> (4) seems like it's doing the opposite of (3), whereas I'd have expected
> it to do the same.  It doesn't like microMIPS has the equivalent of (2).
> (Hope I'm reading this right.)
> 
> Do you know if this special handling is still needed?  If so, is the
> current behaviour intentional?

 Thanks for the heads-up.

 Hmm, it looks like some cleanup might be in order here, especially the 
microMIPS stuff seems backwards (probably blindly copied from the standard 
MIPS version whereas MIPS16 conditions stand here) and should be fixed.

 Other than that I am fairly sure the behaviour is intentional, so except 
from any microMIPS change to match the MIPS16 variant please refrain from 
fiddling with these bits until I am done with the change proposed here:

http://sourceware.org/ml/binutils/2012-05/msg00183.html
http://sourceware.org/ml/binutils/2012-06/msg00114.html

  Maciej


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Is this "gdb disassembler" code still needed?
  2013-06-26 19:08 ` Maciej W. Rozycki
@ 2013-06-26 22:10   ` Richard Sandiford
  2013-07-31 16:24     ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2013-06-26 22:10 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>  Hmm, it looks like some cleanup might be in order here, especially the 
> microMIPS stuff seems backwards (probably blindly copied from the standard 
> MIPS version whereas MIPS16 conditions stand here) and should be fixed.
>
>  Other than that I am fairly sure the behaviour is intentional, so except 
> from any microMIPS change to match the MIPS16 variant please refrain from 
> fiddling with these bits until I am done with the change proposed here:
>
> http://sourceware.org/ml/binutils/2012-05/msg00183.html
> http://sourceware.org/ml/binutils/2012-06/msg00114.html

OK, thanks.  I'll change microMIPS to match MIPS16 like you say.

Richard


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Is this "gdb disassembler" code still needed?
  2013-06-26 22:10   ` Richard Sandiford
@ 2013-07-31 16:24     ` Maciej W. Rozycki
  2013-07-31 17:24       ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2013-07-31 16:24 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gdb

On Wed, 26 Jun 2013, Richard Sandiford wrote:

> >  Other than that I am fairly sure the behaviour is intentional, so except 
> > from any microMIPS change to match the MIPS16 variant please refrain from 
> > fiddling with these bits until I am done with the change proposed here:
> >
> > http://sourceware.org/ml/binutils/2012-05/msg00183.html
> > http://sourceware.org/ml/binutils/2012-06/msg00114.html
> 
> OK, thanks.  I'll change microMIPS to match MIPS16 like you say.

 What happened to this code after your recent changes?  I get rubbish in 
disassembly now, e.g.:

Dump of assembler code for function t1:
   0x00400991 <+0>:	save	a0-a3,32,ra,s1
   0x00400995 <+4>:	addiu	s1,sp,16
   0x00400997 <+6>:	lw	v1,0x4009f4 <t1+99>
   0x00400999 <+8>:	lw	v0,0x4009f0 <t1+95>
   0x0040099b <+10>:	move	t9,v1
   0x0040099d <+12>:	move	t8,v0
   0x0040099f <+14>:	lw	v0,24(s1)
   0x004009a1 <+16>:	move	a1,t9
   0x004009a3 <+18>:	move	a0,t8
   0x004009a5 <+20>:	jal	0x400da0 <exit@mips16plt+15>
   0x004009a9 <+24>:	nop
[...]

  Maciej


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Is this "gdb disassembler" code still needed?
  2013-07-31 16:24     ` Maciej W. Rozycki
@ 2013-07-31 17:24       ` Richard Sandiford
  2013-07-31 21:58         ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2013-07-31 17:24 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Wed, 26 Jun 2013, Richard Sandiford wrote:
>
>> >  Other than that I am fairly sure the behaviour is intentional, so except 
>> > from any microMIPS change to match the MIPS16 variant please refrain from 
>> > fiddling with these bits until I am done with the change proposed here:
>> >
>> > http://sourceware.org/ml/binutils/2012-05/msg00183.html
>> > http://sourceware.org/ml/binutils/2012-06/msg00114.html
>> 
>> OK, thanks.  I'll change microMIPS to match MIPS16 like you say.
>
>  What happened to this code after your recent changes?  I get rubbish in 
> disassembly now, e.g.:
>
> Dump of assembler code for function t1:
>    0x00400991 <+0>:	save	a0-a3,32,ra,s1
>    0x00400995 <+4>:	addiu	s1,sp,16
>    0x00400997 <+6>:	lw	v1,0x4009f4 <t1+99>
>    0x00400999 <+8>:	lw	v0,0x4009f0 <t1+95>
>    0x0040099b <+10>:	move	t9,v1
>    0x0040099d <+12>:	move	t8,v0
>    0x0040099f <+14>:	lw	v0,24(s1)
>    0x004009a1 <+16>:	move	a1,t9
>    0x004009a3 <+18>:	move	a0,t8
>    0x004009a5 <+20>:	jal	0x400da0 <exit@mips16plt+15>
>    0x004009a9 <+24>:	nop
> [...]

Gah, sorry.  The code is still there, but I think in the MIPS16 case it's
being thwarted by an even base address.  Does this fix it?  (microMIPS
should be OK.)

Richard


opcodes/
	* mips-dis.c (print_mips16_insn_arg): Include ISA bit in base address.

Index: opcodes/mips-dis.c
===================================================================
--- opcodes/mips-dis.c	2013-07-31 18:16:42.972774949 +0100
+++ opcodes/mips-dis.c	2013-07-31 18:19:37.661036158 +0100
@@ -1565,7 +1565,7 @@ print_mips16_insn_arg (struct disassembl
 	     }
 	}
 
-      print_insn_arg (info, state, opcode, operand, baseaddr, uval);
+      print_insn_arg (info, state, opcode, operand, baseaddr + 1, uval);
       break;
     }
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Is this "gdb disassembler" code still needed?
  2013-07-31 17:24       ` Richard Sandiford
@ 2013-07-31 21:58         ` Maciej W. Rozycki
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2013-07-31 21:58 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gdb

On Wed, 31 Jul 2013, Richard Sandiford wrote:

> Gah, sorry.  The code is still there, but I think in the MIPS16 case it's
> being thwarted by an even base address.  Does this fix it?  (microMIPS
> should be OK.)

 Yes it does, thanks for a quick fix!

  Maciej


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-07-31 21:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-26 18:35 Is this "gdb disassembler" code still needed? Richard Sandiford
2013-06-26 19:08 ` Maciej W. Rozycki
2013-06-26 22:10   ` Richard Sandiford
2013-07-31 16:24     ` Maciej W. Rozycki
2013-07-31 17:24       ` Richard Sandiford
2013-07-31 21:58         ` Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox