Fix/gdbstub continuation crash

Sming/SmingCore/SimpleTimer.h

@@ -37,6 +37,8 @@ extern "C" {
 */
 #define MAX_OS_TIMER_INTERVAL_US 268435000
 
+typedef os_timer_func_t* SimpleTimerCallback;
+
 class SimpleTimer
 {
 public:
@@ -89,7 +91,7 @@ class SimpleTimer
      *  @param  interrupt Function to be called on timer trigger
      *  @note   Classic c-type callback method
      */
-	void setCallback(os_timer_func_t callback, void* arg = nullptr)
+	void setCallback(SimpleTimerCallback callback, void* arg = nullptr)
 	{
 		stop();
 		ets_timer_setfn(&osTimer, callback, arg);

Sming/appspecific/gdb/gdb_hooks.cpp

@@ -120,10 +120,7 @@ void dumpExceptionInfo()
 				"***** Fatal exception %u"),
 			 reg.cause);
 	if(reg.cause <= EXCCAUSE_MAX) {
-		char name[32];
-		memcpy_P(name, exceptionNames[reg.cause], sizeof(name));
-		name[sizeof(name) - 1] = '\0';
-		m_printf(_F(" (%s)"), name);
+		m_printf(_F(" (%s)"), exceptionNames[reg.cause]);
 	}
 	m_puts("\r\n");
 

Sming/appspecific/gdb/gdbstub-entry.S

@@ -213,6 +213,10 @@ gdbstub_init_debug_entry:
 	ASATTR_GDBFN
 	.align 4
 gdbstub_icount_ena_single_step:
+
+//Make sure we're running at sufficiently high interrupt level to prevent early triggering
+	rsil a2,XCHAL_DEBUGLEVEL
+
 	movi	a3, XCHAL_DEBUGLEVEL //Only count steps in non-debug mode
 	movi	a2, -2
 	wsr		a3, ICOUNTLEVEL

Sming/gdb/GdbPacket.cpp

@@ -15,14 +15,19 @@
 // Send the start of a packet; reset checksum calculation.
 void ATTR_GDBEXTERNFN GdbPacket::start()
 {
+#if GDBSTUB_ENABLE_DEBUG
+	m_puts(_F("> PKT "));
+#endif
 	gdbSendChar('$');
 }
 
 void ATTR_GDBEXTERNFN GdbPacket::end()
 {
 	gdbSendChar('#');
 	writeHexByte(checksum);
-	debug_i("> PKT [%u]", packetLength);
+#if GDBSTUB_ENABLE_DEBUG
+	m_puts("\r\n");
+#endif
 }
 
 void ATTR_GDBEXTERNFN GdbPacket::writeChar(char c)

Sming/gdb/gdbcmds

@@ -1,5 +1,5 @@
 # Enable this if you want to log all traffic between GDB and the stub
-# set remotelogfile gdb_rsp_logfile.txt
+#set remotelogfile gdb_rsp_logfile.txt
 
 # ESP8266 HW limits the number of breakpoints/watchpoints
 set remote hardware-breakpoint-limit 1
@@ -29,6 +29,9 @@ file out/build/app_0.out
 # See https://github.com/jcmvbkbc/esp-elf-rom
 add-symbol-file out/build/bootrom.elf 0x40000000 -readnow
 
+# Useful for lower-level debugging to see the result of stepping through assembler code
+#set disassemble-next-line on
+
 # Change the following to your serial port and baud
 #set serial baud 115200
 #target remote /dev/ttyUSB0

Sming/gdb/gdbhostio.cpp

@@ -113,7 +113,7 @@ void ATTR_GDBEXTERNFN gdbHandleHostIo(char* commandBuffer, unsigned cmdLen)
 		debug_i("File:pread(%d, %u, %u)", fd, count, offset);
 
 		packet.writeChar('F');
-		if(fileSeek(fd, offset, eSO_FileStart) == offset) {
+		if(fileSeek(fd, offset, eSO_FileStart) == int(offset)) {
 			count = fileRead(fd, commandBuffer, count);
 			if(int(count) >= 0) {
 				packet.writeHexWord16(count);
@@ -139,7 +139,7 @@ void ATTR_GDBEXTERNFN gdbHandleHostIo(char* commandBuffer, unsigned cmdLen)
 		debug_i("File:pwrite(%d, %u, %u)", fd, offset, size);
 
 		packet.writeChar('F');
-		if(fileSeek(fd, offset, eSO_FileStart) == offset) {
+		if(fileSeek(fd, offset, eSO_FileStart) == int(offset)) {
 			int count = fileWrite(fd, data, size);
 			if(count >= 0) {
 				packet.writeHexWord16(count);

Sming/gdb/gdbstub-cfg.h

@@ -54,15 +54,6 @@
 #define GDBSTUB_ENABLE_DEBUG 0
 #endif
 
-/*
- * The HardwareTimer defaults to non-maskable mode, and will continue to operate whilst paused.
- * Set this value to 1 to disable during a debugging exception, and restore it afterwards.
- * Note that to debug HardwareTimer callback routines, the timer must be initialise in maskable mode.
- */
-#ifndef GDBSTUB_PAUSE_HARDWARE_TIMER
-#define GDBSTUB_PAUSE_HARDWARE_TIMER 0
-#endif
-
 /*
  * Espressif provide a patched version of GDB which emits only those registered present in the lx106.
  * Set to 0 if an unpatched version of GDB is used.
@@ -142,7 +133,7 @@
  * If undefined, calls will do nothing and return -1.
  */
 #ifndef GDBSTUB_ENABLE_SYSCALL
-#define GDBSTUB_ENABLE_SYSCALL 1
+#define GDBSTUB_ENABLE_SYSCALL 0
 #endif
 
 /*
@@ -178,7 +169,7 @@
  * Set to 0 to wait indefinitely.
  */
 #ifndef GDBSTUB_UART_READ_TIMEOUT
-#define GDBSTUB_UART_READ_TIMEOUT 2000
+#define GDBSTUB_UART_READ_TIMEOUT 0
 #endif
 
 /*

Sming/gdb/gdbstub.cpp

@@ -195,6 +195,11 @@ static ERRNO_T ATTR_GDBEXTERNFN writeMemoryBlock(uint32_t addr, const void* data
  */
 static void ATTR_GDBEXTERNFN sendReason()
 {
+	if(gdb_state.syscall == syscall_active) {
+		// GDB ignores this whilst processing a system call - we'll send when it's completed via 'F' response
+		return;
+	}
+
 	gdbFlushUserData();
 	GdbPacket packet;
 	packet.writeChar('T');
@@ -300,12 +305,18 @@ static unsigned ATTR_GDBEXTERNFN readCommand()
 	while((c = gdbReceiveChar()) != '#') { // end of packet, checksum follows
 		if(c < 0) {
 			gdbSendChar('-');
+#if GDBSTUB_ENABLE_DEBUG
+			m_puts("\r\n");
 			debug_e("CMD TIMEOUT");
+#endif
 			return 0;
 		}
 		if(c == '$') {
 			// Wut, restart packet?
+#if GDBSTUB_ENABLE_DEBUG
+			m_puts("\r\n");
 			debug_e("Unexpected '$' received");
+#endif
 			checksum = 0;
 			cmdLen = 0;
 			continue;
@@ -318,21 +329,26 @@ static unsigned ATTR_GDBEXTERNFN readCommand()
 		}
 		if(cmdLen >= MAX_COMMAND_LENGTH) {
 			// Received more than the size of the command buffer
+#if GDBSTUB_ENABLE_DEBUG
+			m_puts("\r\n");
 			debug_e("Command '%c' buffer overflow", commandBuffer[0]);
+#endif
 			return 0;
 		}
 		commandBuffer[cmdLen++] = c;
 	}
 	commandBuffer[cmdLen] = '\0';
 
-#if GDBSTUB_ENABLE_DEBUG
-	debug_i("cmd '%c', len %u", commandBuffer[0], cmdLen);
-#endif
-
 	// Read checksum and verify
 	char checksumChars[] = {char(gdbReceiveChar()), char(gdbReceiveChar()), '\0'};
 	const char* ptr = checksumChars;
 	auto receivedChecksum = GdbPacket::readHexValue(ptr);
+
+#if GDBSTUB_ENABLE_DEBUG
+	m_puts("\r\n");
+	debug_i("cmd '%c', len %u", commandBuffer[0], cmdLen);
+#endif
+
 	if(receivedChecksum == checksum) {
 		// Acknowledge the command
 		gdbSendChar('+');
@@ -423,14 +439,7 @@ static GdbResult ATTR_GDBEXTERNFN handleCommand(unsigned cmdLen)
 		auto regnum = GdbPacket::readHexValue(data);
 		auto regPtr = getSavedReg(regnum);
 #if GDBSTUB_ENABLE_DEBUG
-		char regName[16];
-		/*
- * @todo Trying to read flash here sometimes causes a LEVEL1 interrupt exception (even though
- * we haven't hooked it?!) According to the docs. we need to look at the INTERRUPT register
- * and INTENABLE to determine the actual cause.
- */
-		regName[0] = '\0';
-//		memcpy_P(regName, registerNames[regnum], sizeof(regName));
+		const char* regName = (regPtr == nullptr) ? PSTR("(unsupported)") : registerNames[regnum];
 #endif
 
 		if(commandChar == 'p') { // read
@@ -542,14 +551,31 @@ static GdbResult ATTR_GDBEXTERNFN handleCommand(unsigned cmdLen)
 #if GDBSTUB_ENABLE_SYSCALL
 	/*
 	 * A file (or console) I/O request has finished.
+	 *
+	 * This can happen whilst the application is running, or when it has stopped due to
+	 * exception or debug break.
+	 *
+	 * Whilst an I/O request is in progress, GDB will ignore anything we send so if already
+	 * paused we need to call sendReason() again.
+	 *
+	 * If the user hit Ctrl+C then the syscall completes and returns true to tell us this.
+	 *
+	 * Note that GDB expects us to be paused during the entire 'F' request / reply operation,
+	 * but that makes console reading a lot less useful for us so don't actually pause until
+	 * the start of the response packet (indicated by DBGFLAG_PACKET_STARTED).
 	 */
 	case 'F':
 		if(gdb_syscall_complete(data)) {
 			// Ctrl+C was pressed
 			sendReason();
 			return ST_OK;
-		} else {
+		} else if(bitRead(gdb_state.flags, DBGFLAG_PACKET_STARTED)) {
+			// Paused to deal with packet response, continue running normally
 			return ST_CONT;
+		} else {
+			// Paused for some other reason, keep debugging
+			sendReason();
+			return ST_OK;
 		}
 #endif
 
@@ -618,7 +644,7 @@ static GdbResult ATTR_GDBEXTERNFN handleCommand(unsigned cmdLen)
 					access = 2; // write
 				else if(idx == '3')
 					access = 1; // read
-				else if(idx == '4')
+				else			// can only be idx == '4' as we've checked above
 					access = 3; // access
 				if(len == 1)
 					mask = 0x3F;
@@ -701,6 +727,8 @@ static GdbResult ATTR_GDBEXTERNFN handleCommand(unsigned cmdLen)
 */
 void ATTR_GDBEXTERNFN commandLoop(bool waitForStart, bool allowDetach)
 {
+	debug_i(">> ENTER CMDLOOP");
+
 	bool initiallyAttached = gdb_state.attached;
 
 	while(true) {
@@ -727,6 +755,8 @@ void ATTR_GDBEXTERNFN commandLoop(bool waitForStart, bool allowDetach)
 	if(gdb_state.attached != initiallyAttached) {
 		System.queueCallback(TaskCallback(gdb_on_attach), gdb_state.attached);
 	}
+
+	debug_i("<< LEAVE CMDLOOP");
 }
 
 /*
@@ -780,31 +810,14 @@ static void ATTR_GDBEXTERNFN emulLdSt()
 	}
 }
 
-/**
- * @brief If the hardware timer is operating using non-maskable interrupts, we must explicitly stop it
- * @param pause true to stop the timer, false to resume it
- */
-static void pauseHardwareTimer(bool pause)
-{
-#if GDBSTUB_PAUSE_HARDWARE_TIMER
-	static bool edgeIntEnable;
-	if(pause) {
-		edgeIntEnable = bitRead(READ_PERI_REG(EDGE_INT_ENABLE_REG), 1);
-		TM1_EDGE_INT_DISABLE();
-	} else if(edgeIntEnable) {
-		TM1_EDGE_INT_ENABLE();
-	}
-#endif
-}
-
 // Main exception handler
 static void __attribute__((noinline)) gdbstub_handle_debug_exception_flash()
 {
+	debug_i(">> DBG 0x%02x, PC = %p", gdb_state.flags, gdbstub_savedRegs.pc);
+
 	bool isEnabled = gdb_state.enabled;
 
 	if(isEnabled) {
-		pauseHardwareTimer(true);
-
 		bitSet(gdb_state.flags, DBGFLAG_DEBUG_EXCEPTION);
 
 		if(singleStepPs >= 0) {
@@ -849,10 +862,6 @@ static void __attribute__((noinline)) gdbstub_handle_debug_exception_flash()
 	}
 
 	gdb_state.flags = 0;
-
-	if(isEnabled) {
-		pauseHardwareTimer(false);
-	}
 }
 
 // We just caught a debug exception and need to handle it. This is called from an assembly routine in gdbstub-entry.S
@@ -869,14 +878,12 @@ void gdbstub_handle_exception()
 		return;
 	}
 
-	pauseHardwareTimer(true);
 	bitSet(gdb_state.flags, DBGFLAG_SYSTEM_EXCEPTION);
 
 	sendReason();
 	commandLoop(true, false);
 
 	gdb_state.flags = 0;
-	pauseHardwareTimer(false);
 }
 #endif
 
@@ -891,7 +898,6 @@ void ATTR_GDBINIT gdbstub_init()
 	SD(ENABLE_EXCEPTION_DUMP);
 	SD(ENABLE_CRASH_DUMP);
 	SD(GDBSTUB_ENABLE_DEBUG);
-	SD(GDBSTUB_PAUSE_HARDWARE_TIMER);
 	SD(GDBSTUB_GDB_PATCHED);
 	SD(GDBSTUB_USE_OWN_STACK);
 	SD(GDBSTUB_BREAK_ON_EXCEPTION);

Sming/gdb/gdbstub.h

@@ -13,9 +13,6 @@
 #ifndef _GDB_GDBSTUB_H_
 #define _GDB_GDBSTUB_H_
 
-// Always optimise GDB stub code for size, regardless of application settings
-#pragma GCC optimize("Os")
-
 #include "gdbstub-internal.h"
 #include <gdb_hooks.h>
 #include "BitManipulations.h"
@@ -67,4 +64,9 @@ extern const uint8_t gdb_exception_signals[];
 void gdbstub_init();
 void gdbstub_handle_exception();
 
+#if GDBSTUB_ENABLE_DEBUG == 0
+// Optimise GDB stub code for size, regardless of application settings
+#pragma GCC optimize("Os")
+#endif
+
 #endif /* _GDB_GDBSTUB_H_ */

Sming/gdb/gdbuart.cpp

@@ -312,7 +312,6 @@ static void IRAM_ATTR gdb_uart_callback(uart_t* uart, uint32_t status)
 	// RX FIFO Full or RX FIFO Timeout ?
 	if(status & (_BV(UIFF) | _BV(UITO))) {
 #if GDBSTUB_ENABLE_UART2
-		bool isUserData = false;
 		auto rxbuf = user_uart == nullptr ? nullptr : user_uart->rx_buffer;
 #endif
 
@@ -361,7 +360,7 @@ static void IRAM_ATTR gdb_uart_callback(uart_t* uart, uint32_t status)
 				// This event doesn't apply to user uart
 				bitClear(user_uart_status, UIFF);
 				bitClear(user_uart_status, UITO);
-			} else if(rxbuf->getFreeSpace() > UART_RX_FIFO_SIZE + user_uart->rx_headroom) {
+			} else if(rxbuf->getFreeSpace() > size_t(UART_RX_FIFO_SIZE) + user_uart->rx_headroom) {
 				bitClear(user_uart_status, UIFF);
 			}
 		}