From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24894 invoked by alias); 28 Jul 2012 07:05:30 -0000 Received: (qmail 24880 invoked by uid 22791); 28 Jul 2012 07:05:27 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD 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; Sat, 28 Jul 2012 07:05:06 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6S755pC008301 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 28 Jul 2012 03:05:05 -0400 Received: from host2.jankratochvil.net (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q6S74wDS024292 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Sat, 28 Jul 2012 03:05:01 -0400 Date: Sat, 28 Jul 2012 07:05:00 -0000 From: Jan Kratochvil To: Jean-Marc Saffroy Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] refreshed patch for PR 11804 and PR 9904 Message-ID: <20120728070451.GA2167@host2.jankratochvil.net> References: 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: 2012-07/txt/msg00715.txt.bz2 Hello, > which I've refreshed for gdb 7.4.1. So here it is, this is unfortunately not sufficient, 7.4.1 is too old. The patch is not applicable to FSF GDB HEAD - to be accepted. See how to download HEAD: http://sources.redhat.com/gdb/current/ Primarily linux_nat_find_memory_regions is now reworked and renamed as linux_find_memory_regions as GDB can now gcore also from remotely run process via gdbserver. The testcase should be verified it also works with gdbserver afterwards, see gdb/testsuite/boards/native-gdbserver.exp. Also there should be descriptive comments before each modified function. I can write those otherwise. On Wed, 18 Jul 2012 02:50:08 +0200, Jean-Marc Saffroy wrote: static int objfile_find_memory_regions (find_memory_region_ftype func, void *obfd) /* Call callback function for each objfile section. */ ALL_OBJSECTIONS (objfile, objsec) Here is missing a check: if (separate_debug_objfile_backlink != NULL) continue; as separate debug info files (in /usr/lib/debug, sometimes with .debug extension) are irrelevant for gcore. > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c [...] > @@ -4370,6 +4370,40 @@ read_mapping (FILE *mapfile, [...] > + if (fscanf (mapfile, "%64s%lu kB\n", keyword, &number) != 2) > + { > + warning (_("Error parsing /proc/PID/{s,}maps file")); > + do > + ch = fgetc (mapfile); > + while (ch != EOF && ch != '\n'); > + break; > + } > + if (number != 0 && (strcmp (keyword, "Shared_Dirty:") == 0 > + || strcmp (keyword, "Private_Dirty:") == 0 > + || strcmp (keyword, "Swap:") == 0)) The patch is primarily missing the check for "Anonymous:" entry in recent Linux kernels, described by Ulrich Weigand in: http://sourceware.org/ml/gdb-patches/2010-09/msg00398.html Therefore if "Anonymous:" entry is not present at all (=not supported by Linux kernel on that host) we must write out very every area as we have no way to verify for swap-cached pages (having Anonymous > 0). Sure if "Anonymous:" is present and it is > 0 it also must be written out. You can reproduce the "Anonymous:" problem with http://people.redhat.com/jkratoch/relroswap.c although it is not suitable for GDB testsuite as it needs to do huge memory allocations to swap out (+swap in) the testcase pages. (If you have a better idea how to make "Anonymous:" entry non-zero it is sure welcome. I have tried madvise (MADV_DONTNEED) but it does not work.) > + *modified = 1; > + else if (*modified == -1) > + { > + /* Valid line proves an smaps file is being read in. */ > + *modified = 0; > + } > + } > + } > + > return (ret != 0 && ret != EOF); > } > [...] > --- /dev/null > +++ b/gdb/testsuite/gdb.base/gcore-relro.exp > @@ -0,0 +1,80 @@ > +# Copyright 2010 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +if {[skip_shlib_tests]} { > + return 0 > +} > + > +set testfile "gcore-relro" Is hould use 'standard_testfile' like other testcases recently modified by Tom Tromey. > +set srcmainfile ${testfile}-main.c > +set srclibfile ${testfile}-lib.c > +set libfile ${objdir}/${subdir}/${testfile}-lib.so > +set objfile ${objdir}/${subdir}/${testfile}-main.o > +set executable ${testfile}-main > +set binfile ${objdir}/${subdir}/${executable} > +set gcorefile ${objdir}/${subdir}/${executable}.gcore > + > +if { [gdb_compile_shlib ${srcdir}/${subdir}/${srclibfile} ${libfile} {debug}] != "" > + || [gdb_compile ${srcdir}/${subdir}/${srcmainfile} ${objfile} object {debug}] != "" } { > + untested ${testfile}.exp > + return -1 > +} > +set opts [list debug shlib=${libfile} additional_flags=-Wl,-z,relro] > +if { [gdb_compile ${objfile} ${binfile} executable $opts] != "" } { > + unsupported "-Wl,-z,relro compilation failed" > + return -1 > +} > + > +clean_restart $executable > +gdb_load_shlibs $libfile [...] Thanks, Jan