On 22.03.21 06:21, Simon Glass wrote:
At present if sandbox crashes it prints a message and tries to exit. But with the recently introduced signal handler, it often seems to get stuck in a loop until the stack overflows:
Segmentation violation
Segmentation violation
Segmentation violation
Segmentation violation
Segmentation violation
Segmentation violation
Segmentation violation ...
Hello Simon,
do you have a reproducible example? I never have seen this.
Corrupting gd could cause an endless recursive loop, as these lines follow printing the observed string:
printf("pc = 0x%lx, ", pc); printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);
If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c, recursion cannot occur anymore. If a segmentation violation occurs inside the handler it will be delegated to the default handler.
Furthermore we could consider removing the signal handler at the start of os_signal_action().
Best regards
Heinrich
The signal handler is only useful for a few tests, as I understand it. Make it optional.
Signed-off-by: Simon Glass sjg@chromium.org
arch/sandbox/cpu/start.c | 18 +++++++++++++++--- arch/sandbox/include/asm/state.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index e87365e800d..72fe293e8bc 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -389,6 +389,16 @@ static int sandbox_cmdline_cb_select_unittests(struct sandbox_state *state, } SANDBOX_CMDLINE_OPT_SHORT(select_unittests, 'k', 1, "Select unit tests to run");
+static int sandbox_cmdline_cb_signals(struct sandbox_state *state,
const char *arg)+{
- state->handle_signals = true;
- return 0;
+} +SANDBOX_CMDLINE_OPT_SHORT(signals, 'S', 0,
"Handle signals (such as SIGSEGV) in sandbox");static void setup_ram_buf(struct sandbox_state *state) { /* Zero the RAM buffer if we didn't read it, to keep valgrind happy */ @@ -472,9 +482,11 @@ int main(int argc, char *argv[]) if (ret) goto err;
- ret = os_setup_signal_handlers();
- if (ret)
goto err;
- if (state->handle_signals) {
ret = os_setup_signal_handlers();if (ret)goto err;- }
#if CONFIG_VAL(SYS_MALLOC_F_LEN) gd->malloc_base = CONFIG_MALLOC_F_ADDR; diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index bca13069824..1c4c571e28d 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -93,6 +93,7 @@ struct sandbox_state { bool ram_buf_read; /* true if we read the RAM buffer */ bool run_unittests; /* Run unit tests */ const char *select_unittests; /* Unit test to run */
bool handle_signals; /* Handle signals within sandbox */
/* Pointer to information for each SPI bus/cs */ struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]