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; } --------------------------------------
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?
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
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.
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.
*** Bug 16868 has been marked as a duplicate of this bug. ***
Created attachment 7559 [details] Ugly fix
Comment on attachment 7559 [details] 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 ?
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.
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)
(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.
@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?
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 ?
(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.
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
(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.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "gdb and binutils". The branch, master has been updated via 6ef284bd18c31645eb3ec4e7691a0f07100d6b4e (commit) from 270c9937446ca5273caf7fb102bcdba9ed7cff41 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6ef284bd18c31645eb3ec4e7691a0f07100d6b4e commit 6ef284bd18c31645eb3ec4e7691a0f07100d6b4e Author: Simon Marchi <simon.marchi@ericsson.com> Date: Mon Jun 2 17:10:36 2014 -0400 PR mi/15806: Fix quoting of async events Original patch: https://sourceware.org/ml/gdb-patches/2014-04/msg00552.html New in v2: * In remote.c:escape_buffer, pass '\\' to fputstrn_unfiltered/printchar to make sure backslashes are escaped in remote debug output. * Updated function documentation for printchar. See updated ChangeLog below. -------------------- 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-06-02 Simon Marchi <simon.marchi@ericsson.com> PR mi/15806 * utils.c (printchar): Don't escape at all if quoter is NUL. Update function documentation to clarify effect of parameter QUOTER. * remote.c (escape_buffer): Pass '\\' as the quoter to fputstrn_unfiltered. * mi/mi-interp.c (mi_solib_loaded): Use ui_out_field_* functions to generate the output. (mi_solib_unloaded): Same. gdb/testsuite/ChangeLog: 2014-06-02 Simon Marchi <simon.marchi@ericsson.com> * gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify): Fix erroneous dprintf expected input. ----------------------------------------------------------------------- Summary of changes: gdb/ChangeLog | 12 +++++ gdb/mi/mi-interp.c | 57 +++++++++++++----------- gdb/remote.c | 2 +- gdb/testsuite/ChangeLog | 5 ++ gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp | 2 +- gdb/utils.c | 10 +++- 6 files changed, 58 insertions(+), 30 deletions(-)
Fixed in 6ef284bd18c31645eb3ec4e7691a0f07100d6b4e.