From patchwork Fri Jun 8 12:12:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Rainer Orth X-Patchwork-Id: 27710 Received: (qmail 18263 invoked by alias); 8 Jun 2018 12:13:02 -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 18247 invoked by uid 89); 8 Jun 2018 12:13:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-19.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy=BEFORE, race, revived, operates X-HELO: smtp.CeBiTec.Uni-Bielefeld.DE Received: from smtp.CeBiTec.Uni-Bielefeld.DE (HELO smtp.CeBiTec.Uni-Bielefeld.DE) (129.70.160.84) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Jun 2018 12:13:00 +0000 Received: from localhost (localhost.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id EE62A1C5C for ; Fri, 8 Jun 2018 14:12:57 +0200 (CEST) Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (malfoy.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 5Tb23HMcRcRq for ; Fri, 8 Jun 2018 14:12:54 +0200 (CEST) Received: from lokon.CeBiTec.Uni-Bielefeld.DE (lokon.CeBiTec.Uni-Bielefeld.DE [129.70.161.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPS id 388391C5B for ; Fri, 8 Jun 2018 14:12:54 +0200 (CEST) Received: (from ro@localhost) by lokon.CeBiTec.Uni-Bielefeld.DE (8.15.2+Sun/8.15.2/Submit) id w58CCrSH002375; Fri, 8 Jun 2018 14:12:53 +0200 (MEST) From: Rainer Orth To: gdb-patches@sourceware.org Subject: Don't search for $SHELL in procfs.c Date: Fri, 08 Jun 2018 14:12:53 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (usg-unix-v) MIME-Version: 1.0 When building gdb mainline on Solaris with g++ 8.1 with the fixes in https://sourceware.org/ml/gdb-patches/2018-06/msg00196.html one failure remains: /vol/src/gnu/gdb/gdb/dist/gdb/procfs.c: In member function ‘virtual void procfs_target::create_inferior(const char*, const string&, char**, int)’: /vol/src/gnu/gdb/gdb/dist/gdb/procfs.c:3098:12: error: ‘char* std::strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation] strncpy (tryname, p, len); ~~~~~~~~^~~~~~~~~~~~~~~~~ /vol/src/gnu/gdb/gdb/dist/gdb/procfs.c:3097:19: note: length computed here len = strlen (p); ~~~~~~~^~~ cc1plus: all warnings being treated as errors I didn't even try to get rid of this, but revived an older patch of mine to rip out the code to search for $SHELL in $PATH. procfs.c is the only caller of fork_inferior that does this, and I don't see why Solaris should be special here. Since all callers call fork_inferior with shell_file_arg = NULL now, one might even think about getting rid of that arg as a followup. Built on 64-bit Solaris 11.5/x86 (amd64-pc-solaris2.11), make check running. Ok for mainline if that passes? Thanks. Rainer # HG changeset patch # Parent b84d0dfb82ddb3346ad186f337d57d34c8edff38 Don't search for $SHELL in procfs.c diff --git a/gdb/procfs.c b/gdb/procfs.c --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -3053,92 +3053,16 @@ procfs_set_exec_trap (void) /* This function is called BEFORE gdb forks the inferior process. Its only real responsibility is to set things up for the fork, and tell GDB which two functions to call after the fork (one for the parent, - and one for the child). - - This function does a complicated search for a unix shell program, - which it then uses to parse arguments and environment variables to - be sent to the child. I wonder whether this code could not be - abstracted out and shared with other unix targets such as - inf-ptrace? */ + and one for the child). */ static void procfs_create_inferior (struct target_ops *ops, const char *exec_file, const std::string &allargs, char **env, int from_tty) { - char *shell_file = getenv ("SHELL"); - char *tryname; int pid; - if (shell_file != NULL && strchr (shell_file, '/') == NULL) - { - - /* We will be looking down the PATH to find shell_file. If we - just do this the normal way (via execlp, which operates by - attempting an exec for each element of the PATH until it - finds one which succeeds), then there will be an exec for - each failed attempt, each of which will cause a PR_SYSEXIT - stop, and we won't know how to distinguish the PR_SYSEXIT's - for these failed execs with the ones for successful execs - (whether the exec has succeeded is stored at that time in the - carry bit or some such architecture-specific and - non-ABI-specified place). - - So I can't think of anything better than to search the PATH - now. This has several disadvantages: (1) There is a race - condition; if we find a file now and it is deleted before we - exec it, we lose, even if the deletion leaves a valid file - further down in the PATH, (2) there is no way to know exactly - what an executable (in the sense of "capable of being - exec'd") file is. Using access() loses because it may lose - if the caller is the superuser; failing to use it loses if - there are ACLs or some such. */ - - const char *p; - const char *p1; - /* FIXME-maybe: might want "set path" command so user can change what - path is used from within GDB. */ - const char *path = getenv ("PATH"); - int len; - struct stat statbuf; - - if (path == NULL) - path = "/bin:/usr/bin"; - - tryname = (char *) alloca (strlen (path) + strlen (shell_file) + 2); - for (p = path; p != NULL; p = p1 ? p1 + 1: NULL) - { - p1 = strchr (p, ':'); - if (p1 != NULL) - len = p1 - p; - else - len = strlen (p); - strncpy (tryname, p, len); - tryname[len] = '\0'; - strcat (tryname, "/"); - strcat (tryname, shell_file); - if (access (tryname, X_OK) < 0) - continue; - if (stat (tryname, &statbuf) < 0) - continue; - if (!S_ISREG (statbuf.st_mode)) - /* We certainly need to reject directories. I'm not quite - as sure about FIFOs, sockets, etc., but I kind of doubt - that people want to exec() these things. */ - continue; - break; - } - if (p == NULL) - /* Not found. This must be an error rather than merely passing - the file to execlp(), because execlp() would try all the - exec()s, causing GDB to get confused. */ - error (_("procfs:%d -- Can't find shell %s in PATH"), - __LINE__, shell_file); - - shell_file = tryname; - } - pid = fork_inferior (exec_file, allargs, env, procfs_set_exec_trap, - NULL, NULL, shell_file, NULL); + NULL, NULL, NULL, NULL); /* We have something that executes now. We'll be running through the shell at this point (if startup-with-shell is true), but the