From patchwork Thu Nov 21 19:37:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 36102 Received: (qmail 114737 invoked by alias); 21 Nov 2019 19:37:48 -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 114726 invoked by uid 89); 21 Nov 2019 19:37:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=cwd, msdos, MSDOS, BuildBot X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Nov 2019 19:37:46 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574365064; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qx2YOEssTJuR3deoeuFBmx+z8F6PHicCVjGpYs97das=; b=Cv5lbZ0xQoww/VFZAyLyXrj0czRZlTorwOQhfOiWlf0SGi2/8fgkS4cr0UuVlFo6DNHu+H oHFVAmvXFk5pOxMakLj9ceRAd5FmvObo3ZMM/T3xJHt6bGPmd7YOPcnzGQcx9QCNMT7Q5j HwQXr+eK8jTlNHpwbqs8G0up2fd4WSU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-390-P_QKaQODPCCczTW2mK-xnA-1; Thu, 21 Nov 2019 14:37:43 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 41A3D184CAC1; Thu, 21 Nov 2019 19:37:42 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5E8C744F8F; Thu, 21 Nov 2019 19:37:39 +0000 (UTC) From: Sergio Durigan Junior To: GDB Patches Cc: Tom Tromey , Sergio Durigan Junior Subject: [PATCH v2] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613) Date: Thu, 21 Nov 2019 14:37:29 -0500 Message-Id: <20191121193729.26315-1-sergiodj@redhat.com> In-Reply-To: <20190710234615.14800-1-sergiodj@redhat.com> References: <20190710234615.14800-1-sergiodj@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-IsSubscribed: yes Changes from v1: - Adjust all spots where 'current_directory' was being used with 'concat' to determine the path of a file. - Expand 'gdb_abspath' comment and mention what happens when 'current_directory' is NULL. Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1728147 Ref.: https://sourceware.org/bugzilla/show_bug.cgi?id=23613 Hi, This bug has been reported against Fedora GDB, but there's also an upstream bug. The problem reported is that GDB segfaults when the working directory is deleted. It's pretty use to reproduce it: mkdir bla cd bla rmdir ../bla gdb echo Debugging the problem is a bit tricky, because, since the current directory doesn't exist anymore, a corefile cannot be saved there. After a few attempts, I came up with the following: gdb -ex 'shell mkdir bla' -ex 'cd bla' -ex 'shell rmdir ../bla' -ex 'r echo' ./gdb/gdb This assumes that you're inside a build directory which contains ./gdb/gdb, of course. After investigating it, I found that the problem happens at gdb_abspath, where we're dereferencing 'current_directory' without checking if it's NULL: ... (concat (current_directory, IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1]) ? "" : SLASH_STRING, ... So I fixed the problem with the patch below. The idea is that, if 'current_directory' is NULL, then the final string returned should be just the "path". After fixing the bug, I found a similar one reported against our bugzilla: PR gdb/23613. The problem is the same, but the reproducer is a bit different. I really tried writing a testcase for this, but unfortunately it's apparently not possible to start GDB inside a non-existent directory with DejaGNU. I regression tested this patch on the BuildBot, and no regressions were found. gdb/ChangeLog: 2019-11-21 Sergio Durigan Junior https://bugzilla.redhat.com/show_bug.cgi?id=1728147 PR gdb/23613 * bsd-kvm.c (bsd_kvm_target_open): Use 'gdb_abspath'. * corelow.c: Include 'gdbsupport/pathstuff.h'. (core_target_open): Use 'gdb_abspath'. * gdbsupport/pathstuff.c (gdb_abspath): Guard against 'current_directory == NULL' case. * gdbsupport/pathstuff.h (gdb_abspath): Expand comment and explain what happens when 'current_directory' is NULL. * go32-nat.c (go32_nat_target::wait): Check if 'current_directory' is NULL before call to 'chdir'. * source.c (add_path): Use 'gdb_abspath'. * top.c: Include 'gdbsupport/pathstuff.h'. (init_history): Use 'gdb_abspath'. (set_history_filename): Likewise. * tracefile-tfile.c: Include 'gdbsupport/pathstuff.h'. (tfile_target_open): Use 'gdb_abspath'. Change-Id: Ibb0932fa25bc5c2d3ae4a7f64bd7f32885ca403b --- gdb/bsd-kvm.c | 7 +++---- gdb/corelow.c | 4 ++-- gdb/gdbsupport/pathstuff.c | 2 +- gdb/gdbsupport/pathstuff.h | 5 ++++- gdb/go32-nat.c | 3 ++- gdb/source.c | 3 +-- gdb/top.c | 18 ++++++++++++------ gdb/tracefile-tfile.c | 4 ++-- 8 files changed, 27 insertions(+), 19 deletions(-) diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c index 21f978728d..895686ef62 100644 --- a/gdb/bsd-kvm.c +++ b/gdb/bsd-kvm.c @@ -114,14 +114,13 @@ bsd_kvm_target_open (const char *arg, int from_tty) if (arg) { - char *temp; - filename = tilde_expand (arg); if (filename[0] != '/') { - temp = concat (current_directory, "/", filename, (char *)NULL); + gdb::unique_xmalloc_ptr temp (gdb_abspath (filename)); + xfree (filename); - filename = temp; + filename = temp.release (); } } diff --git a/gdb/corelow.c b/gdb/corelow.c index fa1661ee61..c656306fe5 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -43,6 +43,7 @@ #include "gdb_bfd.h" #include "completer.h" #include "gdbsupport/filestuff.h" +#include "gdbsupport/pathstuff.h" #ifndef O_LARGEFILE #define O_LARGEFILE 0 @@ -373,8 +374,7 @@ core_target_open (const char *arg, int from_tty) gdb::unique_xmalloc_ptr filename (tilde_expand (arg)); if (!IS_ABSOLUTE_PATH (filename.get ())) - filename.reset (concat (current_directory, "/", - filename.get (), (char *) NULL)); + filename = gdb_abspath (filename.get ()); flags = O_BINARY | O_LARGEFILE; if (write_files) diff --git a/gdb/gdbsupport/pathstuff.c b/gdb/gdbsupport/pathstuff.c index fafecd543d..aa51e8f36e 100644 --- a/gdb/gdbsupport/pathstuff.c +++ b/gdb/gdbsupport/pathstuff.c @@ -134,7 +134,7 @@ gdb_abspath (const char *path) if (path[0] == '~') return gdb_tilde_expand_up (path); - if (IS_ABSOLUTE_PATH (path)) + if (IS_ABSOLUTE_PATH (path) || current_directory == NULL) return make_unique_xstrdup (path); /* Beware the // my son, the Emacs barfs, the botch that catch... */ diff --git a/gdb/gdbsupport/pathstuff.h b/gdb/gdbsupport/pathstuff.h index 7d7c8cd028..e84685b654 100644 --- a/gdb/gdbsupport/pathstuff.h +++ b/gdb/gdbsupport/pathstuff.h @@ -44,7 +44,10 @@ extern gdb::unique_xmalloc_ptr Contrary to "gdb_realpath", this function uses CURRENT_DIRECTORY for the path expansion. This may lead to scenarios the current - working directory (CWD) is different than CURRENT_DIRECTORY. */ + working directory (CWD) is different than CURRENT_DIRECTORY. + + If CURRENT_DIRECTORY is NULL, this function returns a copy of + PATH. */ extern gdb::unique_xmalloc_ptr gdb_abspath (const char *path); diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c index 8c71bd7650..94972a2f5c 100644 --- a/gdb/go32-nat.c +++ b/gdb/go32-nat.c @@ -507,7 +507,8 @@ go32_nat_target::wait (ptid_t ptid, struct target_waitstatus *status, } getcwd (child_cwd, sizeof (child_cwd)); /* in case it has changed */ - chdir (current_directory); + if (current_directory != NULL) + chdir (current_directory); if (a_tss.tss_irqn == 0x21) { diff --git a/gdb/source.c b/gdb/source.c index d9cd5f32ca..b456a517e3 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -540,8 +540,7 @@ add_path (const char *dirname, char **which_path, int parse_separators) new_name_holder.reset (concat (name, ".", (char *) NULL)); #endif else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$') - new_name_holder.reset (concat (current_directory, SLASH_STRING, name, - (char *) NULL)); + new_name_holder = gdb_abspath (name); else new_name_holder.reset (savestring (name, p - name)); name = new_name_holder.get (); diff --git a/gdb/top.c b/gdb/top.c index 2953eac819..0893fe057f 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -54,6 +54,7 @@ #include "gdb_select.h" #include "gdbsupport/scope-exit.h" #include "gdbarch.h" +#include "gdbsupport/pathstuff.h" /* readline include files. */ #include "readline/readline.h" @@ -1982,12 +1983,13 @@ init_history (void) that was read. */ #ifdef __MSDOS__ /* No leading dots in file names are allowed on MSDOS. */ - history_filename = concat (current_directory, "/_gdb_history", - (char *)NULL); + const char *fname = "_gdb_history"; #else - history_filename = concat (current_directory, "/.gdb_history", - (char *)NULL); + const char *fname = ".gdb_history"; #endif + + gdb::unique_xmalloc_ptr temp (gdb_abspath (fname)); + history_filename = temp.release (); } read_history (history_filename); } @@ -2065,8 +2067,12 @@ set_history_filename (const char *args, directories the file written will be the same as the one that was read. */ if (!IS_ABSOLUTE_PATH (history_filename)) - history_filename = reconcat (history_filename, current_directory, "/", - history_filename, (char *) NULL); + { + gdb::unique_xmalloc_ptr temp (gdb_abspath (history_filename)); + + xfree (history_filename); + history_filename = temp.release (); + } } static void diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c index 5c3837ec5b..0953b03781 100644 --- a/gdb/tracefile-tfile.c +++ b/gdb/tracefile-tfile.c @@ -32,6 +32,7 @@ #include "xml-tdesc.h" #include "target-descriptions.h" #include "gdbsupport/buffer.h" +#include "gdbsupport/pathstuff.h" #include #ifndef O_LARGEFILE @@ -470,8 +471,7 @@ tfile_target_open (const char *arg, int from_tty) gdb::unique_xmalloc_ptr filename (tilde_expand (arg)); if (!IS_ABSOLUTE_PATH (filename.get ())) - filename.reset (concat (current_directory, "/", filename.get (), - (char *) NULL)); + filename = gdb_abspath (filename.get ()); flags = O_BINARY | O_LARGEFILE; flags |= O_RDONLY;