From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id rTqUA2Iq+F9NJwAAWB0awg (envelope-from ) for ; Fri, 08 Jan 2021 04:48:18 -0500 Received: by simark.ca (Postfix, from userid 112) id EFBC41E99A; Fri, 8 Jan 2021 04:48:17 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 784481E4F4; Fri, 8 Jan 2021 04:48:17 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2EED73858025; Fri, 8 Jan 2021 09:48:17 +0000 (GMT) Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id 5E5D13858025 for ; Fri, 8 Jan 2021 09:48:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5E5D13858025 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x433.google.com with SMTP id i9so8374373wrc.4 for ; Fri, 08 Jan 2021 01:48:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=ytoBwakEiz9owbTWY36I8AqdWvTguA4WAYmaIyOw0kI=; b=HTknfztG+5uAQsi7PFvweo6HKb/8c30IX0EiJBZT6r2SinI4ScdFew+7zIrbu7Rr6V BFMLhg5hVyawDVmC/zfpdPC6PJqBU4IimGjBOSQhp75+L8gTH7vDbD+Mjjxs9GiVQ/pm 0A72GFkNN2pJKuIGS+ZC/R7fn7P1VT617pnd4tkyIu9cYXYxOQOO8EX7tWKeX+pK70lz WobCcjd/ygu9A1CFYyTbdJd7Nlbk1aGAeRsXBCIllwuWlQMswXAvmtFmqLCOesJuD5uW 1/VPCJXrIpPPKynUKM/P5e/ru3xO98CrMAOvBoqDcam8fMrHXeCVmlm8oJ4qQ0AeriM2 SK8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=ytoBwakEiz9owbTWY36I8AqdWvTguA4WAYmaIyOw0kI=; b=VWh/d0JAiOpfUqbMZ4O7oNB6zoPud+BNWTG+smf+SwtnWRF7a0cPivGQrk+Oyb072u zcDuZPvCVVfgxK74pq6y/FWGE0Lr/wkj21YUZnGTIXnVkUBAZm6CWF6L12ssNrIsg04G EKlDOFGGZG5HAJbGFpLkmaBnOF+aC7DTJuEopXldwShMk85zlgiyW3eg4s4LlWUbkeUu EZ2RvCNsQNZm/baH6N9Q+NgYMkhV14j5bm3/LKrV+9n9CuwvHv/Wpznmhbfz5R8tzfj4 A8CRZ/awQG6TY9ucyKpzPdalIUsNGKGX1l5XcQNO8VnmucLQohg79pcMGvXCNSM/TgQk 1wAA== X-Gm-Message-State: AOAM530wcAyccXYhqlW4qizI3Ml9FCbJvaWiYkC/qqV17FJrnb2P2qvW BYXbiVcfE4iZaRmNHW3RtBbjBg== X-Google-Smtp-Source: ABdhPJzYAjmUTomfbprj2J2P0t4ZjS2StdZD1p46oyE/R1O9PQ+c8CyPXo67vr5Cl2KxU1oRIDdeLw== X-Received: by 2002:adf:d238:: with SMTP id k24mr2764335wrh.414.1610099292477; Fri, 08 Jan 2021 01:48:12 -0800 (PST) Received: from localhost (host86-166-129-230.range86-166.btcentralplus.com. [86.166.129.230]) by smtp.gmail.com with ESMTPSA id m17sm14452925wrn.0.2021.01.08.01.48.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jan 2021 01:48:11 -0800 (PST) Date: Fri, 8 Jan 2021 09:48:11 +0000 From: Andrew Burgess To: Simon Marchi Subject: Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir Message-ID: <20210108094811.GT2945@embecosm.com> References: <20210107205725.2010353-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210107205725.2010353-1-simon.marchi@efficios.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 09:46:03 up 30 days, 14:30, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org, sourcewarebugz@kyber.fi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Simon Marchi via Gdb-patches [2021-01-07 15:57:25 -0500]: > As reported in PR 27157, if some environment variables read at startup > by GDB are defined but empty, we hit the assert in gdb_abspath: > > $ XDG_CACHE_HOME= ./gdb -nx --data-directory=data-directory -q > AddressSanitizer:DEADLYSIGNAL > ================================================================= > ==2007040==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000001b0 (pc 0x5639d4aa4127 bp 0x7ffdac232c00 sp 0x7ffdac232bf0 T0) > ==2007040==The signal is caused by a READ memory access. > ==2007040==Hint: address points to the zero page. > #0 0x5639d4aa4126 in target_stack::top() const /home/smarchi/src/binutils-gdb/gdb/target.h:1334 > #1 0x5639d4aa41f1 in inferior::top_target() /home/smarchi/src/binutils-gdb/gdb/inferior.h:369 > #2 0x5639d4a70b1f in current_top_target() /home/smarchi/src/binutils-gdb/gdb/target.c:120 > #3 0x5639d4b00591 in gdb_readline_wrapper_cleanup::gdb_readline_wrapper_cleanup() /home/smarchi/src/binutils-gdb/gdb/top.c:1046 > #4 0x5639d4afab31 in gdb_readline_wrapper(char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1104 > #5 0x5639d4ccce2c in defaulted_query /home/smarchi/src/binutils-gdb/gdb/utils.c:893 > #6 0x5639d4ccd6af in query(char const*, ...) /home/smarchi/src/binutils-gdb/gdb/utils.c:985 > #7 0x5639d4ccaec1 in internal_vproblem /home/smarchi/src/binutils-gdb/gdb/utils.c:373 > #8 0x5639d4ccb3d1 in internal_verror(char const*, int, char const*, __va_list_tag*) /home/smarchi/src/binutils-gdb/gdb/utils.c:439 > #9 0x5639d5151a92 in internal_error(char const*, int, char const*, ...) /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55 > #10 0x5639d5162ab4 in gdb_abspath(char const*) /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:132 > #11 0x5639d5162fac in get_standard_cache_dir[abi:cxx11]() /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:228 > #12 0x5639d3e76a81 in _initialize_index_cache() /home/smarchi/src/binutils-gdb/gdb/dwarf2/index-cache.c:325 > #13 0x5639d4dbbe92 in initialize_all_files() /home/smarchi/build/binutils-gdb/gdb/init.c:321 > #14 0x5639d4b00259 in gdb_init(char*) /home/smarchi/src/binutils-gdb/gdb/top.c:2344 > #15 0x5639d4440715 in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:950 > #16 0x5639d444252e in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1229 > #17 0x5639d44425cf in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1254 > #18 0x5639d3923371 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32 > #19 0x7fa002d3f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) > #20 0x5639d392314d in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x4d414d) > > gdb_abspath doesn't handle empty strings, so handle this case in the > callers. If a variable is defined but empty, I think it's reasonable in > this case to just ignore it, as if it was not defined. > > Note that this sometimes also lead to a segfault, because the failed > assertion happens very early during startup, before things are fully > initialized. > > gdbsupport/ChangeLog: > > PR gdb/27157 > * pathstuff.cc (get_standard_cache_dir, get_standard_config_dir, > find_gdb_home_config_file): Add empty string check. > > gdb/testsuite/ChangeLog: > > PR gdb/27157 > * gdb.base/empty-host-env-vars.exp: New test. > > Change-Id: I8654d8e97e74e1dff6d308c111ae4b1bbf07bef9 > --- > .../gdb.base/empty-host-env-vars.exp | 28 +++++++++++++++++++ > gdbsupport/pathstuff.cc | 14 +++++----- > 2 files changed, 35 insertions(+), 7 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp > > diff --git a/gdb/testsuite/gdb.base/empty-host-env-vars.exp b/gdb/testsuite/gdb.base/empty-host-env-vars.exp > new file mode 100644 > index 00000000000..54cd4b92ccf > --- /dev/null > +++ b/gdb/testsuite/gdb.base/empty-host-env-vars.exp > @@ -0,0 +1,28 @@ > +# Copyright 2021 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 . > + > +# GDB reads some environment variables on startup, make sure it behaves > +# correctly if these variables are defined but empty. > + > +foreach_with_prefix env_var_name {HOME XDG_CACHE_HOME LOCALAPPDATA XDG_CONFIG_HOME } { > + # Restore the original state of the environment variable. > + save_vars env($env_var_name) { > + set env($env_var_name) {} > + clean_restart > + > + # Check that GDB is really started and working. > + gdb_test "print 1" " = 1" > + } > +} I wonder if it's worth including the case where ALL of the environment variables are empty as well, just to check that there's no issues with GDB falling through all the checks? Otherwise, LGTM. Thanks, Andrew > diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc > index 51ab8c651b2..ad13900819e 100644 > --- a/gdbsupport/pathstuff.cc > +++ b/gdbsupport/pathstuff.cc > @@ -222,7 +222,7 @@ get_standard_cache_dir () > > #ifndef __APPLE__ > const char *xdg_cache_home = getenv ("XDG_CACHE_HOME"); > - if (xdg_cache_home != NULL) > + if (xdg_cache_home != NULL && xdg_cache_home[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (xdg_cache_home)); > @@ -231,7 +231,7 @@ get_standard_cache_dir () > #endif > > const char *home = getenv ("HOME"); > - if (home != NULL) > + if (home != NULL && home[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (home)); > @@ -240,14 +240,14 @@ get_standard_cache_dir () > > #ifdef WIN32 > const char *win_home = getenv ("LOCALAPPDATA"); > - if (win_home != NULL) > + if (win_home != NULL && win_home[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (win_home)); > return string_printf ("%s/gdb", abs.get ()); > } > #endif > - > + > return {}; > } > > @@ -289,7 +289,7 @@ get_standard_config_dir () > > #ifndef __APPLE__ > const char *xdg_config_home = getenv ("XDG_CONFIG_HOME"); > - if (xdg_config_home != NULL) > + if (xdg_config_home != NULL && xdg_config_home[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (xdg_config_home)); > @@ -298,7 +298,7 @@ get_standard_config_dir () > #endif > > const char *home = getenv ("HOME"); > - if (home != NULL) > + if (home != NULL && home[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (home)); > @@ -340,7 +340,7 @@ find_gdb_home_config_file (const char *name, struct stat *buf) > } > > const char *homedir = getenv ("HOME"); > - if (homedir != nullptr) > + if (homedir != nullptr && homedir[0] != '\0') > { > /* Make sure the path is absolute and tilde-expanded. */ > gdb::unique_xmalloc_ptr abs (gdb_abspath (homedir)); > -- > 2.29.2 >