From patchwork Fri Aug 28 17:20:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 8498 Received: (qmail 87635 invoked by alias); 28 Aug 2015 17:20:37 -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 87605 invoked by uid 89); 28 Aug 2015 17:20:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: e06smtp15.uk.ibm.com Received: from e06smtp15.uk.ibm.com (HELO e06smtp15.uk.ibm.com) (195.75.94.111) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 28 Aug 2015 17:20:34 +0000 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 28 Aug 2015 18:20:28 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp15.uk.ibm.com (192.168.101.145) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 28 Aug 2015 18:20:26 +0100 X-MailFrom: uweigand@de.ibm.com X-RcptTo: gdb-patches@sourceware.org Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 638631B08061 for ; Fri, 28 Aug 2015 18:21:59 +0100 (BST) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t7SHKNdm20054088 for ; Fri, 28 Aug 2015 17:20:26 GMT Received: from d06av05.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t7SHKEN5011943 for ; Fri, 28 Aug 2015 11:20:14 -0600 Received: from oc7340732750.ibm.com (icon-9-164-150-178.megacenter.de.ibm.com [9.164.150.178]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t7SHKDfe011843; Fri, 28 Aug 2015 11:20:13 -0600 Received: by oc7340732750.ibm.com (Postfix, from userid 500) id E17E48775; Fri, 28 Aug 2015 19:20:03 +0200 (CEST) Subject: Re: Cell multi-arch symbol lookup broken (Re: [PATCH] solib_global_lookup: Fetch arch from objfile.) To: xdje42@gmail.com (Doug Evans) Date: Fri, 28 Aug 2015 19:20:03 +0200 (CEST) From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: from "Doug Evans" at Aug 28, 2015 09:05:58 AM MIME-Version: 1.0 Message-Id: <20150828172003.E17E48775@oc7340732750.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15082817-0021-0000-0000-0000051F75BE Doug Evans wrote: > > Now, the "target_gdbarch" is a bit of a special case. It used > > to have a distinct semantics: the architecture used at the > > target interface level. (Think of it as the architecture that > > defines the contents of the register packets of the remote > > gdbserver protocol.) However, since the remote protocol was > > extended to actually support using *multiple* different > > architectures, there's no longer a need for target_gdbarch > > as a distinct concept. And in fact, it's now simply defined > > as the architecture of the current inferior: > > > > struct gdbarch * > > target_gdbarch (void) > > { > > return current_inferior ()->gdbarch; > > } > > > > I'd be in favor of inlining this function into every user, > > and starting to replace "current_inferior" with any given > > inferior that we may be actually operating on in these > > places. > > I'm all for removal of references to global state (where appropriate) > and passing in the needed context. > IWBN to also clean up the types while we're at it. Sure. I had thought of leaving the "gdbarch" name for the target side (which is where it originated from), and some new name for the symbol side. This means there'd be no change here (since this is and remains all target side). > > Maybe one way to make this obvious would be to change solib_ops > > to take an inferior instead of a gdbarch as argument ... > > In the particular case of solib_ops it does seem like > gdbarch is the wrong "this/self" parameter. I've checked in the patch to revert your change. On top of that, I'd now suggest something along the following lines to implement the above suggestion. At a later stage, we could then add "inferior" arguments to the solib routines and push calls to current_inferior up to the callers. Does this look like a reasonable first step to you? Bye, Ulrich ChangeLog: * solib.c (solib_ops): Take "struct inferior *" argument instead of "struct gdbarch *". (solib_find_1, solib_map_sections, clear_so, free_so, update_solib_list, solib_add, solib_keep_data_in_core, clear_solib, solib_create_inferior_hook, in_solib_dynsym_resolve_code, update_solib_breakpoints, handle_solib_event, reload_shared_libraries, solib_global_lookup): Update callers. diff --git a/gdb/solib.c b/gdb/solib.c index c46116d..e0e58fb 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -63,9 +63,9 @@ solib_init (struct obstack *obstack) } static const struct target_so_ops * -solib_ops (struct gdbarch *gdbarch) +solib_ops (struct inferior *inf) { - const struct target_so_ops **ops = gdbarch_data (gdbarch, solib_data); + const struct target_so_ops **ops = gdbarch_data (inf->gdbarch, solib_data); return *ops; } @@ -152,7 +152,7 @@ show_solib_search_path (struct ui_file *file, int from_tty, static char * solib_find_1 (char *in_pathname, int *fd, int is_solib) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); int found_file = -1; char *temp_pathname = NULL; const char *fskind = effective_target_file_system_kind (); @@ -533,7 +533,7 @@ solib_bfd_open (char *pathname) static int solib_map_sections (struct so_list *so) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); char *filename; struct target_section *p; struct cleanup *old_chain; @@ -605,7 +605,7 @@ solib_map_sections (struct so_list *so) static void clear_so (struct so_list *so) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); if (so->sections) { @@ -645,7 +645,7 @@ clear_so (struct so_list *so) void free_so (struct so_list *so) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); clear_so (so); ops->free_so (so); @@ -760,7 +760,7 @@ solib_used (const struct so_list *const known) static void update_solib_list (int from_tty, struct target_ops *target) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); struct so_list *inferior = ops->current_sos(); struct so_list *gdb, **gdb_link; @@ -1035,7 +1035,7 @@ solib_add (const char *pattern, int from_tty, if (loaded_any_symbols) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); /* Getting new symbols may change our opinion about what is frameless. */ @@ -1208,7 +1208,7 @@ solib_name_from_address (struct program_space *pspace, CORE_ADDR address) int solib_keep_data_in_core (CORE_ADDR vaddr, unsigned long size) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); if (ops->keep_data_in_core) return ops->keep_data_in_core (vaddr, size); @@ -1221,7 +1221,7 @@ solib_keep_data_in_core (CORE_ADDR vaddr, unsigned long size) void clear_solib (void) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); /* This function is expected to handle ELF shared libraries. It is also used on Solaris, which can run either ELF or a.out binaries @@ -1268,7 +1268,7 @@ clear_solib (void) void solib_create_inferior_hook (int from_tty) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); ops->solib_create_inferior_hook (from_tty); } @@ -1279,7 +1279,7 @@ solib_create_inferior_hook (int from_tty) int in_solib_dynsym_resolve_code (CORE_ADDR pc) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); return ops->in_dynsym_resolve_code (pc); } @@ -1315,7 +1315,7 @@ no_shared_libraries (char *ignored, int from_tty) void update_solib_breakpoints (void) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); if (ops->update_breakpoints != NULL) ops->update_breakpoints (); @@ -1326,7 +1326,7 @@ update_solib_breakpoints (void) void handle_solib_event (void) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); if (ops->handle_event != NULL) ops->handle_event (); @@ -1423,7 +1423,7 @@ reload_shared_libraries (char *ignored, int from_tty, reload_shared_libraries_1 (from_tty); - ops = solib_ops (target_gdbarch ()); + ops = solib_ops (current_inferior ()); /* Creating inferior hooks here has two purposes. First, if we reload shared libraries then the address of solib breakpoint we've computed @@ -1516,7 +1516,7 @@ solib_global_lookup (struct objfile *objfile, const char *name, const domain_enum domain) { - const struct target_so_ops *ops = solib_ops (target_gdbarch ()); + const struct target_so_ops *ops = solib_ops (current_inferior ()); if (ops->lookup_lib_global_symbol != NULL) return ops->lookup_lib_global_symbol (objfile, name, domain);