From patchwork Mon Apr 14 17:00:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 537 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 0BAE6360075 for ; Mon, 14 Apr 2014 10:00:31 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14314964) id 9756A62B02985; Mon, 14 Apr 2014 10:00:31 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id E182962B02996 for ; Mon, 14 Apr 2014 10:00:30 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:subject:to:date:from:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=c7g Iu7t9w1X4a4hz9YRZ8K6bDZI79SfePPLnc+xHqIZruM8iw4fL08wKVMb0vbOdRu4 SpW5UnL22tCewWKncn6Fv+2h2/bAmiqTtfc7MXZ6JmQXR34O2im3VbDaVgPEUeEr upWkFR1KhtMy8AA3H2fVLLKV2Y1Lmz1IfBLA3zec= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:subject:to:date:from:mime-version :content-type:content-transfer-encoding; s=default; bh=ofKrvVZKy oXr7xZPM2PWcbnvopg=; b=mxEEfQALYYk0rfx1uYq3PAH82kRXMnQNNVYEOOAfr 7THEr0HSjKMJVoS36MhqTdVlIr17t65uCLHynzW94rNV5XdlxqNC5AEv29JEOeiK pz3n6HwLTTAj9AByqX/2mTbKGsInekNeLNkIlfekt7n39IzQbNCrc4iVJ6BKpYFR lQ= Received: (qmail 25211 invoked by alias); 14 Apr 2014 17:00:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25187 invoked by uid 89); 14 Apr 2014 17:00:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, MSGID_FROM_MTA_HEADER, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: e06smtp10.uk.ibm.com Received: from e06smtp10.uk.ibm.com (HELO e06smtp10.uk.ibm.com) (195.75.94.106) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 14 Apr 2014 17:00:23 +0000 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 Apr 2014 18:00:19 +0100 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp10.uk.ibm.com (192.168.101.140) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 14 Apr 2014 18:00:17 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id F2BEF17D8056 for ; Mon, 14 Apr 2014 18:01:11 +0100 (BST) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s3EH0Gev61603912 for ; Mon, 14 Apr 2014 17:00:16 GMT Received: from d06av02.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s3EH0Ghi020773 for ; Mon, 14 Apr 2014 11:00:16 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with SMTP id s3EH0F7v020732 for ; Mon, 14 Apr 2014 11:00:15 -0600 Message-Id: <201404141700.s3EH0F7v020732@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Mon, 14 Apr 2014 19:00:15 +0200 Subject: [RFC] Use address_from_register in dwarf2-frame.c:read_addr_from_reg To: gdb-patches@sourceware.org Date: Mon, 14 Apr 2014 19:00:15 +0200 (CEST) From: "Ulrich Weigand" MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14041417-4966-0000-0000-0000091164D6 X-DH-Original-To: gdb@patchwork.siddhesh.in Hello, with recent compilers I'm seeing lots of failures in the GDB testsuite on SPU, because debug data now tends to use features more frequently that rely on the debugger using DWARF frame unwinding. I had never enabled that on spu, and just used prologue parsing instead, which shows up as a problem now ... However, I cannot simply enable DWARF unwinding, since dwarf2-frame.c common code does not handle the situation well (that we have on SPU) where the stack and/or frame pointer are maintained in *vector* registers. This is because read_addr_from_reg is hard-coded to assume that such pointers can be read from registers via a simple get_frame_register / unpack_pointer operation. Now, there *is* a routine address_from_register that calls into the appropriate tdep routines to handle pointer values in "weird" registers like on SPU, but it turns out I cannot simply change dwarf2-frame.c to use address_from_register. This is because address_from_register uses value_from_register to create a (temporary) value, and that routine at some point calls get_frame_id in order to set up that value's VALUE_FRAME_ID entry. However, the dwarf2-frame.c read_addr_from_reg routine will be called during early unwinding (to unwind the frame's CFA), at which point the frame's ID is not actually known yet! This would cause an assert. I not completely sure what the best fix is. One way might be to recognize that VALUE_FRAME_ID is only needed in the value returned by value_from_register if that value is later used as an lvalue. But this is obviously never done to the temporary value used in address_from_register. So, if we could change address_from_register to not call value_from_register but instead accept constructing a value that doesn't have VALUE_FRAME_ID set, things should be fine. To do that, we can change the value_from_register callback to accept a FRAME_ID instead of a FRAME; the only existing uses of the FRAME argument were either to extract its frame ID, or its gdbarch. (To keep a way of getting at the latter, we also change the callback's type from "f" to "m".) Together with the required follow-on changes in the existing value_from_register implementations (including the default one), this seems to fix the problem. As another minor interface cleanup, I've removed the explicit TYPE argument from address_from_register. This routine really always uses a default pointer type, and in the new implementation it -to some extent- relies on that fact, in that it will now no longer handle types that require gdbarch_convert_register_p handling. Does this sound reasonable? Comments or alternative suggestions would be appreciated! Tested on powerpc64-linux and spu-elf with no regressions. If there are no objections, I'm planning on committing this later this week. Bye, Ulrich ChangeLog: * gdbarch.sh (value_from_register): Make class "m" instead of "f". Replace FRAME argument with FRAME_ID. * gdbarch.c, gdbarch.h: Regenerate. * findvar.c (default_value_from_register): Add GDBARCH argument; replace FRAME by FRAME_ID. No longer call get_frame_id. (value_from_register): Update call to gdbarch_value_from_register. * value.h (default_value_from_register): Update prototype. * s390-linux-tdep.c (s390_value_from_register): Update interface and call to default_value_from_register. * spu-tdep.c (spu_value_from_register): Likewise. * findvar.c (address_from_register): Remove TYPE argument. Do not call value_from_register; use gdbarch_value_from_register with null_frame_id instead. * value.h (address_from_register): Update prototype. * dwarf2-frame.c (read_addr_from_reg): Use address_from_register. * dwarf2loc.c (dwarf_expr_read_addr_from_reg): Update for address_from_register interface change. Index: binutils-gdb/gdb/dwarf2-frame.c =================================================================== --- binutils-gdb.orig/gdb/dwarf2-frame.c +++ binutils-gdb/gdb/dwarf2-frame.c @@ -291,15 +291,9 @@ read_addr_from_reg (void *baton, int reg { struct frame_info *this_frame = (struct frame_info *) baton; struct gdbarch *gdbarch = get_frame_arch (this_frame); - int regnum; - gdb_byte *buf; - - regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, reg); - - buf = alloca (register_size (gdbarch, regnum)); - get_frame_register (this_frame, regnum, buf); + int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, reg); - return unpack_pointer (register_type (gdbarch, regnum), buf); + return address_from_register (regnum, this_frame); } /* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback. */ Index: binutils-gdb/gdb/dwarf2loc.c =================================================================== --- binutils-gdb.orig/gdb/dwarf2loc.c +++ binutils-gdb/gdb/dwarf2loc.c @@ -317,13 +317,9 @@ dwarf_expr_read_addr_from_reg (void *bat { struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton; struct gdbarch *gdbarch = get_frame_arch (debaton->frame); - CORE_ADDR result; - int regnum; + int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, dwarf_regnum); - regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, dwarf_regnum); - result = address_from_register (builtin_type (gdbarch)->builtin_data_ptr, - regnum, debaton->frame); - return result; + return address_from_register (regnum, debaton->frame); } /* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback. */ Index: binutils-gdb/gdb/findvar.c =================================================================== --- binutils-gdb.orig/gdb/findvar.c +++ binutils-gdb/gdb/findvar.c @@ -625,15 +625,14 @@ read_var_value (struct symbol *var, stru /* Install default attributes for register values. */ struct value * -default_value_from_register (struct type *type, int regnum, - struct frame_info *frame) +default_value_from_register (struct gdbarch *gdbarch, struct type *type, + int regnum, struct frame_id frame_id) { - struct gdbarch *gdbarch = get_frame_arch (frame); int len = TYPE_LENGTH (type); struct value *value = allocate_value (type); VALUE_LVAL (value) = lval_register; - VALUE_FRAME_ID (value) = get_frame_id (frame); + VALUE_FRAME_ID (value) = frame_id; VALUE_REGNUM (value) = regnum; /* Any structure stored in more than one register will always be @@ -741,7 +740,8 @@ value_from_register (struct type *type, else { /* Construct the value. */ - v = gdbarch_value_from_register (gdbarch, type, regnum, frame); + v = gdbarch_value_from_register (gdbarch, type, + regnum, get_frame_id (frame)); /* Get the data. */ read_frame_register_value (v, frame); @@ -750,18 +750,30 @@ value_from_register (struct type *type, return v; } -/* Return contents of register REGNUM in frame FRAME as address, - interpreted as value of type TYPE. Will abort if register - value is not available. */ +/* Return contents of register REGNUM in frame FRAME as address. + Will abort if register value is not available. */ CORE_ADDR -address_from_register (struct type *type, int regnum, struct frame_info *frame) +address_from_register (int regnum, struct frame_info *frame) { + struct gdbarch *gdbarch = get_frame_arch (frame); + struct type *type = builtin_type (gdbarch)->builtin_data_ptr; struct value *value; CORE_ADDR result; - value = value_from_register (type, regnum, frame); - gdb_assert (value); + /* This routine may be called during early unwinding, at a time + where the ID of FRAME is not yet known. Calling value_from_register + would therefore abort in get_frame_id. However, since we only need + a temporary value that is never used as lvalue, we actually do not + really need to set its VALUE_FRAME_ID. Therefore, we re-implement + the core of value_from_register, but use the null_frame_id. + + This works only if we do not require a special conversion routine, + which is true for plain pointer types for all current targets. */ + gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type)); + + value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id); + read_frame_register_value (value, frame); if (value_optimized_out (value)) { Index: binutils-gdb/gdb/gdbarch.c =================================================================== --- binutils-gdb.orig/gdb/gdbarch.c +++ binutils-gdb/gdb/gdbarch.c @@ -2381,13 +2381,13 @@ set_gdbarch_value_to_register (struct gd } struct value * -gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_info *frame) +gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id) { gdb_assert (gdbarch != NULL); gdb_assert (gdbarch->value_from_register != NULL); if (gdbarch_debug >= 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_value_from_register called\n"); - return gdbarch->value_from_register (type, regnum, frame); + return gdbarch->value_from_register (gdbarch, type, regnum, frame_id); } void Index: binutils-gdb/gdb/gdbarch.h =================================================================== --- binutils-gdb.orig/gdb/gdbarch.h +++ binutils-gdb/gdb/gdbarch.h @@ -422,12 +422,12 @@ extern void gdbarch_value_to_register (s extern void set_gdbarch_value_to_register (struct gdbarch *gdbarch, gdbarch_value_to_register_ftype *value_to_register); /* Construct a value representing the contents of register REGNUM in - frame FRAME, interpreted as type TYPE. The routine needs to + frame FRAME_ID, interpreted as type TYPE. The routine needs to allocate and return a struct value with all value attributes (but not the value contents) filled in. */ -typedef struct value * (gdbarch_value_from_register_ftype) (struct type *type, int regnum, struct frame_info *frame); -extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_info *frame); +typedef struct value * (gdbarch_value_from_register_ftype) (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id); +extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id); extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register); typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf); Index: binutils-gdb/gdb/gdbarch.sh =================================================================== --- binutils-gdb.orig/gdb/gdbarch.sh +++ binutils-gdb/gdb/gdbarch.sh @@ -500,10 +500,10 @@ m:int:convert_register_p:int regnum, str f:int:register_to_value:struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep:frame, regnum, type, buf, optimizedp, unavailablep:0 f:void:value_to_register:struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf:frame, regnum, type, buf:0 # Construct a value representing the contents of register REGNUM in -# frame FRAME, interpreted as type TYPE. The routine needs to +# frame FRAME_ID, interpreted as type TYPE. The routine needs to # allocate and return a struct value with all value attributes # (but not the value contents) filled in. -f:struct value *:value_from_register:struct type *type, int regnum, struct frame_info *frame:type, regnum, frame::default_value_from_register::0 +m:struct value *:value_from_register:struct type *type, int regnum, struct frame_id frame_id:type, regnum, frame_id::default_value_from_register::0 # m:CORE_ADDR:pointer_to_address:struct type *type, const gdb_byte *buf:type, buf::unsigned_pointer_to_address::0 m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0 Index: binutils-gdb/gdb/s390-linux-tdep.c =================================================================== --- binutils-gdb.orig/gdb/s390-linux-tdep.c +++ binutils-gdb/gdb/s390-linux-tdep.c @@ -380,11 +380,11 @@ s390_pseudo_register_write (struct gdbar registers, even though we are otherwise a big-endian platform. */ static struct value * -s390_value_from_register (struct type *type, int regnum, - struct frame_info *frame) +s390_value_from_register (struct gdbarch *gdbarch, struct type *type, + int regnum, struct frame_id frame_id) { - struct value *value = default_value_from_register (type, regnum, frame); - + struct value *value = default_value_from_register (gdbarch, type, + regnum, frame_id); check_typedef (type); if (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM Index: binutils-gdb/gdb/spu-tdep.c =================================================================== --- binutils-gdb.orig/gdb/spu-tdep.c +++ binutils-gdb/gdb/spu-tdep.c @@ -314,10 +314,11 @@ spu_pseudo_register_write (struct gdbarc /* Value conversion -- access scalar values at the preferred slot. */ static struct value * -spu_value_from_register (struct type *type, int regnum, - struct frame_info *frame) +spu_value_from_register (struct gdbarch *gdbarch, struct type *type, + int regnum, struct frame_id frame_id) { - struct value *value = default_value_from_register (type, regnum, frame); + struct value *value = default_value_from_register (gdbarch, type, + regnum, frame_id); int len = TYPE_LENGTH (type); if (regnum < SPU_NUM_GPRS && len < 16) Index: binutils-gdb/gdb/value.h =================================================================== --- binutils-gdb.orig/gdb/value.h +++ binutils-gdb/gdb/value.h @@ -579,9 +579,10 @@ extern struct value *value_from_contents CORE_ADDR); extern struct value *value_from_contents (struct type *, const gdb_byte *); -extern struct value *default_value_from_register (struct type *type, +extern struct value *default_value_from_register (struct gdbarch *gdbarch, + struct type *type, int regnum, - struct frame_info *frame); + struct frame_id frame_id); extern void read_frame_register_value (struct value *value, struct frame_info *frame); @@ -589,7 +590,7 @@ extern void read_frame_register_value (s extern struct value *value_from_register (struct type *type, int regnum, struct frame_info *frame); -extern CORE_ADDR address_from_register (struct type *type, int regnum, +extern CORE_ADDR address_from_register (int regnum, struct frame_info *frame); extern struct value *value_of_variable (struct symbol *var,