From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13881 invoked by alias); 8 Aug 2011 21:24:36 -0000 Received: (qmail 13728 invoked by uid 22791); 8 Aug 2011 21:24:34 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP,TW_XS 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; Mon, 08 Aug 2011 21:24:17 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p78LOGUR012873 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 8 Aug 2011 17:24:16 -0400 Received: from host1.jankratochvil.net (ovpn-116-17.ams2.redhat.com [10.36.116.17]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p78LOEbO024832 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 8 Aug 2011 17:24:16 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p78LODu7020962; Mon, 8 Aug 2011 23:24:13 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p78LOCqP020960; Mon, 8 Aug 2011 23:24:12 +0200 Date: Mon, 08 Aug 2011 21:24:00 -0000 From: Jan Kratochvil To: Paul Pluzhnikov Cc: gdb-patches@sourceware.org Subject: Re: [patch] Implement qXfer:libraries for Linux/gdbserver Message-ID: <20110808212412.GB19337@host1.jankratochvil.net> References: <20110808183613.GA31998@host1.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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-08/txt/msg00157.txt.bz2 On Mon, 08 Aug 2011 20:55:40 +0200, Paul Pluzhnikov wrote: > On Mon, Aug 8, 2011 at 11:36 AM, Jan Kratochvil > wrote: [...] > The trouble is that solib-target.c's notion of what info is in solib->lm_info > is radically different from solib-svr4.c's notion, each having a private > and distinct 'struct lm_info'. Un-tangling that seemed troublesome. I agree the solib-svr4.c split looks a bit complex to me. > > It would make libexpat mandatory even for the linux-nat.c usage so libexpat > > could be bundler into the sourceware tree. > > I think it's highly desirable to keep the current solib-srv4 code working > (e.g. in case you have a gdbserver that answers with empty list; either > because it is old, or because it's for a platform that hasn't been updated). > > Given above, you can always fall back to it when libexpat is not detected. I tried to copy how windows-nat.c works, which even in local mode produces and consumes the XML document, exchanged internally as TARGET_OBJECT_LIBRARIES. Therefore the core svr4_current_sos function would also produce TARGET_OBJECT_LIBRARIES XML document, even in local run. Sure other solutions exist but this unifies the remote and local codepath the most. I do not think it is any problem to require + bundle libexpat, there was even some FAQ of people with broken GDB for ARM remote targets before some proper error messages were added indicating libexpat is missing for GDB. > The producers would have to stay separate anyway, as the details of getting > the list differ greatly. While it is a detail it saves 46 LoC (Lines of Code) and it IMO also makes the code more readable. Getting the list and producing the XML are two different parts of code. > Yes, the two isolated consumers are a bit bothersome. Unfortunately the solib-svr4.c part is not so trivial as this one but IMO it should be also done, I could get to it only later. Thanks, Jan gdb/gdbserver/ 2011-08-08 Jan Kratochvil * inferiors.c (clear_all_dlls): New function. (clear_inferiors): Move there the code from here, call it here. * linux-low.c (linux_qxfer_libraries): Rename to ... (linux_refresh_libraries): ... here. Drop all the XML handling. Call clear_all_dlls and loaded_dll instead. Clear DLLS_CHANGED. (linux_target_ops): Rename linux_qxfer_libraries to linux_refresh_libraries. * server.c (handle_qxfer_libraries): Likewise. Continue execution even if REFRESH_LIBRARIES exists. * server.h (clear_all_dlls): New prototype. * target.h (struct target_ops): Rename linux_qxfer_libraries to linux_refresh_libraries. Drop its parameters and return type. --- a/gdb/gdbserver/inferiors.c +++ b/gdb/gdbserver/inferiors.c @@ -312,14 +312,22 @@ unloaded_dll (const char *name, CORE_ADDR base_addr) #define clear_list(LIST) \ do { (LIST)->head = (LIST)->tail = NULL; } while (0) +/* Clear ALL_DLLS. */ + void -clear_inferiors (void) +clear_all_dlls (void) { - for_each_inferior (&all_threads, free_one_thread); for_each_inferior (&all_dlls, free_one_dll); + clear_list (&all_dlls); +} +void +clear_inferiors (void) +{ + for_each_inferior (&all_threads, free_one_thread); clear_list (&all_threads); - clear_list (&all_dlls); + + clear_all_dlls (); current_inferior = NULL; } --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -4939,16 +4939,13 @@ struct link_map_offsets /* Construct qXfer:libraries:read reply. */ -static int -linux_qxfer_libraries (const char *annex, unsigned char *readbuf, - unsigned const char *writebuf, - CORE_ADDR offset, int len) +static void +linux_refresh_libraries (void) { - char *document; - unsigned document_len; struct process_info_private *const priv = current_process ()->private; char filename[PATH_MAX]; - int pid, is_elf64; + int pid, is_elf64, ptr_size, r_version; + CORE_ADDR lm_addr, lm_prev, l_name, l_addr, l_next, l_prev; static const struct link_map_offsets lmo_32bit_offsets = { @@ -4971,34 +4968,17 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf, }; const struct link_map_offsets *lmo; - if (writebuf != NULL) - return -2; - if (readbuf == NULL) - return -1; - pid = lwpid_of (get_thread_lwp (current_inferior)); xsnprintf (filename, sizeof filename, "/proc/%d/exe", pid); is_elf64 = elf_64_file_p (filename); lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets; + ptr_size = is_elf64 ? 8 : 4; if (priv->r_debug == 0) priv->r_debug = get_r_debug (pid, is_elf64); if (priv->r_debug == (CORE_ADDR) -1 || priv->r_debug == 0) - { - document = xstrdup ("\n"); - } - else - { - int allocated = 1024; - char *p; - const int ptr_size = is_elf64 ? 8 : 4; - CORE_ADDR lm_addr, lm_prev, l_name, l_addr, l_next, l_prev; - int r_version; - - document = xmalloc (allocated); - strcpy (document, ""); - p = document + strlen (document); + return; r_version = 0; if (linux_read_memory (priv->r_debug + lmo->r_version_offset, @@ -5007,7 +4987,7 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf, || r_version != 1) { warning ("unexpected r_debug version %d", r_version); - goto done; + return; } if (read_one_ptr (priv->r_debug + lmo->r_map_offset, @@ -5015,9 +4995,11 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf, { warning ("unable to read r_map from 0x%lx", (long) priv->r_debug + lmo->r_map_offset); - goto done; + return; } + clear_all_dlls (); + lm_prev = 0; while (read_one_ptr (lm_addr + lmo->l_name_offset, &l_name, ptr_size) == 0 @@ -5042,33 +5024,7 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf, libname[0] = '\0'; linux_read_memory (l_name, libname, sizeof (libname)); if (libname[0] != '\0') - { - size_t len = strlen ((char *) libname); - char *name; - - while (allocated < p - document + len + 100) - { - /* Expand to guarantee sufficient storage. */ - uintptr_t document_len = p - document; - - document = xrealloc (document, 2 * allocated); - allocated *= 2; - p = document + document_len; - } - - strcpy (p, ""); - p = p + strlen (p); - } + loaded_dll ((const char *) libname, l_addr); if (l_next == 0) break; @@ -5076,18 +5032,9 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf, lm_prev = lm_addr; lm_addr = l_next; } - done: - strcpy (p, ""); - } - document_len = strlen (document + offset); - if (len > document_len) - len = document_len; - - memcpy (readbuf, document + offset, len); - xfree (document); - - return len; + /* The library notification response is not expected for GNU/Linux. */ + dlls_changed = 0; } @@ -5150,7 +5097,7 @@ static struct target_ops linux_target_ops = { linux_stabilize_threads, linux_install_fast_tracepoint_jump_pad, linux_emit_ops, - linux_qxfer_libraries + linux_refresh_libraries }; static void --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -918,8 +918,8 @@ handle_qxfer_libraries (const char *annex, if (annex[0] != '\0' || !target_running ()) return -1; - if (the_target->qxfer_libraries != NULL) - return the_target->qxfer_libraries (annex, readbuf, writebuf, offset, len); + if (the_target->refresh_libraries != NULL) + the_target->refresh_libraries (); /* Over-estimate the necessary memory. Assume that every character in the library name must be escaped. */ --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -260,6 +260,7 @@ ptid_t thread_id_to_gdb_id (ptid_t); ptid_t thread_to_gdb_id (struct thread_info *); ptid_t gdb_id_to_thread_id (ptid_t); struct thread_info *gdb_id_to_thread (unsigned int); +void clear_all_dlls (void); void clear_inferiors (void); struct inferior_list_entry *find_inferior (struct inferior_list *, --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -374,10 +374,8 @@ struct target_ops Returns NULL if bytecode compilation is not supported. */ struct emit_ops *(*emit_ops) (void); - /* Read solib info. */ - int (*qxfer_libraries) (const char *annex, unsigned char *readbuf, - unsigned const char *writebuf, - CORE_ADDR offset, int len); + /* Refresh ALL_DLLS. */ + void (*refresh_libraries) (void); }; extern struct target_ops *the_target;