Raymond Toy pushed to branch issue-97-define-ud2-inst at cmucl / cmucl

Commits: 07da205f by Raymond Toy at 2021-03-28T08:02:04-07:00 Remove debugging prints

- - - - - a99cd825 by Raymond Toy at 2021-03-28T08:02:24-07:00 Clean up and add comments on how things work.

- - - - -

4 changed files:

- src/code/debug-int.lisp - src/lisp/breakpoint.c - src/lisp/x86-arch.c - src/lisp/x86-arch.h

Changes:

===================================== src/code/debug-int.lisp ===================================== @@ -4477,9 +4477,6 @@ The result is a symbol or nil if the routine cannot be found." ;;; (defun handle-breakpoint (offset component signal-context) (let ((data (breakpoint-data component offset nil))) - (format t "(handle-breakpoint ~A ~A ~A)~%" - offset component signal-context) - (format t " data = ~A~%" data) (unless data (error (intl:gettext "Unknown breakpoint in ~S at offset ~S.") (debug-function-name (debug-function-from-pc component offset))

===================================== src/lisp/breakpoint.c ===================================== @@ -192,8 +192,6 @@ compute_offset(os_context_t * scp, lispobj code, boolean function_end) static int compute_offset(os_context_t * scp, lispobj code, boolean function_end) { - extern unsigned int debug_handlers; - DPRINTF(debug_handlers, (stderr, "compute_offset: code = 0x%lx\n", code));

if (code == NIL) @@ -257,8 +255,6 @@ handle_breakpoint(int signal, int subcode, os_context_t * scp) void handle_breakpoint(int signal, int subcode, os_context_t * scp) { - extern unsigned int debug_handlers; - lispobj code, scp_sap = alloc_sap(scp);

fake_foreign_function_call(scp);

===================================== src/lisp/x86-arch.c ===================================== @@ -153,7 +153,8 @@ arch_skip_instruction(os_context_t * context) /* Get and skip the lisp error code. */ char* pc = (char *) SC_PC(context);

- pc += 2; /* skip 0x0f and 0x0b */ + /* Skip over the UD2 inst (0x0f, 0x0b) */ + pc += 2;

code = *pc++; SC_PC(context) = (unsigned long) pc; @@ -173,6 +174,8 @@ arch_skip_instruction(os_context_t * context) break;

case trap_Breakpoint: + lose("Unexpected breakpoint trap in arch_skip_instruction\n"); + break; case trap_FunctionEndBreakpoint: break;

@@ -231,6 +234,11 @@ arch_remove_breakpoint(void *pc, unsigned long orig_inst) (stderr, "arch_remove_breakpoint: %p orig %lx\n", pc, orig_inst)); unsigned char *ptr = (unsigned char *) pc; + /* + * Just restore all the bytes from orig_inst. Should we just + * re-install just the one byte that was taken by the int3 + * instruction? + */ ptr[0] = orig_inst & 0xff; ptr[1] = (orig_inst >> 8) & 0xff; ptr[2] = (orig_inst >> 16) & 0xff; @@ -267,8 +275,12 @@ arch_do_displaced_inst(os_context_t * context, unsigned long orig_inst)

*((char *) pc) = orig_inst & 0xff;

+ /* + * If we have the SC_EFLAGS macro, we can enable single-stepping + * by setting the bit. Otherwise, we need a more complicated way + * of enabling single-stepping. + */ #ifdef SC_EFLAGS - /* Enable single-stepping */ SC_EFLAGS(context) |= 0x100; #else

@@ -305,6 +317,11 @@ arch_do_displaced_inst(os_context_t * context, unsigned long orig_inst) }

+/* + * Handles the break instruction from lisp, which is now UD2 followed + * by the trap code. In particular, this does not handle the + * breakpoint traps. + */ void sigill_handler(HANDLER_ARGS) { @@ -322,9 +339,6 @@ sigill_handler(HANDLER_ARGS) *((unsigned char*)SC_PC(context) + 3), *((unsigned char*)SC_PC(context) + 4)));

- if (single_stepping) { - lose("sigill handler with single-stepping enabled?\n"); - }

/* This is just for info in case monitor wants to print an approx */ current_control_stack_pointer = (unsigned long *) SC_SP(os_context); @@ -346,6 +360,14 @@ sigill_handler(HANDLER_ARGS) DPRINTF(debug_handlers, (stderr, "pc %x\n", *(unsigned short *)SC_PC(context)));

+ /* + * Make sure the trapping instruction is UD2. Abort if not. + * + * TODO: aborting is probably not the best idea. Could get here + * from other illegal instructions in, say, C code? Maybe we + * should call interrupt_handle_now, as we do below for an unknown + * trap code? + */ if (*(unsigned short *) SC_PC(context) == 0x0b0f) { trap = *(((char *)SC_PC(context)) + 2); } else { @@ -413,6 +435,9 @@ sigill_handler(HANDLER_ARGS) } }

+/* + * Handles the breakpoint trap (int3) and also single-stepping + */ void sigtrap_handler(HANDLER_ARGS) { @@ -464,6 +489,13 @@ sigtrap_handler(HANDLER_ARGS)

DPRINTF(debug_handlers, (stderr, "*C break\n"));

+ /* + * The int3 instruction causes a trap that leaves us just after + * the instruction. Backup one so we're at the beginning. This + * is really important so that when we handle the breakpoint, the + * offset of the instruction matches where Lisp thinks the + * breakpoint was placed. + */ SC_PC(os_context) -= 1;

handle_breakpoint(signal, CODE(code), os_context);

===================================== src/lisp/x86-arch.h ===================================== @@ -9,6 +9,13 @@ extern int arch_support_sse2(void); extern boolean os_support_sse2(void);

+/* + * Set to non-zero to enable debug prints for debugging the sigill and + * sigtrap handlers and for debugging breakpoints. + */ +extern unsigned int debug_handlers; + + /* * Define macro to allocate a local array of the appropriate size * where the fpu state can be stored.

View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/d039eb90c6e86457c946c9f...