Re: [RFC] New GDB Port CR16

Mike Frysinger
2012-08-27T21:10:14Z

[-- Attachment #1: Type: Text/Plain, Size: 2246 bytes --]

On Monday 27 August 2012 02:36:15 Kaushik Phatak wrote:
> Please find attached a patch that adds support for the National
> Semiconductor CR16 architecture to GDB. The sim patch is already present
> in the sources. This patch enhances the sim to support breakpoints for sim
> based debugging.

this one diff has multiple unrelated changes. might be nice if you were to
split them up and commit them separately. the sim/common/ changes could be
merged now for example. the other cr16 sim changes should be split up from
the gdb changes so that they may be merged independently.

> --- gdb_src.orig/sim/common/gennltvals.sh
> +++ ./gdb_src/sim/common/gennltvals.sh
>
> --- gdb_src.orig/sim/common/nltvals.def
> +++ ./gdb_src/sim/common/nltvals.def
>
> --- gdb_src.orig/sim/cr16/cr16_sim.h
> +++ ./gdb_src/sim/cr16/cr16_sim.h
>
> --- gdb_src.orig/sim/cr16/interp.c
> +++ ./gdb_src/sim/cr16/interp.c

changes to these files look fine

> --- gdb_src.orig/sim/cr16/simops.c
> +++ ./gdb_src/sim/cr16/simops.c
> void
> OP_C_C ()

unrelated, but that should be "(void)". i imagine this sim port probably has
a bunch of those bugs lurking though.

> @@ -5465,9 +5467,24 @@ OP_C_C ()
> #endif
>
> default:
> + a = OP[0];
> + switch(a)
> + {
> + case TRAP_BREAKPOINT:
> + State.exception = SIGTRAP;
> + tmp = (PC);
> + JMP(tmp);
> + trace_output_void ();
> + break;
> + case SIGTRAP: // supervisor call ?
> + State.exception = SIG_CR16_EXIT;
> + trace_output_void ();
> + break;
> + default:
> cr16_callback->error (cr16_callback, "Unknown syscall %d", FUNC);
> + break;
> + }

pretty sure the indentation here is incorrect. the case statements inside
this new switch statement are not indented far enough.

also, please do not use C++ style comments //. use /* */ instead.

> --- gdb_src.orig/opcodes/cr16-dis.c
> +++ ./gdb_src/opcodes/cr16-dis.c
>
> -static int
> +int match_opcode (void);
> +
> +int
> match_opcode (void)

this new local prototype makes no sense. just delete it.

> -static void
> +void make_instruction (void);
> +
> +void
> make_instruction (void)

same here
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]