From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1303 invoked by alias); 13 May 2011 15:44:07 -0000 Received: (qmail 1294 invoked by uid 22791); 13 May 2011 15:44:05 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 13 May 2011 15:43:48 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4DFhkPZ029693 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 13 May 2011 11:43:46 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p4DFhkJx014337; Fri, 13 May 2011 11:43:46 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p4DFhjYC007142; Fri, 13 May 2011 11:43:45 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id E0A97378BA5; Fri, 13 May 2011 09:43:44 -0600 (MDT) From: Tom Tromey To: Jan Kratochvil Cc: Ulrich Weigand , gdb-patches@sourceware.org Subject: Re: Regression: Re: RFC: implement typed DWARF stack References: <201105120003.p4C03V9u022585@d06av02.portsmouth.uk.ibm.com> <20110513075220.GA7000@host1.jankratochvil.net> Date: Fri, 13 May 2011 15:44:00 -0000 In-Reply-To: <20110513075220.GA7000@host1.jankratochvil.net> (Jan Kratochvil's message of "Fri, 13 May 2011 09:52:20 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2011-05/txt/msg00323.txt.bz2 Jan> This is 15MB .sum diff of regressions, nothing works now. Yeah, I apologize for that. I didn't run the final version through the tester, neglecting that I had made a major change. Here is the fix I am checking in. The bug was that we were calling value_free_to_mark but then using values from the value list. The fix is to be more careful about ordering cleanups. Built and regtested on the buildbot. Jan> +# This test can only be run on x86 targets. Jan> +if { ![istarget i?86-*] } { Jan> + return 0 Jan> +} Jan> It will not run on x86_64 box with -m32 while it could, I would Jan> prefer there also x86_64 target with is_ilp32_target conditional. I will make this change separately. Tom 2011-05-13 Tom Tromey * utils.c (do_value_free): New function. (make_cleanup_value_free): Likewise. * dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Handle value freeing correctly. (dwarf2_loc_desc_needs_frame): Call make_cleanup_value_free_to_mark. * dwarf2expr.h (struct dwarf_expr_context) : Remove field. * dwarf2expr.c (free_dwarf_expr_context): Don't call value_free_to_mark. (new_dwarf_expr_context): Don't call value_mark. * dwarf2-frame.c (execute_stack_op): Call make_cleanup_value_free_to_mark. * defs.h (make_cleanup_value_free): Declare. diff --git a/gdb/defs.h b/gdb/defs.h index 771d3ad..38a2fcf 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -362,6 +362,7 @@ extern struct cleanup * make_cleanup_restore_ui_file (struct ui_file **variable); extern struct cleanup *make_cleanup_value_free_to_mark (struct value *); +extern struct cleanup *make_cleanup_value_free (struct value *); extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c index 4e4e6d9..fe48713 100644 --- a/gdb/dwarf2-frame.c +++ b/gdb/dwarf2-frame.c @@ -402,6 +402,7 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size, ctx = new_dwarf_expr_context (); old_chain = make_cleanup_free_dwarf_expr_context (ctx); + make_cleanup_value_free_to_mark (value_mark ()); ctx->gdbarch = get_frame_arch (this_frame); ctx->addr_size = addr_size; diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c index 0c0760b..b42f289 100644 --- a/gdb/dwarf2expr.c +++ b/gdb/dwarf2expr.c @@ -104,7 +104,6 @@ new_dwarf_expr_context (void) retval->num_pieces = 0; retval->pieces = 0; retval->max_recursion_depth = 0x100; - retval->mark = value_mark (); return retval; } @@ -113,7 +112,6 @@ new_dwarf_expr_context (void) void free_dwarf_expr_context (struct dwarf_expr_context *ctx) { - value_free_to_mark (ctx->mark); xfree (ctx->stack); xfree (ctx->pieces); xfree (ctx); diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index 281c65b..676b54b 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -81,10 +81,6 @@ struct dwarf_expr_context /* Offset used to relocate DW_OP_addr argument. */ CORE_ADDR offset; - /* The evaluator is value-based, and frees values up to this point - when the expression context is destroyed. */ - struct value *mark; - /* An opaque argument provided by the caller, which will be passed to all of the callback functions. */ void *baton; diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 04f16e9..64cc5e5 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -1086,7 +1086,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, struct value *retval; struct dwarf_expr_baton baton; struct dwarf_expr_context *ctx; - struct cleanup *old_chain; + struct cleanup *old_chain, *value_chain; struct objfile *objfile = dwarf2_per_cu_objfile (per_cu); volatile struct gdb_exception ex; @@ -1106,6 +1106,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, ctx = new_dwarf_expr_context (); old_chain = make_cleanup_free_dwarf_expr_context (ctx); + value_chain = make_cleanup_value_free_to_mark (value_mark ()); ctx->gdbarch = get_objfile_arch (objfile); ctx->addr_size = dwarf2_per_cu_addr_size (per_cu); @@ -1128,6 +1129,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, { if (ex.error == NOT_AVAILABLE_ERROR) { + do_cleanups (old_chain); retval = allocate_value (type); mark_value_bytes_unavailable (retval, 0, TYPE_LENGTH (type)); return retval; @@ -1150,6 +1152,9 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, c = allocate_piece_closure (per_cu, ctx->num_pieces, ctx->pieces, ctx->addr_size); + /* We must clean up the value chain after creating the piece + closure but before allocating the result. */ + do_cleanups (value_chain); retval = allocate_computed_value (type, &pieced_value_funcs, c); VALUE_FRAME_ID (retval) = frame_id; set_value_offset (retval, byte_offset); @@ -1166,6 +1171,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, if (byte_offset != 0) error (_("cannot use offset on synthetic pointer to register")); + do_cleanups (value_chain); if (gdb_regnum != -1) retval = value_from_register (type, gdb_regnum, frame); else @@ -1179,6 +1185,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, CORE_ADDR address = dwarf_expr_fetch_address (ctx, 0); int in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0); + do_cleanups (value_chain); retval = allocate_value_lazy (type); VALUE_LVAL (retval) = lval_memory; if (in_stack_memory) @@ -1201,6 +1208,13 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, val_bytes += byte_offset; n -= byte_offset; + /* Preserve VALUE because we are going to free values back + to the mark, but we still need the value contents + below. */ + value_incref (value); + do_cleanups (value_chain); + make_cleanup_value_free (value); + retval = allocate_value (type); contents = value_contents_raw (retval); if (n > TYPE_LENGTH (type)) @@ -1218,6 +1232,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, if (byte_offset + TYPE_LENGTH (type) > n) invalid_synthetic_pointer (); + do_cleanups (value_chain); retval = allocate_value (type); contents = value_contents_raw (retval); @@ -1231,6 +1246,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, break; case DWARF_VALUE_OPTIMIZED_OUT: + do_cleanups (value_chain); retval = allocate_value (type); VALUE_LVAL (retval) = not_lval; set_value_optimized_out (retval, 1); @@ -1353,6 +1369,7 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, unsigned short size, ctx = new_dwarf_expr_context (); old_chain = make_cleanup_free_dwarf_expr_context (ctx); + make_cleanup_value_free_to_mark (value_mark ()); ctx->gdbarch = get_objfile_arch (objfile); ctx->addr_size = dwarf2_per_cu_addr_size (per_cu); diff --git a/gdb/utils.c b/gdb/utils.c index 3e4a54d..6fd220a 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -448,6 +448,22 @@ make_cleanup_value_free_to_mark (struct value *mark) return make_my_cleanup (&cleanup_chain, do_value_free_to_mark, mark); } +/* Helper for make_cleanup_value_free. */ + +static void +do_value_free (void *value) +{ + value_free (value); +} + +/* Free VALUE. */ + +struct cleanup * +make_cleanup_value_free (struct value *value) +{ + return make_my_cleanup (&cleanup_chain, do_value_free, value); +} + struct cleanup * make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, void *arg, void (*free_arg) (void *))