This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: gdbarch.sh -- new field


David Taylor wrote:
> 
> In proceed (infrun.c), we see the code:
> 
>       /* Check breakpoint_here_p first, because breakpoint_here_p is fast
>          (it just checks internal GDB data structures) and STEP_SKIPS_DELAY
>          is slow (it needs to read memory from the target).  */
>       if (STEP_SKIPS_DELAY_P
>           && breakpoint_here_p (read_pc () + 4)
>           && STEP_SKIPS_DELAY (read_pc ()))
> 
> and later in proceed there's the code:
> 
>         /* Did we fail to remove breakpoints?  If so, try
>            to set the PC past the bp.  (There's at least
>            one situation in which we can fail to remove
>            the bp's: On HP-UX's that use ttrace, we can't
>            change the address space of a vforking child
>            process until the child exits (well, okay, not
>            then either :-) or execs. */
>         if (remove_status != 0)
>           {
>             write_pc_pid (stop_pc - DECR_PC_AFTER_BREAK + 4, ecs->pid);
>           }
> 
> Both of these are wrong -- they assume that all instructions for all
> processors are 4 bytes long. 
>
> Not all processors have fixed length
> instructions; and not all processors with fixed length instructions
> fix the length at 4 bytes.

Correct, except that the first is only used for MIPS, 
and the second (should) only (be) used for HP.  Still,
you're right that they're horrible...

> I propose the addition of a new function field to gdbarch / new macro:
> 
>     addr_next_inst / ADDR_NEXT_INST (pc)
> 
> which given the address of an instruction (pc), returns the address of
> the next instruction (pc + 4 in the above).

Not a bad idea at all, and probably greatly useful in 
some other contexts.  Pretty ugly to implement for ia32, 
but easy for a lot of targets.

> There would then be a default "address of next instruction" function
> for those architectures that don't supply one that simply returns its
> argument plus 4.
> 
> For the non multi-arch case, we would have:
> 
>     #if !defined(ADDR_NEXT_INST)
>     #define ADDR_NEXT_INST(pc)  ((pc) + 4)
>     #endif
> 
> Then the two sections of code above would become:
> 
>       if (STEP_SKIPS_DELAY_P
>           && breakpoint_here_p (ADDR_NEXT_INST (read_pc ()))
>           && STEP_SKIPS_DELAY (read_pc ()))
> 
> and
> 
>         if (remove_status != 0)
>           {
>             write_pc_pid (ADDR_NEXT_INST (stop_pc - DECR_PC_AFTER_BREAK),
>                           ecs->pid);
>           }
> 
> respectively.
> 
> I don't think that the processor I'm currently working on is bothered
> by either chunk of code, but as I was stepping through some code and
> saw the above it bothered me (partly because the processor has 2 byte
> instructions).

If your processor is not MIPS or HP, you're safe.

> If I hear no objections to the change, I'll generate an actual patch
> and post it here.
> 
> Comments?

Go for it.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]