From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27379 invoked by alias); 10 Jan 2010 21:43:17 -0000 Received: (qmail 27355 invoked by uid 22791); 10 Jan 2010 21:43:15 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS 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; Sun, 10 Jan 2010 21:43:10 +0000 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0ALh8Gd022538 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 10 Jan 2010 16:43:09 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx04.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o0ALgH8P021536 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sun, 10 Jan 2010 16:43:07 -0500 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id o0ALgF6J001803 for ; Sun, 10 Jan 2010 22:42:15 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id o0ALg8e8001784 for gdb-patches@sourceware.org; Sun, 10 Jan 2010 22:42:08 +0100 Date: Sun, 10 Jan 2010 21:43:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: Re: [patch 14/15] PIE: Fix back valgrind --db-attach=yes [fixup] Message-ID: <20100110214208.GA1435@host0.dyn.jankratochvil.net> References: <20091109210114.GO19138@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091109210114.GO19138@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.20 (2009-08-17) 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: 2010-01/txt/msg00233.txt.bz2 Hi, small fix-up of unstable results of the testcase: +gdb_test "" "" "eat first prompt" ------------------------------------------------------------------------------ `valgrind --db-attach=yes' will run GDB on the crashed PID which is only a virtual emulated program inside a virtual machine running at that PID. The /proc/PID/exe program is a valgrind tool (such as `memcheck'). valgrind spawns GDB using gdb /proc/PID/fd/FD PID where it provides virtual ELF image corresponding to the emulated process (and not `memcheck' itself). It worked OK so far but with the PIE support GDB still reads /proc/PID/auxv which corresponds to `memcheck' itself instead of the virtual ELF image. This way it finds incorrect PIE displacement and breaks the debugging. As discussed with Roland McGrath valgrind already provides even the virtual AUXV vector with properly provided AT_ENTRY value. AUXV vector can be besides /proc/PID/auxv found also in the initial `_start' function parameters in the order (argc, argv, envp array, auxv array). ld.so stores this auxv array into its `_dl_auxv' variable. One possibility is to backtrace to `_start' and find AUXV there. Unfortunately when the initial thread already pthread_exit()s the program can be still perfectly running but GDB can no longer backtrace to reach `_start'. Using the `_dl_auxv' variable has a problem it is located in ld.so and not the main executable. This brings in a chicken-and-egg problem (the primary issue of the former Red Hat PIE patch) one needs a relocated executable to find its shared libraries where it finds the relocation displacement (this is not so true as one can find the shared libraries even without the relocation but the code has many code paths where some of them require the relocation). As a best compromise after various attempts found to just use `_dl_auxv' during attachment and not during since-`_start' inferior startup. This means GDB would have problems only in the case of `valgrind --db-attach=yes' to inferior in a startup pre-main() phase before even before filling in `_dl_auxv'. svr4_solib_create_inferior_hook conditional is only some simplification / performance optimization to avoid relocation to invalid displacement. svr4_special_symbol_handling has no conditional as that time all the solib stuff is initialized and that time it is most safe to query the displacement. It is safe to adjust the main executable displacement multiple times (which should never happen) and it is cheap to try to adjust the displacement to an already set value. The testcase could possibly try some PIE/prelinking/debuginfos combinations. It does not. Thanks, Jan gdb/ Support Valgrind attachments broken by the PIE support. * auxv.c: Include gdbcore.h. (procfs_xfer_auxv): Make static. Reduce its comment. Drop its parameters ops, object and annex. Remove their assertions. (ld_so_xfer_auxv, memory_xfer_auxv): New function. * auxv.h (procfs_xfer_auxv): Remove comment. Rename to ... (memory_xfer_auxv): ... here. * linux-nat.c (linux_xfer_partial): Rename procfs_xfer_auxv to memory_xfer_auxv. * procfs.c (procfs_xfer_partial): Likewise. * solib-svr4.c (svr4_relocate_main_executable): New prototype. (svr4_special_symbol_handling): Call svr4_relocate_main_executable. (svr4_solib_create_inferior_hook): Conditionalize the svr4_relocate_main_executable call. gdb/testsuite/ * gdb.base/valgrind-db-attach.exp, gdb.base/valgrind-db-attach.c: New. --- a/gdb/auxv.c +++ b/gdb/auxv.c @@ -25,6 +25,7 @@ #include "inferior.h" #include "valprint.h" #include "gdb_assert.h" +#include "gdbcore.h" #include "auxv.h" #include "elf/common.h" @@ -33,15 +34,11 @@ #include -/* This function is called like a to_xfer_partial hook, but must be - called with TARGET_OBJECT_AUXV. It handles access via - /proc/PID/auxv, which is a common method for native targets. */ +/* This function handles access via /proc/PID/auxv, which is a common method + for native targets. */ -LONGEST -procfs_xfer_auxv (struct target_ops *ops, - enum target_object object, - const char *annex, - gdb_byte *readbuf, +static LONGEST +procfs_xfer_auxv (gdb_byte *readbuf, const gdb_byte *writebuf, ULONGEST offset, LONGEST len) @@ -50,9 +47,6 @@ procfs_xfer_auxv (struct target_ops *ops, int fd; LONGEST n; - gdb_assert (object == TARGET_OBJECT_AUXV); - gdb_assert (readbuf || writebuf); - pathname = xstrprintf ("/proc/%d/auxv", PIDGET (inferior_ptid)); fd = open (pathname, writebuf != NULL ? O_WRONLY : O_RDONLY); xfree (pathname); @@ -72,6 +66,143 @@ procfs_xfer_auxv (struct target_ops *ops, return n; } +/* This function handles access via ld.so's symbol `_dl_auxv'. */ + +static LONGEST +ld_so_xfer_auxv (gdb_byte *readbuf, + const gdb_byte *writebuf, + ULONGEST offset, + LONGEST len) +{ + struct minimal_symbol *msym; + CORE_ADDR data_address, pointer_address; + struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr; + size_t ptr_size = TYPE_LENGTH (ptr_type); + size_t auxv_pair_size = 2 * ptr_size; + gdb_byte *ptr_buf = alloca (ptr_size); + LONGEST retval; + size_t block; + + msym = lookup_minimal_symbol ("_dl_auxv", NULL, NULL); + if (msym == NULL) + return -1; + + if (MSYMBOL_SIZE (msym) != ptr_size) + return -1; + + /* POINTER_ADDRESS is a location where the `_dl_auxv' variable resides. + DATA_ADDRESS is the inferior value present in `_dl_auxv', therefore the + real inferior AUXV address. */ + + pointer_address = SYMBOL_VALUE_ADDRESS (msym); + + data_address = read_memory_typed_address (pointer_address, ptr_type); + + /* Possibly still not initialized such as during an inferior startup. */ + if (data_address == 0) + return -1; + + data_address += offset; + + if (writebuf != NULL) + { + if (target_write_memory (data_address, writebuf, len) == 0) + return len; + else + return -1; + } + + /* Stop if trying to read past the existing AUXV block. The final AT_NULL + was already returned before. */ + + if (offset >= auxv_pair_size) + { + if (target_read_memory (data_address - auxv_pair_size, ptr_buf, + ptr_size) != 0) + return -1; + + if (extract_typed_address (ptr_buf, ptr_type) == AT_NULL) + return 0; + } + + retval = 0; + block = 0x400; + gdb_assert (block % auxv_pair_size == 0); + + while (len > 0) + { + if (block > len) + block = len; + + /* Reading sizes smaller than AUXV_PAIR_SIZE is not supported. Tails + unaligned to AUXV_PAIR_SIZE will not be read during a call (they + should be completed during next read with new/extended buffer). */ + + block &= -auxv_pair_size; + if (block == 0) + return retval; + + if (target_read_memory (data_address, readbuf, block) != 0) + { + if (block <= auxv_pair_size) + return retval; + + block = auxv_pair_size; + continue; + } + + data_address += block; + len -= block; + + /* Check terminal AT_NULL. This function is being called indefinitely + being extended its READBUF until it returns EOF (0). */ + + while (block >= auxv_pair_size) + { + retval += auxv_pair_size; + + if (extract_typed_address (readbuf, ptr_type) == AT_NULL) + return retval; + + readbuf += auxv_pair_size; + block -= auxv_pair_size; + } + } + + return retval; +} + +/* This function is called like a to_xfer_partial hook, but must be + called with TARGET_OBJECT_AUXV. It handles access to AUXV. */ + +LONGEST +memory_xfer_auxv (struct target_ops *ops, + enum target_object object, + const char *annex, + gdb_byte *readbuf, + const gdb_byte *writebuf, + ULONGEST offset, + LONGEST len) +{ + gdb_assert (object == TARGET_OBJECT_AUXV); + gdb_assert (readbuf || writebuf); + + /* ld_so_xfer_auxv is the only function safe for virtual executables being + executed by valgrind's memcheck. As using ld_so_xfer_auxv is problematic + during inferior startup GDB does call it only for attached processes. */ + + if (current_inferior ()->attach_flag != 0) + { + LONGEST retval; + + retval = ld_so_xfer_auxv (readbuf, writebuf, offset, len); + if (retval != -1) + return retval; + } + + return procfs_xfer_auxv (readbuf, writebuf, offset, len); +} + /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR. Return 0 if *READPTR is already at the end of the buffer. Return -1 if there is insufficient buffer for a whole entry. --- a/gdb/auxv.h +++ b/gdb/auxv.h @@ -43,11 +43,7 @@ extern int target_auxv_search (struct target_ops *ops, /* Print the contents of the target's AUXV on the specified file. */ extern int fprint_target_auxv (struct ui_file *file, struct target_ops *ops); -/* This function is called like a to_xfer_partial hook, but must be - called with TARGET_OBJECT_AUXV. It handles access via - /proc/PID/auxv, which is a common method for native targets. */ - -extern LONGEST procfs_xfer_auxv (struct target_ops *ops, +extern LONGEST memory_xfer_auxv (struct target_ops *ops, enum target_object object, const char *annex, gdb_byte *readbuf, --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -4928,7 +4928,7 @@ linux_xfer_partial (struct target_ops *ops, enum target_object object, LONGEST xfer; if (object == TARGET_OBJECT_AUXV) - return procfs_xfer_auxv (ops, object, annex, readbuf, writebuf, + return memory_xfer_auxv (ops, object, annex, readbuf, writebuf, offset, len); if (object == TARGET_OBJECT_OSDATA) --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -4377,7 +4377,7 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object, #ifdef NEW_PROC_API case TARGET_OBJECT_AUXV: - return procfs_xfer_auxv (ops, object, annex, readbuf, writebuf, + return memory_xfer_auxv (ops, object, annex, readbuf, writebuf, offset, len); #endif --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -50,6 +50,7 @@ static struct link_map_offsets *svr4_fetch_link_map_offsets (void); static int svr4_have_link_map_offsets (void); +static void svr4_relocate_main_executable (void); /* Link map info to include in an allocated so_list entry */ @@ -1483,6 +1484,7 @@ enable_break (struct svr4_info *info) static void svr4_special_symbol_handling (void) { + svr4_relocate_main_executable (); } /* Relocate the main executable. This function should be called upon @@ -1655,7 +1657,8 @@ svr4_solib_create_inferior_hook (void) info = get_svr4_info (); /* Relocate the main executable if necessary. */ - svr4_relocate_main_executable (); + if (current_inferior ()->attach_flag == 0) + svr4_relocate_main_executable (); if (!svr4_have_link_map_offsets ()) return; --- /dev/null +++ b/gdb/testsuite/gdb.base/valgrind-db-attach.c @@ -0,0 +1,30 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2009 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 . */ + +#include + +int main() +{ + void *p; + + p = malloc (1); + if (p == NULL) + return 1; + free (p); + free (p); /* double-free */ + return 0; +} --- /dev/null +++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp @@ -0,0 +1,76 @@ +# Copyright 2009 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 . + +set test valgrind-db-attach +set srcfile $test.c +set executable $test +set binfile ${objdir}/${subdir}/${executable} +if {[build_executable $test.exp $executable $srcfile {debug}] == -1} { + return -1 +} + +gdb_exit + +# remote_spawn breaks the command on each whitespace despite possible quoting. +# Use backslash-escaped whitespace there instead: + +set db_command "--db-command=$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts] %f %p" +regsub -all " " $db_command "\\ " db_command + +set test "spawn valgrind" +set cmd "valgrind --db-attach=yes $db_command $binfile" +set res [remote_spawn host $cmd]; +if { $res < 0 || $res == "" } { + verbose -log "Spawning $cmd failed." + setup_xfail *-*-* + fail $test + return -1 +} +pass $test +# Declare GDB now as running. +set gdb_spawn_id -1 + +set test "valgrind started" +# The trailing '.' differs for different memcheck versions. +gdb_test_multiple "" $test { + -re "Memcheck, a memory error detector\\.?\r\n" { + pass $test + } + -re "valgrind: failed to start tool 'memcheck' for platform '.*': No such file or directory" { + setup_xfail *-*-* + fail $test + return -1 + } +} + +set double_free [gdb_get_line_number "double-free"] + +gdb_test_multiple "" $test { + -re "Invalid free\\(\\) / delete / delete\\\[\\\]\r\n.*: main \\(${srcfile}:$double_free\\)\r\n.*---- Attach to debugger \\? --- \[^\r\n\]* ---- " { + send_gdb "y\r" + } + -re "---- Attach to debugger \\? --- \[^\r\n\]* ---- " { + send_gdb "n\r" + exp_continue + } +} + +gdb_test "" "" "eat first prompt" + +# Initialization from default_gdb_start. +gdb_test "set height 0" +gdb_test "set width 0" + +gdb_test "bt" "in main \\(.*\\) at .*${srcfile}:$double_free"