public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Fix type_stack leaks in c expression parsing.
@ 2019-02-09 19:50 Philippe Waroquiers
  2019-02-10  0:31 ` Kevin Buettner
  0 siblings, 1 reply; 3+ messages in thread
From: Philippe Waroquiers @ 2019-02-09 19:50 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

Valgrind detects a bunch of leaks in several tests, such as:

==22905== 40 (24 direct, 16 indirect) bytes in 1 blocks are definitely lost in loss record 531 of 3,268
==22905==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
==22905==    by 0x5893AD: get_type_stack() (parse.c:1509)
==22905==    by 0x3F4EAD: c_yyparse() (c-exp.y:1223)
==22905==    by 0x3F71BC: c_parse(parser_state*) (c-exp.y:3308)
==22905==    by 0x588CEA: parse_exp_in_context_1(char const**, unsigned long, block const*, int, int, int*) [clone .constprop.89] (parse.c:1205)
==22905==    by 0x588FA1: parse_exp_in_context (parse.c:1108)
==22905==    by 0x588FA1: parse_exp_1 (parse.c:1099)
==22905==    by 0x588FA1: parse_expression(char const*) (parse.c:1247)
...

==22395== 456 (168 direct, 288 indirect) bytes in 7 blocks are definitely lost in loss record 2,658 of 2,978
==22395==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
==22395==    by 0x5893AD: get_type_stack() (parse.c:1509)
==22395==    by 0x3F4ECF: c_yyparse() (c-exp.y:1230)
==22395==    by 0x3F71BC: c_parse(parser_state*) (c-exp.y:3308)
==22395==    by 0x588CEA: parse_exp_in_context_1(char const**, unsigned long, block const*, int, int, int*) [clone .constprop.89] (parse.c:1205)
==22395==    by 0x588FA1: parse_exp_in_context (parse.c:1108)
==22395==    by 0x588FA1: parse_exp_1 (parse.c:1099)
==22395==    by 0x588FA1: parse_expression(char const*) (parse.c:1247)
==22395==    by 0x67BB9D: whatis_exp(char const*, int) (typeprint.c:515)
...

==22395== VALGRIND_GDB_ERROR_BEGIN
==22395== 144 (24 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 1,016 of 2,978
==22395==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
==22395==    by 0x5893AD: get_type_stack() (parse.c:1509)
==22395==    by 0x3F4E8A: c_yyparse() (c-exp.y:1217)
==22395==    by 0x3F71BC: c_parse(parser_state*) (c-exp.y:3308)
==22395==    by 0x588CEA: parse_exp_in_context_1(char const**, unsigned long, block const*, int, int, int*) [clone .constprop.89] (parse.c:1205)
==22395==    by 0x588FA1: parse_exp_in_context (parse.c:1108)
==22395==    by 0x588FA1: parse_exp_1 (parse.c:1099)
==22395==    by 0x588FA1: parse_expression(char const*) (parse.c:1247)
==22395==    by 0x67BB9D: whatis_exp(char const*, int) (typeprint.c:515)
...

Fix these by storing the allocated type_stack in the cpstate->type_stacks
vector.

Tested on debian/amd64, natively and under valgrind.

gdb/ChangeLog
2019-02-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* c-exp.y (direct_abs_decl): emplace_back type_stack.
---
 gdb/c-exp.y | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index f3ef23c75a..508e9ef28d 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1215,12 +1215,14 @@ direct_abs_decl: '(' abs_decl ')'
 			  push_type_int ($2);
 			  push_type (tp_array);
 			  $$ = get_type_stack ();
+			  cpstate->type_stacks.emplace_back ($$);
 			}
 	|	array_mod
 			{
 			  push_type_int ($1);
 			  push_type (tp_array);
 			  $$ = get_type_stack ();
+			  cpstate->type_stacks.emplace_back ($$);
 			}
 
 	| 	direct_abs_decl func_mod
@@ -1228,11 +1230,13 @@ direct_abs_decl: '(' abs_decl ')'
 			  push_type_stack ($1);
 			  push_typelist ($2);
 			  $$ = get_type_stack ();
+			  cpstate->type_stacks.emplace_back ($$);
 			}
 	|	func_mod
 			{
 			  push_typelist ($1);
 			  $$ = get_type_stack ();
+			  cpstate->type_stacks.emplace_back ($$);
 			}
 	;
 
-- 
2.20.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFA] Fix type_stack leaks in c expression parsing.
  2019-02-09 19:50 [RFA] Fix type_stack leaks in c expression parsing Philippe Waroquiers
@ 2019-02-10  0:31 ` Kevin Buettner
  2019-02-10 12:18   ` Philippe Waroquiers
  0 siblings, 1 reply; 3+ messages in thread
From: Kevin Buettner @ 2019-02-10  0:31 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

On Sat,  9 Feb 2019 20:50:00 +0100
Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:

> Fix these by storing the allocated type_stack in the cpstate->type_stacks
> vector.
> 
> Tested on debian/amd64, natively and under valgrind.
> 
> gdb/ChangeLog
> 2019-02-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* c-exp.y (direct_abs_decl): emplace_back type_stack.

The actual changes in your patch look good to me.

One nit regarding the ChangeLog entry though - I think we strive to
make our ChangeLog remarks look like sentences where the first word
is capitalized.  So... maybe something like:

 	* c-exp.y (direct_abs_decl): Use emplace_back to record the
	type_stack.

If you can think of a better ChangeLog comment, that's fine too.

Kevin

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFA] Fix type_stack leaks in c expression parsing.
  2019-02-10  0:31 ` Kevin Buettner
@ 2019-02-10 12:18   ` Philippe Waroquiers
  0 siblings, 0 replies; 3+ messages in thread
From: Philippe Waroquiers @ 2019-02-10 12:18 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On Sat, 2019-02-09 at 17:31 -0700, Kevin Buettner wrote:
> The actual changes in your patch look good to me.
> 
> One nit regarding the ChangeLog entry though - I think we strive to
> make our ChangeLog remarks look like sentences where the first word
> is capitalized.  So... maybe something like:
> 
>  	* c-exp.y (direct_abs_decl): Use emplace_back to record the
> 	type_stack.
> 
> If you can think of a better ChangeLog comment, that's fine too.
> 
> Kevin

Thanks for the review, pushed after having fixed the ChangeLog
as suggested.

Philippe

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-02-10 12:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-09 19:50 [RFA] Fix type_stack leaks in c expression parsing Philippe Waroquiers
2019-02-10  0:31 ` Kevin Buettner
2019-02-10 12:18   ` Philippe Waroquiers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).