[RFA] Fix memory leak in gdbserver

[RFA] Fix memory leak in gdbserver

[Doug Evans]

Hi.

This fixes a memory leak in gdbserver.

Ok to check in?

2009-04-29  Doug Evans  <dje@google.com>

	* inferiors.c (remove_process): Fix memory leak, free process.
	* linux-low.c (linux_remove_process): New function.
	(linux_kill): Call it instead of remove_process.
	(linux_detach, linux_wait_1): Ditto.

Index: inferiors.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/inferiors.c,v
retrieving revision 1.20
diff -u -p -r1.20 inferiors.c
--- inferiors.c	3 Apr 2009 20:15:51 -0000	1.20
+++ inferiors.c	30 Apr 2009 01:23:29 -0000
@@ -427,12 +427,17 @@ add_process (int pid, int attached)
   return process;
 }
 
+/* Remove a process from the common process list and free the memory
+   allocated for it.
+   The caller is responsible for freeing private data first.  */
+
 void
 remove_process (struct process_info *process)
 {
   clear_symbol_cache (&process->symbol_cache);
   free_all_breakpoints (process);
   remove_inferior (&all_processes, &process->head);
+  free (process);
 }
 
 struct process_info *
Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.97
diff -u -p -r1.97 linux-low.c
--- linux-low.c	3 Apr 2009 11:40:02 -0000	1.97
+++ linux-low.c	30 Apr 2009 06:30:01 -0000
@@ -182,6 +182,16 @@ linux_add_process (int pid, int attached
   return proc;
 }
 
+/* Remove a process from the common process list,
+   also freeing all private data.  */
+
+static void
+linux_remove_process (struct process_info *process)
+{
+  free (process->private);
+  remove_process (process);
+}
+
 /* Handle a GNU/Linux extended wait response.  If we see a clone
    event, we need to add the new LWP to our list (and not report the
    trap to higher layers).  */
@@ -565,7 +575,7 @@ linux_kill (int pid)
     } while (lwpid > 0 && WIFSTOPPED (wstat));
 
   delete_lwp (lwp);
-  remove_process (process);
+  linux_remove_process (process);
   return 0;
 }
 
@@ -654,7 +664,7 @@ linux_detach (int pid)
 
   delete_all_breakpoints ();
   find_inferior (&all_threads, linux_detach_one_lwp, &pid);
-  remove_process (process);
+  linux_remove_process (process);
   return 0;
 }
 
@@ -1273,7 +1283,7 @@ retry:
 	  struct process_info *process = find_process_pid (pid);
 
 	  delete_lwp (lwp);
-	  remove_process (process);
+	  linux_remove_process (process);
 
 	  current_inferior = NULL;
 

Re: [RFA] Fix memory leak in gdbserver

[Pedro Alves]

On Thursday 30 April 2009 07:38:36, Doug Evans wrote:

> 2009-04-29  Doug Evans  <dje@google.com>
> 
> 	* inferiors.c (remove_process): Fix memory leak, free process.
> 	* linux-low.c (linux_remove_process): New function.
> 	(linux_kill): Call it instead of remove_process.
> 	(linux_detach, linux_wait_1): Ditto.

Ok, thanks.

-- 
Pedro Alves

Build failure for x86_64-pc-mingw32 target

[Kai Tietz]

Hello,

I noticed a failure while building gdb for x64 target. It seems to be an 
new regression. 

x86_64-pc-mingw32-gcc -g -O2 -D__USE_MINGW_ACCESS   -I. -I../../src/gdb 
-I../../
src/gdb/common -I../../src/gdb/config 
-DLOCALEDIR="\"/usr/local/share/locale\""
-DHAVE_CONFIG_H -I../../src/gdb/../include/opcode 
-I../../src/gdb/../readline/..
 -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include 
-I../libdecnumber -I
../../src/gdb/../libdecnumber -I./../intl -I../../src/gdb/gnulib -Ignulib 
-DMI_
OUT=1  -Wall -Wdeclaration-after-statement -Wpointer-arith 
-Wformat-nonliteral -
Wno-pointer-sign -Wno-unused -Wno-switch -Wno-char-subscripts -Wno-format 
-Werro
r -c -o i386-tdep.o -MT i386-tdep.o -MMD -MP -MF .deps/i386-tdep.Tpo 
../../src/g
db/i386-tdep.c
cc1: warnings being treated as errors
../../src/gdb/i386-tdep.c: In function 'i386_process_record':
../../src/gdb/i386-tdep.c:2996: error: implicit declaration of function 
'bzero'
../../src/gdb/i386-tdep.c:2996: error: incompatible implicit declaration 
of buil
t-in function 'bzero'
make[2]: *** [i386-tdep.o] Error 1

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: Build failure for x86_64-pc-mingw32 target

[Hui Zhu]

mingw32 don't have bzero?
Could you please help me try to build some code that have bzero in mingw?

Thanks,
Hui

On Wed, May 6, 2009 at 16:50, Kai Tietz <Kai.Tietz@onevision.com> wrote:
> Hello,
>
> I noticed a failure while building gdb for x64 target. It seems to be an
> new regression.
>
> x86_64-pc-mingw32-gcc -g -O2 -D__USE_MINGW_ACCESS   -I. -I../../src/gdb
> -I../../
> src/gdb/common -I../../src/gdb/config
> -DLOCALEDIR="\"/usr/local/share/locale\""
> -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode
> -I../../src/gdb/../readline/..
>  -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include
> -I../libdecnumber -I
> ../../src/gdb/../libdecnumber -I./../intl -I../../src/gdb/gnulib -Ignulib
> -DMI_
> OUT=1  -Wall -Wdeclaration-after-statement -Wpointer-arith
> -Wformat-nonliteral -
> Wno-pointer-sign -Wno-unused -Wno-switch -Wno-char-subscripts -Wno-format
> -Werro
> r -c -o i386-tdep.o -MT i386-tdep.o -MMD -MP -MF .deps/i386-tdep.Tpo
> ../../src/g
> db/i386-tdep.c
> cc1: warnings being treated as errors
> ../../src/gdb/i386-tdep.c: In function 'i386_process_record':
> ../../src/gdb/i386-tdep.c:2996: error: implicit declaration of function
> 'bzero'
> ../../src/gdb/i386-tdep.c:2996: error: incompatible implicit declaration
> of buil
> t-in function 'bzero'
> make[2]: *** [i386-tdep.o] Error 1
>
> Cheers,
> Kai
>
> |  (\_/)  This is Bunny. Copy and paste Bunny
> | (='.'=) into your signature to help him gain
> | (")_(") world domination.
>
>

Re: Build failure for x86_64-pc-mingw32 target

[Pedro Alves]

On Wednesday 06 May 2009 10:05:09, Hui Zhu wrote:
> mingw32 don't have bzero?
> Could you please help me try to build some code that have bzero in mingw?

Please don't use bzero, use memset instead.  Patch doing that pre-approved.

(Kai, please don't reply to unrelated threads to start a new topic.  Start
a new thread instead.)

-- 
Pedro Alves

Re: Build failure for x86_64-pc-mingw32 target

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

> Date: Wed, 6 May 2009 17:05:09 +0800
> From: Hui Zhu <teawater@gmail.com>
> 
> mingw32 don't have bzero?
> Could you please help me try to build some code that have bzero in mingw?

Probably best to replace bzero with memset.

> On Wed, May 6, 2009 at 16:50, Kai Tietz <Kai.Tietz@onevision.com> wrote:
> > Hello,
> >
> > I noticed a failure while building gdb for x64 target. It seems to be an
> > new regression.
> >
> > x86_64-pc-mingw32-gcc -g -O2 -D__USE_MINGW_ACCESS   -I. -I../../src/gdb
> > -I../../
> > src/gdb/common -I../../src/gdb/config
> > -DLOCALEDIR="\"/usr/local/share/locale\""
> > -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode
> > -I../../src/gdb/../readline/..
> >  -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include
> > -I../libdecnumber -I
> > ../../src/gdb/../libdecnumber -I./../intl -I../../src/gdb/gnulib -Ignulib
> > -DMI_
> > OUT=1  -Wall -Wdeclaration-after-statement -Wpointer-arith
> > -Wformat-nonliteral -
> > Wno-pointer-sign -Wno-unused -Wno-switch -Wno-char-subscripts -Wno-format
> > -Werro
> > r -c -o i386-tdep.o -MT i386-tdep.o -MMD -MP -MF .deps/i386-tdep.Tpo
> > ../../src/g
> > db/i386-tdep.c
> > cc1: warnings being treated as errors
> > ../../src/gdb/i386-tdep.c: In function 'i386_process_record':
> > ../../src/gdb/i386-tdep.c:2996: error: implicit declaration of function
> > 'bzero'
> > ../../src/gdb/i386-tdep.c:2996: error: incompatible implicit declaration
> > of buil
> > t-in function 'bzero'
> > make[2]: *** [i386-tdep.o] Error 1
> >
> > Cheers,
> > Kai
> >
> > |  (\_/)  This is Bunny. Copy and paste Bunny
> > | (='.'=) into your signature to help him gain
> > | (")_(") world domination.
> >
> >
> 
> 

Re: Build failure for x86_64-pc-mingw32 target

[Hui Zhu]

OK.  I will post a patch to change bzero to memset.

Thanks for your help guys.

Hui

On Wed, May 6, 2009 at 17:38, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 06 May 2009 10:05:09, Hui Zhu wrote:
>> mingw32 don't have bzero?
>> Could you please help me try to build some code that have bzero in mingw?
>
> Please don't use bzero, use memset instead.  Patch doing that pre-approved.
>
> (Kai, please don't reply to unrelated threads to start a new topic.  Start
> a new thread instead.)
>
> --
> Pedro Alves
>

Re: Build failure for x86_64-pc-mingw32 target

[Hui Zhu]

The patch is checked in.

2009-05-06  Hui Zhu  <teawater@gmail.com>

	* i386-tdep.c (i386_process_record): Change bzero to memset.

Thanks,
Hui

RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.271
retrieving revision 1.272
diff -u -r1.271 -r1.272
--- src/gdb/i386-tdep.c	2009/05/01 08:09:16	1.271
+++ src/gdb/i386-tdep.c	2009/05/06 11:16:40	1.272
@@ -2993,7 +2993,7 @@
   uint32_t opcode;
   struct i386_record_s ir;

-  bzero (&ir, sizeof (struct i386_record_s));
+  memset (&ir, 0, sizeof (struct i386_record_s));
   ir.regcache = regcache;
   ir.addr = addr;
   ir.aflag = 1;


On Wed, May 6, 2009 at 18:46, Hui Zhu <teawater@gmail.com> wrote:
> OK.  I will post a patch to change bzero to memset.
>
> Thanks for your help guys.
>
> Hui
>
> On Wed, May 6, 2009 at 17:38, Pedro Alves <pedro@codesourcery.com> wrote:
>> On Wednesday 06 May 2009 10:05:09, Hui Zhu wrote:
>>> mingw32 don't have bzero?
>>> Could you please help me try to build some code that have bzero in mingw?
>>
>> Please don't use bzero, use memset instead.  Patch doing that pre-approved.
>>
>> (Kai, please don't reply to unrelated threads to start a new topic.  Start
>> a new thread instead.)
>>
>> --
>> Pedro Alves
>>
>

Re: Build failure for x86_64-pc-mingw32 target

[Kai Tietz]

gdb-patches-owner@sourceware.org wrote on 06.05.2009 13:20:10:

> The patch is checked in.
> 
> 2009-05-06  Hui Zhu  <teawater@gmail.com>
> 
>    * i386-tdep.c (i386_process_record): Change bzero to memset.
> 
> Thanks,
> Hui
> 
> RCS file: /cvs/src/src/gdb/i386-tdep.c,v
> retrieving revision 1.271
> retrieving revision 1.272
> diff -u -r1.271 -r1.272
> --- src/gdb/i386-tdep.c   2009/05/01 08:09:16   1.271
> +++ src/gdb/i386-tdep.c   2009/05/06 11:16:40   1.272
> @@ -2993,7 +2993,7 @@
>    uint32_t opcode;
>    struct i386_record_s ir;
> 
> -  bzero (&ir, sizeof (struct i386_record_s));
> +  memset (&ir, 0, sizeof (struct i386_record_s));
>    ir.regcache = regcache;
>    ir.addr = addr;
>    ir.aflag = 1;
> 
> 
> On Wed, May 6, 2009 at 18:46, Hui Zhu <teawater@gmail.com> wrote:
> > OK.  I will post a patch to change bzero to memset.
> >
> > Thanks for your help guys.
> >
> > Hui
> >
> > On Wed, May 6, 2009 at 17:38, Pedro Alves <pedro@codesourcery.com> 
wrote:
> >> On Wednesday 06 May 2009 10:05:09, Hui Zhu wrote:
> >>> mingw32 don't have bzero?
> >>> Could you please help me try to build some code that have bzero in 
mingw?
> >>
> >> Please don't use bzero, use memset instead.  Patch doing that 
pre-approved.
> >>
> >> (Kai, please don't reply to unrelated threads to start a new topic. 
 Start
> >> a new thread instead.)
> >>
> >> --
> >> Pedro Alves
> >>
> >
> 

Hello,

I test build for x64 and it works now again.

Pedro, sorry. Wasn't by intention.

Hui, thanks for fixing it,

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

[ARI] Add "bzero" (was: "Re: Build failure for x86_64-pc-mingw32 target")

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Pierre,

Would you mind adding bzero to the list of functions in the ARI that
should not be used? I think this would be useful to have. The following
patch should work, but I'm not sure how you test it, and i'm also not
sure about the right category...

Thanks!
-- 
Joel

[-- Attachment #2: bzero-ari.diff --]
[-- Type: text/x-diff, Size: 664 bytes --]

Index: gdb_ari.sh
===================================================================
RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
retrieving revision 1.89
diff -u -p -r1.89 gdb_ari.sh
--- gdb_ari.sh	20 Apr 2009 15:55:23 -0000	1.89
+++ gdb_ari.sh	6 May 2009 16:00:22 -0000
@@ -1028,6 +1028,14 @@ Do not use xvasprintf(), instead use xst
 
 # More generic memory operations
 
+BEGIN { doc["bzero"] = "\
+Do not use bzero(), instead use memset()"
+    category["bzero"] = ari_regression
+}
+/(^|[^_[:alnum:]])bzero[[:space:]]*\(/ {
+    fail("bzero")
+}
+
 BEGIN { doc["strdup"] = "\
 Do not use strdup(), instead use xstrdup()";
     category["strdup"] = ari_regression

Re: [ARI] Add "bzero" (was: "Re: Build failure for x86_64-pc-mingw32 target")

[Joel Brobecker]

Pierre also informed me privately that he checked in the change below,
after testing it with tonight's snapshot.

> Index: gdb_ari.sh
> ===================================================================
> RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
> retrieving revision 1.89
> diff -u -p -r1.89 gdb_ari.sh
> --- gdb_ari.sh	20 Apr 2009 15:55:23 -0000	1.89
> +++ gdb_ari.sh	6 May 2009 16:00:22 -0000
> @@ -1028,6 +1028,14 @@ Do not use xvasprintf(), instead use xst
>  
>  # More generic memory operations
>  
> +BEGIN { doc["bzero"] = "\
> +Do not use bzero(), instead use memset()"
> +    category["bzero"] = ari_regression
> +}
> +/(^|[^_[:alnum:]])bzero[[:space:]]*\(/ {
> +    fail("bzero")
> +}
> +
>  BEGIN { doc["strdup"] = "\
>  Do not use strdup(), instead use xstrdup()";
>      category["strdup"] = ari_regression


-- 
Joel

Re: [ARI] Add "bzero" (was: "Re: Build failure for x86_64-pc-mingw32 target")

[Eli Zaretskii]

> Date: Wed, 6 May 2009 09:02:50 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> Would you mind adding bzero to the list of functions in the ARI that
> should not be used?

Do we already have bcopy there?  If not, we should add that, too, I
think.

Re: [ARI] Add "bzero" (was: "Re: Build failure for x86_64-pc-mingw32 target")

[Joel Brobecker]

> Do we already have bcopy there?  If not, we should add that, too, I
> think.

Yes, I just double-checked, we do have a rule for bcopy :)

-- 
Joel

Re: [ARI] Add "bzero"

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Would you mind adding bzero to the list of functions in the ARI that
Joel> should not be used? I think this would be useful to have. The following
Joel> patch should work, but I'm not sure how you test it, and i'm also not
Joel> sure about the right category...

For this specific case, and some others like it, perhaps using #pragma
poison would be better.  That would result in a compile time error,
rather than a report on a web page.

Tom

Re: [ARI] Add "bzero"

[Joel Brobecker]

> Joel> Would you mind adding bzero to the list of functions in the ARI that
> Joel> should not be used? I think this would be useful to have. The following
> Joel> patch should work, but I'm not sure how you test it, and i'm also not
> Joel> sure about the right category...
> 
> For this specific case, and some others like it, perhaps using #pragma
> poison would be better.  That would result in a compile time error,
> rather than a report on a web page.

Sounds like a nice suggestion, indeed. Nice to have you back, Tom :).
I'll leave that in my list of things to do. Should be pretty quick to do.

-- 
Joel

Re: [ARI] Add "bzero"

[Joel Brobecker]

> For this specific case, and some others like it, perhaps using #pragma
> poison would be better.  That would result in a compile time error,
> rather than a report on a web page.

I really liked the idea and gave it a quick try, but I don't know
how to make it work. I thought I would put the pragma at the end of
defs.h, since defs.h is always included.  But defs.h is also always
the first file to be included, and so any #include that follows it
which references any of the poisoned identifier will cause an error.

Do you see another way?

Thanks,
-- 
Joel

Re: [ARI] Add "bzero"

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Tom> For this specific case, and some others like it, perhaps using #pragma
Tom> poison would be better.  That would result in a compile time error,
Tom> rather than a report on a web page.

Joel> I really liked the idea and gave it a quick try, but I don't know
Joel> how to make it work. I thought I would put the pragma at the end of
Joel> defs.h, since defs.h is always included.  But defs.h is also always
Joel> the first file to be included, and so any #include that follows it
Joel> which references any of the poisoned identifier will cause an error.

Joel> Do you see another way?

Nope.  In gcc I think the possibly-offending system headers are all
included in a single header file ("system.h"), and then the poisoning
is done at the end of that file.

Tom