diff mbox

Fix GDB crash in dprintf.exp

Message ID 86zivppysh.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Jan. 28, 2016, 2:24 p.m. UTC 
  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.
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.  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