From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3134 invoked by alias); 23 Jan 2007 14:45:35 -0000 Received: (qmail 3117 invoked by uid 22791); 23 Jan 2007 14:45:32 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Tue, 23 Jan 2007 14:45:26 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1H9Mtn-0002TN-KO for gdb-patches@sourceware.org; Tue, 23 Jan 2007 09:45:23 -0500 Date: Tue, 23 Jan 2007 14:45:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: [rfc] Plug memory leaks during gdbarch initialization Message-ID: <20070123144523.GA6339@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.13 (2006-08-11) 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: 2007-01/txt/msg00477.txt.bz2 This patch fixes a number of memory leaks I found with valgrind. There are more, but this should cover everything that gets repeated for each gdbarch. Some of these are simple omissions, but one in particular requires care: we no longer call build_gdbtypes to build the per-gdbarch types during _initialize_gdbtypes. Two targets that I spotted relied on this, being i386-tdep.c and spu-tdep.c; they constructed new SIMD types based on builtin_type_v2_int and other standard vectors, before the vectors were guaranteed to be available. I moved them to be per-gdbarch like the types they depend on. Of course while doing this I realized that there's still a problem. Now they are created during gdbarch initialization, but the only reason it used to work still applies: alphabetical order. gdbtypes.c's constructor still gets called before i386-tdep.c's because it was registered first. The patch is an improvement, but doesn't fix that wart. I don't really want to check it in without fixing the underlying problem but I don't see a good way... anyone have a suggestion? -- Daniel Jacobowitz CodeSourcery 2007-01-23 Daniel Jacobowitz * gdbtypes.c (build_gdbtypes): Move registration of opaque-type-resolution to ... (_initialize_gdbtypes): ... here. Don't call build_gdbtypes. Register swaps for builtin_type_true_char, builtin_type_bool, and builtin_type_vec64. * parse.c (_initialize_parse): Don't call build_parse. Register swap for msym_tls_symbol_type. * f-lang.c (_initialize_f_language): Don't call build_fortran_types. Don't create or register builtin_type_string. * i386-tdep.c (_initialize_i386_tdep): Don't call i386_init_types. Register swaps for builtin types. * remote.c (add_packet_config_cmd): Free allocated strings. * spu-tdep.c (_initialize_spu_tdep): Don't call spu_init_vector_type. Register swap for builtin type. --- gdb/f-lang.c | 8 -------- gdb/gdbtypes.c | 25 +++++++++++++------------ gdb/i386-tdep.c | 7 ++++++- gdb/parse.c | 3 +-- gdb/remote.c | 2 ++ gdb/spu-tdep.c | 3 ++- 6 files changed, 24 insertions(+), 24 deletions(-) Index: src/gdb/gdbtypes.c =================================================================== --- src.orig/gdb/gdbtypes.c 2007-01-23 09:00:02.000000000 -0500 +++ src/gdb/gdbtypes.c 2007-01-23 09:03:04.000000000 -0500 @@ -3419,16 +3419,6 @@ build_gdbtypes (void) 0, "bool", (struct objfile *) NULL); - /* Add user knob for controlling resolution of opaque types */ - add_setshow_boolean_cmd ("opaque-type-resolution", class_support, - &opaque_type_resolution, _("\ -Set resolution of opaque struct/class/union types (if set before loading symbols)."), _("\ -Show resolution of opaque struct/class/union types (if set before loading symbols)."), NULL, - NULL, - show_opaque_type_resolution, - &setlist, &showlist); - opaque_type_resolution = 1; - /* Build SIMD types. */ builtin_type_v4sf = init_simd_type ("__builtin_v4sf", builtin_type_float, "f", 4); @@ -3719,8 +3709,6 @@ _initialize_gdbtypes (void) TYPE_FLAG_UNSIGNED, "uint128_t", (struct objfile *) NULL); - build_gdbtypes (); - gdbtypes_data = gdbarch_data_register_post_init (gdbtypes_post_init); /* FIXME - For the moment, handle types by swapping them in and out. @@ -3734,6 +3722,7 @@ _initialize_gdbtypes (void) a different architecture. */ DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_void); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_char); + DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_true_char); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_short); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_int); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_long); @@ -3750,6 +3739,7 @@ _initialize_gdbtypes (void) DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_complex); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_double_complex); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_string); + DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_bool); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v4sf); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v4si); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v16qi); @@ -3767,6 +3757,7 @@ _initialize_gdbtypes (void) DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v2_int32); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v8_int8); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_v4_int16); + DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_vec64); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_vec128); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_void_data_ptr); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_void_func_ptr); @@ -3889,4 +3880,14 @@ When enabled, ranking of the functions i NULL, show_overload_debug, &setdebuglist, &showdebuglist); + + /* Add user knob for controlling resolution of opaque types */ + add_setshow_boolean_cmd ("opaque-type-resolution", class_support, + &opaque_type_resolution, _("\ +Set resolution of opaque struct/class/union types (if set before loading symbols)."), _("\ +Show resolution of opaque struct/class/union types (if set before loading symbols)."), NULL, + NULL, + show_opaque_type_resolution, + &setlist, &showlist); + opaque_type_resolution = 1; } Index: src/gdb/parse.c =================================================================== --- src.orig/gdb/parse.c 2007-01-23 09:00:02.000000000 -0500 +++ src/gdb/parse.c 2007-01-23 09:03:04.000000000 -0500 @@ -1405,14 +1405,13 @@ _initialize_parse (void) type_stack = (union type_stack_elt *) xmalloc (type_stack_size * sizeof (*type_stack)); - build_parse (); - /* FIXME - For the moment, handle types by swapping them in and out. Should be using the per-architecture data-pointer and a large struct. */ DEPRECATED_REGISTER_GDBARCH_SWAP (msym_text_symbol_type); DEPRECATED_REGISTER_GDBARCH_SWAP (msym_data_symbol_type); DEPRECATED_REGISTER_GDBARCH_SWAP (msym_unknown_symbol_type); + DEPRECATED_REGISTER_GDBARCH_SWAP (msym_tls_symbol_type); deprecated_register_gdbarch_swap (NULL, 0, build_parse); add_setshow_zinteger_cmd ("expression", class_maintenance, Index: src/gdb/f-lang.c =================================================================== --- src.orig/gdb/f-lang.c 2007-01-23 09:00:02.000000000 -0500 +++ src/gdb/f-lang.c 2007-01-23 09:03:04.000000000 -0500 @@ -567,8 +567,6 @@ build_fortran_types (void) void _initialize_f_language (void) { - build_fortran_types (); - DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_character); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_logical); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_logical_s1); @@ -582,14 +580,8 @@ _initialize_f_language (void) DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_complex_s16); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_complex_s32); DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_f_void); - DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_string); deprecated_register_gdbarch_swap (NULL, 0, build_fortran_types); - builtin_type_string = - init_type (TYPE_CODE_STRING, TARGET_CHAR_BIT / TARGET_CHAR_BIT, - 0, - "character string", (struct objfile *) NULL); - add_language (&f_language_defn); } Index: src/gdb/i386-tdep.c =================================================================== --- src.orig/gdb/i386-tdep.c 2007-01-23 09:00:02.000000000 -0500 +++ src/gdb/i386-tdep.c 2007-01-23 09:03:04.000000000 -0500 @@ -2521,5 +2521,10 @@ is \"default\"."), /* Initialize the i386-specific register groups & types. */ i386_init_reggroups (); - i386_init_types(); + + DEPRECATED_REGISTER_GDBARCH_SWAP (i386_eflags_type); + DEPRECATED_REGISTER_GDBARCH_SWAP (i386_mmx_type); + DEPRECATED_REGISTER_GDBARCH_SWAP (i386_sse_type); + DEPRECATED_REGISTER_GDBARCH_SWAP (i386_mxcsr_type); + deprecated_register_gdbarch_swap (NULL, 0, i386_init_types); } Index: src/gdb/remote.c =================================================================== --- src.orig/gdb/remote.c 2007-01-23 09:00:02.000000000 -0500 +++ src/gdb/remote.c 2007-01-23 09:03:04.000000000 -0500 @@ -784,6 +784,8 @@ add_packet_config_cmd (struct packet_con set_remote_protocol_packet_cmd, show_remote_protocol_packet_cmd, &remote_set_cmdlist, &remote_show_cmdlist); + xfree (set_doc); + xfree (show_doc); /* set/show remote NAME-packet {auto,on,off} -- legacy. */ if (legacy) { Index: src/gdb/spu-tdep.c =================================================================== --- src.orig/gdb/spu-tdep.c 2007-01-23 09:23:42.000000000 -0500 +++ src/gdb/spu-tdep.c 2007-01-23 09:24:40.000000000 -0500 @@ -1107,5 +1107,6 @@ _initialize_spu_tdep (void) { register_gdbarch_init (bfd_arch_spu, spu_gdbarch_init); - spu_init_vector_type (); + DEPRECATED_REGISTER_GDBARCH_SWAP (spu_builtin_type_vec128); + deprecated_register_gdbarch_swap (NULL, 0, spu_init_vector_type); }