From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31165 invoked by alias); 13 Jan 2009 19:09:49 -0000 Received: (qmail 31157 invoked by uid 22791); 13 Jan 2009 19:09:47 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,MSGID_FROM_MTA_HEADER,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mtagate3.de.ibm.com (HELO mtagate3.de.ibm.com) (195.212.29.152) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 13 Jan 2009 19:08:55 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate3.de.ibm.com (8.13.8/8.13.8) with ESMTP id n0DJ8oeZ193610 for ; Tue, 13 Jan 2009 19:08:50 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id n0DJ8oFO3149860 for ; Tue, 13 Jan 2009 20:08:50 +0100 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n0DJ8oG9020540 for ; Tue, 13 Jan 2009 20:08:50 +0100 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id n0DJ8ouI020537 for ; Tue, 13 Jan 2009 20:08:50 +0100 Message-Id: <200901131908.n0DJ8ouI020537@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Tue, 13 Jan 2009 20:08:50 +0100 Subject: [rfc] Remove deprecated_safe_get_selected_frame call from read_var_value To: gdb-patches@sourceware.org Date: Tue, 13 Jan 2009 19:09:00 -0000 From: "Ulrich Weigand" MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2009-01/txt/msg00293.txt.bz2 Hello, it turns out the patch in: http://sourceware.org/ml/gdb-patches/2008-08/msg00565.html doesn't really fix the problem, because read_var_value will use the default selected frame when called with a NULL frame, so it might generate lval_register values even in that case. However, as noted in a comment, this behaviour of read_var_value is questionable in any case: /* FIXME drow/2003-09-06: this call to the selected frame should be pushed upwards to the callers. */ I've reviewed all callers of read_var_value, and most either already pass in a non-NULL frame value, or else deliberately pass in NULL because they query a global/static variable. The only exceptions are value_of_variable, where the get_selected_frame call can be easily pushed to, and locate_var_value, which sometimes does get called with a NULL frame, even for frame-local variables. Note that code like: else if (symbol_read_needs_frame (var)) return locate_var_value (var, block_innermost_frame (exp->elts[pc + 1].block)); else return locate_var_value (var, NULL); in eval.c:evaluate_subexp_for_address doesn't have quite the effect one might think it has, because the parsers may set exp->elts[..].block to NULL to indicate the "current block"; in this case the block_innermost_frame call will return a NULL frame as well. This can cause assertion failures when attempting to print the address of a variable that is held in a register. On the other hand, locate_var_value is only called from eval.c routines, where we'd really prefer an interface like value_of_variable instead of an interface like read_var_value. The following patch therefore removes locate_var_value in favor of a new address_of_variable routine -- which because of lazy evaluation now simply calls value_of_variable. This fixes the problems mentioned above. Tested with no regression on powerpc-linux. Comments welcome -- does this look right the right thing to do? Bye, Ulrich ChangeLog: * value.h (address_of_variable): Add prototype. (locate_var_value): Remove prototype. * findvar.c (read_var_value): Do not attempt to default frame to selected frame. (locate_var_value): Remove function. * valops.c (value_of_variable): Retrieve selected frame for symbols that require a frame when called with NULL block. * valops.c (address_of_variable): New function. * eval.c (evaluate_subexp_for_address): Call address_of_variable instead of calling locate_var_value. (evaluate_subexp_with_coercion): Likewise. Index: gdb/eval.c =================================================================== RCS file: /cvs/src/src/gdb/eval.c,v retrieving revision 1.104 diff -c -p -r1.104 eval.c *** gdb/eval.c 3 Jan 2009 05:57:51 -0000 1.104 --- gdb/eval.c 13 Jan 2009 18:27:59 -0000 *************** evaluate_subexp_for_address (struct expr *** 2560,2572 **** return value_zero (type, not_lval); } - else if (symbol_read_needs_frame (var)) - return - locate_var_value - (var, - block_innermost_frame (exp->elts[pc + 1].block)); else ! return locate_var_value (var, NULL); case OP_SCOPE: tem = longest_to_int (exp->elts[pc + 2].longconst); --- 2560,2567 ---- return value_zero (type, not_lval); } else ! return address_of_variable (var, exp->elts[pc + 1].block); case OP_SCOPE: tem = longest_to_int (exp->elts[pc + 2].longconst); *************** evaluate_subexp_with_coercion (struct ex *** 2620,2625 **** --- 2615,2621 ---- int pc; struct value *val; struct symbol *var; + struct type *type; pc = (*pos); op = exp->elts[pc].opcode; *************** evaluate_subexp_with_coercion (struct ex *** 2628,2641 **** { case OP_VAR_VALUE: var = exp->elts[pc + 2].symbol; ! if (TYPE_CODE (check_typedef (SYMBOL_TYPE (var))) == TYPE_CODE_ARRAY && CAST_IS_CONVERSION) { (*pos) += 4; ! val = ! locate_var_value ! (var, block_innermost_frame (exp->elts[pc + 1].block)); ! return value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (check_typedef (SYMBOL_TYPE (var)))), val); } /* FALLTHROUGH */ --- 2624,2636 ---- { case OP_VAR_VALUE: var = exp->elts[pc + 2].symbol; ! type = check_typedef (SYMBOL_TYPE (var)); ! if (TYPE_CODE (type) == TYPE_CODE_ARRAY && CAST_IS_CONVERSION) { (*pos) += 4; ! val = address_of_variable (var, exp->elts[pc + 1].block); ! return value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (type)), val); } /* FALLTHROUGH */ Index: gdb/findvar.c =================================================================== RCS file: /cvs/src/src/gdb/findvar.c,v retrieving revision 1.120 diff -c -p -r1.120 findvar.c *** gdb/findvar.c 3 Jan 2009 05:57:51 -0000 1.120 --- gdb/findvar.c 13 Jan 2009 18:27:59 -0000 *************** symbol_read_needs_frame (struct symbol * *** 382,389 **** /* Given a struct symbol for a variable, and a stack frame id, read the value of the variable and return a (pointer to a) struct value containing the value. ! If the variable cannot be found, return a zero pointer. ! If FRAME is NULL, use the selected frame. */ struct value * read_var_value (struct symbol *var, struct frame_info *frame) --- 382,388 ---- /* Given a struct symbol for a variable, and a stack frame id, read the value of the variable and return a (pointer to a) struct value containing the value. ! If the variable cannot be found, return a zero pointer. */ struct value * read_var_value (struct symbol *var, struct frame_info *frame) *************** read_var_value (struct symbol *var, stru *** 405,414 **** len = TYPE_LENGTH (type); ! /* FIXME drow/2003-09-06: this call to the selected frame should be ! pushed upwards to the callers. */ ! if (frame == NULL) ! frame = deprecated_safe_get_selected_frame (); switch (SYMBOL_CLASS (var)) { --- 404,411 ---- len = TYPE_LENGTH (type); ! if (symbol_read_needs_frame (var)) ! gdb_assert (frame); switch (SYMBOL_CLASS (var)) { *************** address_from_register (struct type *type *** 657,714 **** return result; } - - - /* Given a struct symbol for a variable or function, - and a stack frame id, - return a (pointer to a) struct value containing the properly typed - address. */ - - struct value * - locate_var_value (struct symbol *var, struct frame_info *frame) - { - struct gdbarch *gdbarch; - CORE_ADDR addr = 0; - struct type *type = SYMBOL_TYPE (var); - struct value *lazy_value; - - /* Evaluate it first; if the result is a memory address, we're fine. - Lazy evaluation pays off here. */ - - lazy_value = read_var_value (var, frame); - if (lazy_value == 0) - error (_("Address of \"%s\" is unknown."), SYMBOL_PRINT_NAME (var)); - - if ((VALUE_LVAL (lazy_value) == lval_memory && value_lazy (lazy_value)) - || TYPE_CODE (type) == TYPE_CODE_FUNC) - { - struct value *val; - - addr = VALUE_ADDRESS (lazy_value); - val = value_from_pointer (lookup_pointer_type (type), addr); - return val; - } - - /* Not a memory address; check what the problem was. */ - switch (VALUE_LVAL (lazy_value)) - { - case lval_register: - gdb_assert (frame); - gdbarch = get_frame_arch (frame); - gdb_assert (gdbarch_register_name - (gdbarch, VALUE_REGNUM (lazy_value)) != NULL - && *gdbarch_register_name - (gdbarch, VALUE_REGNUM (lazy_value)) != '\0'); - error (_("Address requested for identifier " - "\"%s\" which is in register $%s"), - SYMBOL_PRINT_NAME (var), - gdbarch_register_name (gdbarch, VALUE_REGNUM (lazy_value))); - break; - - default: - error (_("Can't take address of \"%s\" which isn't an lvalue."), - SYMBOL_PRINT_NAME (var)); - break; - } - return 0; /* For lint -- never reached */ - } --- 654,656 ---- Index: gdb/valops.c =================================================================== RCS file: /cvs/src/src/gdb/valops.c,v retrieving revision 1.206 diff -c -p -r1.206 valops.c *** gdb/valops.c 13 Jan 2009 10:34:31 -0000 1.206 --- gdb/valops.c 13 Jan 2009 18:28:00 -0000 *************** struct value * *** 988,998 **** value_of_variable (struct symbol *var, struct block *b) { struct value *val; ! struct frame_info *frame = NULL; ! if (!b) ! frame = NULL; /* Use selected frame. */ ! else if (symbol_read_needs_frame (var)) { frame = block_innermost_frame (b); if (!frame) --- 988,1000 ---- value_of_variable (struct symbol *var, struct block *b) { struct value *val; ! struct frame_info *frame; ! if (!symbol_read_needs_frame (var)) ! frame = NULL; ! else if (!b) ! frame = get_selected_frame (_("No frame selected.")); ! else { frame = block_innermost_frame (b); if (!frame) *************** value_of_variable (struct symbol *var, s *** 1013,1018 **** --- 1015,1068 ---- return val; } + struct value * + address_of_variable (struct symbol *var, struct block *b) + { + struct type *type = SYMBOL_TYPE (var); + struct value *val; + + /* Evaluate it first; if the result is a memory address, we're fine. + Lazy evaluation pays off here. */ + + val = value_of_variable (var, b); + + if ((VALUE_LVAL (val) == lval_memory && value_lazy (val)) + || TYPE_CODE (type) == TYPE_CODE_FUNC) + { + CORE_ADDR addr = VALUE_ADDRESS (val); + return value_from_pointer (lookup_pointer_type (type), addr); + } + + /* Not a memory address; check what the problem was. */ + switch (VALUE_LVAL (val)) + { + case lval_register: + { + struct frame_info *frame; + const char *regname; + + frame = frame_find_by_id (VALUE_FRAME_ID (val)); + gdb_assert (frame); + + regname = gdbarch_register_name (get_frame_arch (frame), + VALUE_REGNUM (val)); + gdb_assert (regname && *regname); + + error (_("Address requested for identifier " + "\"%s\" which is in register $%s"), + SYMBOL_PRINT_NAME (var), regname); + break; + } + + default: + error (_("Can't take address of \"%s\" which isn't an lvalue."), + SYMBOL_PRINT_NAME (var)); + break; + } + + return val; + } + /* Return one if VAL does not live in target memory, but should in order to operate on it. Otherwise return zero. */ Index: gdb/value.h =================================================================== RCS file: /cvs/src/src/gdb/value.h,v retrieving revision 1.128 diff -c -p -r1.128 value.h *** gdb/value.h 13 Jan 2009 10:34:31 -0000 1.128 --- gdb/value.h 13 Jan 2009 18:28:00 -0000 *************** extern CORE_ADDR address_from_register ( *** 310,315 **** --- 310,317 ---- extern struct value *value_of_variable (struct symbol *var, struct block *b); + extern struct value *address_of_variable (struct symbol *var, struct block *b); + extern struct value *value_of_register (int regnum, struct frame_info *frame); struct value *value_of_register_lazy (struct frame_info *frame, int regnum); *************** extern int symbol_read_needs_frame (stru *** 319,327 **** extern struct value *read_var_value (struct symbol *var, struct frame_info *frame); - extern struct value *locate_var_value (struct symbol *var, - struct frame_info *frame); - extern struct value *allocate_value (struct type *type); extern struct value *allocate_value_lazy (struct type *type); extern void allocate_value_contents (struct value *value); --- 321,326 ---- -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com