Re: [patch] add trace capabilities to arm sim

[Richard Earnshaw <rearnsha@arm.com>]

I haven't seen anyone else comment on this, so I'll stick my nose in ;-)

I can't approve or reject this patch, but:


> 2001-02-23  DJ Delorie  <dj@redhat.com>
> 
> 	* armemu.c (ARMul_Emulate): Add hook for tracing.
> 	* wrapper.c (current_bfd): New, remembers which bfd we're
> 	simulating.
> 	(sim_dis_read): New, helper for tracing.
> 	(remove_useless_symbols): New, helper for tracing.  From objdump.
> 	(compare_symbols): Ditto.
> 	(sim_trace_one_arm_insn): New, trace one instruction using
> 	disassembler.
> 	(sim_trace): Make it do something useful now.
> 	(sim_create_inferior): Save current bfd.
> 	(sim_open): Ditto.
> 	(sim_load): Ditto.
> 	(sim_do_command): Handle the `trace' command.
> 
> 	* wrapper.c (ARMul_Debug): Return insn just in case we're called.
> 

> --- armemu.c	2001/02/24 00:24:39
> *************** ARMul_Emulate26 (register ARMul_State * 
> *** 377,382 ****
> --- 377,383 ----
>         if (instr == 0)
>   	abort ();
>   #endif
> +       sim_trace_one_arm_insn (pc, instr);

This will bump up the cost of simulating each instruction by the overhead 
of this call plus any code within that always gets executed.

Since tracing is going to slow the simulation to a crawl, I think it would 
be better to consider this an "exceptional condition"; that is, put the 
code inside the immediately following test:

>   
>         if (state->Exception)
>   	{			/* Any exceptions */

and then bump state->Exception (which behaves pretty much like a semaphore 
count on the number of extra things to go look at) when tracing needs to 
run.

> + void
> + sim_trace_one_arm_insn(pc, insn)
> + 	      int pc;
> + 	      unsigned int insn;
> + {
> +   static int initted = 0;
> +   static asymbol **symtab = 0;
> +   static int symcount = 0;
> +   static int last_sym = -1;
> +   static struct disassemble_info info;
> +   int storage, sym, bestsym, bestaddr;
> +   int min, max, i, pnl;
> +   static ARMword prevregs[16];
> + 
> +   if (insn == 0)
> +     {
> +       state->Emulate = STOP;

why should executing the (legal) instruction

	andeq	r0, r0, r0

cause the simulation to halt (OK, it's not very useful, but it is a valid 
NOP)?

> +       return;
> +     }
> +   if (current_bfd == 0)
> +     return;
> +   if (!do_tracing)
> +     return;

Shouldn't this be the first thing you check?

>   int
>   sim_trace (sd)
>        SIM_DESC sd ATTRIBUTE_UNUSED;

It is used now... (this is also where you might bump/unbump state->
Exception).

> ! {
> !   do_tracing = 1;
> !   sim_resume(sd, 0, 0);
> !   do_tracing = 0;
>     return 1;
>   }
>   

You'd also need to do it here (but a bit more subtly...

>   sim_do_command (sd, cmd)
>        SIM_DESC sd ATTRIBUTE_UNUSED;
>        char *cmd ATTRIBUTE_UNUSED;
> ! {
> !   if (strcmp(cmd, "trace") == 0)
> !     {
> !       do_tracing = !do_tracing;
> !       if (do_tracing)
> ! 	(*sim_callback->printf_filtered) (sim_callback,
> ! 					  "Tracing on.\n");
> !       else
> ! 	(*sim_callback->printf_filtered) (sim_callback,
> ! 					  "Tracing off.\n");
> !     }
> !   else
> ! 	(*sim_callback->printf_filtered) (sim_callback,
> ! 					  "Unknown command, only `trace' allowed.\n");
>   }
>   
>   

R.