From d2828a38f3a596618d70b07d23bffda1648fbdea Mon Sep 17 00:00:00 2001 From: fnasser Date: Wed, 29 Nov 2000 00:27:46 +0000 Subject: [PATCH] 2000-10-23 Fernando Nasser From 2000-10-19 Steven Johnson Note: The original patch used asprintf/vasprintf. I changed it to use the new xasprintf/xvasprintf gdb functions. * gdbtk.c (TclDebug, gdbtk_init): Replaced the vast majority of sprintf/vsprintf calls with asprintf and vasprintf respectively. Should prevent any possible buffer overruns possible with fixed size sprintf buffers. Specifically fixes a problem with long filenames and clearing breakpoints overflowing their buffers when using sprintf, causing a segfault. Generically should also prevent any other similar problems from occuring. * gdbtk-cmds.c (sprintf_append_element_to_obj, get_pc_register, gdb_get_tracepoint_info, gdb_load_disassembly, gdbtk_load_source, gdbtk_load_asm, gdb_set_bp, gdb_set_bp_addr, gdb_get_breakpoint_info, gdb_selected_frame, gdb_selected_block, gdb_get_blocks): Ditto. * gdbtk-hooks.c (gdbtk_warning, gdbtk_ignorable_warning, gdbtk_readline_begin, gdbtk_set_hook, breakpoint_notify, gdbtk_query, tracepoint_notify, gdbtk_error_begin, gdbtk_annotate_signal): Ditto. --- gdb/gdbtk/generic/ChangeLog | 21 ++++++++ gdb/gdbtk/generic/gdbtk-cmds.c | 107 ++++++++++++++++++++++++---------------- gdb/gdbtk/generic/gdbtk-hooks.c | 55 +++++++++++++-------- gdb/gdbtk/generic/gdbtk.c | 16 +++--- 4 files changed, 131 insertions(+), 68 deletions(-) diff --git a/gdb/gdbtk/generic/ChangeLog b/gdb/gdbtk/generic/ChangeLog index 4a8c538a62..d3c48a1966 100644 --- a/gdb/gdbtk/generic/ChangeLog +++ b/gdb/gdbtk/generic/ChangeLog @@ -1,3 +1,24 @@ +2000-10-23 Fernando Nasser + + From 2000-10-19 Steven Johnson + Note: The original patch used asprintf/vasprintf. I changed it to + use the new xasprintf/xvasprintf gdb functions. + * gdbtk.c (TclDebug, gdbtk_init): Replaced the vast majority of + sprintf/vsprintf calls with asprintf and vasprintf respectively. + Should prevent any possible buffer overruns possible with + fixed size sprintf buffers. Specifically fixes a problem with long + filenames and clearing breakpoints overflowing their buffers when + using sprintf, causing a segfault. Generically should also prevent + any other similar problems from occuring. + * gdbtk-cmds.c (sprintf_append_element_to_obj, get_pc_register, + gdb_get_tracepoint_info, gdb_load_disassembly, gdbtk_load_source, + gdbtk_load_asm, gdb_set_bp, gdb_set_bp_addr, gdb_get_breakpoint_info, + gdb_selected_frame, gdb_selected_block, gdb_get_blocks): Ditto. + * gdbtk-hooks.c (gdbtk_warning, gdbtk_ignorable_warning, + gdbtk_readline_begin, gdbtk_set_hook, breakpoint_notify, + gdbtk_query, tracepoint_notify, gdbtk_error_begin, + gdbtk_annotate_signal): Ditto. + 2000-10-23 Fernando Nasser * gdbtk-hooks.c (x_event): Only process events if the target is diff --git a/gdb/gdbtk/generic/gdbtk-cmds.c b/gdb/gdbtk/generic/gdbtk-cmds.c index f11f631d0d..c1535d4991 100644 --- a/gdb/gdbtk/generic/gdbtk-cmds.c +++ b/gdb/gdbtk/generic/gdbtk-cmds.c @@ -545,13 +545,14 @@ static void sprintf_append_element_to_obj (Tcl_Obj * objp, char *format,...) { va_list args; - char buf[1024]; + char *buf; va_start (args, format); - vsprintf (buf, format, args); + xvasprintf (&buf, format, args); Tcl_ListObjAppendElement (NULL, objp, Tcl_NewStringObj (buf, -1)); + free(buf); } /* @@ -2008,10 +2009,11 @@ get_pc_register (clientData, interp, objc, objv) int objc; Tcl_Obj *CONST objv[]; { - char buff[64]; + char *buff; - sprintf (buff, "0x%llx", (long long) read_register (PC_REGNUM)); + xasprintf (&buff, "0x%llx", (long long) read_register (PC_REGNUM)); Tcl_SetStringObj (result_ptr->obj_ptr, buff, -1); + free(buff); return TCL_OK; } @@ -2180,9 +2182,10 @@ gdb_get_tracepoint_info (clientData, interp, objc, objv) if (tp == NULL) { - char buff[64]; - sprintf (buff, "Tracepoint #%d does not exist", tpnum); + char *buff; + xasprintf (&buff, "Tracepoint #%d does not exist", tpnum); Tcl_SetStringObj (result_ptr->obj_ptr, buff, -1); + free(buff); return TCL_ERROR; } @@ -2202,7 +2205,7 @@ gdb_get_tracepoint_info (clientData, interp, objc, objv) Tcl_NewIntObj (sal.line)); { char *tmp; - asprintf (&tmp, "0x%s", paddr_nz (tp->address)); + xasprintf (&tmp, "0x%s", paddr_nz (tp->address)); Tcl_ListObjAppendElement (interp, result_ptr->obj_ptr, Tcl_NewStringObj (tmp, -1)); free (tmp); @@ -2597,14 +2600,16 @@ gdb_load_disassembly (clientData, interp, objc, objv) into the Tcl result. */ if (ret_val == TCL_OK) { - char buffer[256]; + char *buffer; Tcl_Obj *limits_obj[2]; - sprintf (buffer, "0x%s", paddr_nz (low)); + xasprintf (&buffer, "0x%s", paddr_nz (low)); limits_obj[0] = Tcl_NewStringObj (buffer, -1); + free(buffer); - sprintf (buffer, "0x%s", paddr_nz (high)); + xasprintf (&buffer, "0x%s", paddr_nz (high)); limits_obj[1] = Tcl_NewStringObj (buffer, -1); + free(buffer); Tcl_DecrRefCount (result_ptr->obj_ptr); result_ptr->obj_ptr = Tcl_NewListObj (2, limits_obj); @@ -2620,7 +2625,7 @@ gdbtk_load_source (ClientData clientData, struct symtab *symtab, int { struct disassembly_client_data *client_data = (struct disassembly_client_data *) clientData; - char buffer[18]; + char *buffer; int index_len; index_len = Tcl_DStringLength (&client_data->src_to_line_prefix); @@ -2695,11 +2700,12 @@ gdbtk_load_source (ClientData clientData, struct symtab *symtab, int /* FIXME: Convert to Tcl_SetVar2Ex when we move to 8.2. This will allow us avoid converting widget_line_no into a string. */ - sprintf (buffer, "%d", client_data->widget_line_no); + xasprintf (&buffer, "%d", client_data->widget_line_no); Tcl_SetVar2 (client_data->interp, client_data->map_arr, Tcl_DStringValue (&client_data->src_to_line_prefix), buffer, 0); + free(buffer); Tcl_DStringSetLength (&client_data->src_to_line_prefix, index_len); } @@ -2801,7 +2807,7 @@ gdbtk_load_asm (clientData, pc, di) if (*client_data->map_arr != '\0') { - char buffer[16]; + char *buffer; /* Run the command, then add an entry to the map array in the caller's scope. */ @@ -2811,7 +2817,7 @@ gdbtk_load_asm (clientData, pc, di) /* FIXME: Convert to Tcl_SetVar2Ex when we move to 8.2. This will allow us avoid converting widget_line_no into a string. */ - sprintf (buffer, "%d", client_data->widget_line_no); + xasprintf (&buffer, "%d", client_data->widget_line_no); Tcl_SetVar2 (client_data->interp, client_data->map_arr, Tcl_DStringValue (&client_data->pc_to_line_prefix), @@ -2828,6 +2834,7 @@ gdbtk_load_asm (clientData, pc, di) Tcl_DStringSetLength (&client_data->pc_to_line_prefix, pc_to_line_len); Tcl_DStringSetLength (&client_data->line_to_pc_prefix, line_to_pc_len); + free(buffer); } do_cleanups (old_chain); @@ -3684,7 +3691,7 @@ gdb_set_bp (clientData, interp, objc, objv) struct symtab_and_line sal; int line, ret, thread = -1; struct breakpoint *b; - char buf[64], *typestr; + char *buf, *typestr; Tcl_DString cmd; enum bpdisp disp; @@ -3744,27 +3751,31 @@ gdb_set_bp (clientData, interp, objc, objv) b->thread = thread; /* FIXME: this won't work for duplicate basenames! */ - sprintf (buf, "%s:%d", basename (Tcl_GetStringFromObj (objv[1], NULL)), + xasprintf (&buf, "%s:%d", basename (Tcl_GetStringFromObj (objv[1], NULL)), line); b->addr_string = strsave (buf); + free(buf); /* now send notification command back to GUI */ Tcl_DStringInit (&cmd); Tcl_DStringAppend (&cmd, "gdbtk_tcl_breakpoint create ", -1); - sprintf (buf, "%d", b->number); + xasprintf (&buf, "%d", b->number); Tcl_DStringAppendElement (&cmd, buf); - sprintf (buf, "0x%lx", (long) sal.pc); + free(buf); + xasprintf (&buf, "0x%lx", (long) sal.pc); Tcl_DStringAppendElement (&cmd, buf); Tcl_DStringAppendElement (&cmd, Tcl_GetStringFromObj (objv[2], NULL)); Tcl_DStringAppendElement (&cmd, Tcl_GetStringFromObj (objv[1], NULL)); Tcl_DStringAppendElement (&cmd, bpdisp[b->disposition]); - sprintf (buf, "%d", b->enable); + free(buf); + xasprintf (&buf, "%d", b->enable); Tcl_DStringAppendElement (&cmd, buf); - sprintf (buf, "%d", b->thread); + free(buf); + xasprintf (&buf, "%d", b->thread); Tcl_DStringAppendElement (&cmd, buf); - + free(buf); ret = Tcl_Eval (interp, Tcl_DStringValue (&cmd)); Tcl_DStringFree (&cmd); @@ -3796,7 +3807,7 @@ gdb_set_bp_addr (clientData, interp, objc, objv) int line, ret, thread = -1; long addr; struct breakpoint *b; - char *filename, *typestr, buf[64]; + char *filename, *typestr, *buf; Tcl_DString cmd; enum bpdisp disp; @@ -3848,7 +3859,7 @@ gdb_set_bp_addr (clientData, interp, objc, objv) b->disposition = disp; b->thread = thread; - sprintf (buf, "*(0x%lx)", addr); + xasprintf (&buf, "*(0x%lx)", addr); b->addr_string = strsave (buf); /* now send notification command back to GUI */ @@ -3856,11 +3867,14 @@ gdb_set_bp_addr (clientData, interp, objc, objv) Tcl_DStringInit (&cmd); Tcl_DStringAppend (&cmd, "gdbtk_tcl_breakpoint create ", -1); - sprintf (buf, "%d", b->number); + free(buf); + xasprintf (&buf, "%d", b->number); Tcl_DStringAppendElement (&cmd, buf); - sprintf (buf, "0x%lx", addr); + free(buf); + xasprintf (&buf, "0x%lx", addr); Tcl_DStringAppendElement (&cmd, buf); - sprintf (buf, "%d", b->line_number); + free(buf); + xasprintf (&buf, "%d", b->line_number); Tcl_DStringAppendElement (&cmd, buf); filename = symtab_to_filename (sal.symtab); @@ -3868,13 +3882,16 @@ gdb_set_bp_addr (clientData, interp, objc, objv) filename = ""; Tcl_DStringAppendElement (&cmd, filename); Tcl_DStringAppendElement (&cmd, bpdisp[b->disposition]); - sprintf (buf, "%d", b->enable); + free(buf); + xasprintf (&buf, "%d", b->enable); Tcl_DStringAppendElement (&cmd, buf); - sprintf (buf, "%d", b->thread); + free(buf); + xasprintf (&buf, "%d", b->thread); Tcl_DStringAppendElement (&cmd, buf); ret = Tcl_Eval (interp, Tcl_DStringValue (&cmd)); Tcl_DStringFree (&cmd); + free(buf); return ret; } @@ -4013,9 +4030,10 @@ gdb_get_breakpoint_info (clientData, interp, objc, objv) if (!b || b->type != bp_breakpoint) { - char err_buf[64]; - sprintf (err_buf, "Breakpoint #%d does not exist.", bpnum); + char *err_buf; + xasprintf (&err_buf, "Breakpoint #%d does not exist.", bpnum); Tcl_SetStringObj (result_ptr->obj_ptr, err_buf, -1); + free(err_buf); return TCL_ERROR; } @@ -4308,15 +4326,16 @@ gdb_selected_frame (clientData, interp, objc, objv) int objc; Tcl_Obj *CONST objv[]; { - char frame[32]; + char *frame; if (selected_frame == NULL) - strcpy (frame, ""); + xasprintf (&frame, "%s",""); else - sprintf (frame, "0x%s", paddr_nz (FRAME_FP (selected_frame))); + xasprintf (&frame, "0x%s", paddr_nz (FRAME_FP (selected_frame))); Tcl_SetStringObj (result_ptr->obj_ptr, frame, -1); + free(frame); return TCL_OK; } @@ -4338,20 +4357,20 @@ gdb_selected_block (clientData, interp, objc, objv) int objc; Tcl_Obj *CONST objv[]; { - char start[32]; - char end[32]; + char *start = NULL; + char *end = NULL; if (selected_frame == NULL) { - strcpy (start, ""); - strcpy (end, ""); + xasprintf (&start, "%s", ""); + xasprintf (&end, "%s", ""); } else { struct block *block; block = get_frame_block (selected_frame); - sprintf (start, "0x%s", paddr_nz (BLOCK_START (block))); - sprintf (end, "0x%s", paddr_nz (BLOCK_END (block))); + xasprintf (&start, "0x%s", paddr_nz (BLOCK_START (block))); + xasprintf (&end, "0x%s", paddr_nz (BLOCK_END (block))); } Tcl_SetListObj (result_ptr->obj_ptr, 0, NULL); @@ -4360,6 +4379,8 @@ gdb_selected_block (clientData, interp, objc, objv) Tcl_ListObjAppendElement (interp, result_ptr->obj_ptr, Tcl_NewStringObj (end, -1)); + free(start); + free(end); return TCL_OK; } @@ -4435,16 +4456,18 @@ gdb_get_blocks (clientData, interp, objc, objv) if (!junk && pc < BLOCK_END (block)) { - char addr[32]; + char *addr; Tcl_Obj *elt = Tcl_NewListObj (0, NULL); - sprintf (addr, "0x%s", paddr_nz (BLOCK_START (block))); + xasprintf (&addr, "0x%s", paddr_nz (BLOCK_START (block))); Tcl_ListObjAppendElement (interp, elt, Tcl_NewStringObj (addr, -1)); - sprintf (addr, "0x%s", paddr_nz (BLOCK_END (block))); + free(addr); + xasprintf (&addr, "0x%s", paddr_nz (BLOCK_END (block))); Tcl_ListObjAppendElement (interp, elt, Tcl_NewStringObj (addr, -1)); Tcl_ListObjAppendElement (interp, result_ptr->obj_ptr, elt); + free(addr); } if (BLOCK_FUNCTION (block)) diff --git a/gdb/gdbtk/generic/gdbtk-hooks.c b/gdb/gdbtk/generic/gdbtk-hooks.c index 98fd8a8c9d..7de761790f 100644 --- a/gdb/gdbtk/generic/gdbtk-hooks.c +++ b/gdb/gdbtk/generic/gdbtk-hooks.c @@ -292,10 +292,12 @@ gdbtk_warning (warning, args) const char *warning; va_list args; { - char buf[200]; + char *buf; - vsprintf (buf, warning, args); + xvasprintf (&buf, warning, args); gdbtk_two_elem_cmd ("gdbtk_tcl_warning", buf); + + free(buf); } @@ -325,10 +327,11 @@ gdbtk_ignorable_warning (class, warning) const char *class; const char *warning; { - char buf[512]; - sprintf (buf, "gdbtk_tcl_ignorable_warning {%s} {%s}", class, warning); + char *buf; + xasprintf (&buf, "gdbtk_tcl_ignorable_warning {%s} {%s}", class, warning); if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK) report_error (); + free(buf); } static void @@ -469,11 +472,12 @@ static void gdbtk_readline_begin (char *format,...) { va_list args; - char buf[200]; + char *buf; va_start (args, format); - vsprintf (buf, format, args); + xvasprintf (&buf, format, args); gdbtk_two_elem_cmd ("gdbtk_tcl_readline_begin", buf); + free(buf); } static char * @@ -538,7 +542,7 @@ gdbtk_set_hook (struct cmd_list_element *cmdblk) { Tcl_DString cmd; char *p; - char buffer[30]; + char *buffer = NULL; Tcl_DStringInit (&cmd); Tcl_DStringAppendElement (&cmd, "run_hooks"); @@ -550,7 +554,7 @@ gdbtk_set_hook (struct cmd_list_element *cmdblk) while (p && *p) { char *q = strchr (p, ' '); - char save; + char save = '\0'; if (q) { save = *q; @@ -581,12 +585,12 @@ gdbtk_set_hook (struct cmd_list_element *cmdblk) case var_uinteger: case var_zinteger: - sprintf (buffer, "%u", *(unsigned int *) cmdblk->var); + xasprintf (&buffer, "%u", *(unsigned int *) cmdblk->var); Tcl_DStringAppendElement (&cmd, buffer); break; case var_integer: - sprintf (buffer, "%d", *(int *) cmdblk->var); + xasprintf (&buffer, "%d", *(int *) cmdblk->var); Tcl_DStringAppendElement (&cmd, buffer); break; @@ -600,6 +604,11 @@ gdbtk_set_hook (struct cmd_list_element *cmdblk) report_error (); Tcl_DStringFree (&cmd); + + if (buffer != NULL) + { + free(buffer); + } } /* The next three functions use breakpoint_notify to allow the GUI @@ -640,7 +649,7 @@ breakpoint_notify (b, action) struct breakpoint *b; const char *action; { - char buf[256]; + char *buf; int v; struct symtab_and_line sal; char *filename; @@ -655,21 +664,24 @@ breakpoint_notify (b, action) if (filename == NULL) filename = ""; - sprintf (buf, "gdbtk_tcl_breakpoint %s %d 0x%lx %d {%s} {%s} %d %d", + xasprintf (&buf, "gdbtk_tcl_breakpoint %s %d 0x%lx %d {%s} {%s} %d %d", action, b->number, (long) b->address, b->line_number, filename, bpdisp[b->disposition], b->enable, b->thread); if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK) report_error (); + free(buf); } int gdbtk_load_hash (const char *section, unsigned long num) { - char buf[128]; - sprintf (buf, "Download::download_hash %s %ld", section, num); + char *buf; + xasprintf (&buf, "Download::download_hash %s %ld", section, num); if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK) report_error (); + free(buf); + return atoi (gdbtk_interp->result); } @@ -721,11 +733,12 @@ gdbtk_query (query, args) const char *query; va_list args; { - char buf[200]; + char *buf; long val; - vsprintf (buf, query, args); + xvasprintf (&buf, query, args); gdbtk_two_elem_cmd ("gdbtk_tcl_query", buf); + free(buf); val = atol (gdbtk_interp->result); return val; @@ -769,7 +782,7 @@ tracepoint_notify (tp, action) struct tracepoint *tp; const char *action; { - char buf[256]; + char *buf; int v; struct symtab_and_line sal; char *filename; @@ -781,11 +794,12 @@ tracepoint_notify (tp, action) filename = symtab_to_filename (sal.symtab); if (filename == NULL) filename = "N/A"; - sprintf (buf, "gdbtk_tcl_tracepoint %s %d 0x%lx %d {%s} %d", action, tp->number, + xasprintf (&buf, "gdbtk_tcl_tracepoint %s %d 0x%lx %d {%s} %d", action, tp->number, (long) tp->address, sal.line, filename, tp->pass_count); if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK) report_error (); + free(buf); } /* @@ -886,7 +900,7 @@ gdbtk_error_begin () static void gdbtk_annotate_signal () { - char buf[128]; + char *buf; /* Inform gui that the target has stopped. This is a necessary stop button evil. We don't want signal notification @@ -894,10 +908,11 @@ gdbtk_annotate_signal () timeout. */ Tcl_Eval (gdbtk_interp, "gdbtk_stop_idle_callback"); - sprintf (buf, "gdbtk_signal %s {%s}", target_signal_to_name (stop_signal), + xasprintf (&buf, "gdbtk_signal %s {%s}", target_signal_to_name (stop_signal), target_signal_to_string (stop_signal)); if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK) report_error (); + free(buf); } static void diff --git a/gdb/gdbtk/generic/gdbtk.c b/gdb/gdbtk/generic/gdbtk.c index 829118db01..1dd2af3de5 100644 --- a/gdb/gdbtk/generic/gdbtk.c +++ b/gdb/gdbtk/generic/gdbtk.c @@ -196,7 +196,7 @@ void TclDebug (char level, const char *fmt,...) { va_list args; - char buf[10000], *v[3], *merge, *priority; + char *buf, *v[3], *merge, *priority; switch (level) { @@ -215,17 +215,19 @@ TclDebug (char level, const char *fmt,...) va_start (args, fmt); + + xvasprintf (&buf, fmt, args); + va_end (args); + v[0] = "dbug"; v[1] = priority; v[2] = buf; - vsprintf (buf, fmt, args); - va_end (args); - merge = Tcl_Merge (3, v); if (Tcl_Eval (gdbtk_interp, merge) != TCL_OK) Tcl_BackgroundError (gdbtk_interp); Tcl_Free (merge); + free(buf); } @@ -359,7 +361,7 @@ gdbtk_init (argv0) { struct cleanup *old_chain; int found_main; - char s[5]; + char *s; Tcl_Obj *auto_path_elem, *auto_path_name; /* If there is no DISPLAY environment variable, Tk_Init below will fail, @@ -389,8 +391,10 @@ gdbtk_init (argv0) /* Set up some globals used by gdb to pass info to gdbtk for start up options and the like */ - sprintf (s, "%d", inhibit_gdbinit); + xasprintf (&s, "%d", inhibit_gdbinit); Tcl_SetVar2 (gdbtk_interp, "GDBStartup", "inhibit_prefs", s, TCL_GLOBAL_ONLY); + free(s); + /* Note: Tcl_SetVar2() treats the value as read-only (making a copy). Unfortunatly it does not mark the parameter as ``const''. */ -- 2.11.0