diff mbox

Fix GDB crash in dprintf.exp

Message ID 1452859520-19444-1-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi Jan. 15, 2016, 12:05 p.m. UTC 
  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

Comments

Pedro Alves Jan. 28, 2016, 11:47 a.m. UTC | #1 
On 01/15/2016 12:05 PM, Yao Qi wrote:

> 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.

I agree.  build_target_condition_list has the same problem though.
(we could probably merge these functions).

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.

> I have a vague memory that the code here was discussed before, but I
> can't find the mail in archive.
> 

No idea.

Thanks,
Pedro Alves
diff mbox

Patch

=================================================================
==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.

Regression tested on aarch64-linux.

I have a vague memory that the code here was discussed before, but I
can't find the mail in archive.

gdb:

2016-01-15  Yao Qi  <yao.qi@linaro.org>

	* breakpoint.c (build_target_command_list): Don't call continue
	if aexpr is NULL.
---
 gdb/breakpoint.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 72da4ef..9937b92 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2552,9 +2552,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