From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13664 invoked by alias); 16 Dec 2008 08:43:49 -0000 Received: (qmail 13652 invoked by uid 22791); 16 Dec 2008 08:43:48 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 16 Dec 2008 08:43:12 +0000 Received: from zps36.corp.google.com (zps36.corp.google.com [172.25.146.36]) by smtp-out.google.com with ESMTP id mBG8hABq009347 for ; Tue, 16 Dec 2008 00:43:10 -0800 Received: from localhost (ruffy.corp.google.com [172.18.118.116]) by zps36.corp.google.com with ESMTP id mBG8h9on010087 for ; Tue, 16 Dec 2008 00:43:09 -0800 Received: by localhost (Postfix, from userid 67641) id 3EDAC1C7A11; Tue, 16 Dec 2008 00:43:09 -0800 (PST) To: gdb-patches@sourceware.org Subject: [RFA] fixes to xmalloc for gdbserver patch Message-Id: <20081216084309.3EDAC1C7A11@localhost> Date: Tue, 16 Dec 2008 08:43:00 -0000 From: dje@google.com (Doug Evans) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-12/txt/msg00301.txt.bz2 Oops. I forgot to watch for calls to malloc that are checked and report failure back to gdb. This add a couple more checks for malloc failure and reporting the failure back to gdb instead of dying, as well as a couple of fixmes for some minor leaks. It also adds a freeargv function which is used in server.c. Fixing some of the memory leaks could use it too, except a bit more work is required because new_argv[0] can be NULL (which would cause freeargv to miss the later elements). Ok to check in? 2008-12-16 Doug Evans * regcache.c (new_register_cache): No need to check result of xcalloc. * server.c (handle_search_memory): Back out calls to xmalloc, result is checked and error is returned to user upon failure. (handle_query): Ditto. Add more checks for result of malloc. (handle_v_cont): Check result of malloc, report error back to user upon failure. (handle_v_run): Ditto. Call freeargv. * server.h (freeargv): Declare. * utils.c (freeargv): New fn. Index: regcache.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/regcache.c,v retrieving revision 1.16 diff -u -p -r1.16 regcache.c --- regcache.c 14 Dec 2008 20:51:04 -0000 1.16 +++ regcache.c 16 Dec 2008 08:29:35 -0000 @@ -100,8 +100,6 @@ new_register_cache (void) in case there are registers the target never fetches. This way they'll read as zero instead of garbage. */ regcache->registers = xcalloc (1, register_bytes); - if (regcache->registers == NULL) - fatal ("Could not allocate register cache."); regcache->registers_valid = 0; Index: server.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/server.c,v retrieving revision 1.82 diff -u -p -r1.82 server.c --- server.c 14 Dec 2008 20:51:04 -0000 1.82 +++ server.c 16 Dec 2008 08:29:35 -0000 @@ -436,7 +436,7 @@ handle_search_memory (char *own_buf, int CORE_ADDR found_addr; int cmd_name_len = sizeof ("qSearch:memory:") - 1; - pattern = xmalloc (packet_len); + pattern = malloc (packet_len); if (pattern == NULL) { error ("Unable to allocate memory to perform the search"); @@ -460,7 +460,7 @@ handle_search_memory (char *own_buf, int if (search_space_len < search_buf_size) search_buf_size = search_space_len; - search_buf = xmalloc (search_buf_size); + search_buf = malloc (search_buf_size); if (search_buf == NULL) { free (pattern); @@ -575,7 +575,7 @@ handle_query (char *own_buf, int packet_ return; if (len > PBUFSIZ - 2) len = PBUFSIZ - 2; - spu_buf = xmalloc (len + 1); + spu_buf = malloc (len + 1); if (!spu_buf) return; @@ -604,7 +604,7 @@ handle_query (char *own_buf, int packet_ require_running (own_buf); strcpy (own_buf, "E00"); - spu_buf = xmalloc (packet_len - 15); + spu_buf = malloc (packet_len - 15); if (!spu_buf) return; if (decode_xfer_write (own_buf + 16, packet_len - 16, &annex, @@ -648,7 +648,12 @@ handle_query (char *own_buf, int packet_ more. */ if (len > PBUFSIZ - 2) len = PBUFSIZ - 2; - data = xmalloc (len + 1); + data = malloc (len + 1); + if (data == NULL) + { + write_enn (own_buf); + return; + } n = (*the_target->read_auxv) (ofs, data, len + 1); if (n < 0) write_enn (own_buf); @@ -726,7 +731,12 @@ handle_query (char *own_buf, int packet_ for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next) total_len += 128 + 6 * strlen (((struct dll_info *) dll_ptr)->name); - document = xmalloc (total_len); + document = malloc (total_len); + if (document == NULL) + { + write_enn (own_buf); + return; + } strcpy (document, "\n"); p = document + strlen (document); @@ -782,7 +792,7 @@ handle_query (char *own_buf, int packet_ return; if (len > PBUFSIZ - 2) len = PBUFSIZ - 2; - workbuf = xmalloc (len + 1); + workbuf = malloc (len + 1); if (!workbuf) return; @@ -895,9 +905,15 @@ handle_query (char *own_buf, int packet_ /* Handle "monitor" commands. */ if (strncmp ("qRcmd,", own_buf, 6) == 0) { - char *mon = xmalloc (PBUFSIZ); + char *mon = malloc (PBUFSIZ); int len = strlen (own_buf + 6); + if (mon == NULL) + { + write_enn (own_buf); + return; + } + if ((len % 2) != 0 || unhexify (mon, own_buf + 6, len / 2) != len / 2) { write_enn (own_buf); @@ -975,7 +991,9 @@ handle_v_cont (char *own_buf, char *stat /* Allocate room for one extra action, for the default remain-stopped behavior; if no default action is in the list, we'll need the extra slot. */ - resume_info = xmalloc ((n + 1) * sizeof (resume_info[0])); + resume_info = malloc ((n + 1) * sizeof (resume_info[0])); + if (resume_info == NULL) + goto err; default_action.thread = -1; default_action.leave_stopped = 1; @@ -1097,7 +1115,7 @@ handle_v_attach (char *own_buf, char *st static int handle_v_run (char *own_buf, char *status, int *signal) { - char *p, **pp, *next_p, **new_argv; + char *p, *next_p, **new_argv; int i, new_argc; new_argc = 0; @@ -1107,7 +1125,13 @@ handle_v_run (char *own_buf, char *statu new_argc++; } - new_argv = xcalloc (new_argc + 2, sizeof (char *)); + new_argv = calloc (new_argc + 2, sizeof (char *)); + if (new_argv == NULL) + { + write_enn (own_buf); + return 0; + } + i = 0; for (p = own_buf + strlen ("vRun;"); *p; p = next_p) { @@ -1119,6 +1143,7 @@ handle_v_run (char *own_buf, char *statu new_argv[i] = NULL; else { + /* FIXME: Fail request if out of memory instead of dying. */ new_argv[i] = xmalloc (1 + (next_p - p) / 2); unhexify (new_argv[i], p, (next_p - p) / 2); new_argv[i][(next_p - p) / 2] = '\0'; @@ -1137,20 +1162,22 @@ handle_v_run (char *own_buf, char *statu if (program_argv == NULL) { + /* FIXME: new_argv memory leak */ write_enn (own_buf); return 0; } - new_argv[0] = xstrdup (program_argv[0]); + new_argv[0] = strdup (program_argv[0]); + if (new_argv[0] == NULL) + { + /* FIXME: new_argv memory leak */ + write_enn (own_buf); + return 0; + } } - /* Free the old argv. */ - if (program_argv) - { - for (pp = program_argv; *pp != NULL; pp++) - free (*pp); - free (program_argv); - } + /* Free the old argv and install the new one. */ + freeargv (program_argv); program_argv = new_argv; *signal = start_inferior (program_argv, status); Index: server.h =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/server.h,v retrieving revision 1.48 diff -u -p -r1.48 server.h --- server.h 14 Dec 2008 20:51:04 -0000 1.48 +++ server.h 16 Dec 2008 08:29:35 -0000 @@ -276,6 +276,7 @@ char *target_signal_to_name (enum target void *xmalloc (size_t) ATTR_MALLOC; void *xcalloc (size_t, size_t) ATTR_MALLOC; char *xstrdup (const char *) ATTR_MALLOC; +void freeargv (char **argv); void perror_with_name (char *string); void error (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2); void fatal (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2); Index: utils.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/utils.c,v retrieving revision 1.15 diff -u -p -r1.15 utils.c --- utils.c 14 Dec 2008 20:51:04 -0000 1.15 +++ utils.c 16 Dec 2008 08:29:35 -0000 @@ -87,6 +87,23 @@ xstrdup (const char *s) return ret; } +/* Free a standard argv vector. */ + +void +freeargv (char **vector) +{ + char **scan; + + if (vector != NULL) + { + for (scan = vector; *scan != NULL; scan++) + { + free (*scan); + } + free (vector); + } +} + /* Print the system error message for errno, and also mention STRING as the file name for which the error was encountered. Then return to command level. */