From patchwork Fri Dec 6 20:44:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Terekhov, Mikhail via Gdb-patches" X-Patchwork-Id: 36575 Received: (qmail 109994 invoked by alias); 6 Dec 2019 20:44:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 109977 invoked by uid 89); 6 Dec 2019 20:44:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=11710, 3046 X-HELO: mail-yb1-f201.google.com Received: from mail-yb1-f201.google.com (HELO mail-yb1-f201.google.com) (209.85.219.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2019 20:44:31 +0000 Received: by mail-yb1-f201.google.com with SMTP id t12so1663586ybc.0 for ; Fri, 06 Dec 2019 12:44:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=YBAqqPSzGb3uHv2NpyozbpKiZmDR8OetGnR9sd9NIa0=; b=IpcUhHkhUh4JQCfWP4Mie6piHCNy+1Awd1VkDXS2Oq1QzRm8MAUUP5c1yOWG5WhH1R Q8WlhoglOLWkna0FMmgyvxGFoLoFKkgS/NOmVOJvpMvSSLIOu0qkPr0vt/MDxbqr8ZYE e4ZdGcSFrFrMOe34RbIm8RyBzbWs4pUjCdrerr+zdTVXDUrdPXYMagH66nxe7Jy6b5GN k6uACbph/y74Ns8DcMSsWAAk1XoMO4juPuqKoAUD0hDdYjf3WDivhxbcIzJmZh9Nvn9y /5bK8EyPsI0Ows0GwrRuW/UKKJGN1efARzXyVEa3oCfj6ZDWLlXVR3H+i4FpdI650eWG Fmzw== Date: Fri, 6 Dec 2019 14:44:26 -0600 In-Reply-To: Message-Id: <20191206204426.121447-1-cbiesinger@google.com> Mime-Version: 1.0 References: Subject: [PATCH v3] Replace the remaining uses of strerror with safe_strerror X-Patchwork-Original-From: "Christian Biesinger via gdb-patches" From: "Terekhov, Mikhail via Gdb-patches" Reply-To: Christian Biesinger To: gdb-patches@sourceware.org Cc: Christian Biesinger X-IsSubscribed: yes [Should now work for musl, though I have no easy way to test.] To do that, this patch makes IPA compile safe-strerror as well. Because it doesn't use Gnulib, it calls the Glibc version of strerror_r directly. As I understand it, IPA only needs to work on Linux, so this should be safe. Consequently this patch also removes the configure checks for strerror. Depends on https://sourceware.org/ml/gdb-patches/2019-11/msg00912.html gdb/ChangeLog: 2019-11-26 Christian Biesinger * config.in: Regenerate. * configure: Regenerate. * gdbsupport/agent.c (gdb_connect_sync_socket): Call safe_strerror instead of strerror. * gdbsupport/common.m4: Don't check for strerror. * gdbsupport/safe-strerror.c: If IN_PROCESS_AGENT is defined, call the glibc version of strerror_r. gdb/gdbserver/ChangeLog: 2019-11-26 Christian Biesinger * Makefile.in: Add safe-strerror.c to gdbreplay and IPA, and change UNDO_GNULIB_CFLAGS to undo strerror_r instead of strerror. * config.in: Regenerate. * configure: Regenerate. * configure.ac: Don't check for strerror. * linux-i386-ipa.c (initialize_fast_tracepoint_trampoline_buffer): Call safe_strerror instead of strerror. * server.h (strerror): Remove this now-unnecessary declaration. * tracepoint.c (init_named_socket): Call safe_strerror instead of strerror. (gdb_agent_helper_thread): Likewise. * utils.c (perror_with_name): Likewise. Change-Id: I74848f072dcde75cb55c435ef9398dc8f958cd73 --- gdb/config.in | 4 ---- gdb/configure | 12 +----------- gdb/gdbserver/Makefile.in | 4 +++- gdb/gdbserver/config.in | 4 ---- gdb/gdbserver/configure | 22 +--------------------- gdb/gdbserver/configure.ac | 2 +- gdb/gdbserver/linux-i386-ipa.c | 4 ++-- gdb/gdbserver/server.h | 6 ------ gdb/gdbserver/tracepoint.c | 12 ++++++------ gdb/gdbserver/utils.c | 2 +- gdb/gdbsupport/agent.c | 4 ++-- gdb/gdbsupport/common.m4 | 2 +- gdb/gdbsupport/safe-strerror.c | 8 ++++++++ 13 files changed, 26 insertions(+), 60 deletions(-) diff --git a/gdb/config.in b/gdb/config.in index 1caf76481b..cb886ba8e1 100644 --- a/gdb/config.in +++ b/gdb/config.in @@ -117,10 +117,6 @@ don't. */ #undef HAVE_DECL_SNPRINTF -/* Define to 1 if you have the declaration of `strerror', and to 0 if you - don't. */ -#undef HAVE_DECL_STRERROR - /* Define to 1 if you have the declaration of `strstr', and to 0 if you don't. */ #undef HAVE_DECL_STRSTR diff --git a/gdb/configure b/gdb/configure index 6b64619df7..324eb50944 100755 --- a/gdb/configure +++ b/gdb/configure @@ -13630,17 +13630,7 @@ fi done - ac_fn_c_check_decl "$LINENO" "strerror" "ac_cv_have_decl_strerror" "$ac_includes_default" -if test "x$ac_cv_have_decl_strerror" = xyes; then : - ac_have_decl=1 -else - ac_have_decl=0 -fi - -cat >>confdefs.h <<_ACEOF -#define HAVE_DECL_STRERROR $ac_have_decl -_ACEOF -ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default" + ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default" if test "x$ac_cv_have_decl_strstr" = xyes; then : ac_have_decl=1 else diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in index e79d24f96f..10e004039f 100644 --- a/gdb/gdbserver/Makefile.in +++ b/gdb/gdbserver/Makefile.in @@ -304,6 +304,7 @@ GDBREPLAY_OBS = \ gdbsupport/errors.o \ gdbsupport/netstuff.o \ gdbsupport/print-utils.o \ + gdbsupport/safe-strerror.o \ gdbreplay.o \ utils.o \ version.o @@ -430,6 +431,7 @@ IPA_OBJS = \ gdbsupport/format-ipa.o \ gdbsupport/print-utils-ipa.o \ gdbsupport/rsp-low-ipa.o \ + gdbsupport/safe-strerror-ipa.o \ gdbsupport/tdesc-ipa.o \ regcache-ipa.o \ remote-utils-ipa.o \ @@ -557,7 +559,7 @@ UST_CFLAGS = $(ustinc) -DCONFIG_UST_GDB_INTEGRATION # Undo gnulib replacements for the IPA shared library build. # The gnulib headers are still needed, but gnulib is not linked # into the IPA lib so replacement apis don't work. -UNDO_GNULIB_CFLAGS = -Drpl_strerror=strerror +UNDO_GNULIB_CFLAGS = -Drpl_strerror_r=strerror_r # Note, we only build the IPA if -fvisibility=hidden is supported in # the first place. diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in index 14cf7099e3..b1e58a3941 100644 --- a/gdb/gdbserver/config.in +++ b/gdb/gdbserver/config.in @@ -54,10 +54,6 @@ don't. */ #undef HAVE_DECL_SNPRINTF -/* Define to 1 if you have the declaration of `strerror', and to 0 if you - don't. */ -#undef HAVE_DECL_STRERROR - /* Define to 1 if you have the declaration of `strstr', and to 0 if you don't. */ #undef HAVE_DECL_STRSTR diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure index da17f8cb2c..ccffde80ed 100755 --- a/gdb/gdbserver/configure +++ b/gdb/gdbserver/configure @@ -7022,17 +7022,7 @@ fi done - ac_fn_c_check_decl "$LINENO" "strerror" "ac_cv_have_decl_strerror" "$ac_includes_default" -if test "x$ac_cv_have_decl_strerror" = xyes; then : - ac_have_decl=1 -else - ac_have_decl=0 -fi - -cat >>confdefs.h <<_ACEOF -#define HAVE_DECL_STRERROR $ac_have_decl -_ACEOF -ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default" + ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default" if test "x$ac_cv_have_decl_strstr" = xyes; then : ac_have_decl=1 else @@ -8427,16 +8417,6 @@ _ACEOF -ac_fn_c_check_decl "$LINENO" "strerror" "ac_cv_have_decl_strerror" "$ac_includes_default" -if test "x$ac_cv_have_decl_strerror" = xyes; then : - ac_have_decl=1 -else - ac_have_decl=0 -fi - -cat >>confdefs.h <<_ACEOF -#define HAVE_DECL_STRERROR $ac_have_decl -_ACEOF ac_fn_c_check_decl "$LINENO" "perror" "ac_cv_have_decl_perror" "$ac_includes_default" if test "x$ac_cv_have_decl_perror" = xyes; then : ac_have_decl=1 diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac index 07c9bd7fa7..4a6d88ebd1 100644 --- a/gdb/gdbserver/configure.ac +++ b/gdb/gdbserver/configure.ac @@ -158,7 +158,7 @@ LIBS="$old_LIBS" libiberty_INIT -AC_CHECK_DECLS([strerror, perror, vasprintf, vsnprintf]) +AC_CHECK_DECLS([perror, vasprintf, vsnprintf]) AC_CHECK_MEMBERS([struct stat.st_blocks, struct stat.st_blksize]) diff --git a/gdb/gdbserver/linux-i386-ipa.c b/gdb/gdbserver/linux-i386-ipa.c index 2e3d1ac7fe..4bbc1b5f51 100644 --- a/gdb/gdbserver/linux-i386-ipa.c +++ b/gdb/gdbserver/linux-i386-ipa.c @@ -210,7 +210,7 @@ initialize_fast_tracepoint_trampoline_buffer (void) if (!f) { snprintf (buf, sizeof (buf), "mmap_min_addr open failed: %s", - strerror (errno)); + safe_strerror (errno)); set_trampoline_buffer_space (0, 0, buf); return; } @@ -233,7 +233,7 @@ initialize_fast_tracepoint_trampoline_buffer (void) else { snprintf (buf, IPA_BUFSIZ, "low-64K-buffer mmap() failed: %s", - strerror (errno)); + safe_strerror (errno)); set_trampoline_buffer_space (0, 0, buf); } } diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index e01c4f146e..3326159dfe 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -29,12 +29,6 @@ gdb_static_assert (sizeof (CORE_ADDR) >= sizeof (void *)); #include "gdbsupport/version.h" -#if !HAVE_DECL_STRERROR -#ifndef strerror -extern char *strerror (int); /* X3.159-1989 4.11.6.2 */ -#endif -#endif - #if !HAVE_DECL_PERROR #ifndef perror extern void perror (const char *); diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index 2bd75dfb4a..9f8a49174c 100644 --- a/gdb/gdbserver/tracepoint.c +++ b/gdb/gdbserver/tracepoint.c @@ -6879,7 +6879,7 @@ init_named_socket (const char *name) result = fd = socket (PF_UNIX, SOCK_STREAM, 0); if (result == -1) { - warning ("socket creation failed: %s", strerror (errno)); + warning ("socket creation failed: %s", safe_strerror (errno)); return -1; } @@ -6895,7 +6895,7 @@ init_named_socket (const char *name) result = unlink (name); if (result == -1) { - warning ("unlink failed: %s", strerror (errno)); + warning ("unlink failed: %s", safe_strerror (errno)); close (fd); return -1; } @@ -6905,7 +6905,7 @@ init_named_socket (const char *name) result = bind (fd, (struct sockaddr *) &addr, sizeof (addr)); if (result == -1) { - warning ("bind failed: %s", strerror (errno)); + warning ("bind failed: %s", safe_strerror (errno)); close (fd); return -1; } @@ -6913,7 +6913,7 @@ init_named_socket (const char *name) result = listen (fd, 1); if (result == -1) { - warning ("listen: %s", strerror (errno)); + warning ("listen: %s", safe_strerror (errno)); close (fd); return -1; } @@ -7219,7 +7219,7 @@ gdb_agent_helper_thread (void *arg) if (fd < 0) { warning ("Accept returned %d, error: %s", - fd, strerror (errno)); + fd, safe_strerror (errno)); break; } @@ -7231,7 +7231,7 @@ gdb_agent_helper_thread (void *arg) if (ret == -1) { warning ("reading socket (fd=%d) failed with %s", - fd, strerror (errno)); + fd, safe_strerror (errno)); close (fd); break; } diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c index 6a0e7a7a0f..b72dd0ec2b 100644 --- a/gdb/gdbserver/utils.c +++ b/gdb/gdbserver/utils.c @@ -47,7 +47,7 @@ perror_with_name (const char *string) const char *err; char *combined; - err = strerror (errno); + err = safe_strerror (errno); if (err == NULL) err = "unknown error"; diff --git a/gdb/gdbsupport/agent.c b/gdb/gdbsupport/agent.c index 6d55f586b8..50e9500ffc 100644 --- a/gdb/gdbsupport/agent.c +++ b/gdb/gdbsupport/agent.c @@ -149,7 +149,7 @@ gdb_connect_sync_socket (int pid) res = fd = gdb_socket_cloexec (PF_UNIX, SOCK_STREAM, 0); if (res == -1) { - warning (_("error opening sync socket: %s"), strerror (errno)); + warning (_("error opening sync socket: %s"), safe_strerror (errno)); return -1; } @@ -168,7 +168,7 @@ gdb_connect_sync_socket (int pid) { warning (_("error connecting sync socket (%s): %s. " "Make sure the directory exists and that it is writable."), - path, strerror (errno)); + path, safe_strerror (errno)); close (fd); return -1; } diff --git a/gdb/gdbsupport/common.m4 b/gdb/gdbsupport/common.m4 index 4f2bb5218d..c61753fb8e 100644 --- a/gdb/gdbsupport/common.m4 +++ b/gdb/gdbsupport/common.m4 @@ -35,7 +35,7 @@ AC_DEFUN([GDB_AC_COMMON], [ AC_CHECK_FUNCS([fdwalk getrlimit pipe pipe2 socketpair sigaction \ sigprocmask]) - AC_CHECK_DECLS([strerror, strstr]) + AC_CHECK_DECLS([strstr]) # Check for std::thread. This does not work on some platforms, like # mingw and DJGPP. diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c index c37db579f7..6d30e4633b 100644 --- a/gdb/gdbsupport/safe-strerror.c +++ b/gdb/gdbsupport/safe-strerror.c @@ -27,11 +27,19 @@ safe_strerror (int errnum) { static thread_local char buf[1024]; +#if defined(IN_PROCESS_AGENT) && defined(__GLIBC__) + /* IPA does not use Gnulib, but only supports Linux, so we can safely + call the GNU version of strerror_r here if using glibc. It is documented + not to return NULL. musl uses the portable signature, so should + use the other branch of the #if. */ + return strerror_r (errnum, buf, sizeof (buf)); +#else /* Assign the return value to an int, so we get an error if we accidentally get the wrong version of this function (glibc has two of them...). */ int ret = strerror_r (errnum, buf, sizeof (buf)); if (ret == 0) return buf; +#endif xsnprintf (buf, sizeof buf, "(undocumented errno %d)", errnum); return buf;