From patchwork Fri Jan 19 20:32:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 84468 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 32D123857728 for ; Fri, 19 Jan 2024 20:33:18 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 0CBE13858C2A for ; Fri, 19 Jan 2024 20:32:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0CBE13858C2A Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0CBE13858C2A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705696372; cv=none; b=oN0HB7GWfICE/dkzes4WuKRsHAKLtxaOH46H1p3wxiwrLrVGh+9hi5YU4DesWHSkrZzZN73hjb5RuPWD3tPUYsbJJEnuPSBQD4RkN/cXEFgdnlek8ZnPuNEIrVYEyMOPnfcBqZMVxR3MHyqMhiq41TB1Yh/Yln8EVHhMvXpgJQI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705696372; c=relaxed/simple; bh=WnBDFkA8MlFAlx+ZjsZliV7gRzoSVVTG0XOX4QbpQNg=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=vCVOwtE35fWQZ0+70UtiWZKtTlOUGiqlGUFNmu+22uW5dKGP1Pyf5Jz82QCHHsfVh0nICFM68Jvrfmtuv1lDit/XlAa7v3UOu4pxrX8JSI1Y9XxaAnxKnJtNu4b5RH4G0t7miUu+34uWmLtTMzAvDeyrErwjosTEqfsUuD0UZqI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from smarchi-efficios.internal.efficios.com (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 623891E092; Fri, 19 Jan 2024 15:32:47 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v2] gdb: remove SYMBOL_*_OPS macros Date: Fri, 19 Jan 2024 15:32:32 -0500 Message-ID: <20240119203246.83805-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Spam-Status: No, score=-3495.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_STOCKGEN, SPF_HELO_PASS, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org New in v2: - remove one use of auto in findvar.c - fix formatting in frame.c Remove SYMBOL_BLOCK_OPS, SYMBOL_COMPUTED_OPS and SYMBOL_REGISTER_OPS, in favor of methods on struct symbol. More changes could be done here to improve the design and make things safer, but I just wanted to do a straightforward change to remove the macros for now. Change-Id: I27adb74a28ea3c0dc9a85c2953413437cd95ad21 Reviewed-by: Kevin Buettner --- gdb/ax-gdb.c | 14 +++++--------- gdb/compile/compile-c-symbols.c | 11 +++++------ gdb/dwarf2/loc.c | 23 +++++++++++------------ gdb/dwarf2/read.c | 2 +- gdb/eval.c | 7 ++++--- gdb/findvar.c | 13 +++++++------ gdb/frame.c | 15 +++++++++------ gdb/printcmd.c | 10 +++++----- gdb/stack.c | 16 +++++++--------- gdb/symtab.h | 25 +++++++++++++++++++------ gdb/tracepoint.c | 16 +++++++--------- 11 files changed, 80 insertions(+), 72 deletions(-) base-commit: 74d8fa2df7e7844f9b1b8fac27fe7d0b82100ab0 diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c index deadbc1bc022..c56d48683105 100644 --- a/gdb/ax-gdb.c +++ b/gdb/ax-gdb.c @@ -521,11 +521,9 @@ gen_var_ref (struct agent_expr *ax, struct axs_value *value, struct symbol *var) value->type = check_typedef (var->type ()); value->optimized_out = 0; - if (SYMBOL_COMPUTED_OPS (var) != NULL) - { - SYMBOL_COMPUTED_OPS (var)->tracepoint_var_ref (var, ax, value); - return; - } + if (const symbol_computed_ops *computed_ops = var->computed_ops (); + computed_ops != nullptr) + return computed_ops->tracepoint_var_ref (var, ax, value); /* I'm imitating the code in read_var_value. */ switch (var->aclass ()) @@ -587,8 +585,7 @@ gen_var_ref (struct agent_expr *ax, struct axs_value *value, struct symbol *var) this as an lvalue or rvalue, the caller will generate the right code. */ value->kind = axs_lvalue_register; - value->u.reg - = SYMBOL_REGISTER_OPS (var)->register_number (var, ax->gdbarch); + value->u.reg = var->register_ops ()->register_number (var, ax->gdbarch); break; /* A lot like LOC_REF_ARG, but the pointer lives directly in a @@ -596,8 +593,7 @@ gen_var_ref (struct agent_expr *ax, struct axs_value *value, struct symbol *var) because it's just like any other case where the thing has a real address. */ case LOC_REGPARM_ADDR: - ax_reg (ax, - SYMBOL_REGISTER_OPS (var)->register_number (var, ax->gdbarch)); + ax_reg (ax, var->register_ops ()->register_number (var, ax->gdbarch)); value->kind = axs_lvalue_memory; break; diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c index f55438d9ccbd..9e15709c4e54 100644 --- a/gdb/compile/compile-c-symbols.c +++ b/gdb/compile/compile-c-symbols.c @@ -563,7 +563,8 @@ generate_c_for_for_one_variable (compile_instance *compiler, stream->write (local_file.c_str (), local_file.size ()); } - if (SYMBOL_COMPUTED_OPS (sym) != NULL) + if (const symbol_computed_ops *computed_ops = sym->computed_ops (); + computed_ops != nullptr) { gdb::unique_xmalloc_ptr generated_name = c_symbol_substitution_name (sym); @@ -571,11 +572,9 @@ generate_c_for_for_one_variable (compile_instance *compiler, occurs in the middle. */ string_file local_file; - SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, &local_file, - gdbarch, - registers_used, - pc, - generated_name.get ()); + computed_ops->generate_c_location (sym, &local_file, gdbarch, + registers_used, pc, + generated_name.get ()); stream->write (local_file.c_str (), local_file.size ()); } else diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index 062bd12909fa..8a350b402587 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -515,14 +515,15 @@ locexpr_get_frame_base (struct symbol *framefunc, frame_info_ptr frame) /* If this method is called, then FRAMEFUNC is supposed to be a DWARF block. Thus, it's supposed to provide the find_frame_base_location method as well. */ - gdb_assert (SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location != NULL); + gdb_assert (framefunc->block_ops ()->find_frame_base_location != nullptr); gdbarch = get_frame_arch (frame); type = builtin_type (gdbarch)->builtin_data_ptr; dlbaton = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (framefunc); - SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location - (framefunc, get_frame_pc (frame), &start, &length); + framefunc->block_ops ()->find_frame_base_location (framefunc, + get_frame_pc (frame), + &start, &length); result = dwarf2_evaluate_loc_desc (type, frame, start, length, dlbaton->per_cu, dlbaton->per_objfile); @@ -572,14 +573,15 @@ loclist_get_frame_base (struct symbol *framefunc, frame_info_ptr frame) /* If this method is called, then FRAMEFUNC is supposed to be a DWARF block. Thus, it's supposed to provide the find_frame_base_location method as well. */ - gdb_assert (SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location != NULL); + gdb_assert (framefunc->block_ops ()->find_frame_base_location != nullptr); gdbarch = get_frame_arch (frame); type = builtin_type (gdbarch)->builtin_data_ptr; dlbaton = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (framefunc); - SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location - (framefunc, get_frame_pc (frame), &start, &length); + framefunc->block_ops ()->find_frame_base_location (framefunc, + get_frame_pc (frame), + &start, &length); result = dwarf2_evaluate_loc_desc (type, frame, start, length, dlbaton->per_cu, dlbaton->per_objfile); @@ -606,12 +608,9 @@ void func_get_frame_base_dwarf_block (struct symbol *framefunc, CORE_ADDR pc, const gdb_byte **start, size_t *length) { - if (SYMBOL_BLOCK_OPS (framefunc) != NULL) - { - const struct symbol_block_ops *ops_block = SYMBOL_BLOCK_OPS (framefunc); - - ops_block->find_frame_base_location (framefunc, pc, start, length); - } + if (const symbol_block_ops *block_ops = framefunc->block_ops (); + block_ops != nullptr) + block_ops->find_frame_base_location (framefunc, pc, start, length); else *length = 0; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 7691fe0050b6..69dbeae7ea1a 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -18693,7 +18693,7 @@ var_decode_location (struct attribute *attr, struct symbol *sym, dwarf2_symbol_mark_computed (attr, sym, cu, 0); - if (SYMBOL_COMPUTED_OPS (sym)->location_has_loclist) + if (sym->computed_ops ()->location_has_loclist) cu->has_loclist = true; } diff --git a/gdb/eval.c b/gdb/eval.c index 495effe2d039..1529063b7a99 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -1070,13 +1070,14 @@ eval_op_var_entry_value (struct type *expect_type, struct expression *exp, if (noside == EVAL_AVOID_SIDE_EFFECTS) return value::zero (sym->type (), not_lval); - if (SYMBOL_COMPUTED_OPS (sym) == NULL - || SYMBOL_COMPUTED_OPS (sym)->read_variable_at_entry == NULL) + const symbol_computed_ops *computed_ops = sym->computed_ops (); + if (computed_ops == nullptr + || computed_ops->read_variable_at_entry == nullptr) error (_("Symbol \"%s\" does not have any specific entry value"), sym->print_name ()); frame_info_ptr frame = get_selected_frame (NULL); - return SYMBOL_COMPUTED_OPS (sym)->read_variable_at_entry (sym, frame); + return computed_ops->read_variable_at_entry (sym, frame); } /* Helper function that implements the body of OP_VAR_MSYM_VALUE. */ diff --git a/gdb/findvar.c b/gdb/findvar.c index 37b859c2347d..2fcfccda1c7a 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -318,8 +318,9 @@ address_to_signed_pointer (struct gdbarch *gdbarch, struct type *type, enum symbol_needs_kind symbol_read_needs (struct symbol *sym) { - if (SYMBOL_COMPUTED_OPS (sym) != NULL) - return SYMBOL_COMPUTED_OPS (sym)->get_symbol_read_needs (sym); + if (const symbol_computed_ops *computed_ops = sym->computed_ops (); + computed_ops != nullptr) + return computed_ops->get_symbol_read_needs (sym); switch (sym->aclass ()) { @@ -498,8 +499,8 @@ language_defn::read_var_value (struct symbol *var, if (frame != NULL) frame = get_hosting_frame (var, var_block, frame); - if (SYMBOL_COMPUTED_OPS (var) != NULL) - return SYMBOL_COMPUTED_OPS (var)->read_variable (var, frame); + if (const symbol_computed_ops *computed_ops = var->computed_ops ()) + return computed_ops->read_variable (var, frame); switch (var->aclass ()) { @@ -621,8 +622,8 @@ language_defn::read_var_value (struct symbol *var, case LOC_REGISTER: case LOC_REGPARM_ADDR: { - int regno = SYMBOL_REGISTER_OPS (var) - ->register_number (var, get_frame_arch (frame)); + const symbol_register_ops *reg_ops = var->register_ops (); + int regno = reg_ops->register_number (var, get_frame_arch (frame)); if (var->aclass () == LOC_REGPARM_ADDR) addr = value_as_address diff --git a/gdb/frame.c b/gdb/frame.c index d95d63eb0f60..29cda7da7495 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -3146,12 +3146,15 @@ frame_follow_static_link (frame_info_ptr frame) /* If we don't know how to compute FRAME's base address, don't give up: maybe the frame we are looking for is upper in the stack frame. */ - if (framefunc != NULL - && SYMBOL_BLOCK_OPS (framefunc) != NULL - && SYMBOL_BLOCK_OPS (framefunc)->get_frame_base != NULL - && (SYMBOL_BLOCK_OPS (framefunc)->get_frame_base (framefunc, frame) - == upper_frame_base)) - break; + if (framefunc != nullptr) + { + if (const symbol_block_ops *block_ops = framefunc->block_ops (); + (block_ops != nullptr + && block_ops->get_frame_base != nullptr + && (block_ops->get_frame_base (framefunc, frame) + == upper_frame_base))) + break; + } } return frame; diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 7a638ab8b894..f4c79ed6d247 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -1643,10 +1643,10 @@ info_address_command (const char *exp, int from_tty) section = NULL; gdbarch = sym->arch (); - if (SYMBOL_COMPUTED_OPS (sym) != NULL) + if (const symbol_computed_ops *computed_ops = sym->computed_ops (); + computed_ops != nullptr) { - SYMBOL_COMPUTED_OPS (sym)->describe_location (sym, context_pc, - gdb_stdout); + computed_ops->describe_location (sym, context_pc, gdb_stdout); gdb_printf (".\n"); return; } @@ -1684,7 +1684,7 @@ info_address_command (const char *exp, int from_tty) architecture at this point. We assume the objfile architecture will contain all the standard registers that occur in debug info in that objfile. */ - regno = SYMBOL_REGISTER_OPS (sym)->register_number (sym, gdbarch); + regno = sym->register_ops ()->register_number (sym, gdbarch); if (sym->is_argument ()) gdb_printf (_("an argument in register %s"), @@ -1712,7 +1712,7 @@ info_address_command (const char *exp, int from_tty) case LOC_REGPARM_ADDR: /* Note comment at LOC_REGISTER. */ - regno = SYMBOL_REGISTER_OPS (sym)->register_number (sym, gdbarch); + regno = sym->register_ops ()->register_number (sym, gdbarch); gdb_printf (_("address of an argument in register %s"), gdbarch_register_name (gdbarch, regno)); break; diff --git a/gdb/stack.c b/gdb/stack.c index 96a9cd480ac5..31b89342bd18 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -547,18 +547,16 @@ read_frame_arg (const frame_print_options &fp_opts, } } - if (SYMBOL_COMPUTED_OPS (sym) != NULL - && SYMBOL_COMPUTED_OPS (sym)->read_variable_at_entry != NULL - && fp_opts.print_entry_values != print_entry_values_no - && (fp_opts.print_entry_values != print_entry_values_if_needed - || !val || val->optimized_out ())) + if (const symbol_computed_ops *computed_ops = sym->computed_ops (); + (computed_ops != nullptr + && computed_ops->read_variable_at_entry != nullptr + && fp_opts.print_entry_values != print_entry_values_no + && (fp_opts.print_entry_values != print_entry_values_if_needed || !val + || val->optimized_out ()))) { try { - const struct symbol_computed_ops *ops; - - ops = SYMBOL_COMPUTED_OPS (sym); - entryval = ops->read_variable_at_entry (sym, frame); + entryval = computed_ops->read_variable_at_entry (sym, frame); } catch (const gdb_exception_error &except) { diff --git a/gdb/symtab.h b/gdb/symtab.h index eecd999b7e6f..729c003f1c10 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1267,6 +1267,21 @@ struct symbol : public general_symbol_info, public allocate_on_obstack return symbol_impls[this->m_aclass_index]; } + const symbol_block_ops *block_ops () const + { + return this->impl ().ops_block; + } + + const symbol_computed_ops *computed_ops () const + { + return this->impl ().ops_computed; + } + + const symbol_register_ops *register_ops () const + { + return this->impl ().ops_register; + } + address_class aclass () const { return this->impl ().aclass; @@ -1543,17 +1558,15 @@ struct block_symbol /* Note: There is no accessor macro for symbol.owner because it is "private". */ -#define SYMBOL_COMPUTED_OPS(symbol) ((symbol)->impl ().ops_computed) -#define SYMBOL_BLOCK_OPS(symbol) ((symbol)->impl ().ops_block) -#define SYMBOL_REGISTER_OPS(symbol) ((symbol)->impl ().ops_register) #define SYMBOL_LOCATION_BATON(symbol) (symbol)->aux_value inline const block * symbol::value_block () const { - if (SYMBOL_BLOCK_OPS (this) != nullptr - && SYMBOL_BLOCK_OPS (this)->get_block_value != nullptr) - return SYMBOL_BLOCK_OPS (this)->get_block_value (this); + if (const symbol_block_ops *block_ops = this->block_ops (); + block_ops != nullptr && block_ops->get_block_value != nullptr) + return block_ops->get_block_value (this); + return m_value.block; } diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index 8f04f1a95018..abe8d2b45f44 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -943,7 +943,7 @@ collection_list::collect_symbol (struct symbol *sym, add_memrange (gdbarch, memrange_absolute, offset, len, scope); break; case LOC_REGISTER: - reg = SYMBOL_REGISTER_OPS (sym)->register_number (sym, gdbarch); + reg = sym->register_ops ()->register_number (sym, gdbarch); if (info_verbose) gdb_printf ("LOC_REG[parm] %s: ", sym->print_name ()); add_local_register (gdbarch, reg, scope); @@ -2502,10 +2502,10 @@ info_scope_command (const char *args_in, int from_tty) gdb_printf ("Symbol %s is ", symname); - if (SYMBOL_COMPUTED_OPS (sym) != NULL) - SYMBOL_COMPUTED_OPS (sym)->describe_location (sym, - block->entry_pc (), - gdb_stdout); + if (const symbol_computed_ops *computed_ops = sym->computed_ops (); + computed_ops != nullptr) + computed_ops->describe_location (sym, block->entry_pc (), + gdb_stdout); else { switch (sym->aclass ()) @@ -2539,8 +2539,7 @@ info_scope_command (const char *args_in, int from_tty) We assume the objfile architecture will contain all the standard registers that occur in debug info in that objfile. */ - regno = SYMBOL_REGISTER_OPS (sym)->register_number (sym, - gdbarch); + regno = sym->register_ops ()->register_number (sym, gdbarch); if (sym->is_argument ()) gdb_printf ("an argument in register $%s", @@ -2563,8 +2562,7 @@ info_scope_command (const char *args_in, int from_tty) break; case LOC_REGPARM_ADDR: /* Note comment at LOC_REGISTER. */ - regno = SYMBOL_REGISTER_OPS (sym)->register_number (sym, - gdbarch); + regno = sym->register_ops ()->register_number (sym, gdbarch); gdb_printf ("the address of an argument, in register $%s", gdbarch_register_name (gdbarch, regno)); break;