* [PATCH v2 1/6] Unify shell-finding logic
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-19 23:27 ` Sergio Durigan Junior
2018-10-18 22:31 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed several places in gdb that were using getenv("SHELL") and
then falling back to "/bin/sh" if it returned NULL. This unifies
these into a single function.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* procfs.c (procfs_target::create_inferior): Use get_shell.
* cli/cli-cmds.c (shell_escape): Use get_shell.
* windows-nat.c (windows_nat_target::create_inferior): Use
get_shell.
* common/pathstuff.c (get_shell): New function.
* nat/fork-inferior.c (SHELL_FILE, get_startup_shell): Remove.
(fork_inferior): Use get_shell.
* common/pathstuff.h (get_shell): Declare.
---
gdb/ChangeLog | 11 +++++++++++
gdb/cli/cli-cmds.c | 6 ++----
gdb/common/pathstuff.c | 12 ++++++++++++
gdb/common/pathstuff.h | 4 ++++
gdb/nat/fork-inferior.c | 21 ++-------------------
gdb/procfs.c | 4 ++--
gdb/windows-nat.c | 5 ++---
7 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b871e476d3..135f550b80 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -50,6 +50,7 @@
#include "cli/cli-utils.h"
#include "extension.h"
+#include "common/pathstuff.h"
#ifdef TUI
#include "tui/tui.h" /* For tui_active et.al. */
@@ -726,13 +727,10 @@ shell_escape (const char *arg, int from_tty)
if ((pid = vfork ()) == 0)
{
- const char *p, *user_shell;
+ const char *p, *user_shell = get_shell ();
close_most_fds ();
- if ((user_shell = getenv ("SHELL")) == NULL)
- user_shell = "/bin/sh";
-
/* Get the name of the shell for arg0. */
p = lbasename (user_shell);
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 82905c9e68..6d8e53f4e1 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -190,3 +190,15 @@ get_standard_cache_dir ()
return {};
}
+
+/* See common/pathstuff.h. */
+
+const char *
+get_shell ()
+{
+ const char *ret = getenv ("SHELL");
+ if (ret == NULL)
+ ret = "/bin/sh";
+
+ return ret;
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index a43b963651..40fbda8d85 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -64,4 +64,8 @@ extern bool contains_dir_separator (const char *path);
extern std::string get_standard_cache_dir ();
+/* Return the file name of the user's shell. */
+
+extern const char *get_shell ();
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 40cd05a0f8..f1032b43c9 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -24,16 +24,13 @@
#include "target/target.h"
#include "common-inferior.h"
#include "common-gdbthread.h"
+#include "common/pathstuff.h"
#include "signals-state-save-restore.h"
#include "gdb_tilde_expand.h"
#include <vector>
extern char **environ;
-/* Default shell file to be used if 'startup-with-shell' is set but
- $SHELL is not. */
-#define SHELL_FILE "/bin/sh"
-
/* Build the argument vector for execv(3). */
class execv_argv
@@ -265,20 +262,6 @@ execv_argv::init_for_shell (const char *exec_file,
m_argv.push_back (NULL);
}
-/* Return the shell that must be used to startup the inferior. The
- first attempt is the environment variable SHELL; if it is not set,
- then we default to SHELL_FILE. */
-
-static const char *
-get_startup_shell ()
-{
- const char *ret = getenv ("SHELL");
- if (ret == NULL)
- ret = SHELL_FILE;
-
- return ret;
-}
-
/* See nat/fork-inferior.h. */
pid_t
@@ -316,7 +299,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
/* Figure out what shell to start up the user program under. */
if (shell_file == NULL)
- shell_file = get_startup_shell ();
+ shell_file = get_shell ();
gdb_assert (shell_file != NULL);
}
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 6ffe569e69..ca381a71ae 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3035,11 +3035,11 @@ procfs_target::create_inferior (const char *exec_file,
const std::string &allargs,
char **env, int from_tty)
{
- char *shell_file = getenv ("SHELL");
+ const char *shell_file = get_shell ();
char *tryname;
int pid;
- if (shell_file != NULL && strchr (shell_file, '/') == NULL)
+ if (strchr (shell_file, '/') == NULL)
{
/* We will be looking down the PATH to find shell_file. If we
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 0047a26418..8292cf4212 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -68,6 +68,7 @@
#include "complaints.h"
#include "inf-child.h"
#include "gdb_tilde_expand.h"
+#include "common/pathstuff.h"
#define AdjustTokenPrivileges dyn_AdjustTokenPrivileges
#define DebugActiveProcessStop dyn_DebugActiveProcessStop
@@ -2578,9 +2579,7 @@ windows_nat_target::create_inferior (const char *exec_file,
}
else
{
- sh = getenv ("SHELL");
- if (!sh)
- sh = "/bin/sh";
+ sh = get_shell ();
if (cygwin_conv_path (CCP_POSIX_TO_WIN_W, sh, shell, __PMAX) < 0)
error (_("Error starting executable via shell: %d"), errno);
#ifdef __USEWIDE
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/6] Unify shell-finding logic
2018-10-18 22:31 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
@ 2018-10-19 23:27 ` Sergio Durigan Junior
2018-10-27 17:41 ` Tom Tromey
0 siblings, 1 reply; 20+ messages in thread
From: Sergio Durigan Junior @ 2018-10-19 23:27 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Thursday, October 18 2018, Tom Tromey wrote:
> I noticed several places in gdb that were using getenv("SHELL") and
> then falling back to "/bin/sh" if it returned NULL. This unifies
> these into a single function.
Not sure if I reviewed this one before, but it LGTM (with a very small
nit below). It also can be pushed independently, IMO.
Thanks,
> gdb/ChangeLog
> 2018-10-18 Tom Tromey <tom@tromey.com>
>
> * procfs.c (procfs_target::create_inferior): Use get_shell.
> * cli/cli-cmds.c (shell_escape): Use get_shell.
> * windows-nat.c (windows_nat_target::create_inferior): Use
> get_shell.
> * common/pathstuff.c (get_shell): New function.
> * nat/fork-inferior.c (SHELL_FILE, get_startup_shell): Remove.
> (fork_inferior): Use get_shell.
> * common/pathstuff.h (get_shell): Declare.
> ---
> gdb/ChangeLog | 11 +++++++++++
> gdb/cli/cli-cmds.c | 6 ++----
> gdb/common/pathstuff.c | 12 ++++++++++++
> gdb/common/pathstuff.h | 4 ++++
> gdb/nat/fork-inferior.c | 21 ++-------------------
> gdb/procfs.c | 4 ++--
> gdb/windows-nat.c | 5 ++---
> 7 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index b871e476d3..135f550b80 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -50,6 +50,7 @@
> #include "cli/cli-utils.h"
>
> #include "extension.h"
> +#include "common/pathstuff.h"
>
> #ifdef TUI
> #include "tui/tui.h" /* For tui_active et.al. */
> @@ -726,13 +727,10 @@ shell_escape (const char *arg, int from_tty)
>
> if ((pid = vfork ()) == 0)
> {
> - const char *p, *user_shell;
> + const char *p, *user_shell = get_shell ();
>
> close_most_fds ();
>
> - if ((user_shell = getenv ("SHELL")) == NULL)
> - user_shell = "/bin/sh";
> -
> /* Get the name of the shell for arg0. */
> p = lbasename (user_shell);
>
> diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
> index 82905c9e68..6d8e53f4e1 100644
> --- a/gdb/common/pathstuff.c
> +++ b/gdb/common/pathstuff.c
> @@ -190,3 +190,15 @@ get_standard_cache_dir ()
>
> return {};
> }
> +
> +/* See common/pathstuff.h. */
> +
> +const char *
> +get_shell ()
> +{
> + const char *ret = getenv ("SHELL");
> + if (ret == NULL)
> + ret = "/bin/sh";
> +
> + return ret;
> +}
> diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
> index a43b963651..40fbda8d85 100644
> --- a/gdb/common/pathstuff.h
> +++ b/gdb/common/pathstuff.h
> @@ -64,4 +64,8 @@ extern bool contains_dir_separator (const char *path);
>
> extern std::string get_standard_cache_dir ();
>
> +/* Return the file name of the user's shell. */
WDYT about explicitly mentioning here that "user's shell" means "the
SHELL environment variable"?
> +
> +extern const char *get_shell ();
> +
> #endif /* PATHSTUFF_H */
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 40cd05a0f8..f1032b43c9 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -24,16 +24,13 @@
> #include "target/target.h"
> #include "common-inferior.h"
> #include "common-gdbthread.h"
> +#include "common/pathstuff.h"
> #include "signals-state-save-restore.h"
> #include "gdb_tilde_expand.h"
> #include <vector>
>
> extern char **environ;
>
> -/* Default shell file to be used if 'startup-with-shell' is set but
> - $SHELL is not. */
> -#define SHELL_FILE "/bin/sh"
> -
> /* Build the argument vector for execv(3). */
>
> class execv_argv
> @@ -265,20 +262,6 @@ execv_argv::init_for_shell (const char *exec_file,
> m_argv.push_back (NULL);
> }
>
> -/* Return the shell that must be used to startup the inferior. The
> - first attempt is the environment variable SHELL; if it is not set,
> - then we default to SHELL_FILE. */
> -
> -static const char *
> -get_startup_shell ()
> -{
> - const char *ret = getenv ("SHELL");
> - if (ret == NULL)
> - ret = SHELL_FILE;
> -
> - return ret;
> -}
> -
> /* See nat/fork-inferior.h. */
>
> pid_t
> @@ -316,7 +299,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
>
> /* Figure out what shell to start up the user program under. */
> if (shell_file == NULL)
> - shell_file = get_startup_shell ();
> + shell_file = get_shell ();
>
> gdb_assert (shell_file != NULL);
> }
> diff --git a/gdb/procfs.c b/gdb/procfs.c
> index 6ffe569e69..ca381a71ae 100644
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -3035,11 +3035,11 @@ procfs_target::create_inferior (const char *exec_file,
> const std::string &allargs,
> char **env, int from_tty)
> {
> - char *shell_file = getenv ("SHELL");
> + const char *shell_file = get_shell ();
> char *tryname;
> int pid;
>
> - if (shell_file != NULL && strchr (shell_file, '/') == NULL)
> + if (strchr (shell_file, '/') == NULL)
> {
>
> /* We will be looking down the PATH to find shell_file. If we
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 0047a26418..8292cf4212 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
> @@ -68,6 +68,7 @@
> #include "complaints.h"
> #include "inf-child.h"
> #include "gdb_tilde_expand.h"
> +#include "common/pathstuff.h"
>
> #define AdjustTokenPrivileges dyn_AdjustTokenPrivileges
> #define DebugActiveProcessStop dyn_DebugActiveProcessStop
> @@ -2578,9 +2579,7 @@ windows_nat_target::create_inferior (const char *exec_file,
> }
> else
> {
> - sh = getenv ("SHELL");
> - if (!sh)
> - sh = "/bin/sh";
> + sh = get_shell ();
> if (cygwin_conv_path (CCP_POSIX_TO_WIN_W, sh, shell, __PMAX) < 0)
> error (_("Error starting executable via shell: %d"), errno);
> #ifdef __USEWIDE
> --
> 2.17.1
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/6] Unify shell-finding logic
2018-10-19 23:27 ` Sergio Durigan Junior
@ 2018-10-27 17:41 ` Tom Tromey
0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-10-27 17:41 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: Tom Tromey, gdb-patches
>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
Sergio> Not sure if I reviewed this one before, but it LGTM (with a very small
Sergio> nit below). It also can be pushed independently, IMO.
>> +/* Return the file name of the user's shell. */
Sergio> WDYT about explicitly mentioning here that "user's shell" means "the
Sergio> SHELL environment variable"?
I added some text here.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
2018-10-18 22:31 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-29 16:01 ` Simon Marchi
2018-10-18 22:31 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This moves mkdir_recursive from dwarf-index-cache.c to
common/filestuff.c, and also changes it to return a boolean that says
whether or not it worked.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* unittests/mkdir-recursive-selftests.c: New file.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/mkdir-recursive-selftests.c.
* dwarf-index-cache.c (mkdir_recursive): Move to
common/filestuff.c.
(index_cache::store): Check return value of mkdir_recursive.
(create_dir_and_check, test_mkdir_recursive): Move to new file.
(_initialize_index_cache): Don't register test.
* common/filestuff.h (mkdir_recursive): Declare.
* common/filestuff.c (mkdir_recursive): Move from
dwarf-index-cache.c. Return bool.
---
gdb/ChangeLog | 14 +++
gdb/Makefile.in | 1 +
gdb/common/filestuff.c | 45 +++++++++
gdb/common/filestuff.h | 10 ++
gdb/dwarf-index-cache.c | 114 ++--------------------
gdb/unittests/mkdir-recursive-selftests.c | 89 +++++++++++++++++
6 files changed, 165 insertions(+), 108 deletions(-)
create mode 100644 gdb/unittests/mkdir-recursive-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0bb203f45f..4ab0da5c61 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -419,6 +419,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/optional-selftests.c \
unittests/parse-connection-spec-selftests.c \
unittests/ptid-selftests.c \
+ unittests/mkdir-recursive-selftests.c \
unittests/rsp-low-selftests.c \
unittests/scoped_fd-selftests.c \
unittests/scoped_mmap-selftests.c \
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index dfd86f9fbb..d4bd1a8cb8 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -447,3 +447,48 @@ is_regular_file (const char *name, int *errno_ptr)
*errno_ptr = EINVAL;
return false;
}
+
+/* See common/filestuff.h. */
+
+bool
+mkdir_recursive (const char *dir)
+{
+ gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
+ char * const start = holder.get ();
+ char *component_start = start;
+ char *component_end = start;
+
+ while (1)
+ {
+ /* Find the beginning of the next component. */
+ while (*component_start == '/')
+ component_start++;
+
+ /* Are we done? */
+ if (*component_start == '\0')
+ return true;
+
+ /* Find the slash or null-terminator after this component. */
+ component_end = component_start;
+ while (*component_end != '/' && *component_end != '\0')
+ component_end++;
+
+ /* Temporarily replace the slash with a null terminator, so we can create
+ the directory up to this component. */
+ char saved_char = *component_end;
+ *component_end = '\0';
+
+ /* If we get EEXIST and the existing path is a directory, then we're
+ happy. If it exists, but it's a regular file and this is not the last
+ component, we'll fail at the next component. If this is the last
+ component, the caller will fail with ENOTDIR when trying to
+ open/create a file under that path. */
+ if (mkdir (start, 0700) != 0)
+ if (errno != EEXIST)
+ return false;
+
+ /* Restore the overwritten char. */
+ *component_end = saved_char;
+ component_start = component_end;
+ }
+}
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index e9328f5358..ecfc18d600 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -122,4 +122,14 @@ typedef std::unique_ptr<DIR, gdb_dir_deleter> gdb_dir_up;
we're expecting a regular file. */
extern bool is_regular_file (const char *name, int *errno_ptr);
+
+/* A cheap (as in low-quality) recursive mkdir. Try to create all the
+ parents directories up to DIR and DIR itself. Stop if we hit an
+ error along the way. There is no attempt to remove created
+ directories in case of failure.
+
+ Returns false on failure and sets errno. */
+
+extern bool mkdir_recursive (const char *dir);
+
#endif /* FILESTUFF_H */
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 966630b60c..bac98f9db8 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -45,53 +45,6 @@ index_cache global_index_cache;
static cmd_list_element *set_index_cache_prefix_list;
static cmd_list_element *show_index_cache_prefix_list;
-/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
- directories up to DIR and DIR itself. Stop if we hit an error along the way.
- There is no attempt to remove created directories in case of failure. */
-
-static void
-mkdir_recursive (const char *dir)
-{
- gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
- char * const start = holder.get ();
- char *component_start = start;
- char *component_end = start;
-
- while (1)
- {
- /* Find the beginning of the next component. */
- while (*component_start == '/')
- component_start++;
-
- /* Are we done? */
- if (*component_start == '\0')
- return;
-
- /* Find the slash or null-terminator after this component. */
- component_end = component_start;
- while (*component_end != '/' && *component_end != '\0')
- component_end++;
-
- /* Temporarily replace the slash with a null terminator, so we can create
- the directory up to this component. */
- char saved_char = *component_end;
- *component_end = '\0';
-
- /* If we get EEXIST and the existing path is a directory, then we're
- happy. If it exists, but it's a regular file and this is not the last
- component, we'll fail at the next component. If this is the last
- component, the caller will fail with ENOTDIR when trying to
- open/create a file under that path. */
- if (mkdir (start, 0700) != 0)
- if (errno != EEXIST)
- return;
-
- /* Restore the overwritten char. */
- *component_end = saved_char;
- component_start = component_end;
- }
-}
-
/* Default destructor of index_cache_resource. */
index_cache_resource::~index_cache_resource () = default;
@@ -160,7 +113,12 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
TRY
{
/* Try to create the containing directory. */
- mkdir_recursive (m_dir.c_str ());
+ if (!mkdir_recursive (m_dir.c_str ()))
+ {
+ warning (_("index cache: could not make cache directory: %s\n"),
+ safe_strerror (errno));
+ return;
+ }
if (debug_index_cache)
printf_unfiltered ("index cache: writing index cache for objfile %s\n",
@@ -346,62 +304,6 @@ show_index_cache_stats_command (const char *arg, int from_tty)
indent, global_index_cache.n_misses ());
}
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
-namespace selftests
-{
-
-/* Try to create DIR using mkdir_recursive and make sure it exists. */
-
-static bool
-create_dir_and_check (const char *dir)
-{
- mkdir_recursive (dir);
-
- struct stat st;
- if (stat (dir, &st) != 0)
- perror_with_name (("stat"));
-
- return (st.st_mode & S_IFDIR) != 0;
-}
-
-/* Test mkdir_recursive. */
-
-static void
-test_mkdir_recursive ()
-{
- char base[] = "/tmp/gdb-selftests-XXXXXX";
-
- if (mkdtemp (base) == NULL)
- perror_with_name (("mkdtemp"));
-
- /* Try not to leave leftover directories. */
- struct cleanup_dirs {
- cleanup_dirs (const char *base)
- : m_base (base)
- {}
-
- ~cleanup_dirs () {
- rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
- rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
- rmdir (string_printf ("%s/a/b", m_base).c_str ());
- rmdir (string_printf ("%s/a", m_base).c_str ());
- rmdir (m_base);
- }
-
- private:
- const char *m_base;
- } cleanup_dirs (base);
-
- std::string dir = string_printf ("%s/a/b", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-
- dir = string_printf ("%s/a/b/c//d/e/", base);
- SELF_CHECK (create_dir_and_check (dir.c_str ()));
-}
-}
-#endif /* GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
-
void
_initialize_index_cache ()
{
@@ -456,8 +358,4 @@ _initialize_index_cache ()
When non-zero, debugging output for the index cache is displayed."),
NULL, NULL,
&setdebuglist, &showdebuglist);
-
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
- selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
-#endif
}
diff --git a/gdb/unittests/mkdir-recursive-selftests.c b/gdb/unittests/mkdir-recursive-selftests.c
new file mode 100644
index 0000000000..d501f8e081
--- /dev/null
+++ b/gdb/unittests/mkdir-recursive-selftests.c
@@ -0,0 +1,89 @@
+/* Self tests for scoped_fd for GDB, the GNU debugger.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+
+#include "common/filestuff.h"
+#include "selftest.h"
+
+namespace selftests {
+namespace mkdir_recursive {
+
+/* Try to create DIR using mkdir_recursive and make sure it exists. */
+
+static bool
+create_dir_and_check (const char *dir)
+{
+ ::mkdir_recursive (dir);
+
+ struct stat st;
+ if (stat (dir, &st) != 0)
+ perror_with_name (("stat"));
+
+ return (st.st_mode & S_IFDIR) != 0;
+}
+
+/* Test mkdir_recursive. */
+
+static void
+test ()
+{
+ char base[] = "/tmp/gdb-selftests-XXXXXX";
+
+ if (mkdtemp (base) == NULL)
+ perror_with_name (("mkdtemp"));
+
+ /* Try not to leave leftover directories. */
+ struct cleanup_dirs {
+ cleanup_dirs (const char *base)
+ : m_base (base)
+ {}
+
+ ~cleanup_dirs () {
+ rmdir (string_printf ("%s/a/b/c/d/e", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c/d", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b/c", m_base).c_str ());
+ rmdir (string_printf ("%s/a/b", m_base).c_str ());
+ rmdir (string_printf ("%s/a", m_base).c_str ());
+ rmdir (m_base);
+ }
+
+ private:
+ const char *m_base;
+ } cleanup_dirs (base);
+
+ std::string dir = string_printf ("%s/a/b", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+
+ dir = string_printf ("%s/a/b/c//d/e/", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+}
+
+}
+}
+
+void
+_initialize_mkdir_recursive_selftests ()
+{
+#if defined (HAVE_MKDTEMP)
+ selftests::register_test ("mkdir_recursive",
+ selftests::mkdir_recursive::test);
+#endif
+}
+
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-18 22:31 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
@ 2018-10-29 16:01 ` Simon Marchi
2018-10-29 22:16 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-10-29 16:01 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-10-18 6:30 p.m., Tom Tromey wrote:
> This moves mkdir_recursive from dwarf-index-cache.c to
> common/filestuff.c, and also changes it to return a boolean that says
> whether or not it worked.
>
> gdb/ChangeLog
> 2018-10-18 Tom Tromey <tom@tromey.com>
>
> * unittests/mkdir-recursive-selftests.c: New file.
> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> unittests/mkdir-recursive-selftests.c.
> * dwarf-index-cache.c (mkdir_recursive): Move to
> common/filestuff.c.
> (index_cache::store): Check return value of mkdir_recursive.
> (create_dir_and_check, test_mkdir_recursive): Move to new file.
> (_initialize_index_cache): Don't register test.
> * common/filestuff.h (mkdir_recursive): Declare.
> * common/filestuff.c (mkdir_recursive): Move from
> dwarf-index-cache.c. Return bool.
I just stumbled on a build failure caused by this patch, when building for mingw:
CXX unittests/mkdir-recursive-selftests.o
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
if (mkdtemp (base) == NULL)
^
The function in the original code was guarded by HAVE_MKDTEMP (the register_test
call still is). So I guess the function should be put back in an ifdef, or we
should import the mkdtemp gnulib module (I don't remember why I chose not to do
that).
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-29 16:01 ` Simon Marchi
@ 2018-10-29 22:16 ` Simon Marchi
2018-10-30 20:55 ` Tom Tromey
0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-10-29 22:16 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-10-29 12:00 p.m., Simon Marchi wrote:
> On 2018-10-18 6:30 p.m., Tom Tromey wrote:
>> This moves mkdir_recursive from dwarf-index-cache.c to
>> common/filestuff.c, and also changes it to return a boolean that says
>> whether or not it worked.
>>
>> gdb/ChangeLog
>> 2018-10-18 Tom Tromey <tom@tromey.com>
>>
>> * unittests/mkdir-recursive-selftests.c: New file.
>> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
>> unittests/mkdir-recursive-selftests.c.
>> * dwarf-index-cache.c (mkdir_recursive): Move to
>> common/filestuff.c.
>> (index_cache::store): Check return value of mkdir_recursive.
>> (create_dir_and_check, test_mkdir_recursive): Move to new file.
>> (_initialize_index_cache): Don't register test.
>> * common/filestuff.h (mkdir_recursive): Declare.
>> * common/filestuff.c (mkdir_recursive): Move from
>> dwarf-index-cache.c. Return bool.
>
> I just stumbled on a build failure caused by this patch, when building for mingw:
>
> CXX unittests/mkdir-recursive-selftests.o
> /home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
> /home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
> if (mkdtemp (base) == NULL)
> ^
>
> The function in the original code was guarded by HAVE_MKDTEMP (the register_test
> call still is). So I guess the function should be put back in an ifdef, or we
> should import the mkdtemp gnulib module (I don't remember why I chose not to do
> that).
>
> Simon
>
What do you think about this?
From 4819c8481fe543df991c940137f80a238592c086 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 29 Oct 2018 16:35:01 -0400
Subject: [PATCH] Import mkdtemp gnulib module, fix mingw build
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Building with mingw currently fails:
CXX unittests/mkdir-recursive-selftests.o
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c: In function ‘void selftests::mkdir_recursive::test()’:
/home/emaisin/src/binutils-gdb/gdb/unittests/mkdir-recursive-selftests.c:49:20: error: ‘mkdtemp’ was not declared in this scope
if (mkdtemp (base) == NULL)
^
Commit
e418a61a67a ("Move mkdir_recursive to common/filestuff.c")
moved this code, but also removed the HAVE_MKDTEMP guard which prevented
the mkdtemp call to be compiled on mingw.
We can either put back the HAVE_MKDTEMP ifdef, or import the gnulib
mkdtemp module, which provides the function for mingw. Since the
mkdir_recursive is susceptible to be used on mingw at some point, I
think it would be nice to have it tested on mingw, so I did the latter.
Once built, I tested it on Windows (copied the resulting gdb.exe on a
Windows machine, ran it, and ran "maint selftest mkdir_recursive"). It
failed, because the temporary directory is hardcoded to "/tmp/...". I
therefore added and used a new get_standard_temp_dir function, which
returns an appropriate temporary directory for the host platform.
gdb/ChangeLog:
* common/pathstuff.c (get_standard_temp_dir): New.
* common/pathstuff.h (get_standard_temp_dir): New.
* config.in: Re-generate.
* configure: Re-generate.
* configure.ac: Don't check for mkdtemp.
* gnulib/aclocal-m4-deps.mk: Re-generate.
* gnulib/aclocal.m4: Re-generate.
* gnulib/config.in: Re-generate.
* gnulib/configure: Re-generate.
* gnulib/import/Makefile.am: Re-generate.
* gnulib/import/Makefile.in: Re-generate.
* gnulib/import/m4/gnulib-cache.m4: Re-generate.
* gnulib/import/m4/gnulib-comp.m4: Re-generate.
* gnulib/import/m4/mkdtemp.m4: New file.
* gnulib/import/mkdtemp.c: New file.
* gnulib/update-gnulib.sh (IMPORTED_GNULIB_MODULES):
Add mkdtemp module.
* unittests/mkdir-recursive-selftests.c (test): Use
get_standard_temp_dir.
(_initialize_mkdir_recursive_selftests): Remove HAVE_MKDTEMP
ifdef.
* compile/compile.c (get_compile_file_tempdir): Likewise.
---
gdb/common/pathstuff.c | 21 ++++++++++
gdb/common/pathstuff.h | 9 +++++
gdb/compile/compile.c | 4 --
gdb/config.in | 3 --
gdb/configure | 2 +-
gdb/configure.ac | 2 +-
gdb/gnulib/aclocal-m4-deps.mk | 1 +
gdb/gnulib/aclocal.m4 | 1 +
gdb/gnulib/config.in | 6 +++
gdb/gnulib/configure | 47 +++++++++++++++++++++++
gdb/gnulib/import/Makefile.am | 11 +++++-
gdb/gnulib/import/Makefile.in | 20 +++++-----
gdb/gnulib/import/m4/gnulib-cache.m4 | 3 +-
gdb/gnulib/import/m4/gnulib-comp.m4 | 9 +++++
gdb/gnulib/import/m4/mkdtemp.m4 | 20 ++++++++++
gdb/gnulib/import/mkdtemp.c | 39 +++++++++++++++++++
gdb/gnulib/update-gnulib.sh | 1 +
gdb/unittests/mkdir-recursive-selftests.c | 15 ++++----
18 files changed, 187 insertions(+), 27 deletions(-)
create mode 100644 gdb/gnulib/import/m4/mkdtemp.m4
create mode 100644 gdb/gnulib/import/mkdtemp.c
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 48ff861edae..c0c575f3fb4 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -193,6 +193,27 @@ get_standard_cache_dir ()
/* See common/pathstuff.h. */
+std::string
+get_standard_temp_dir ()
+{
+#ifdef WIN32
+ char *tmp = getenv ("TMP");
+ if (tmp != nullptr)
+ return tmp;
+
+ tmp = getenv ("TEMP");
+ if (tmp != nullptr)
+ return tmp;
+
+ error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
+
+#else
+ return "/tmp";
+#endif
+}
+
+/* See common/pathstuff.h. */
+
const char *
get_shell ()
{
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index d57aafff079..18af733ab98 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -66,6 +66,15 @@ extern bool contains_dir_separator (const char *path);
extern std::string get_standard_cache_dir ();
+/* Get the usual temporary directory for the current platform.
+
+ On Windows, this is the TMP or TEMP environment variable. On the rest,
+ it's /tmp.
+
+ Throw an exception on error. */
+
+extern std::string get_standard_temp_dir ();
+
/* Return the file name of the user's shell. Normally this comes from
the SHELL environment variable. */
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index ec644a7b5a8..f6f49e0833f 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -395,11 +395,7 @@ get_compile_file_tempdir (void)
strcpy (tname, TEMPLATE);
#undef TEMPLATE
-#ifdef HAVE_MKDTEMP
tempdir_name = mkdtemp (tname);
-#else
- error (_("Command not supported on this host."));
-#endif
if (tempdir_name == NULL)
perror_with_name (_("Could not make temporary directory"));
diff --git a/gdb/config.in b/gdb/config.in
index f0d14143520..deb9d4a996e 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -288,9 +288,6 @@
/* Define to 1 if you have the <memory.h> header file. */
#undef HAVE_MEMORY_H
-/* Define to 1 if you have the `mkdtemp' function. */
-#undef HAVE_MKDTEMP
-
/* Define to 1 if you have a working `mmap' system call. */
#undef HAVE_MMAP
diff --git a/gdb/configure b/gdb/configure
index 3652455322f..0f3ec4520aa 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13320,7 +13320,7 @@ for ac_func in getauxval getrusage getuid getgid \
sigaction sigprocmask sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack mkdtemp setns
+ ptrace64 sigaltstack setns
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/configure.ac b/gdb/configure.ac
index b2343a90e5f..e1ea60660b9 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1359,7 +1359,7 @@ AC_CHECK_FUNCS([getauxval getrusage getuid getgid \
sigaction sigprocmask sigsetmask socketpair \
ttrace wborder wresize setlocale iconvlist libiconvlist btowc \
setrlimit getrlimit posix_madvise waitpid \
- ptrace64 sigaltstack mkdtemp setns])
+ ptrace64 sigaltstack setns])
AM_LANGINFO_CODESET
GDB_AC_COMMON
diff --git a/gdb/gnulib/aclocal-m4-deps.mk b/gdb/gnulib/aclocal-m4-deps.mk
index 5b2c6cc5ea3..ffa003d7613 100644
--- a/gdb/gnulib/aclocal-m4-deps.mk
+++ b/gdb/gnulib/aclocal-m4-deps.mk
@@ -80,6 +80,7 @@ aclocal_m4_deps = \
import/m4/mempcpy.m4 \
import/m4/memrchr.m4 \
import/m4/mkdir.m4 \
+ import/m4/mkdtemp.m4 \
import/m4/mkostemp.m4 \
import/m4/mmap-anon.m4 \
import/m4/mode_t.m4 \
diff --git a/gdb/gnulib/aclocal.m4 b/gdb/gnulib/aclocal.m4
index 861caf6692c..e480633394d 100644
--- a/gdb/gnulib/aclocal.m4
+++ b/gdb/gnulib/aclocal.m4
@@ -1353,6 +1353,7 @@ m4_include([import/m4/memmem.m4])
m4_include([import/m4/mempcpy.m4])
m4_include([import/m4/memrchr.m4])
m4_include([import/m4/mkdir.m4])
+m4_include([import/m4/mkdtemp.m4])
m4_include([import/m4/mkostemp.m4])
m4_include([import/m4/mmap-anon.m4])
m4_include([import/m4/mode_t.m4])
diff --git a/gdb/gnulib/config.in b/gdb/gnulib/config.in
index d23d208cb28..2f1a5405506 100644
--- a/gdb/gnulib/config.in
+++ b/gdb/gnulib/config.in
@@ -203,6 +203,9 @@
/* Define to 1 when the gnulib module memrchr should be tested. */
#undef GNULIB_TEST_MEMRCHR
+/* Define to 1 when the gnulib module mkdtemp should be tested. */
+#undef GNULIB_TEST_MKDTEMP
+
/* Define to 1 when the gnulib module mkostemp should be tested. */
#undef GNULIB_TEST_MKOSTEMP
@@ -548,6 +551,9 @@
when it succeeds. */
#undef HAVE_MINIMALLY_WORKING_GETCWD
+/* Define to 1 if you have the `mkdtemp' function. */
+#undef HAVE_MKDTEMP
+
/* Define to 1 if you have the 'mkostemp' function. */
#undef HAVE_MKOSTEMP
diff --git a/gdb/gnulib/configure b/gdb/gnulib/configure
index 5d7f8aa004f..340c622cb39 100644
--- a/gdb/gnulib/configure
+++ b/gdb/gnulib/configure
@@ -5841,6 +5841,7 @@ fi
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
+ # Code from module mkdtemp:
# Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
@@ -21891,6 +21892,52 @@ $as_echo "#define FUNC_MKDIR_DOT_BUG 1" >>confdefs.h
fi
+ for ac_func in mkdtemp
+do :
+ ac_fn_c_check_func "$LINENO" "mkdtemp" "ac_cv_func_mkdtemp"
+if test "x$ac_cv_func_mkdtemp" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_MKDTEMP 1
+_ACEOF
+
+fi
+done
+
+ if test $ac_cv_func_mkdtemp = no; then
+ HAVE_MKDTEMP=0
+ fi
+
+ if test $HAVE_MKDTEMP = 0; then
+
+
+
+
+
+
+
+
+ gl_LIBOBJS="$gl_LIBOBJS mkdtemp.$ac_objext"
+
+ :
+
+ fi
+
+
+
+
+
+ GNULIB_MKDTEMP=1
+
+
+
+
+
+$as_echo "#define GNULIB_TEST_MKDTEMP 1" >>confdefs.h
+
+
+
+
+
diff --git a/gdb/gnulib/import/Makefile.am b/gdb/gnulib/import/Makefile.am
index 608c2c725cf..9dca489ee10 100644
--- a/gdb/gnulib/import/Makefile.am
+++ b/gdb/gnulib/import/Makefile.am
@@ -21,7 +21,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
AUTOMAKE_OPTIONS = 1.9.6 gnits
@@ -1223,6 +1223,15 @@ EXTRA_libgnu_a_SOURCES += mkdir.c
## end gnulib module mkdir
+## begin gnulib module mkdtemp
+
+
+EXTRA_DIST += mkdtemp.c
+
+EXTRA_libgnu_a_SOURCES += mkdtemp.c
+
+## end gnulib module mkdtemp
+
## begin gnulib module mkostemp
diff --git a/gdb/gnulib/import/Makefile.in b/gdb/gnulib/import/Makefile.in
index 8b0487ccc75..f433c363485 100644
--- a/gdb/gnulib/import/Makefile.in
+++ b/gdb/gnulib/import/Makefile.in
@@ -35,7 +35,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
@@ -192,6 +192,7 @@ am__aclocal_m4_deps = $(top_srcdir)/import/m4/00gnulib.m4 \
$(top_srcdir)/import/m4/mempcpy.m4 \
$(top_srcdir)/import/m4/memrchr.m4 \
$(top_srcdir)/import/m4/mkdir.m4 \
+ $(top_srcdir)/import/m4/mkdtemp.m4 \
$(top_srcdir)/import/m4/mkostemp.m4 \
$(top_srcdir)/import/m4/mmap-anon.m4 \
$(top_srcdir)/import/m4/mode_t.m4 \
@@ -1515,9 +1516,9 @@ EXTRA_DIST = m4/gnulib-cache.m4 alloca.c alloca.in.h arpa_inet.in.h \
malloca.valgrind math.in.h mbrtowc.c mbsinit.c \
mbsrtowcs-impl.h mbsrtowcs-state.c mbsrtowcs.c memchr.c \
memchr.valgrind memmem.c str-two-way.h mempcpy.c memrchr.c \
- mkdir.c mkostemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
- msvc-nothrow.h netinet_in.in.h open.c openat.c openat.h \
- dirent-private.h opendir.c pathmax.h rawmemchr.c \
+ mkdir.c mkdtemp.c mkostemp.c msvc-inval.c msvc-inval.h \
+ msvc-nothrow.c msvc-nothrow.h netinet_in.in.h open.c openat.c \
+ openat.h dirent-private.h opendir.c pathmax.h rawmemchr.c \
rawmemchr.valgrind dirent-private.h readdir.c readlink.c \
realloc.c rename.c dirent-private.h rewinddir.c rmdir.c \
same-inode.h save-cwd.h secure_getenv.c setenv.c signal.in.h \
@@ -1587,11 +1588,11 @@ EXTRA_libgnu_a_SOURCES = alloca.c openat-proc.c canonicalize-lgpl.c \
gettimeofday.c glob.c inet_ntop.c isnan.c isnand.c isnan.c \
isnanl.c lstat.c malloc.c mbrtowc.c mbsinit.c \
mbsrtowcs-state.c mbsrtowcs.c memchr.c memmem.c mempcpy.c \
- memrchr.c mkdir.c mkostemp.c msvc-inval.c msvc-nothrow.c \
- open.c openat.c opendir.c rawmemchr.c readdir.c readlink.c \
- realloc.c rename.c rewinddir.c rmdir.c secure_getenv.c \
- setenv.c stat.c strchrnul.c strdup.c strerror.c \
- strerror-override.c strstr.c strtok_r.c unsetenv.c
+ memrchr.c mkdir.c mkdtemp.c mkostemp.c msvc-inval.c \
+ msvc-nothrow.c open.c openat.c opendir.c rawmemchr.c readdir.c \
+ readlink.c realloc.c rename.c rewinddir.c rmdir.c \
+ secure_getenv.c setenv.c stat.c strchrnul.c strdup.c \
+ strerror.c strerror-override.c strstr.c strtok_r.c unsetenv.c
# Use this preprocessor expression to decide whether #include_next works.
# Do not rely on a 'configure'-time test for this, since the expression
@@ -1722,6 +1723,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mempcpy.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memrchr.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdir.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdtemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkostemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-inval.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-nothrow.Po@am__quote@
diff --git a/gdb/gnulib/import/m4/gnulib-cache.m4 b/gdb/gnulib/import/m4/gnulib-cache.m4
index 8cefb67be7c..4cbd1a76fa4 100644
--- a/gdb/gnulib/import/m4/gnulib-cache.m4
+++ b/gdb/gnulib/import/m4/gnulib-cache.m4
@@ -27,7 +27,7 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkdtemp mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
# Specification in the form of a few gnulib-tool.m4 macro invocations:
gl_LOCAL_DIR([])
@@ -48,6 +48,7 @@ gl_MODULES([
memchr
memmem
mkdir
+ mkdtemp
mkostemp
pathmax
rawmemchr
diff --git a/gdb/gnulib/import/m4/gnulib-comp.m4 b/gdb/gnulib/import/m4/gnulib-comp.m4
index 2c55958eb7e..1700bdd3cf0 100644
--- a/gdb/gnulib/import/m4/gnulib-comp.m4
+++ b/gdb/gnulib/import/m4/gnulib-comp.m4
@@ -121,6 +121,7 @@ AC_DEFUN([gl_EARLY],
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
+ # Code from module mkdtemp:
# Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
@@ -444,6 +445,12 @@ AC_DEFUN([gl_INIT],
if test $REPLACE_MKDIR = 1; then
AC_LIBOBJ([mkdir])
fi
+ gl_FUNC_MKDTEMP
+ if test $HAVE_MKDTEMP = 0; then
+ AC_LIBOBJ([mkdtemp])
+ gl_PREREQ_MKDTEMP
+ fi
+ gl_STDLIB_MODULE_INDICATOR([mkdtemp])
gl_FUNC_MKOSTEMP
if test $HAVE_MKOSTEMP = 0; then
AC_LIBOBJ([mkostemp])
@@ -845,6 +852,7 @@ AC_DEFUN([gl_FILE_LIST], [
lib/mempcpy.c
lib/memrchr.c
lib/mkdir.c
+ lib/mkdtemp.c
lib/mkostemp.c
lib/msvc-inval.c
lib/msvc-inval.h
@@ -992,6 +1000,7 @@ AC_DEFUN([gl_FILE_LIST], [
m4/mempcpy.m4
m4/memrchr.m4
m4/mkdir.m4
+ m4/mkdtemp.m4
m4/mkostemp.m4
m4/mmap-anon.m4
m4/mode_t.m4
diff --git a/gdb/gnulib/import/m4/mkdtemp.m4 b/gdb/gnulib/import/m4/mkdtemp.m4
new file mode 100644
index 00000000000..a2edd395005
--- /dev/null
+++ b/gdb/gnulib/import/m4/mkdtemp.m4
@@ -0,0 +1,20 @@
+# mkdtemp.m4 serial 8
+dnl Copyright (C) 2001-2003, 2006-2007, 2009-2016 Free Software Foundation,
+dnl Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_MKDTEMP],
+[
+ AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+ AC_CHECK_FUNCS([mkdtemp])
+ if test $ac_cv_func_mkdtemp = no; then
+ HAVE_MKDTEMP=0
+ fi
+])
+
+# Prerequisites of lib/mkdtemp.c
+AC_DEFUN([gl_PREREQ_MKDTEMP],
+[:
+])
diff --git a/gdb/gnulib/import/mkdtemp.c b/gdb/gnulib/import/mkdtemp.c
new file mode 100644
index 00000000000..c1b05fd8ada
--- /dev/null
+++ b/gdb/gnulib/import/mkdtemp.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 1999, 2001-2003, 2006-2007, 2009-2016 Free Software
+ Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+/* Extracted from misc/mkdtemp.c. */
+
+#include <config.h>
+
+/* Specification. */
+#include <stdlib.h>
+
+#include "tempname.h"
+
+/* Generate a unique temporary directory from XTEMPLATE.
+ The last six characters of XTEMPLATE must be "XXXXXX";
+ they are replaced with a string that makes the filename unique.
+ The directory is created, mode 700, and its name is returned.
+ (This function comes from OpenBSD.) */
+char *
+mkdtemp (char *xtemplate)
+{
+ if (gen_tempname (xtemplate, 0, 0, GT_DIR))
+ return NULL;
+ else
+ return xtemplate;
+}
diff --git a/gdb/gnulib/update-gnulib.sh b/gdb/gnulib/update-gnulib.sh
index 5129450577d..e67565a0e72 100755
--- a/gdb/gnulib/update-gnulib.sh
+++ b/gdb/gnulib/update-gnulib.sh
@@ -46,6 +46,7 @@ IMPORTED_GNULIB_MODULES="\
memchr \
memmem \
mkdir \
+ mkdtemp \
mkostemp \
pathmax \
rawmemchr \
diff --git a/gdb/unittests/mkdir-recursive-selftests.c b/gdb/unittests/mkdir-recursive-selftests.c
index d501f8e0817..5a5c453ad6a 100644
--- a/gdb/unittests/mkdir-recursive-selftests.c
+++ b/gdb/unittests/mkdir-recursive-selftests.c
@@ -21,6 +21,8 @@
#include "common/filestuff.h"
#include "selftest.h"
+#include "common/byte-vector.h"
+#include "common/pathstuff.h"
namespace selftests {
namespace mkdir_recursive {
@@ -44,9 +46,10 @@ create_dir_and_check (const char *dir)
static void
test ()
{
- char base[] = "/tmp/gdb-selftests-XXXXXX";
+ std::string tmp = get_standard_temp_dir () + "/gdb-selftests";
+ gdb::char_vector base = make_temp_filename (tmp);
- if (mkdtemp (base) == NULL)
+ if (mkdtemp (base.data ()) == NULL)
perror_with_name (("mkdtemp"));
/* Try not to leave leftover directories. */
@@ -66,12 +69,12 @@ test ()
private:
const char *m_base;
- } cleanup_dirs (base);
+ } cleanup_dirs (base.data ());
- std::string dir = string_printf ("%s/a/b", base);
+ std::string dir = string_printf ("%s/a/b", base.data ());
SELF_CHECK (create_dir_and_check (dir.c_str ()));
- dir = string_printf ("%s/a/b/c//d/e/", base);
+ dir = string_printf ("%s/a/b/c//d/e/", base.data ());
SELF_CHECK (create_dir_and_check (dir.c_str ()));
}
@@ -81,9 +84,7 @@ test ()
void
_initialize_mkdir_recursive_selftests ()
{
-#if defined (HAVE_MKDTEMP)
selftests::register_test ("mkdir_recursive",
selftests::mkdir_recursive::test);
-#endif
}
--
2.19.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-29 22:16 ` Simon Marchi
@ 2018-10-30 20:55 ` Tom Tromey
2018-10-30 21:04 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-10-30 20:55 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> What do you think about this?
Thanks for doing this.
Simon> /* See common/pathstuff.h. */
Simon> +std::string
Simon> +get_standard_temp_dir ()
Simon> +{
Simon> +#ifdef WIN32
Simon> + char *tmp = getenv ("TMP");
Simon> + if (tmp != nullptr)
Simon> + return tmp;
Simon> +
Simon> + tmp = getenv ("TEMP");
Simon> + if (tmp != nullptr)
Simon> + return tmp;
Simon> +
Simon> + error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
Simon> +
Simon> +#else
Simon> + return "/tmp";
It would be normal to look at TMPDIR on unix systems.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-30 20:55 ` Tom Tromey
@ 2018-10-30 21:04 ` Simon Marchi
2018-10-31 15:03 ` Tom Tromey
0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-10-30 21:04 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-10-30 4:55 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> What do you think about this?
>
> Thanks for doing this.
>
> Simon> /* See common/pathstuff.h. */
>
> Simon> +std::string
> Simon> +get_standard_temp_dir ()
> Simon> +{
> Simon> +#ifdef WIN32
> Simon> + char *tmp = getenv ("TMP");
> Simon> + if (tmp != nullptr)
> Simon> + return tmp;
> Simon> +
> Simon> + tmp = getenv ("TEMP");
> Simon> + if (tmp != nullptr)
> Simon> + return tmp;
> Simon> +
> Simon> + error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
> Simon> +
> Simon> +#else
> Simon> + return "/tmp";
>
> It would be normal to look at TMPDIR on unix systems.
Ah, indeed. How is it with this little fixup on top of the patch?
From e8b9d0eabfb9bd85afcf42af9bdfb6a5bde66fc2 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 30 Oct 2018 17:01:50 -0400
Subject: [PATCH] fixup
---
gdb/common/pathstuff.c | 4 ++++
gdb/common/pathstuff.h | 5 +++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index c0c575f3fb4..166a21593eb 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -208,6 +208,10 @@ get_standard_temp_dir ()
error (_("Couldn't find temp dir path, both TMP and TEMP are unset."));
#else
+ char *tmp = getenv ("TMPDIR");
+ if (tmp != nullptr)
+ return tmp;
+
return "/tmp";
#endif
}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 18af733ab98..f29349e8b28 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -68,8 +68,9 @@ extern std::string get_standard_cache_dir ();
/* Get the usual temporary directory for the current platform.
- On Windows, this is the TMP or TEMP environment variable. On the rest,
- it's /tmp.
+ On Windows, this is the TMP or TEMP environment variable.
+
+ On the rest, this is the TMPDIR environment variable, if defined, else /tmp.
Throw an exception on error. */
--
2.19.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-30 21:04 ` Simon Marchi
@ 2018-10-31 15:03 ` Tom Tromey
2018-11-01 19:44 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-10-31 15:03 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>> It would be normal to look at TMPDIR on unix systems.
Simon> Ah, indeed. How is it with this little fixup on top of the patch?
Seems reasonable, thanks!
Simon> + char *tmp = getenv ("TMPDIR");
A tiny nit: you can use const char * here.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c
2018-10-31 15:03 ` Tom Tromey
@ 2018-11-01 19:44 ` Simon Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-11-01 19:44 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
On 2018-10-31 11:03, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
>>> It would be normal to look at TMPDIR on unix systems.
>
> Simon> Ah, indeed. How is it with this little fixup on top of the
> patch?
>
> Seems reasonable, thanks!
>
> Simon> + char *tmp = getenv ("TMPDIR");
>
> A tiny nit: you can use const char * here.
>
> Tom
Pushed with that fixed, thanks!
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
2018-10-18 22:31 ` [PATCH v2 1/6] Unify shell-finding logic Tom Tromey
2018-10-18 22:31 ` [PATCH v2 3/6] Move mkdir_recursive to common/filestuff.c Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-18 22:31 ` [PATCH v2 5/6] Do not reopen temporary files Tom Tromey
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently make_temp_filename is a function local to
write_psymtabs_to_index. This patch moves it to pathstuff.c so that
it can be used from other places in gdb.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* dwarf-index-write.c (write_psymtabs_to_index): Move
make_temp_filename to common/pathstuff.c.
* common/pathstuff.h (make_temp_filename): Declare.
* common/pathstuff.c (make_temp_filename): New function, moved
from dwarf-index-write.c.
---
gdb/ChangeLog | 8 ++++++++
gdb/common/pathstuff.c | 11 +++++++++++
gdb/common/pathstuff.h | 7 +++++++
gdb/dwarf-index-write.c | 11 +----------
4 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 6d8e53f4e1..48ff861eda 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -202,3 +202,14 @@ get_shell ()
return ret;
}
+
+/* See common/pathstuff.h. */
+
+gdb::char_vector
+make_temp_filename (const std::string &f)
+{
+ gdb::char_vector filename_temp (f.length () + 8);
+ strcpy (filename_temp.data (), f.c_str ());
+ strcat (filename_temp.data () + f.size (), "-XXXXXX");
+ return filename_temp;
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 40fbda8d85..14fab04805 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -20,6 +20,8 @@
#ifndef PATHSTUFF_H
#define PATHSTUFF_H
+#include "common/byte-vector.h"
+
/* Path utilities. */
/* Return the real path of FILENAME, expanding all the symbolic links.
@@ -68,4 +70,9 @@ extern std::string get_standard_cache_dir ();
extern const char *get_shell ();
+/* Make a filename suitable to pass to mkstemp based on F (e.g.
+ /tmp/foo -> /tmp/foo-XXXXXX). */
+
+extern gdb::char_vector make_temp_filename (const std::string &f);
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index d4585af837..4335c39074 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -24,6 +24,7 @@
#include "common/byte-vector.h"
#include "common/filestuff.h"
#include "common/gdb_unlinker.h"
+#include "common/pathstuff.h"
#include "common/scoped_fd.h"
#include "complaints.h"
#include "dwarf-index-common.h"
@@ -1560,16 +1561,6 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
if (stat (objfile_name (objfile), &st) < 0)
perror_with_name (objfile_name (objfile));
- /* Make a filename suitable to pass to mkstemp based on F (e.g.
- /tmp/foo -> /tmp/foo-XXXXXX). */
- auto make_temp_filename = [] (const std::string &f) -> gdb::char_vector
- {
- gdb::char_vector filename_temp (f.length () + 8);
- strcpy (filename_temp.data (), f.c_str ());
- strcat (filename_temp.data () + f.size (), "-XXXXXX");
- return filename_temp;
- };
-
std::string filename (std::string (dir) + SLASH_STRING + basename
+ (index_kind == dw_index_kind::DEBUG_NAMES
? INDEX5_SUFFIX : INDEX4_SUFFIX));
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 5/6] Do not reopen temporary files
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
` (2 preceding siblings ...)
2018-10-18 22:31 ` [PATCH v2 2/6] Move make_temp_filename to common/pathstuff.c Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-18 22:31 ` [PATCH v2 4/6] Use mkostemp, not mkstemp Tom Tromey
` (2 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The current callers of mkostemp close the file descriptor and then
re-open it with fopen. It seemed better to me to continue to use the
already-opened file descriptor, so this patch rearranges the code a
little in order to do so. It takes care to ensure that the files are
only unlinked after the file descriptor in question is closed, as
before.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* unittests/scoped_fd-selftests.c (test_to_file): New function.
(run_tests): Call test_to_file.
* dwarf-index-write.c (write_psymtabs_to_index): Do not reopen
temporary files.
* common/scoped_fd.h (scoped_fd::to_file): New method.
---
gdb/ChangeLog | 8 +++++
gdb/common/scoped_fd.h | 13 +++++++
gdb/dwarf-index-write.c | 56 ++++++++++++++---------------
gdb/unittests/scoped_fd-selftests.c | 17 +++++++++
4 files changed, 65 insertions(+), 29 deletions(-)
diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h
index c2125bd1af..d20e18a2c0 100644
--- a/gdb/common/scoped_fd.h
+++ b/gdb/common/scoped_fd.h
@@ -21,6 +21,7 @@
#define SCOPED_FD_H
#include <unistd.h>
+#include "filestuff.h"
/* A smart-pointer-like class to automatically close a file descriptor. */
@@ -43,6 +44,18 @@ public:
return fd;
}
+ /* Like release, but return a gdb_file_up that owns the file
+ descriptor. On success, this scoped_fd will be released. On
+ failure, return NULL and leave this scoped_fd in possession of
+ the fd. */
+ gdb_file_up to_file (const char *mode) noexcept
+ {
+ gdb_file_up result (fdopen (m_fd, mode));
+ if (result != nullptr)
+ m_fd = -1;
+ return result;
+ }
+
int get () const noexcept
{
return m_fd;
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index e07bda9c08..8a4c1c7ea4 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -1566,23 +1566,21 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
? INDEX5_SUFFIX : INDEX4_SUFFIX));
gdb::char_vector filename_temp = make_temp_filename (filename);
- gdb::optional<scoped_fd> out_file_fd
- (gdb::in_place, gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY));
- if (out_file_fd->get () == -1)
+ /* Order matters here; we want FILE to be closed before
+ FILENAME_TEMP is unlinked, because on MS-Windows one cannot
+ delete a file that is still open. So, we wrap the unlinker in an
+ optional and emplace it once we know the file name. */
+ gdb::optional<gdb::unlinker> unlink_file;
+ scoped_fd out_file_fd (gdb_mkostemp_cloexec (filename_temp.data (),
+ O_BINARY));
+ if (out_file_fd.get () == -1)
perror_with_name (("mkstemp"));
- FILE *out_file = gdb_fopen_cloexec (filename_temp.data (), "wb").release ();
+ gdb_file_up out_file = out_file_fd.to_file ("wb");
if (out_file == nullptr)
error (_("Can't open `%s' for writing"), filename_temp.data ());
- /* Order matters here; we want FILE to be closed before FILENAME_TEMP is
- unlinked, because on MS-Windows one cannot delete a file that is
- still open. (Don't call anything here that might throw until
- file_closer is created.) We don't need OUT_FILE_FD anymore, so we might
- as well close it now. */
- out_file_fd.reset ();
- gdb::unlinker unlink_file (filename_temp.data ());
- gdb_file_up close_out_file (out_file);
+ unlink_file.emplace (filename_temp.data ());
if (index_kind == dw_index_kind::DEBUG_NAMES)
{
@@ -1590,45 +1588,45 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
+ basename + DEBUG_STR_SUFFIX);
gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
- gdb::optional<scoped_fd> out_file_str_fd
- (gdb::in_place, gdb_mkostemp_cloexec (filename_str_temp.data (),
- O_BINARY));
- if (out_file_str_fd->get () == -1)
+ /* As above, arrange to unlink the file only after the file
+ descriptor has been closed. */
+ gdb::optional<gdb::unlinker> unlink_file_str;
+ scoped_fd out_file_str_fd
+ (gdb_mkostemp_cloexec (filename_str_temp.data (), O_BINARY));
+ if (out_file_str_fd.get () == -1)
perror_with_name (("mkstemp"));
- FILE *out_file_str
- = gdb_fopen_cloexec (filename_str_temp.data (), "wb").release ();
+ gdb_file_up out_file_str = out_file_str_fd.to_file ("wb");
if (out_file_str == nullptr)
error (_("Can't open `%s' for writing"), filename_str_temp.data ());
- out_file_str_fd.reset ();
- gdb::unlinker unlink_file_str (filename_str_temp.data ());
- gdb_file_up close_out_file_str (out_file_str);
+ unlink_file_str.emplace (filename_str_temp.data ());
const size_t total_len
- = write_debug_names (dwarf2_per_objfile, out_file, out_file_str);
- assert_file_size (out_file, filename_temp.data (), total_len);
+ = write_debug_names (dwarf2_per_objfile, out_file.get (),
+ out_file_str.get ());
+ assert_file_size (out_file.get (), filename_temp.data (), total_len);
/* We want to keep the file .debug_str file too. */
- unlink_file_str.keep ();
+ unlink_file_str->keep ();
/* Close and move the str file in place. */
- close_out_file_str.reset ();
+ out_file_str.reset ();
if (rename (filename_str_temp.data (), filename_str.c_str ()) != 0)
perror_with_name (("rename"));
}
else
{
const size_t total_len
- = write_gdbindex (dwarf2_per_objfile, out_file);
- assert_file_size (out_file, filename_temp.data (), total_len);
+ = write_gdbindex (dwarf2_per_objfile, out_file.get ());
+ assert_file_size (out_file.get (), filename_temp.data (), total_len);
}
/* We want to keep the file. */
- unlink_file.keep ();
+ unlink_file->keep ();
/* Close and move the file in place. */
- close_out_file.reset ();
+ out_file.reset ();
if (rename (filename_temp.data (), filename.c_str ()) != 0)
perror_with_name (("rename"));
}
diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c
index fb6a0d675d..6a9c727477 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -65,12 +65,29 @@ test_release ()
SELF_CHECK (close (fd) == 0 || errno != EBADF);
}
+/* Test that the file descriptor can be converted to a FILE *. */
+static void
+test_to_file ()
+{
+ char filename[] = "scoped_fd-selftest-XXXXXX";
+
+ ::scoped_fd sfd (gdb_mkostemp_cloexec (filename));
+ SELF_CHECK (sfd.get () >= 0);
+
+ unlink (filename);
+
+ gdb_file_up file = sfd.to_file ("rw");
+ SELF_CHECK (file != nullptr);
+ SELF_CHECK (sfd.get () == -1);
+}
+
/* Run selftests. */
static void
run_tests ()
{
test_destroy ();
test_release ();
+ test_to_file ();
}
} /* namespace scoped_fd */
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 4/6] Use mkostemp, not mkstemp
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
` (3 preceding siblings ...)
2018-10-18 22:31 ` [PATCH v2 5/6] Do not reopen temporary files Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-18 22:31 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
2018-10-19 23:41 ` [PATCH v2 0/6] A different approach to startup-with-shell " Sergio Durigan Junior
6 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed that gdb could leak file descriptors coming from mkstemp.
This patch fixes the problem by importing the gnulib mkostemp instead,
and then changing gdb to pass O_CLOEXEC.
A small gnulib patch was needed. This has already been accepted
upstream.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
* unittests/scoped_mmap-selftests.c (test_normal): Use
gdb_mkostemp_cloexec.
* unittests/scoped_fd-selftests.c (test_destroy, test_release):
Use gdb_mkostemp_cloexec.
* gnulib/aclocal-m4-deps.mk, gnulib/aclocal.m4,
gnulib/config.in, gnulib/configure,
gnulib/import/Makefile.am, gnulib/import/Makefile.in,
gnulib/import/m4/gnulib-cache.m4,
gnulib/import/m4/gnulib-comp.m4: Update.
* gnulib/import/m4/mkostemp.m4: New file.
* gnulib/import/m4/mkstemp.m4: Remove.
* gnulib/import/mkostemp.c: New file.
* gnulib/import/mkstemp.m4: Remove.
* gnulib/update-gnulib.sh (IMPORTED_GNULIB_MODULES): Remove
mkstemp, add mkostemp. Apply new patch.
* gnulib/import/stdlib.in.h: Apply patch.
* gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch:
New file.
* dwarf-index-write.c (write_psymtabs_to_index): Use
gdb_mkostemp_cloexec.
* common/filestuff.h (gdb_mkostemp_cloexec): New function.
---
gdb/ChangeLog | 24 +++++
gdb/common/filestuff.h | 11 +++
gdb/dwarf-index-write.c | 5 +-
gdb/gnulib/aclocal-m4-deps.mk | 2 +-
gdb/gnulib/aclocal.m4 | 2 +-
gdb/gnulib/config.in | 12 ++-
gdb/gnulib/configure | 95 +++----------------
gdb/gnulib/import/Makefile.am | 10 +-
gdb/gnulib/import/Makefile.in | 18 ++--
gdb/gnulib/import/m4/gnulib-cache.m4 | 4 +-
gdb/gnulib/import/m4/gnulib-comp.m4 | 17 ++--
gdb/gnulib/import/m4/mkostemp.m4 | 23 +++++
gdb/gnulib/import/m4/mkstemp.m4 | 82 ----------------
gdb/gnulib/import/{mkstemp.c => mkostemp.c} | 12 +--
gdb/gnulib/import/stdlib.in.h | 3 +-
...ps-Fix-compilation-error-in-C-mode-o.patch | 38 ++++++++
gdb/gnulib/update-gnulib.sh | 3 +-
gdb/unittests/scoped_fd-selftests.c | 5 +-
gdb/unittests/scoped_mmap-selftests.c | 3 +-
19 files changed, 162 insertions(+), 207 deletions(-)
create mode 100644 gdb/gnulib/import/m4/mkostemp.m4
delete mode 100644 gdb/gnulib/import/m4/mkstemp.m4
rename gdb/gnulib/import/{mkstemp.c => mkostemp.c} (79%)
create mode 100644 gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index ecfc18d600..9a12f83c27 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -20,6 +20,7 @@
#define FILESTUFF_H
#include <dirent.h>
+#include <fcntl.h>
/* Note all the file descriptors which are open when this is called.
These file descriptors will not be closed by close_most_fds. */
@@ -48,6 +49,16 @@ extern void close_most_fds (void);
extern int gdb_open_cloexec (const char *filename, int flags,
/* mode_t */ unsigned long mode);
+/* Like mkstemp, but ensures that the file descriptor is
+ close-on-exec. */
+
+static inline int
+gdb_mkostemp_cloexec (char *name_template, int flags = 0)
+{
+ /* gnulib provides a mkostemp replacement if needed. */
+ return mkostemp (name_template, flags | O_CLOEXEC);
+}
+
/* Convenience wrapper for the above, which takes the filename as an
std::string. */
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index 4335c39074..e07bda9c08 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -1567,7 +1567,7 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
gdb::char_vector filename_temp = make_temp_filename (filename);
gdb::optional<scoped_fd> out_file_fd
- (gdb::in_place, mkstemp (filename_temp.data ()));
+ (gdb::in_place, gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY));
if (out_file_fd->get () == -1)
perror_with_name (("mkstemp"));
@@ -1591,7 +1591,8 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
gdb::optional<scoped_fd> out_file_str_fd
- (gdb::in_place, mkstemp (filename_str_temp.data ()));
+ (gdb::in_place, gdb_mkostemp_cloexec (filename_str_temp.data (),
+ O_BINARY));
if (out_file_str_fd->get () == -1)
perror_with_name (("mkstemp"));
diff --git a/gdb/gnulib/aclocal-m4-deps.mk b/gdb/gnulib/aclocal-m4-deps.mk
index d866b6de89..5b2c6cc5ea 100644
--- a/gdb/gnulib/aclocal-m4-deps.mk
+++ b/gdb/gnulib/aclocal-m4-deps.mk
@@ -80,7 +80,7 @@ aclocal_m4_deps = \
import/m4/mempcpy.m4 \
import/m4/memrchr.m4 \
import/m4/mkdir.m4 \
- import/m4/mkstemp.m4 \
+ import/m4/mkostemp.m4 \
import/m4/mmap-anon.m4 \
import/m4/mode_t.m4 \
import/m4/msvc-inval.m4 \
diff --git a/gdb/gnulib/aclocal.m4 b/gdb/gnulib/aclocal.m4
index 740387ac34..861caf6692 100644
--- a/gdb/gnulib/aclocal.m4
+++ b/gdb/gnulib/aclocal.m4
@@ -1353,7 +1353,7 @@ m4_include([import/m4/memmem.m4])
m4_include([import/m4/mempcpy.m4])
m4_include([import/m4/memrchr.m4])
m4_include([import/m4/mkdir.m4])
-m4_include([import/m4/mkstemp.m4])
+m4_include([import/m4/mkostemp.m4])
m4_include([import/m4/mmap-anon.m4])
m4_include([import/m4/mode_t.m4])
m4_include([import/m4/msvc-inval.m4])
diff --git a/gdb/gnulib/config.in b/gdb/gnulib/config.in
index d32b192e0b..d23d208cb2 100644
--- a/gdb/gnulib/config.in
+++ b/gdb/gnulib/config.in
@@ -95,6 +95,10 @@
whether the gnulib module getcwd shall be considered present. */
#undef GNULIB_GETCWD
+/* Define to a C preprocessor expression that evaluates to 1 or 0, depending
+ whether the gnulib module mkostemp shall be considered present. */
+#undef GNULIB_MKOSTEMP
+
/* Define to a C preprocessor expression that evaluates to 1 or 0, depending
whether the gnulib module openat shall be considered present. */
#undef GNULIB_OPENAT
@@ -199,8 +203,8 @@
/* Define to 1 when the gnulib module memrchr should be tested. */
#undef GNULIB_TEST_MEMRCHR
-/* Define to 1 when the gnulib module mkstemp should be tested. */
-#undef GNULIB_TEST_MKSTEMP
+/* Define to 1 when the gnulib module mkostemp should be tested. */
+#undef GNULIB_TEST_MKOSTEMP
/* Define to 1 when the gnulib module open should be tested. */
#undef GNULIB_TEST_OPEN
@@ -544,8 +548,8 @@
when it succeeds. */
#undef HAVE_MINIMALLY_WORKING_GETCWD
-/* Define to 1 if you have the 'mkstemp' function. */
-#undef HAVE_MKSTEMP
+/* Define to 1 if you have the 'mkostemp' function. */
+#undef HAVE_MKOSTEMP
/* Define to 1 if you have the 'mprotect' function. */
#undef HAVE_MPROTECT
diff --git a/gdb/gnulib/configure b/gdb/gnulib/configure
index dc6c5b6657..5d7f8aa004 100644
--- a/gdb/gnulib/configure
+++ b/gdb/gnulib/configure
@@ -3525,7 +3525,7 @@ gl_func_list="$gl_func_list mbsinit"
gl_func_list="$gl_func_list mbrtowc"
gl_header_list="$gl_header_list sys/mman.h"
gl_func_list="$gl_func_list mprotect"
-gl_func_list="$gl_func_list mkstemp"
+gl_func_list="$gl_func_list mkostemp"
gl_func_list="$gl_func_list openat"
gl_func_list="$gl_func_list link"
gl_func_list="$gl_func_list secure_getenv"
@@ -5841,7 +5841,7 @@ fi
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
- # Code from module mkstemp:
+ # Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
# Code from module multiarch:
@@ -21893,116 +21893,51 @@ $as_echo "#define FUNC_MKDIR_DOT_BUG 1" >>confdefs.h
+
+
:
- if test $ac_cv_func_mkstemp = yes; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for working mkstemp" >&5
-$as_echo_n "checking for working mkstemp... " >&6; }
-if ${gl_cv_func_working_mkstemp+:} false; then :
- $as_echo_n "(cached) " >&6
-else
+ if test $ac_cv_func_mkostemp != yes; then
+ HAVE_MKOSTEMP=0
+ fi
- mkdir conftest.mkstemp
- if test "$cross_compiling" = yes; then :
- case "$host_os" in
- # Guess yes on glibc systems.
- *-gnu*) gl_cv_func_working_mkstemp="guessing yes" ;;
- # If we don't know, assume the worst.
- *) gl_cv_func_working_mkstemp="guessing no" ;;
- esac
+ if test $HAVE_MKOSTEMP = 0; then
-else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-$ac_includes_default
-int
-main ()
-{
-int result = 0;
- int i;
- off_t large = (off_t) 4294967295u;
- if (large < 0)
- large = 2147483647;
- umask (0);
- for (i = 0; i < 70; i++)
- {
- char templ[] = "conftest.mkstemp/coXXXXXX";
- int (*mkstemp_function) (char *) = mkstemp;
- int fd = mkstemp_function (templ);
- if (fd < 0)
- result |= 1;
- else
- {
- struct stat st;
- if (lseek (fd, large, SEEK_SET) != large)
- result |= 2;
- if (fstat (fd, &st) < 0)
- result |= 4;
- else if (st.st_mode & 0077)
- result |= 8;
- if (close (fd))
- result |= 16;
- }
- }
- return result;
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_run "$LINENO"; then :
- gl_cv_func_working_mkstemp=yes
-else
- gl_cv_func_working_mkstemp=no
-fi
-rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
- conftest.$ac_objext conftest.beam conftest.$ac_ext
-fi
- rm -rf conftest.mkstemp
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gl_cv_func_working_mkstemp" >&5
-$as_echo "$gl_cv_func_working_mkstemp" >&6; }
- case "$gl_cv_func_working_mkstemp" in
- *yes) ;;
- *)
- REPLACE_MKSTEMP=1
- ;;
- esac
- else
- HAVE_MKSTEMP=0
- fi
- if test $HAVE_MKSTEMP = 0 || test $REPLACE_MKSTEMP = 1; then
+ gl_LIBOBJS="$gl_LIBOBJS mkostemp.$ac_objext"
+ fi
- gl_LIBOBJS="$gl_LIBOBJS mkstemp.$ac_objext"
+cat >>confdefs.h <<_ACEOF
+#define GNULIB_MKOSTEMP 1
+_ACEOF
- fi
- GNULIB_MKSTEMP=1
+ GNULIB_MKOSTEMP=1
-$as_echo "#define GNULIB_TEST_MKSTEMP 1" >>confdefs.h
+$as_echo "#define GNULIB_TEST_MKOSTEMP 1" >>confdefs.h
diff --git a/gdb/gnulib/import/Makefile.am b/gdb/gnulib/import/Makefile.am
index d1bee0bc02..608c2c725c 100644
--- a/gdb/gnulib/import/Makefile.am
+++ b/gdb/gnulib/import/Makefile.am
@@ -21,7 +21,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
AUTOMAKE_OPTIONS = 1.9.6 gnits
@@ -1223,14 +1223,14 @@ EXTRA_libgnu_a_SOURCES += mkdir.c
## end gnulib module mkdir
-## begin gnulib module mkstemp
+## begin gnulib module mkostemp
-EXTRA_DIST += mkstemp.c
+EXTRA_DIST += mkostemp.c
-EXTRA_libgnu_a_SOURCES += mkstemp.c
+EXTRA_libgnu_a_SOURCES += mkostemp.c
-## end gnulib module mkstemp
+## end gnulib module mkostemp
## begin gnulib module msvc-inval
diff --git a/gdb/gnulib/import/Makefile.in b/gdb/gnulib/import/Makefile.in
index 7e4d278d91..8b0487ccc7 100644
--- a/gdb/gnulib/import/Makefile.in
+++ b/gdb/gnulib/import/Makefile.in
@@ -35,7 +35,7 @@
# the same distribution terms as the rest of that program.
#
# Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
@@ -192,7 +192,7 @@ am__aclocal_m4_deps = $(top_srcdir)/import/m4/00gnulib.m4 \
$(top_srcdir)/import/m4/mempcpy.m4 \
$(top_srcdir)/import/m4/memrchr.m4 \
$(top_srcdir)/import/m4/mkdir.m4 \
- $(top_srcdir)/import/m4/mkstemp.m4 \
+ $(top_srcdir)/import/m4/mkostemp.m4 \
$(top_srcdir)/import/m4/mmap-anon.m4 \
$(top_srcdir)/import/m4/mode_t.m4 \
$(top_srcdir)/import/m4/msvc-inval.m4 \
@@ -1515,7 +1515,7 @@ EXTRA_DIST = m4/gnulib-cache.m4 alloca.c alloca.in.h arpa_inet.in.h \
malloca.valgrind math.in.h mbrtowc.c mbsinit.c \
mbsrtowcs-impl.h mbsrtowcs-state.c mbsrtowcs.c memchr.c \
memchr.valgrind memmem.c str-two-way.h mempcpy.c memrchr.c \
- mkdir.c mkstemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
+ mkdir.c mkostemp.c msvc-inval.c msvc-inval.h msvc-nothrow.c \
msvc-nothrow.h netinet_in.in.h open.c openat.c openat.h \
dirent-private.h opendir.c pathmax.h rawmemchr.c \
rawmemchr.valgrind dirent-private.h readdir.c readlink.c \
@@ -1587,11 +1587,11 @@ EXTRA_libgnu_a_SOURCES = alloca.c openat-proc.c canonicalize-lgpl.c \
gettimeofday.c glob.c inet_ntop.c isnan.c isnand.c isnan.c \
isnanl.c lstat.c malloc.c mbrtowc.c mbsinit.c \
mbsrtowcs-state.c mbsrtowcs.c memchr.c memmem.c mempcpy.c \
- memrchr.c mkdir.c mkstemp.c msvc-inval.c msvc-nothrow.c open.c \
- openat.c opendir.c rawmemchr.c readdir.c readlink.c realloc.c \
- rename.c rewinddir.c rmdir.c secure_getenv.c setenv.c stat.c \
- strchrnul.c strdup.c strerror.c strerror-override.c strstr.c \
- strtok_r.c unsetenv.c
+ memrchr.c mkdir.c mkostemp.c msvc-inval.c msvc-nothrow.c \
+ open.c openat.c opendir.c rawmemchr.c readdir.c readlink.c \
+ realloc.c rename.c rewinddir.c rmdir.c secure_getenv.c \
+ setenv.c stat.c strchrnul.c strdup.c strerror.c \
+ strerror-override.c strstr.c strtok_r.c unsetenv.c
# Use this preprocessor expression to decide whether #include_next works.
# Do not rely on a 'configure'-time test for this, since the expression
@@ -1722,7 +1722,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mempcpy.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memrchr.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkdir.Po@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkstemp.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/mkostemp.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-inval.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/msvc-nothrow.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/open.Po@am__quote@
diff --git a/gdb/gnulib/import/m4/gnulib-cache.m4 b/gdb/gnulib/import/m4/gnulib-cache.m4
index 442aad5563..8cefb67be7 100644
--- a/gdb/gnulib/import/m4/gnulib-cache.m4
+++ b/gdb/gnulib/import/m4/gnulib-cache.m4
@@ -27,7 +27,7 @@
# Specification in the form of a command-line invocation:
-# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkstemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
+# gnulib-tool --import --lib=libgnu --source-base=import --m4-base=import/m4 --doc-base=doc --tests-base=tests --aux-dir=import/extra --no-conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca canonicalize-lgpl dirent dirfd errno fnmatch-gnu frexpl getcwd glob inet_ntop inttypes limits-h lstat memchr memmem mkdir mkostemp pathmax rawmemchr readlink rename setenv signal-h strchrnul strstr strtok_r sys_stat unistd unsetenv update-copyright wchar wctype-h
# Specification in the form of a few gnulib-tool.m4 macro invocations:
gl_LOCAL_DIR([])
@@ -48,7 +48,7 @@ gl_MODULES([
memchr
memmem
mkdir
- mkstemp
+ mkostemp
pathmax
rawmemchr
readlink
diff --git a/gdb/gnulib/import/m4/gnulib-comp.m4 b/gdb/gnulib/import/m4/gnulib-comp.m4
index 21e4383c34..2c55958eb7 100644
--- a/gdb/gnulib/import/m4/gnulib-comp.m4
+++ b/gdb/gnulib/import/m4/gnulib-comp.m4
@@ -121,7 +121,7 @@ AC_DEFUN([gl_EARLY],
# Code from module mempcpy:
# Code from module memrchr:
# Code from module mkdir:
- # Code from module mkstemp:
+ # Code from module mkostemp:
# Code from module msvc-inval:
# Code from module msvc-nothrow:
# Code from module multiarch:
@@ -444,12 +444,13 @@ AC_DEFUN([gl_INIT],
if test $REPLACE_MKDIR = 1; then
AC_LIBOBJ([mkdir])
fi
- gl_FUNC_MKSTEMP
- if test $HAVE_MKSTEMP = 0 || test $REPLACE_MKSTEMP = 1; then
- AC_LIBOBJ([mkstemp])
- gl_PREREQ_MKSTEMP
+ gl_FUNC_MKOSTEMP
+ if test $HAVE_MKOSTEMP = 0; then
+ AC_LIBOBJ([mkostemp])
+ gl_PREREQ_MKOSTEMP
fi
- gl_STDLIB_MODULE_INDICATOR([mkstemp])
+ gl_MODULE_INDICATOR([mkostemp])
+ gl_STDLIB_MODULE_INDICATOR([mkostemp])
AC_REQUIRE([gl_MSVC_INVAL])
if test $HAVE_MSVC_INVALID_PARAMETER_HANDLER = 1; then
AC_LIBOBJ([msvc-inval])
@@ -844,7 +845,7 @@ AC_DEFUN([gl_FILE_LIST], [
lib/mempcpy.c
lib/memrchr.c
lib/mkdir.c
- lib/mkstemp.c
+ lib/mkostemp.c
lib/msvc-inval.c
lib/msvc-inval.h
lib/msvc-nothrow.c
@@ -991,7 +992,7 @@ AC_DEFUN([gl_FILE_LIST], [
m4/mempcpy.m4
m4/memrchr.m4
m4/mkdir.m4
- m4/mkstemp.m4
+ m4/mkostemp.m4
m4/mmap-anon.m4
m4/mode_t.m4
m4/msvc-inval.m4
diff --git a/gdb/gnulib/import/m4/mkostemp.m4 b/gdb/gnulib/import/m4/mkostemp.m4
new file mode 100644
index 0000000000..1f44a0390a
--- /dev/null
+++ b/gdb/gnulib/import/m4/mkostemp.m4
@@ -0,0 +1,23 @@
+# mkostemp.m4 serial 2
+dnl Copyright (C) 2009-2016 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_MKOSTEMP],
+[
+ AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+
+ dnl Persuade glibc <stdlib.h> to declare mkostemp().
+ AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
+
+ AC_CHECK_FUNCS_ONCE([mkostemp])
+ if test $ac_cv_func_mkostemp != yes; then
+ HAVE_MKOSTEMP=0
+ fi
+])
+
+# Prerequisites of lib/mkostemp.c.
+AC_DEFUN([gl_PREREQ_MKOSTEMP],
+[
+])
diff --git a/gdb/gnulib/import/m4/mkstemp.m4 b/gdb/gnulib/import/m4/mkstemp.m4
deleted file mode 100644
index 131e4a7b26..0000000000
--- a/gdb/gnulib/import/m4/mkstemp.m4
+++ /dev/null
@@ -1,82 +0,0 @@
-#serial 23
-
-# Copyright (C) 2001, 2003-2007, 2009-2016 Free Software Foundation, Inc.
-# This file is free software; the Free Software Foundation
-# gives unlimited permission to copy and/or distribute it,
-# with or without modifications, as long as this notice is preserved.
-
-# On some hosts (e.g., HP-UX 10.20, SunOS 4.1.4, Solaris 2.5.1), mkstemp has a
-# silly limit that it can create no more than 26 files from a given template.
-# Other systems lack mkstemp altogether.
-# On OSF1/Tru64 V4.0F, the system-provided mkstemp function can create
-# only 32 files per process.
-# On some hosts, mkstemp creates files with mode 0666, which is a security
-# problem and a violation of POSIX 2008.
-# On systems like the above, arrange to use the replacement function.
-AC_DEFUN([gl_FUNC_MKSTEMP],
-[
- AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
- AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
-
- AC_CHECK_FUNCS_ONCE([mkstemp])
- if test $ac_cv_func_mkstemp = yes; then
- AC_CACHE_CHECK([for working mkstemp],
- [gl_cv_func_working_mkstemp],
- [
- mkdir conftest.mkstemp
- AC_RUN_IFELSE(
- [AC_LANG_PROGRAM(
- [AC_INCLUDES_DEFAULT],
- [[int result = 0;
- int i;
- off_t large = (off_t) 4294967295u;
- if (large < 0)
- large = 2147483647;
- umask (0);
- for (i = 0; i < 70; i++)
- {
- char templ[] = "conftest.mkstemp/coXXXXXX";
- int (*mkstemp_function) (char *) = mkstemp;
- int fd = mkstemp_function (templ);
- if (fd < 0)
- result |= 1;
- else
- {
- struct stat st;
- if (lseek (fd, large, SEEK_SET) != large)
- result |= 2;
- if (fstat (fd, &st) < 0)
- result |= 4;
- else if (st.st_mode & 0077)
- result |= 8;
- if (close (fd))
- result |= 16;
- }
- }
- return result;]])],
- [gl_cv_func_working_mkstemp=yes],
- [gl_cv_func_working_mkstemp=no],
- [case "$host_os" in
- # Guess yes on glibc systems.
- *-gnu*) gl_cv_func_working_mkstemp="guessing yes" ;;
- # If we don't know, assume the worst.
- *) gl_cv_func_working_mkstemp="guessing no" ;;
- esac
- ])
- rm -rf conftest.mkstemp
- ])
- case "$gl_cv_func_working_mkstemp" in
- *yes) ;;
- *)
- REPLACE_MKSTEMP=1
- ;;
- esac
- else
- HAVE_MKSTEMP=0
- fi
-])
-
-# Prerequisites of lib/mkstemp.c.
-AC_DEFUN([gl_PREREQ_MKSTEMP],
-[
-])
diff --git a/gdb/gnulib/import/mkstemp.c b/gdb/gnulib/import/mkostemp.c
similarity index 79%
rename from gdb/gnulib/import/mkstemp.c
rename to gdb/gnulib/import/mkostemp.c
index 90ed78e49e..31c3e4837f 100644
--- a/gdb/gnulib/import/mkstemp.c
+++ b/gdb/gnulib/import/mkostemp.c
@@ -24,7 +24,7 @@
#if !_LIBC
# include "tempname.h"
# define __gen_tempname gen_tempname
-# ifndef __GT_FILE
+# ifndef __GTFILE
# define __GT_FILE GT_FILE
# endif
#endif
@@ -38,13 +38,9 @@
/* Generate a unique temporary file name from XTEMPLATE.
The last six characters of XTEMPLATE must be "XXXXXX";
they are replaced with a string that makes the file name unique.
- Then open the file and return a fd.
-
- If you are creating temporary files which will later be removed,
- consider using the clean-temp module, which avoids several pitfalls
- of using mkstemp directly. */
+ Then open the file and return a fd. */
int
-mkstemp (char *xtemplate)
+mkostemp (char *xtemplate, int flags)
{
- return __gen_tempname (xtemplate, 0, 0, __GT_FILE);
+ return __gen_tempname (xtemplate, 0, flags, __GT_FILE);
}
diff --git a/gdb/gnulib/import/stdlib.in.h b/gdb/gnulib/import/stdlib.in.h
index db3253bd97..8f803a2ea3 100644
--- a/gdb/gnulib/import/stdlib.in.h
+++ b/gdb/gnulib/import/stdlib.in.h
@@ -87,9 +87,10 @@ struct random_data
# endif
#endif
-#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
+#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_MKOSTEMP@ || @GNULIB_MKOSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
/* On Mac OS X 10.3, only <unistd.h> declares mkstemp. */
/* On Mac OS X 10.5, only <unistd.h> declares mkstemps. */
+/* On Mac OS X 10.13, only <unistd.h> declares mkostemp and mkostemps. */
/* On Cygwin 1.7.1, only <unistd.h> declares getsubopt. */
/* But avoid namespace pollution on glibc systems and native Windows. */
# include <unistd.h>
diff --git a/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch b/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
new file mode 100644
index 0000000000..35f917fac4
--- /dev/null
+++ b/gdb/gnulib/patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch
@@ -0,0 +1,38 @@
+From 6954995dd32ea98a1973df31f411f3996bb47dfb Mon Sep 17 00:00:00 2001
+From: Tom Tromey <tom@tromey.com>
+Date: Mon, 1 Oct 2018 14:57:45 -0600
+Subject: [PATCH] mkostemp, mkostemps: Fix compilation error in C++ mode on Mac
+ OS X.
+
+Attempting to use the mkostemp module in gdb caused a build failure
+when using the C++ namespace feature, because mkostemp was not
+declared. On OS X, mkostemp is declared in unistd.h, so this patch
+extends the existing special case in stdlib.in.h to cover mkostemp and
+mkostemps.
+
+* lib/stdlib.in.h: Include <unistd.h> for mkostemp and mkostemps
+on OS X.
+---
+ ChangeLog | 6 ++++++
+ lib/stdlib.in.h | 3 ++-
+ 2 files changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/gdb/gnulib/import/stdlib.in.h b/gdb/gnulib/import/stdlib.in.h
+index db3253bd97..8f803a2ea3 100644
+--- a/gdb/gnulib/import/stdlib.in.h
++++ b/gdb/gnulib/import/stdlib.in.h
+@@ -87,9 +87,10 @@ struct random_data
+ # endif
+ #endif
+
+-#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
++#if (@GNULIB_MKSTEMP@ || @GNULIB_MKSTEMPS@ || @GNULIB_MKOSTEMP@ || @GNULIB_MKOSTEMPS@ || @GNULIB_GETSUBOPT@ || defined GNULIB_POSIXCHECK) && ! defined __GLIBC__ && !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
+ /* On Mac OS X 10.3, only <unistd.h> declares mkstemp. */
+ /* On Mac OS X 10.5, only <unistd.h> declares mkstemps. */
++/* On Mac OS X 10.13, only <unistd.h> declares mkostemp and mkostemps. */
+ /* On Cygwin 1.7.1, only <unistd.h> declares getsubopt. */
+ /* But avoid namespace pollution on glibc systems and native Windows. */
+ # include <unistd.h>
+--
+2.19.0
+
diff --git a/gdb/gnulib/update-gnulib.sh b/gdb/gnulib/update-gnulib.sh
index 09933ab9fe..5129450577 100755
--- a/gdb/gnulib/update-gnulib.sh
+++ b/gdb/gnulib/update-gnulib.sh
@@ -46,7 +46,7 @@ IMPORTED_GNULIB_MODULES="\
memchr \
memmem \
mkdir \
- mkstemp \
+ mkostemp \
pathmax \
rawmemchr \
readlink \
@@ -169,6 +169,7 @@ apply_patches ()
}
apply_patches "patches/0001-Fix-PR-gdb-23558-Use-system-s-getcwd-when-cross-comp.patch"
+apply_patches "patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch"
# Regenerate all necessary files...
aclocal -Iimport/m4 &&
diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c
index d5c0d30abb..fb6a0d675d 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -19,6 +19,7 @@
#include "defs.h"
+#include "common/filestuff.h"
#include "common/scoped_fd.h"
#include "config.h"
#include "selftest.h"
@@ -31,7 +32,7 @@ static void
test_destroy ()
{
char filename[] = "scoped_fd-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
unlink (filename);
@@ -50,7 +51,7 @@ static void
test_release ()
{
char filename[] = "scoped_fd-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
unlink (filename);
diff --git a/gdb/unittests/scoped_mmap-selftests.c b/gdb/unittests/scoped_mmap-selftests.c
index b181e02f50..75d1aeda8a 100644
--- a/gdb/unittests/scoped_mmap-selftests.c
+++ b/gdb/unittests/scoped_mmap-selftests.c
@@ -19,6 +19,7 @@
#include "defs.h"
+#include "common/filestuff.h"
#include "common/scoped_mmap.h"
#include "config.h"
@@ -88,7 +89,7 @@ static void
test_normal ()
{
char filename[] = "scoped_mmapped_file-selftest-XXXXXX";
- int fd = mkstemp (filename);
+ int fd = gdb_mkostemp_cloexec (filename);
SELF_CHECK (fd >= 0);
SELF_CHECK (write (fd, "Hello!", 7) == 7);
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 6/6] Cache a copy of the user's shell on macOS
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
` (4 preceding siblings ...)
2018-10-18 22:31 ` [PATCH v2 4/6] Use mkostemp, not mkstemp Tom Tromey
@ 2018-10-18 22:31 ` Tom Tromey
2018-10-20 2:35 ` Simon Marchi
2018-10-19 23:41 ` [PATCH v2 0/6] A different approach to startup-with-shell " Sergio Durigan Junior
6 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-10-18 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Recent versions of macOS have a feature called System Integrity
Protection. Among other things, This feature prevents ptrace from
tracing certain programs --- for example, the programs in /bin, which
includes typical shells.
This means that startup-with-shell does not work properly. This is PR
cli/23364. Currently there is a workaround in gdb to disable
startup-with-shell when this feature might be in use.
This patch changes gdb to be a bit more precise about when
startup-with-shell will not work, by checking whether the shell
executable is restricted.
If the shell is restricted, then this patch will also cause gdb to
cache a copy of the shell in the gdb cache directory, and then reset
the SHELL environment variable to point to this copy. This lets
startup-with-shell work again.
Tested on High Sierra by trying to start a program using redirection,
and by running startup-with-shell.exp.
gdb/ChangeLog
2018-10-18 Tom Tromey <tom@tromey.com>
PR cli/23364:
* darwin-nat.c (copied_shell): New global.
(may_have_sip): Rename from should_disable_startup_with_shell.
(copy_shell_to_cache, maybe_cache_shell): New functions.
(darwin_nat_target::create_inferior): Update. Use
copied_shell.
---
gdb/ChangeLog | 9 +++
gdb/darwin-nat.c | 151 ++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 153 insertions(+), 7 deletions(-)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index ecc7635d04..1c6f8e061b 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -38,6 +38,7 @@
#include "bfd.h"
#include "bfd/mach-o.h"
+#include <copyfile.h>
#include <sys/ptrace.h>
#include <sys/signal.h>
#include <setjmp.h>
@@ -61,7 +62,11 @@
#include <mach/port.h>
#include "darwin-nat.h"
+#include "filenames.h"
#include "common/filestuff.h"
+#include "common/gdb_unlinker.h"
+#include "common/pathstuff.h"
+#include "common/scoped_fd.h"
#include "nat/fork-inferior.h"
/* Quick overview.
@@ -119,6 +124,10 @@ static int enable_mach_exceptions;
/* Inferior that should report a fake stop event. */
static struct inferior *darwin_inf_fake_stop;
+/* If non-NULL, the shell we actually invoke. See maybe_cache_shell
+ for details. */
+static const char *copied_shell;
+
#define PAGE_TRUNC(x) ((x) & ~(mach_page_size - 1))
#define PAGE_ROUND(x) PAGE_TRUNC((x) + mach_page_size - 1)
@@ -1833,10 +1842,11 @@ darwin_execvp (const char *file, char * const argv[], char * const env[])
posix_spawnp (NULL, argv[0], NULL, &attr, argv, env);
}
-/* Read kernel version, and return TRUE on Sierra or later. */
+/* Read kernel version, and return TRUE if this host may have System
+ Integrity Protection (Sierra or later). */
static bool
-should_disable_startup_with_shell ()
+may_have_sip ()
{
char str[16];
size_t sz = sizeof (str);
@@ -1852,6 +1862,131 @@ should_disable_startup_with_shell ()
return false;
}
+/* A helper for maybe_cache_shell. This copies the shell to the
+ cache. It will throw an exception on any failure. */
+
+static void
+copy_shell_to_cache (const char *shell, const std::string &new_name)
+{
+ scoped_fd from_fd (gdb_open_cloexec (shell, O_RDONLY, 0));
+ if (from_fd.get () < 0)
+ error (_("Could not open shell (%s) for reading: %s"),
+ shell, safe_strerror (errno));
+
+ std::string new_dir = ldirname (new_name.c_str ());
+ if (!mkdir_recursive (new_dir.c_str ()))
+ error (_("Could not make cache directory \"%s\": %s"),
+ new_dir.c_str (), safe_strerror (errno));
+
+ gdb::char_vector temp_name = make_temp_filename (new_name);
+ scoped_fd to_fd (gdb_mkostemp_cloexec (&temp_name[0]));
+ gdb::unlinker unlink_file_on_error (temp_name.data ());
+
+ if (to_fd.get () < 0)
+ error (_("Could not open temporary file \"%s\" for writing: %s"),
+ temp_name.data (), safe_strerror (errno));
+
+ if (fcopyfile (from_fd.get (), to_fd.get (), nullptr,
+ COPYFILE_STAT | COPYFILE_DATA) != 0)
+ error (_("Could not copy shell to cache as \"%s\": %s"),
+ temp_name.data (), safe_strerror (errno));
+
+ /* Be sure that the caching is atomic so that we don't get bad
+ results from multiple copies of gdb running at the same time. */
+ if (rename (temp_name.data (), new_name.c_str ()) != 0)
+ error (_("Could not rename shell cache file to \"%s\": %s"),
+ new_name.c_str (), safe_strerror (errno));
+
+ unlink_file_on_error.keep ();
+}
+
+/* If $SHELL is restricted, try to cache a copy. Starting with El
+ Capitan, macOS introduced System Integrity Protection. Among other
+ things, this prevents certain executables from being ptrace'd. In
+ particular, executables in /bin, like most shells, are affected.
+ To work around this, while preserving command-line glob expansion
+ and redirections, gdb will cache a copy of the shell. Return true
+ if all is well -- either the shell is not subject to SIP or it has
+ been successfully cached. Returns false if something failed. */
+
+static bool
+maybe_cache_shell ()
+{
+ /* SF_RESTRICTED is defined in sys/stat.h and lets us determine if a
+ given file is subject to SIP. */
+#ifdef SF_RESTRICTED
+
+ /* If a check fails we want to revert -- maybe the user deleted the
+ cache while gdb was running, or something like that. */
+ copied_shell = nullptr;
+
+ const char *shell = get_shell ();
+ if (!IS_ABSOLUTE_PATH (shell))
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Normally gdb would try to work around this by caching a copy of your shell,\n\
+but because your shell (%s) is not an absolute path, this is being skipped."),
+ shell);
+ return false;
+ }
+
+ struct stat sb;
+ if (stat (shell, &sb) < 0)
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Normally gdb would try to work around this by caching a copy of your shell,\n\
+but because gdb could not stat your shell (%s), this is being skipped.\n\
+The error was: %s"),
+ shell, safe_strerror (errno));
+ return false;
+ }
+
+ if ((sb.st_flags & SF_RESTRICTED) == 0)
+ return true;
+
+ /* Put the copy somewhere like ~/Library/Caches/gdb/bin/sh. */
+ std::string new_name = get_standard_cache_dir ();
+ if (!IS_DIR_SEPARATOR (new_name.back ()) && !IS_ABSOLUTE_PATH (shell))
+ new_name.push_back ('/');
+ new_name.append (shell);
+
+ /* Maybe it was cached by some earlier gdb. */
+ if (stat (new_name.c_str (), &sb) != 0 || !S_ISREG (sb.st_mode))
+ {
+ TRY
+ {
+ copy_shell_to_cache (shell, new_name);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ warning (_("This version of macOS has System Integrity Protection.\n\
+Because `startup-with-shell' is enabled, gdb tried to work around SIP by\n\
+caching a copy of your shell. However, this failed:\n\
+%s\n\
+If you correct the problem, gdb will automatically try again the next time\n\
+you \"run\". To prevent these attempts, you can use:\n\
+ set startup-with-shell off"),
+ ex.message);
+ return false;
+ }
+ END_CATCH
+
+ printf_filtered (_("Note: this version of macOS has System Integrity Protection.\n\
+Because `startup-with-shell' is enabled, gdb has worked around this by\n\
+caching a copy of your shell. The shell used by \"run\" is now:\n\
+ %s\n"),
+ new_name.c_str ());
+ }
+
+ /* We need to make sure that the new name has the correct lifetime. */
+ static std::string saved_shell = std::move (new_name);
+ copied_shell = saved_shell.c_str ();
+
+#endif /* SF_RESTRICTED */
+
+ return true;
+}
+
void
darwin_nat_target::create_inferior (const char *exec_file,
const std::string &allargs,
@@ -1859,16 +1994,18 @@ darwin_nat_target::create_inferior (const char *exec_file,
{
gdb::optional<scoped_restore_tmpl<int>> restore_startup_with_shell;
- if (startup_with_shell && should_disable_startup_with_shell ())
+ if (startup_with_shell && may_have_sip ())
{
- warning (_("startup-with-shell not supported on this macOS version,"
- " disabling it."));
- restore_startup_with_shell.emplace (&startup_with_shell, 0);
+ if (!maybe_cache_shell ())
+ {
+ warning (_("startup-with-shell is now temporarily disabled"));
+ restore_startup_with_shell.emplace (&startup_with_shell, 0);
+ }
}
/* Do the hard work. */
fork_inferior (exec_file, allargs, env, darwin_ptrace_me,
- darwin_ptrace_him, darwin_pre_ptrace, NULL,
+ darwin_ptrace_him, darwin_pre_ptrace, copied_shell,
darwin_execvp);
}
\f
--
2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/6] Cache a copy of the user's shell on macOS
2018-10-18 22:31 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
@ 2018-10-20 2:35 ` Simon Marchi
2018-10-27 17:43 ` Tom Tromey
0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-10-20 2:35 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-10-18 6:31 p.m., Tom Tromey wrote:
> +/* If $SHELL is restricted, try to cache a copy. Starting with El
> + Capitan, macOS introduced System Integrity Protection. Among other
> + things, this prevents certain executables from being ptrace'd. In
> + particular, executables in /bin, like most shells, are affected.
> + To work around this, while preserving command-line glob expansion
> + and redirections, gdb will cache a copy of the shell. Return true
> + if all is well -- either the shell is not subject to SIP or it has
> + been successfully cached. Returns false if something failed. */
> +
> +static bool
> +maybe_cache_shell ()
> +{
> + /* SF_RESTRICTED is defined in sys/stat.h and lets us determine if a
> + given file is subject to SIP. */
> +#ifdef SF_RESTRICTED
> +
> + /* If a check fails we want to revert -- maybe the user deleted the
> + cache while gdb was running, or something like that. */
> + copied_shell = nullptr;
> +
> + const char *shell = get_shell ();
> + if (!IS_ABSOLUTE_PATH (shell))
> + {
> + warning (_("This version of macOS has System Integrity Protection.\n\
> +Normally gdb would try to work around this by caching a copy of your shell,\n\
> +but because your shell (%s) is not an absolute path, this is being skipped."),
> + shell);
> + return false;
> + }
> +
> + struct stat sb;
> + if (stat (shell, &sb) < 0)
> + {
> + warning (_("This version of macOS has System Integrity Protection.\n\
> +Normally gdb would try to work around this by caching a copy of your shell,\n\
> +but because gdb could not stat your shell (%s), this is being skipped.\n\
> +The error was: %s"),
> + shell, safe_strerror (errno));
> + return false;
> + }
> +
> + if ((sb.st_flags & SF_RESTRICTED) == 0)
> + return true;
> +
> + /* Put the copy somewhere like ~/Library/Caches/gdb/bin/sh. */
> + std::string new_name = get_standard_cache_dir ();
> + if (!IS_DIR_SEPARATOR (new_name.back ()) && !IS_ABSOLUTE_PATH (shell))
I mentioned something about this on the version of the patch, but you might
have missed it since you didn't reply to that specifically, so I'll send
it again just to be sure:
I believe this !IS_ABSOLUTE_PATH check can never be true, since we would
have returned early if it was the case? If so, this append is not needed
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/6] Cache a copy of the user's shell on macOS
2018-10-20 2:35 ` Simon Marchi
@ 2018-10-27 17:43 ` Tom Tromey
0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-10-27 17:43 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>> + /* Put the copy somewhere like ~/Library/Caches/gdb/bin/sh. */
>> + std::string new_name = get_standard_cache_dir ();
>> + if (!IS_DIR_SEPARATOR (new_name.back ()) && !IS_ABSOLUTE_PATH (shell))
Simon> I mentioned something about this on the version of the patch, but you might
Simon> have missed it since you didn't reply to that specifically, so I'll send
Simon> it again just to be sure:
Simon> I believe this !IS_ABSOLUTE_PATH check can never be true, since we would
Simon> have returned early if it was the case? If so, this append is not needed
I will remove the redundant check.
I think I changed this at some point and forgot to update that.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/6] A different approach to startup-with-shell on macOS
2018-10-18 22:31 [PATCH v2 0/6] A different approach to startup-with-shell on macOS Tom Tromey
` (5 preceding siblings ...)
2018-10-18 22:31 ` [PATCH v2 6/6] Cache a copy of the user's shell on macOS Tom Tromey
@ 2018-10-19 23:41 ` Sergio Durigan Junior
2018-10-27 17:58 ` Tom Tromey
6 siblings, 1 reply; 20+ messages in thread
From: Sergio Durigan Junior @ 2018-10-19 23:41 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Thursday, October 18 2018, Tom Tromey wrote:
> Here's version 2 of the series to cache the shell on macOS.
>
> I believe this addresses all review comments.
Thanks, Tom.
I looked at all the patches, and they seem OK to me, to the extent of my
zero-knowledge about proprietary systems.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 20+ messages in thread