[Bug mi/15806] New: file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[Bug mi/15806] New: file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[asmwarrior at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15806

            Bug ID: 15806
           Summary: file path separator becomes "\\\\" instead of "\\"
                    when GDB report breakpoint-modified in MI mode
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mi
          Assignee: unassigned at sourceware dot org
          Reporter: asmwarrior at gmail dot com

Hi, when debug a simple helloworld program under Codeblocks, I have such log
(the log is copied from Codeblocks' gdb-debugger-mi plugin's log, so there are
some log messages which is the plugin itself)

Active debugger config: GDB/MI:Default
start debugger
Selecting target: 
Debug
Adding file: E:\code\cb\test_code\helloworld\bin\Debug\helloworld.exe

[debug]PATH=.;E:\code\gcc\PCXMinGW463\bin;E:\code\gcc\PCXMinGW463;.;F:\cb_sf_git\trunk\src\lib;E:\code\cb\wx\wxWidgets-2.8.12\lib\gcc_dll;F:\cb_sf_git\trunk\src\devel;E:\code\common_bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem;D:\Program
Files\TortoiseSVN\bin

GDB path: E:\code\gcc\PCXMinGW463\bin\gdb.exe
DEBUGGEE path: E:\code\cb\test_code\helloworld\bin\Debug\helloworld.exe
Command-line: E:\code\gcc\PCXMinGW463\bin\gdb.exe -fullname  -quiet
--interpreter=mi -args E:\code\cb\test_code\helloworld\bin\Debug\helloworld.exe
Working dir : E:\code\cb\test_code\helloworld\.
Starting debugger: 

[debug]Executing command: E:\code\gcc\PCXMinGW463\bin\gdb.exe -fullname  -quiet
--interpreter=mi -args E:\code\cb\test_code\helloworld\bin\Debug\helloworld.exe

done

[debug]Executor stopped
[debug]Debugger_GDB_MI::CommitBreakpoints
[debug]ActionsMap::Run -> starting action: 0ADA2DE0 id: 1
[debug]BreakpointAddAction::m_initial_cmd = 10000000000
[debug]cmd==>10000000000-break-insert -f
E:\code\cb\test_code\helloworld\main.cpp:7
[debug]ActionsMap::Run -> starting action: 060E4E40 id: 2
[debug]cmd==>20000000000-exec-arguments 
[debug]ActionsMap::Run -> starting action: 060E5278 id: 3
[debug]cmd==>30000000000-enable-pretty-printing
[debug]ActionsMap::Run -> starting action: 0A841698 id: 4
[debug]cmd==>40000000000-interpreter-exec console "source
E:\\code\\gcc\\PCXMinGW463\\bin\\my.gdb"
[debug]output==>=thread-group-added,id="i1"
[debug]unparsable_output==>~"Reading symbols from
E:\\code\\cb\\test_code\\helloworld\\bin\\Debug\\helloworld.exe..."
[debug]notification event recieved!
[debug]unparsable_output==>~"done.\n"
[debug]unparsable_output==>(gdb)
[debug]output==>10000000000^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x004016ee",func="main()",file="E:\\code\\cb\\test_code\\helloworld\\main.cpp",fullname="E:\\code\\cb\\test_code\\helloworld\\main.cpp",line="7",thread-groups=["i1"],times="0",original-location="E:\\code\\cb\\test_code\\helloworld\\main.cpp:7"}
[debug]unparsable_output==>(gdb)
[debug]output==>20000000000^done
[debug]unparsable_output==>(gdb)
[debug]output==>30000000000^done
[debug]unparsable_output==>(gdb)
[debug]output==>=cmd-param-changed,param="filename-display",value="absolute"
[debug]output==>40000000000^done
[debug]unparsable_output==>(gdb)
[debug]BreakpointAddAction::OnCommandResult: 10000000000
[debug]BreakpointAddAction::breakpoint index is 1
[debug]BreakpointAddAction::Finishing1
[debug]notification event recieved!
[debug]BreakpointAddAction::destructor
[debug]ActionsMap::Run -> starting action: 064400A0 id: 5
[debug]RunAction::OnStart -> -exec-run
[debug]cmd==>50000000000-exec-run
[debug]output==>=thread-group-started,id="i1",pid="3080"
[debug]output==>=thread-created,id="1",group-id="i1"
[debug]unparsable_output==>~"[New Thread 3080.0x824]\n"
[debug]output==>50000000000^running
[debug]output==>*running,thread-id="all"
[debug]unparsable_output==>(gdb)
[debug]output==>=library-loaded,id="C:\\WINDOWS\\system32\\ntdll.dll",target-name="C:\\WINDOWS\\system32\\ntdll.dll",host-name="C:\\WINDOWS\\system32\\ntdll.dll",symbols-loaded="0",thread-group="i1"
[debug]output==>=library-loaded,id="C:\\WINDOWS\\system32\\kernel32.dll",target-name="C:\\WINDOWS\\system32\\kernel32.dll",host-name="C:\\WINDOWS\\system32\\kernel32.dll",symbols-loaded="0",thread-group="i1"
[debug]output==>=library-loaded,id="C:\\WINDOWS\\system32\\msvcrt.dll",target-name="C:\\WINDOWS\\system32\\msvcrt.dll",host-name="C:\\WINDOWS\\system32\\msvcrt.dll",symbols-loaded="0",thread-group="i1"
[debug]notification event recieved!

Found child pid: 3080


[debug]notification event recieved!
[debug]RunAction success, the debugger is !stopped!
[debug]RunAction::Output - type: result
class: running
results:

[debug]Executor started
[debug]notification event recieved!
[debug]notification event recieved!
[debug]notification event recieved!
[debug]notification event recieved!
[debug]output==>=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x004016ee",func="main()",file="E:\\\\code\\\\cb\\\\test_code\\\\helloworld\\\\main.cpp",fullname="E:\\\\code\\\\cb\\\\test_code\\\\helloworld\\\\main.cpp",line="7",thread-groups=["i1"],times="1",original-location="E:\\\\code\\\\cb\\\\test_code\\\\helloworld\\\\main.cpp:7"}
[debug]RunAction::destructor
[debug]output==>*stopped,reason="breakpoint-hit",disp="keep",bkptno="1",frame={addr="0x004016ee",func="main",args=[],file="E:\\code\\cb\\test_code\\helloworld\\main.cpp",fullname="E:\\code\\cb\\test_code\\helloworld\\main.cpp",line="7"},thread-id="1",stopped-threads="all"
[debug]unparsable_output==>(gdb)
[debug]notification event recieved!
[debug]notification event recieved!
[debug]Executor stopped

---------------------------------------------------------------------
Here, I simply set a breakpoint, then start running the inferior, than the bp
get hit, but I see that there is a message from GDB which I believe is wrong:

file="E:\\\\code\\\\cb\\\\test_code\\\\helloworld\\\\main.cpp"

I think it should be:

file="E:\\code\\cb\\test_code\\helloworld\\main.cpp"

---------------------------------------------------------------------
I'm using the latest GDB cvs 2013-07-30 build myself. (MinGW 4.6.3), the
testing code is below:
--------------------------------------
#include <iostream>

using namespace std;

int main()
{
    int a = 0;
    a = 1;
    cout << "Hello world!" << endl;
    return 0;
}
--------------------------------------

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[asmwarrior at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #1 from asmwarrior <asmwarrior at gmail dot com> ---
When I debug gdb under gdb, I found that in the first step that gdb translate a
single backslash to to a double backslash, and in the second step, gdb
translate this again, so we finally get a four backslash.

static void
mi_breakpoint_modified (struct breakpoint *b)
{
  struct mi_interp *mi = top_level_interpreter_data ();
  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
  volatile struct gdb_exception e;

  if (mi_suppress_notification.breakpoint)
    return;

  if (b->number <= 0)
    return;

  target_terminal_ours ();
  fprintf_unfiltered (mi->event_channel,
              "breakpoint-modified");
  /* We want the output from gdb_breakpoint_query to go to
     mi->event_channel.  One approach would be to just call
     gdb_breakpoint_query, and then use mi_out_put to send the current
     content of mi_outout into mi->event_channel.  However, that will
     break if anything is output to mi_uiout prior to calling the
     breakpoint_created notifications.  So, we use
     ui_out_redirect.  */
  ui_out_redirect (mi_uiout, mi->event_channel);
  TRY_CATCH (e, RETURN_MASK_ERROR)
    gdb_breakpoint_query (mi_uiout, b->number, NULL);
  ui_out_redirect (mi_uiout, NULL);

  gdb_flush (mi->event_channel);
}

First step happens in call gdb_breakpoint_query
Second step happens in gdb_flush function call

which the call stack:
[debug]#0  printchar (c=98, do_fputs=0x60efd2 <fputs_unfiltered>,
do_fprintf=0x60c9cb <fprintf_unfiltered>, stream=0x2940a8, quoter=0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\utils.c:1507
[debug]#1  0x0060ba96 in fputstrn_unfiltered (str=0x2f7b238
"breakpoint-modified,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"0x00401497\",func=\"main()\",file=\"E:\\\\code\\\\cb\\\\test_code\\\\debug_gdb2011-12-01\\\\main.cpp\",fullname=\"E:\\\\code\\\\cb\\\\tes"...,
n=312, quoter=0, stream=0x2940a8) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\utils.c:1585
[debug]#2  0x004573c3 in mi_console_raw_packet (data=0x2f11980, buf=0x2f7b238
"breakpoint-modified,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"0x00401497\",func=\"main()\",file=\"E:\\\\code\\\\cb\\\\test_code\\\\debug_gdb2011-12-01\\\\main.cpp\",fullname=\"E:\\\\code\\\\cb\\\\tes"...,
length_buf=312) at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-console.c:120
[debug]#3  0x0060f4b1 in mem_file_put (file=0x2f119d8, write=0x4572e6
<mi_console_raw_packet>, dest=0x2f11980) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\ui-file.c:451
[debug]#4  0x0060ef40 in ui_file_put (file=0x2f119d8, write=0x4572e6
<mi_console_raw_packet>, dest=0x2f11980) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\ui-file.c:217
[debug]#5  0x00457442 in mi_console_file_flush (file=0x2f11938) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-console.c:136
[debug]#6  0x0060eef0 in gdb_flush (file=0x2f11938) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\ui-file.c:197
[debug]#7  0x0045c986 in mi_breakpoint_modified (b=0x2f93ca8) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-interp.c:707
[debug]#8  0x00562e3c in observer_breakpoint_modified_notification_stub
(data=0x45c8a0 <mi_breakpoint_modified>, args_data=0x29df50c) at
f:\build_gdb\gdb\gdbgit\debugbuild1\gdb\observer.inc:611
[debug]#9  0x005623d0 in generic_observer_notify (subject=0x2f11fe0,
args=0x29df50c) at f:\build_gdb\gdb\gdbgit\gdb\gdb\observer.c:167
[debug]#10 0x00562eba in observer_notify_breakpoint_modified (b=0x2f93ca8) at
f:\build_gdb\gdb\gdbgit\debugbuild1\gdb\observer.inc:636
[debug]#11 0x004ad19c in bpstat_stop_status (aspace=0x2f0bce0, bp_addr=4199575,
ptid=..., ws=0x29df80c) at f:\build_gdb\gdb\gdbgit\gdb\gdb\breakpoint.c:5293
[debug]#12 0x00517e0f in handle_inferior_event (ecs=0x29df7fc) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\infrun.c:4208
[debug]#13 0x005153cc in wait_for_inferior () at
f:\build_gdb\gdb\gdbgit\gdb\gdb\infrun.c:2743
[debug]#14 0x005148cb in proceed (addr=2089816591, siggnal=GDB_SIGNAL_0,
step=0) at f:\build_gdb\gdb\gdbgit\gdb\gdb\infrun.c:2324
[debug]#15 0x0050d9f8 in run_command_1 (args=0x0, from_tty=0, tbreak_at_main=0)
at f:\build_gdb\gdb\gdbgit\gdb\gdb\infcmd.c:607
[debug]#16 0x0050da28 in run_command (args=0x0, from_tty=0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\infcmd.c:617
[debug]#17 0x0044b968 in do_cfunc (c=0x2ee7be8, args=0x0, from_tty=0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\cli\cli-decode.c:113
[debug]#18 0x0044e2ce in cmd_func (cmd=0x2ee7be8, args=0x0, from_tty=0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\cli\cli-decode.c:1888
[debug]#19 0x0060832a in execute_command (p=0x32cc11b "", from_tty=0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\top.c:478
[debug]#20 0x004612b9 in mi_execute_cli_command (cmd=0x7842a4
<__PRETTY_FUNCTION__.23215+297> "run", args_p=0, args=0x0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-main.c:2219
[debug]#21 0x0045df2b in mi_cmd_exec_run (command=0x2f9a8e0 "exec-run",
argv=0x2f16668, argc=0) at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-main.c:406
[debug]#22 0x00461173 in mi_cmd_execute (parse=0x32f8198) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-main.c:2171
[debug]#23 0x004608fd in captured_mi_execute_command (uiout=0x2f11ac0,
context=0x32f8198) at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-main.c:1922
[debug]#24 0x00460ca7 in mi_execute_command (cmd=0x32cc1d0 "-exec-run",
from_tty=1) at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-main.c:2041
[debug]#25 0x0045bf4d in mi_execute_command_wrapper (cmd=0x32cc1d0 "-exec-run")
at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-interp.c:311
[debug]#26 0x0045bf60 in mi_execute_command_input_handler (cmd=0x32cc1d0
"-exec-run") at f:\build_gdb\gdb\gdbgit\gdb\gdb\mi\mi-interp.c:319
[debug]#27 0x00532696 in gdb_readline2 (client_data=0x0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\event-top.c:712
[debug]#28 0x00531f00 in stdin_event_handler (error=0, client_data=0x0) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\event-top.c:373
[debug]#29 0x005310e2 in handle_file_event (data=...) at
f:\build_gdb\gdb\gdbgit\gdb\gdb\event-loop.c:768
[debug](More stack frames follow...)
[debug]>>>>>>cb_gdb:

When you look at the #2, you see that the string is correct, file=\"E:\\\\code,
you can see, there is two backslash after the colon. (Note, GDB show char " as
\", and show char\ as \\ )

but in printchar() function, there have extra "\" added in the code snippet:
      if (c == '\\' || c == quoter)
    do_fputs ("\\", stream);
      do_fprintf (stream, "%c", c);

So, finally, the two backslash become four backslash, that's why I see the
wrong log message.
Anyone can suggest a solution?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[asmwarrior at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #2 from asmwarrior <asmwarrior at gmail dot com> ---
Now, I can see the reason:

static void
mi_breakpoint_modified (struct breakpoint *b)
{
  struct mi_interp *mi = top_level_interpreter_data ();
  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
  volatile struct gdb_exception e;

  if (mi_suppress_notification.breakpoint)
    return;

  if (b->number <= 0)
    return;

  target_terminal_ours ();
  fprintf_unfiltered (mi->event_channel,
              "breakpoint-modified");
  /* We want the output from gdb_breakpoint_query to go to
     mi->event_channel.  One approach would be to just call
     gdb_breakpoint_query, and then use mi_out_put to send the current
     content of mi_outout into mi->event_channel.  However, that will
     break if anything is output to mi_uiout prior to calling the
     breakpoint_created notifications.  So, we use
     ui_out_redirect.  */
  ui_out_redirect (mi_uiout, mi->event_channel);
  TRY_CATCH (e, RETURN_MASK_ERROR)
    gdb_breakpoint_query (mi_uiout, b->number, NULL);
  ui_out_redirect (mi_uiout, NULL);

  gdb_flush (mi->event_channel);
}

Look at the comment, there is a redirection of the output stream.
In the first step, gdb_breakpoint_query (mi_uiout, b->number, NULL); GDB just
output the message for the first time, so one backslash becomes two backslash.
Now, after the ui_out_redirect (mi_uiout, NULL);, in gdb_flush, GDB did again,
so two backslash becomes four backslash.

My solution is: we should let the doubling feature only happens in one step.

I just look at other GDB mi messages mechanism, I see that: "gdb_flush
(mi->event_channel);" always does a "doubling backslash", so I suggest disable
this feature in "gdb_breakpoint_query (mi_uiout, b->number, NULL);".

Any ideas?

Thanks.
Yuanhui Zhang

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #3 from asmwarrior <asmwarrior at gmail dot com> ---
Some analysis. 
We have many usage of ui_file redirect in GDB source code

For example:

  ui_out_redirect (mi_uiout, mi->event_channel);
  TRY_CATCH (e, RETURN_MASK_ERROR)
    gdb_breakpoint_query (mi_uiout, b->number, NULL);
  ui_out_redirect (mi_uiout, NULL);

  gdb_flush (mi->event_channel);

The contents(buffer) has two chance of running the function: printchar(). When
printchar see a \, it will replace it with another \, so we get doubled
backslash.

Then, in gdb_flush function, we get another doubled backslash. (the printchar()
function also called in gdb_flush), my idea is that: is it possible to avoid
calling printchar() in the gdb_flush function?), it looks like the
mi_event_channel already have backslash doubled.

Another case is that if a new line character first get chance to becomes \n,
then if printchar() called again, it will becomes \\n.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] file path separator becomes "\\\\" instead of "\\" when GDB report breakpoint-modified in MI mode

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #4 from asmwarrior <asmwarrior at gmail dot com> ---
I did some test by disable the double slash generation in the function call of
gdb_flush (mi->event_channel); Now, the output has changed from:

=breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",a
ddr="0x0040162c",func="main()",file="E:\\\\code\\\\cb\\\\test_code\\\\debug_gdb2
011-12-01\\\\main.cpp",fullname="E:\\\\code\\\\cb\\\\test_code\\\\debug_gdb2011-
12-01\\\\main.cpp",line="26",thread-groups=["i1"],times="0",original-location="m
ain()"}

to

=breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",a
ddr="0x0040162c",func="main()",file="E:\\code\\cb\\test_code\\debug_gdb2011-12-0
1\\main.cpp",fullname="E:\\code\\cb\\test_code\\debug_gdb2011-12-01\\main.cpp",l
ine="26",thread-groups=["i1"],times="0",original-location="main()"}

Looks good, but the side effect is that the other message has changed from:


=library-loaded,id="D:\\mingw-builds\\473\\mingw32\\bin\\libwinpthread-1.dll",ta
rget-name="D:\\mingw-builds\\473\\mingw32\\bin\\libwinpthread-1.dll",host-name="
D:\\mingw-builds\\473\\mingw32\\bin\\libwinpthread-1.dll",symbols-loaded="0",thr
ead-group="i1"

to

=library-loaded,id="D:\mingw-builds\473\mingw32\bin\libwinpthread-1.dll",target-
name="D:\mingw-builds\473\mingw32\bin\libwinpthread-1.dll",host-name="D:\mingw-b
uilds\473\mingw32\bin\libwinpthread-1.dll",symbols-loaded="0",thread-group="i1"

This is bad, because I think the former case in the library-loaded message is
correct.

By looking at the source code:

static void
mi_solib_loaded (struct so_list *solib)
{
  struct mi_interp *mi = top_level_interpreter_data ();

  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);

  gdb_flush (mi->event_channel);
}

I see that fprintf_unfltered just output something like
"D:\mingw-builds\473..." string to the the mi_event_channel, so if double
backslash is disabled in gdb_flush(), we get the result containing
"D:\mingw-builds\473...".

So, I don't have an idea to fix this bug.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #5 from Simon Marchi <simon.marchi at ericsson dot com> ---
*** Bug 16868 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

Simon Marchi <simon.marchi at ericsson dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at ericsson dot com
            Summary|file path separator becomes |Some fields in async MI
                   |"\\\\" instead of "\\" when |events get escaped twice
                   |GDB report                  |
                   |breakpoint-modified in MI   |
                   |mode                        |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #6 from Simon Marchi <simon.marchi at ericsson dot com> ---
Created attachment 7559
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7559&action=edit
Ugly fix

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #7 from Simon Marchi <simon.marchi at ericsson dot com> ---
Comment on attachment 7559
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7559
Ugly fix

As a first step, I made this patch which adds a global variable to disable
temporarily the escaping. It is a very ugly hack, but I think it gives the
behavior we want. Could you test it ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #8 from asmwarrior <asmwarrior at gmail dot com> ---
Hi, Simon, thanks, your patch does not solve my original issue, you patch only
fix an issue in the function: 
static void
mi_breakpoint_created (struct breakpoint *b)

But my problem happens in another function:
static void
mi_breakpoint_modified (struct breakpoint *b)

There are many cases we should fix. (Search the usage of ui_out_redirect
function call, this issue happens in stream re-direction). As I said before,
when a stream get redirected, it has a chance to double the backslash, but
finally gdb_flush (mi->event_channel) function call create another new
backslash.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #9 from asmwarrior <asmwarrior at gmail dot com> ---
Hi, when look at this function:
/* Print the character C on STREAM as part of the contents of a literal
   string whose delimiter is QUOTER.  Note that this routine should only
   be call for printing things which are independent of the language
   of the program being debugged.  */

static void
printchar (int c, void (*do_fputs) (const char *, struct ui_file *),
       void (*do_fprintf) (struct ui_file *, const char *, ...)
       ATTRIBUTE_FPTR_PRINTF_2, struct ui_file *stream, int quoter)
{
  c &= 0xFF;            /* Avoid sign bit follies */

  if (c < 0x20 ||        /* Low control chars */
      (c >= 0x7F && c < 0xA0) ||    /* DEL, High controls */
      (sevenbit_strings && c >= 0x80))
    {                /* high order bit set */
      switch (c)
    {
    case '\n':
      do_fputs ("\\n", stream);
      break;
    case '\b':
      do_fputs ("\\b", stream);
      break;
    case '\t':
      do_fputs ("\\t", stream);
      break;
    case '\f':
      do_fputs ("\\f", stream);
      break;
    case '\r':
      do_fputs ("\\r", stream);
      break;
    case '\033':
      do_fputs ("\\e", stream);
      break;
    case '\007':
      do_fputs ("\\a", stream);
      break;
    default:
      do_fprintf (stream, "\\%.3o", (unsigned int) c);
      break;
    }
    }
  else
    {
      if (c == '\\' || c == quoter)
    do_fputs ("\\", stream);
      do_fprintf (stream, "%c", c);
    }
}

I think we can fix our issue by look at this condition:
1, when   mi->event_channel = mi_console_file_new (raw_stdout, "=", 0); is
created, look at the third argument, it is 0, which means the quoter is 0

2, look at my comment 1, I have a call stack shown, where when gdb_flush
(mi->event_channel); is called, the deeper printchar will have the last
argument quoter = 0.

3, when read the document, I see that "Print the character C on STREAM as part
of the contents of a literal string whose delimiter is QUOTER", this means
usually the QUOTER is " (for c string) or ' (for c char), but what about quoter
== 0?

4, I think if quoter==0, we should not double the backslash.

5, So, we can change the if condition here is like:
      if (c == '\\' ||  (c == quoter && quoter != 0))
    do_fputs ("\\", stream);

6, I initial guess is that if the quoter == 0, we even don't need to escape the
\f \r \n (which is the first part of the printchar() function), but I may be
wrong.


Yuanhui Zhang(asmwarrior)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #10 from asmwarrior <asmwarrior at gmail dot com> ---
(In reply to asmwarrior from comment #9)
> 
> 5, So, we can change the if condition here is like:
>       if (c == '\\' ||  (c == quoter && quoter != 0))
> 	do_fputs ("\\", stream);
> 

Oh, there is a mistake in the above. The correct patch is:
 gdb/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index a8a7cb3..31bdb7e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1539,7 +1539,7 @@ printchar (int c, void (*do_fputs) (const char *, struct
ui_file *),
     }
   else
     {
-      if (c == '\\' || c == quoter)
+      if ( (c == '\\' && quoter != 0) || c == quoter)
     do_fputs ("\\", stream);
       do_fprintf (stream, "%c", c);
     }


But the result is still bad, which lead to some unexpected issue in my comment
4.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
>From gdb-prs-return-15617-listarch-gdb-prs=sources.redhat.com@sourceware.org Fri Apr 25 03:35:21 2014
Return-Path: <gdb-prs-return-15617-listarch-gdb-prs=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-prs@sources.redhat.com
Received: (qmail 23898 invoked by alias); 25 Apr 2014 03:35:20 -0000
Mailing-List: contact gdb-prs-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-prs.sourceware.org>
List-Subscribe: <mailto:gdb-prs-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-prs/>
List-Post: <mailto:gdb-prs@sourceware.org>
List-Help: <mailto:gdb-prs-help@sourceware.org>, <http://sourceware.org/lists.html#faqs>
Sender: gdb-prs-owner@sourceware.org
Delivered-To: mailing list gdb-prs@sourceware.org
Received: (qmail 23864 invoked by uid 48); 25 Apr 2014 03:35:19 -0000
From: "asmwarrior at gmail dot com" <sourceware-bugzilla@sourceware.org>
To: gdb-prs@sourceware.org
Subject: [Bug mi/15806] Some fields in async MI events get escaped twice
Date: Fri, 25 Apr 2014 03:35:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gdb
X-Bugzilla-Component: mi
X-Bugzilla-Version: HEAD
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: asmwarrior at gmail dot com
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P2
X-Bugzilla-Assigned-To: unassigned at sourceware dot org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-15806-4717-86mt7GWtnw@http.sourceware.org/bugzilla/>
In-Reply-To: <bug-15806-4717@http.sourceware.org/bugzilla/>
References: <bug-15806-4717@http.sourceware.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://sourceware.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-q2/txt/msg00123.txt.bz2
Content-length: 1520

https://sourceware.org/bugzilla/show_bug.cgi?id\x15806

--- Comment #11 from asmwarrior <asmwarrior at gmail dot com> ---
@Simon, I think the patch in comment 10 is a good fix. Look at the issue
statement in my comment 4, I think the code below
static void
mi_solib_loaded (struct so_list *solib)
{
  struct mi_interp *mi = top_level_interpreter_data ();

  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);

  gdb_flush (mi->event_channel);
}

should fix itself, not the gdb_flush (mi->event_channel). I mean, since
mi->event_channel is created with quoter = 0, which means when user put some
text in the mi->event_channel, they should do their escape handling themselves.
Once gdb_flush (mi->event_channel) is called, it should not add more
backslashes(escape handling).

What do you think?

--
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #12 from Simon Marchi <simon.marchi at ericsson dot com> ---
Oops, yeah the fix was only for breakpoint-created, sorry about that.

Your comment made me realize that if a library path contains a quote ("), the
result will be wrong. Example:

=library-loaded,id="/tmp/hello"you/libpopt.so.0",target-name="/tmp/hello"you/libpopt.so.0",host-name="/tmp/hello"you/libpopt.so.0",symbols-loaded="0",thread-group="i1"

To reproduce this, I created '/tmp/hello"you', copied some library in it and
modified LD_LIBRARY_PATH so that my test program use this library instead of
the system's version. You can see that the " between hello and you is not
escaped, but should be.

I think that no escaping can be done reliably in gdb_flush. There is no way to
differentiate a quote that should not be escaped from one that should be
escaped. Therefore, I think it should always be done at the moment where the
content is created.

Also, using fprintf_unfiltered to output this is wrong, because the fields are
never escaped. It would probably be better to use the ui_out_field_* functions
everywhere. What do you think ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #13 from asmwarrior <asmwarrior at gmail dot com> ---
(In reply to Simon Marchi from comment #12)
> Oops, yeah the fix was only for breakpoint-created, sorry about that.
Never mind.

> 
> Your comment made me realize that if a library path contains a quote ("),
> the result will be wrong. Example:
> 
> =library-loaded,id="/tmp/hello"you/libpopt.so.0",target-name="/tmp/hello"you/
> libpopt.so.0",host-name="/tmp/hello"you/libpopt.so.0",symbols-loaded="0",
> thread-group="i1"
> 
> To reproduce this, I created '/tmp/hello"you', copied some library in it and
> modified LD_LIBRARY_PATH so that my test program use this library instead of
> the system's version. You can see that the " between hello and you is not
> escaped, but should be.
Yes, look at the function body of mi_solib_loaded (struct so_list *solib). It
firstly use fprintf_unfiltered, which directly copy the contents to the stream
(such as solib->so_original_name, solib->so_original_name). So, it failed to
escape some special chars, such as the " in your file path.

> 
> I think that no escaping can be done reliably in gdb_flush. 
What does this sentence means？ You mean gdb_flush should only copy the contents
to the UI, it should not do escaping, right?


> There is no way
> to differentiate a quote that should not be escaped from one that should be
> escaped. Therefore, I think it should always be done at the moment where the
> content is created.

Correct, gdb_flush know nothing about whether the buffered stream need to be
escaped or not. (unless we write a parser to parse the whole stream, basically,
the stream has some format like xxxx="yyyyyy", the parser know whether it is
inside a string literal or not, but this way(direction) is surely wrong)

> 
> Also, using fprintf_unfiltered to output this is wrong, because the fields
> are never escaped. It would probably be better to use the ui_out_field_*
> functions everywhere. What do you think ?
Yes, I agree with you, the only way to do escaping correctly is call some
ui_out_field_* functions. But this need a lot of code change in GDB.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
>From gdb-prs-return-15624-listarch-gdb-prs=sources.redhat.com@sourceware.org Fri Apr 25 16:59:04 2014
Return-Path: <gdb-prs-return-15624-listarch-gdb-prs=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-prs@sources.redhat.com
Received: (qmail 4382 invoked by alias); 25 Apr 2014 16:59:04 -0000
Mailing-List: contact gdb-prs-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-prs.sourceware.org>
List-Subscribe: <mailto:gdb-prs-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-prs/>
List-Post: <mailto:gdb-prs@sourceware.org>
List-Help: <mailto:gdb-prs-help@sourceware.org>, <http://sourceware.org/lists.html#faqs>
Sender: gdb-prs-owner@sourceware.org
Delivered-To: mailing list gdb-prs@sourceware.org
Received: (qmail 4349 invoked by uid 48); 25 Apr 2014 16:59:03 -0000
From: "palves at redhat dot com" <sourceware-bugzilla@sourceware.org>
To: gdb-prs@sourceware.org
Subject: [Bug server/16255] gdbserver cannot attach to a second inferior that
 is multi-threaded
Date: Fri, 25 Apr 2014 16:59:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gdb
X-Bugzilla-Component: server
X-Bugzilla-Version: 7.4
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: palves at redhat dot com
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P2
X-Bugzilla-Assigned-To: unassigned at sourceware dot org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: cc
Message-ID: <bug-16255-4717-e54y0cKm08@http.sourceware.org/bugzilla/>
In-Reply-To: <bug-16255-4717@http.sourceware.org/bugzilla/>
References: <bug-16255-4717@http.sourceware.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://sourceware.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-q2/txt/msg00130.txt.bz2
Content-length: 513

https://sourceware.org/bugzilla/show_bug.cgi?id\x16255

Pedro Alves <palves at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |palves at redhat dot com

--- Comment #2 from Pedro Alves <palves at redhat dot com> ---
https://sourceware.org/ml/gdb-patches/2014-04/msg00435.html

--
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

Simon Marchi <simon.marchi at ericsson dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7559|0                           |1
        is obsolete|                            |

--- Comment #14 from Simon Marchi <simon.marchi at ericsson dot com> ---
Created attachment 7563
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7563&action=edit
Less ugly patch

Hi Yuanhui,

On top of your fix, I changed mi_solib_loaded to use ui_out_field_* functions,
so that the content gets properly escaped. The patch is attached.

So now, both =breakpoint-modified and =library-loaded should be fine at the
same time. Could you confirm that? If it is ok, we could identify and correct
other functions where fprintf_unfiltered is used directly.

Simon

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[marc.khouzam at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

Marc Khouzam <marc.khouzam at ericsson dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.khouzam at ericsson dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[asmwarrior at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

--- Comment #15 from asmwarrior <asmwarrior at gmail dot com> ---
(In reply to Simon Marchi from comment #14)
> Created attachment 7563 [details]
> Less ugly patch
> 
> Hi Yuanhui,
> 
> On top of your fix, I changed mi_solib_loaded to use ui_out_field_*
> functions, so that the content gets properly escaped. The patch is attached.
> 
> So now, both =breakpoint-modified and =library-loaded should be fine at the
> same time. Could you confirm that? If it is ok, we could identify and
> correct other functions where fprintf_unfiltered is used directly.
> 
> Simon

Hi, Simon, great work, I just tested this patch, and it works fine under
Windows. (the issue in comment 4 is solved) I also prefer using ui_out_field_*
functions.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug mi/15806] Some fields in async MI events get escaped twice

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=15806

Simon Marchi <simon.marchi at ericsson dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #17 from Simon Marchi <simon.marchi at ericsson dot com> ---
Fixed in 6ef284bd18c31645eb3ec4e7691a0f07100d6b4e.

-- 
You are receiving this mail because:
You are on the CC list for the bug.