This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix GDB crash in dprintf.exp
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Thu, 28 Jan 2016 14:24:30 +0000
- Subject: Re: [PATCH] Fix GDB crash in dprintf.exp
- Authentication-results: sourceware.org; auth=none
- References: <1452859520-19444-1-git-send-email-yao dot qi at linaro dot org> <56A9FFD9 dot 1010609 at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
> I agree. build_target_condition_list has the same problem though.
Yes, fix the same problem together, as it crashes GDB too. Patch below
is pushed in.
> (we could probably merge these functions).
>
I'll do that after the 7.11 release.
> There's something else odd in these functions -- the first two loops
> over locations don't check whether the location is enabled, and
> parse the ax anyway if disabled. Isn't that odd? The last loop
> that pushes the bytecode into the vec does take that into account.
Yes, that is odd. The code is less optimal and isn't harmful, so I'll
defer the fix too.
--
Yao (éå)
From 4a6a1ed4a113d386b131938bbc7d66e7b495e73f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 28 Jan 2016 14:16:42 +0000
Subject: [PATCH] Fix GDB crash in dprintf.exp
I see GDB crashes in dprintf.exp on aarch64-linux testing,
(gdb) PASS: gdb.base/dprintf.exp: agent: break 29
set dprintf-style agent^M
(gdb) PASS: gdb.base/dprintf.exp: agent: set dprintf style to agent
continue^M
Continuing.
ASAN:SIGSEGV
=================================================================
==22475==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x000000494820 sp 0x7fff389b83a0 bp 0x62d000082417 T0)
#0 0x49481f in remote_add_target_side_commands /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:9190^M
#1 0x49e576 in remote_add_target_side_commands /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:9174^M
#2 0x49e576 in remote_insert_breakpoint /home/yao/SourceCode/gnu/gdb/git/gdb/remote.c:9240^M
#3 0x5278b7 in insert_bp_location /home/yao/SourceCode/gnu/gdb/git/gdb/breakpoint.c:2734^M
#4 0x52ac09 in insert_breakpoint_locations /home/yao/SourceCode/gnu/gdb/git/gdb/breakpoint.c:3159^M
#5 0x52ac09 in update_global_location_list /home/yao/SourceCode/gnu/gdb/git/gdb/breakpoint.c:12686
the root cause of this problem in this case is about linespec and
symtab which produces additional incorrect location and a NULL is added to
bp_tgt->tcommands. I posted a patch
https://sourceware.org/ml/gdb-patches/2015-12/msg00321.html to fix it
in linespec (the fix causes regression), but GDB still shouldn't add
NULL into bp_tgt->tcommands. The logic of build_target_command_list
looks odd to me. If we get something wrong in parse_cmd_to_aexpr (it
returns NULL), we shouldn't continue, instead we should set flag
null_command_or_parse_error. This is what this patch does. In the
meantime, we find build_target_condition_list has the same problem, so
fix it too.
gdb:
2016-01-28 Yao Qi <yao.qi@linaro.org>
* breakpoint.c (build_target_command_list): Don't call continue
if aexpr is NULL.
(build_target_condition_list): Likewise.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0958c5b..065319c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-01-28 Yao Qi <yao.qi@linaro.org>
+
+ * breakpoint.c (build_target_command_list): Don't call continue
+ if aexpr is NULL.
+ (build_target_condition_list): Likewise.
+
2016-01-27 Kevin Buettner <kevinb@redhat.com>
* rx-tdep.c (rx_push_dummy_call): Treat scalars larger than 8
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7b610ef..afd9065 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2347,12 +2347,6 @@ build_target_condition_list (struct bp_location *bl)
need to parse the condition to bytecodes again. */
aexpr = parse_cond_to_aexpr (bl->address, loc->cond);
loc->cond_bytecode = aexpr;
-
- /* Check if we managed to parse the conditional expression
- correctly. If not, we will not send this condition
- to the target. */
- if (aexpr)
- continue;
}
/* If we have a NULL bytecode expression, it means something
@@ -2553,9 +2547,6 @@ build_target_command_list (struct bp_location *bl)
aexpr = parse_cmd_to_aexpr (bl->address,
loc->owner->extra_string);
loc->cmd_bytecode = aexpr;
-
- if (!aexpr)
- continue;
}
/* If we have a NULL bytecode expression, it means something