Skip to content

Commit

Permalink
Fix/gdbstub continuation crash (#1671)
Browse files Browse the repository at this point in the history
* Define `SimpleTimerCallback` - used in original GDB stub PR but this change was omitted
* Improve gdbstub debugging
* Add missing `return` statements in LiveDebug sample
* Simplify code now that `m_printf` supports flash strings directly
* Add `queueBreak` command
  • Loading branch information
mikee47 authored and slaff committed Apr 14, 2019
1 parent 461bd82 commit eddd414
Show file tree
Hide file tree
Showing 17 changed files with 312 additions and 146 deletions.
13 changes: 7 additions & 6 deletions Sming/Makefile-rboot.mk
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ RBOOT_E2_SECTS ?= .text .data .rodata
RBOOT_E2_USER_ARGS ?= -quiet -bin -boot2
# GDB path
GDB ?= $(ESP_HOME)/xtensa-lx106-elf/bin/xtensa-lx106-elf-gdb
# File containing boot ROM debug symbols for GDB
BOOTROM_ELF := bootrom.elf

## COM port parameters
# Default COM port speed (generic)
Expand Down Expand Up @@ -255,6 +253,7 @@ endif
ifeq ($(ENABLE_GDB), 1)
CFLAGS += -ggdb -DENABLE_GDB=1
MODULES += $(SMING_HOME)/gdb
GDB_SYMBOLS := gdb_symbols
endif

ifeq ($(SMING_RELEASE),1)
Expand Down Expand Up @@ -542,11 +541,13 @@ endef

.PHONY: all checkdirs spiff_update spiff_clean clean kill_term terminal gdb

all: $(USER_LIBDIR)/lib$(LIBSMING).a checkdirs $(LIBMAIN_DST) $(RBOOT_BIN) $(RBOOT_ROM_0) $(RBOOT_ROM_1) $(SPIFF_BIN_OUT) $(FW_FILE_1) $(FW_FILE_2) $(BUILD_BASE)/$(BOOTROM_ELF)
all: $(USER_LIBDIR)/lib$(LIBSMING).a checkdirs $(LIBMAIN_DST) $(RBOOT_BIN) $(RBOOT_ROM_0) $(RBOOT_ROM_1) $(SPIFF_BIN_OUT) $(FW_FILE_1) $(FW_FILE_2) $(GDB_SYMBOLS)

# File contains boot rom symbols required by GDB, put it somewhere easy to find
$(BUILD_BASE)/$(BOOTROM_ELF):
cp $(SMING_HOME)/gdb/$(BOOTROM_ELF) $(BUILD_BASE)
# Copy symbols required by GDB into build directory
gdb_symbols: $(BUILD_BASE)/bootrom.elf

$(BUILD_BASE)/%.elf:
cp $(SMING_HOME)/gdb/symbols/$(notdir $@) $@

$(RBOOT_BIN):
$(MAKE) -C $(THIRD_PARTY_DIR)/rboot RBOOT_GPIO_ENABLED=$(RBOOT_GPIO_ENABLED) RBOOT_SILENT=$(RBOOT_SILENT)
Expand Down
4 changes: 3 additions & 1 deletion Sming/SmingCore/SimpleTimer.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ extern "C" {
*/
#define MAX_OS_TIMER_INTERVAL_US 268435000

typedef os_timer_func_t* SimpleTimerCallback;

class SimpleTimer
{
public:
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 1 addition & 5 deletions Sming/appspecific/gdb/gdb_hooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -243,7 +240,6 @@ static unsigned IRAM_ATTR __gdb_no_op(void)
#define NOOP __attribute__((weak, alias("__gdb_no_op")))

void gdb_enable(bool state) NOOP;
void gdb_do_break(void) NOOP;
GdbState gdb_present(void) NOOP;
void gdb_on_attach(bool attached) NOOP;
void gdb_detach(void) NOOP;
Expand Down
4 changes: 4 additions & 0 deletions Sming/appspecific/gdb/gdbstub-entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion Sming/gdb/GdbPacket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion Sming/gdb/gdbcmds
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Sming/gdb/gdbhostio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
15 changes: 3 additions & 12 deletions Sming/gdb/gdbstub-cfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -102,7 +93,7 @@
* characters are passed through to the application via UART2.
* If your application uses the serial port for terminal (text) communications you should be OK,
* but binary transfers are likely to cause problems and this option should probably be disabled.
* Instead, use GDBSTUB_BREAK_ON_INIT, or call gdbstub_do_break() in your application.
* Instead, use GDBSTUB_BREAK_ON_INIT, or call gdb_do_break() in your application.
*
* Specify:
* 0 to disable Ctrl+C break checking completely
Expand Down Expand Up @@ -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

/*
Expand Down Expand Up @@ -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

/*
Expand Down
Loading

0 comments on commit eddd414

Please sign in to comment.