From patchwork Sun Apr 27 03:08:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 690 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 6188036007D for ; Sat, 26 Apr 2014 20:08:29 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14314964) id E1F0F62E75AB3; Sat, 26 Apr 2014 20:08:26 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id B263E62E60F94 for ; Sat, 26 Apr 2014 20:08:26 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id; q=dns; s= default; b=TRx0LuvKa3SWwAUIVhfy4qRenjkRBJquMmmiz9Gsdre7ZXoTZCUxK 9sXc+D+4zGeD3E1zVs2r7idwyZoa9ZbpVAK706nUweBSU8QXtNHFfqolJPk2p8Oy Ljo1l1hVo6ctwjz4ARMLycKpcw7pOnAX8gVmui4pz3OBtQKlmM6igg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id; s=default; bh=qvxmrILsgLXFYZPTtgboxyBF+k0=; b=FzbdCFeLCFIp9jtjrCnYeKDCk4tw NIiboODD2aUvC/HFHPNSbUw701+c+mWukWwR3rhD7R3jBhmBC8uEm//bvQtv3aXl zkUlSWYXQzUE8urHMVjR79Fqg487zFx1GtlLHadFjKo0gk6wJhJcqLSRQb5veiQy Znsn+OMs8MTwV04= Received: (qmail 29819 invoked by alias); 27 Apr 2014 03:08:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 29798 invoked by uid 89); 27 Apr 2014 03:08:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: smtp.electronicbox.net Received: from smtp.electronicbox.net (HELO smtp.electronicbox.net) (96.127.255.82) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 27 Apr 2014 03:08:15 +0000 Received: from localhost.localdomain (unknown [96.127.221.218]) by smtp.electronicbox.net (Postfix) with ESMTP id 2AF27DF37E; Sat, 26 Apr 2014 22:55:53 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH] PR mi/15806: Fix quoting of async events Date: Sat, 26 Apr 2014 23:08:11 -0400 Message-Id: <1398568091-21253-1-git-send-email-simon.marchi@ericsson.com> X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in The quoting in whatever goes in the event_channel of MI is little bit broken. Link for the lazy: https://sourceware.org/bugzilla/show_bug.cgi?id=15806 Here is an example of a =library-loaded event with an ill-named directory, /tmp/how"are\you (the problem is present with every directory on Windows since it uses backslashes as a path separator). The result will be the following: =library-loaded,id="/tmp/how"are\\you/libexpat.so.1",... The " between 'how' and 'are' should be escaped. Another bad behavior is double escaping in =breakpoint-created, for example: =breakpoint-created,bkpt={...,fullname="/tmp/how\\"are\\\\you/test.c",...} The two backslashes before 'how' should be one and the four before 'you' should be two. The reason for this is that when sending something to an MI console, escaping can take place at two different moments (the actual escaping work is always done in the printchar function): 1. When generating the content, if ui_out_field_* functions are used. Here, fields are automatically quoted with " and properly escaped. At least mi_field_string does it, not sure about mi_field_fmt, I need to investigate further. 2. When gdb_flush is called, to send the data in the buffer of the console to the actual output (stdout). At this point, mi_console_raw_packet takes the whole string in the buffer, quotes it, and escapes all occurences of the quoting character and backslashes. The event_channel does not specify a quoting character, so quotes are not escaped here, only backslashes. The problem with =library-loaded is that it does use fprintf_unfiltered, which doesn't do escaping (so, no #1). When gdb_flush is called, backslashes are escaped (#2). The problem with =breakpoint-created is that it first uses ui_out_field_* functions to generate its output, so backslashes and quotes are escaped there (#1). backslashes are escaped again in #2, leading to an overdose of backslashes. In retrospect, there is no way escaping can be done reliably in mi_console_raw_packet for data that is already formatted, such as event_channel. At this point, there is no way to differentiate quotes that delimit field values from those that should be escaped. In the case of other MI consoles, it is ok since mi_console_raw_packet receives one big string that should be quoted and escaped as a whole. So, first part of the fix: for the MI channels that specify no quoting character, no escaping at all should be done in mi_console_raw_packet (that's the change in printchar, thanks to Yuanhui Zhang for this). For those channels, whoever generates the content is responsible for proper quoting and escaping. This will fix the =breakpoint-created kind of problem. Second part of the fix is to make =library-loaded generate content that is properly escaped. For this, we use ui_out_field_* functions, instead of one big fprintf_unfiltered. =library-unloaded suffered from the same problem so it is modified as well. There might be other events that need fixing too, but that's all I found with a quick scan. Those that use fprintf_unfiltered but whose sole variable data is a %d are not critical, since it won't generate a " or a \. Finally, a test has been fixed, as it was expecting an erroneous output. Otherwise, all other tests that were previously passing still pass (x86-64 linux). gdb/ChangeLog: 2014-04-26 Simon Marchi PR mi/15806 * utils.c (printchar): Don't escape at all if quoter is NUL. * mi/mi-interp.c (mi_solib_loaded): Use ui_out_field_* functions to generate the output. (mi_solib_unloaded): Same. gdb/testsuite/ChangeLog: 2014-04-26 Simon Marchi * gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify): Fix erroneous dprintf expected input. --- gdb/mi/mi-interp.c | 57 ++++++++++++++------------ gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp | 2 +- gdb/utils.c | 2 +- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 25bf0a1..491e7f9 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -764,22 +764,24 @@ static void mi_solib_loaded (struct so_list *solib) { struct mi_interp *mi = top_level_interpreter_data (); + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); target_terminal_ours (); - if (gdbarch_has_global_solist (target_gdbarch ())) - fprintf_unfiltered (mi->event_channel, - "library-loaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",symbols-loaded=\"%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, solib->symbols_loaded); - else - fprintf_unfiltered (mi->event_channel, - "library-loaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",symbols-loaded=\"%d\"," - "thread-group=\"i%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, solib->symbols_loaded, - current_inferior ()->num); + + fprintf_unfiltered (mi->event_channel, "library-loaded"); + + ui_out_redirect (uiout, mi->event_channel); + + ui_out_field_string (uiout, "id", solib->so_original_name); + ui_out_field_string (uiout, "target-name", solib->so_original_name); + ui_out_field_string (uiout, "host-name", solib->so_name); + ui_out_field_int (uiout, "symbols-loaded", solib->symbols_loaded); + if (!gdbarch_has_global_solist (target_gdbarch ())) + { + ui_out_field_fmt (uiout, "thread-group", "i%d", current_inferior ()->num); + } + + ui_out_redirect (uiout, NULL); gdb_flush (mi->event_channel); } @@ -788,20 +790,23 @@ static void mi_solib_unloaded (struct so_list *solib) { struct mi_interp *mi = top_level_interpreter_data (); + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); target_terminal_ours (); - if (gdbarch_has_global_solist (target_gdbarch ())) - fprintf_unfiltered (mi->event_channel, - "library-unloaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\"", - solib->so_original_name, solib->so_original_name, - solib->so_name); - else - fprintf_unfiltered (mi->event_channel, - "library-unloaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",thread-group=\"i%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, current_inferior ()->num); + + fprintf_unfiltered (mi->event_channel, "library-unloaded"); + + ui_out_redirect (uiout, mi->event_channel); + + ui_out_field_string (uiout, "id", solib->so_original_name); + ui_out_field_string (uiout, "target-name", solib->so_original_name); + ui_out_field_string (uiout, "host-name", solib->so_name); + if (!gdbarch_has_global_solist (target_gdbarch ())) + { + ui_out_field_fmt (uiout, "thread-group", "i%d", current_inferior ()->num); + } + + ui_out_redirect (uiout, NULL); gdb_flush (mi->event_channel); } diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp index cb2f7f6..f023e8b 100644 --- a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp +++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp @@ -96,7 +96,7 @@ proc test_insert_delete_modify { } { $test set test "dprintf marker, \"arg\" \"" mi_gdb_test $test \ - {.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\{\"printf \\\\\"arg\\\\\" \\\\\"\"\}.*\}\r\n\^done} \ + {.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\{\"printf \\\"arg\\\" \\\"\"\}.*\}\r\n\^done} \ $test # 2. when modifying condition diff --git a/gdb/utils.c b/gdb/utils.c index a802bfa..746a272 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1515,7 +1515,7 @@ printchar (int c, void (*do_fputs) (const char *, struct ui_file *), } else { - if (c == '\\' || c == quoter) + if (quoter != 0 && (c == '\\' || c == quoter)) do_fputs ("\\", stream); do_fprintf (stream, "%c", c); }