From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11003 invoked by alias); 22 Sep 2014 18:35:16 -0000 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 Received: (qmail 10984 invoked by uid 89); 22 Sep 2014 18:35:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,FAKE_REPLY_C,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 22 Sep 2014 18:35:12 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8MIZ98a025815 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 22 Sep 2014 14:35:09 -0400 Received: from host2.jankratochvil.net (ovpn-116-67.ams2.redhat.com [10.36.116.67]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8MIZ5ER028052 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Mon, 22 Sep 2014 14:35:08 -0400 Date: Mon, 22 Sep 2014 18:35:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: Doug Evans , "gdb-patches@sourceware.org" Subject: Re: time to workaround libc/13097 in fsf gdb? Message-ID: <20140922183505.GA21660@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <541F2B1E.4030909@redhat.com> <541F2311.1040404@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00667.txt.bz2 On Sun, 21 Sep 2014 21:12:17 +0200, Pedro Alves wrote: > Is it really a pain though? 95 lines of gdbarch.* patch + its ChangeLog is really a pain compared to 1 line of C++ virtual override. > Sounds like a predicate like this would work then? > > if (vsyscall_start <= so->lm_info->l_ld && so->lm_info->l_ld < vsyscall_end) Yes, I find this test correct. On Sun, 21 Sep 2014 21:46:38 +0200, Pedro Alves wrote: > Here's a quick update to the patch that does (just) that, for > testing. Doesn't update comments, etc. It works on f20 here. > > WDYT ? Yes, it works for me on kernel-2.6.32-220.el6.x86_64 (also verified your previous patch still displayed the warning). Clarifying it does not fulfill this your suggestion: On Fri, 12 Sep 2014 14:14:36 +0200, Pedro Alves wrote: # I was more inclined to leave the vdso in the shared library list # though, like ldd does, than filtering it out. But I understand it was more a suggestion than requirement for patch acceptance. IMO it was request for an unrelated feature. > --- a/gdb/gdbarch.sh > +++ b/gdb/gdbarch.sh > @@ -1029,6 +1029,10 @@ m:int:insn_is_jump:CORE_ADDR addr:addr::default_insn_is_jump::0 > # Return -1 if there is insufficient buffer for a whole entry. > # Return 1 if an entry was read into *TYPEP and *VALP. > M:int:auxv_parse:gdb_byte **readptr, gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp:readptr, endptr, typep, valp > + > +# Find the address range of the current inferior's vsyscall/vDSO, and > +# write it to *START, *END. Returns true if found, false otherwise. I find unclear the description whether *END is the last byte address or the after-the-last byte address. > +m:int:vsyscall_range:CORE_ADDR *start, CORE_ADDR *end:start, end::default_vsyscall_range::0 > EOF > } > > diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c > index dae59c5..3f28fc9 100644 > --- a/gdb/linux-tdep.c > +++ b/gdb/linux-tdep.c > @@ -1782,6 +1782,54 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch, > return -1; > } > > +/* Arguments for symbol_file_add_from_memory_wrapper. */ > + > +struct find_mapping_size_args > +{ > + CORE_ADDR vaddr; > + size_t size; size_t and not CORE_ADDR? (This patch uses also unsigned long for this value.) > +}; > + > +/* Rummage through mappings to find a mapping size. */ > + > +static int > +find_mapping_size (CORE_ADDR vaddr, unsigned long size, > + int read, int write, int exec, int modified, > + void *data) > +{ > + struct find_mapping_size_args *args = data; > + > + if (vaddr == args->vaddr) > + { > + args->size = size; > + return 1; > + } > + return 0; > +} > + > +/* Implementation of the "vsyscall_range" gdbarch hook. */ > + > +static int > +linux_vsyscall_range (struct gdbarch *gdbarch, CORE_ADDR *start, CORE_ADDR *end) > +{ > + struct find_mapping_size_args args; > + > + if (target_auxv_search (¤t_target, AT_SYSINFO_EHDR, &args.vaddr) <= 0) > + return 0; > + > + /* This is installed by linux_init_abi below, so should always be > + available. */ > + gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ())); Is there a reason for target_gdbarch () and not gdbarch? > + > + args.size = 0; > + gdbarch_find_memory_regions (target_gdbarch (), > + find_mapping_size, &args); > + > + *start = args.vaddr; > + *end = *start + args.size; > + return 1; Here it returns 1 even if the vDSO was not found. It will return *start == *end so the current only caller behaves correctly. But I do not find it fully compliant to its gdbarch.sh documentation. > +} > + > /* To be called from the various GDB_OSABI_LINUX handlers for the > various GNU/Linux architectures and machine types. */ > > @@ -1799,6 +1847,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) > linux_gdb_signal_from_target); > set_gdbarch_gdb_signal_to_target (gdbarch, > linux_gdb_signal_to_target); > + set_gdbarch_vsyscall_range (gdbarch, linux_vsyscall_range); > } > > /* Provide a prototype to silence -Wmissing-prototypes. */ [...] > --- a/gdb/symfile-mem.c > +++ b/gdb/symfile-mem.c > @@ -188,33 +188,16 @@ symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data) > return 0; > } > > -/* Rummage through mappings to find the vsyscall page size. */ > - > -static int > -find_vdso_size (CORE_ADDR vaddr, unsigned long size, > - int read, int write, int exec, int modified, > - void *data) > -{ > - struct symbol_file_add_from_memory_args *args = data; > - > - if (vaddr == args->sysinfo_ehdr) > - { > - args->size = size; > - return 1; > - } > - return 0; > -} > - > /* Try to add the symbols for the vsyscall page, if there is one. > This function is called via the inferior_created observer. */ > > static void > add_vsyscall_page (struct target_ops *target, int from_tty) > { > - CORE_ADDR sysinfo_ehdr; > + CORE_ADDR vsyscall_start, vsyscall_end; > > - if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0 > - && sysinfo_ehdr != (CORE_ADDR) 0) > + if (gdbarch_vsyscall_range (target_gdbarch (), > + &vsyscall_start, &vsyscall_end)) This is a code cleanup part of the patch which could be separate. > { > struct bfd *bfd; > struct symbol_file_add_from_memory_args args; Thanks, Jan