This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Use secure_getenv for GOMP_DEBUG
On 06/27/2017 09:38 AM, Jakub Jelinek wrote:
On Tue, Jun 27, 2017 at 09:17:57AM +0200, Tom de Vries wrote:
This patch uses secure_getenv for GOMP_DEBUG.
It factors out the secure_getenv code from plugin-hsa.c into
libgomp/secure_getenv.h, and reuses it in env.c.
I've added _GNU_SOURCE before the libgomp.h include in env.c to make sure
that secure_getenv (imported from stdlib.h) is available.
I've also added a test-case that sets GOMP_DEBUG to 1 and verifies that some
output is generated.
Build for c-only on x86_64 without accelerator, tested libgomp -m64/-m32.
OK if x86_64 bootstrap and reg-test succeeds?
Thanks,
- Tom
Use secure_getenv for GOMP_DEBUG
--- /dev/null
+++ b/libgomp/secure_getenv.h
@@ -0,0 +1,53 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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, or (at your option)
+any later version.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+#ifndef _SECURE_GETENV_H
+#define _SECURE_GETENV_H 1
+
+/* Secure getenv() which returns NULL if running as SUID/SGID. */
+#ifndef HAVE_SECURE_GETENV
+#ifdef HAVE___SECURE_GETENV
+#define secure_getenv __secure_getenv
+#elif defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \
+ && defined(HAVE_GETGID) && defined(HAVE_GETEGID)
+
+#include <unistd.h>
+
+/* Implementation of secure_getenv() for targets where it is not provided but
+ we have at least means to test real and effective IDs. */
+
+static char *
+secure_getenv (const char *name)
Shouldn't this be static inline char * ? I mean, even at -O0 we don't want
it to be emitted into every TU.
Done.
Another thing is that we probably want to follow what libgfortran does for
the case where secure_getenv isn't available, but __secure_getenv is -
in particular emit this function (if geteuid and getegid are present), but
emit a weakref call to __secure_getenv first if non-NULL.
I've copied the approach from libgfortran.
Build and tested:
- once as is, and
- once with '#ifndef HAVE_SECURE_GETENV' replaced with '#if 1' to
trigger the fallback.
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-set-target-env-var GOMP_DEBUG "1" } */
+
+/* Check that GOMP_DEBUG=1 triggers some output. */
+
+int
+main (void)
+{
+#pragma acc parallel
+ ;
+}
+
+/* { dg-output "GOACC_parallel_keyed" } */
Does dg-set-target-env-var work for remote testing?
No, it'll make the testcase unsupported for remote testing.
[ Discussed before at f.i.
https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01808.html ]
Thanks,
- Tom
Use secure_getenv for GOMP_DEBUG
2017-06-26 Tom de Vries <tom@codesourcery.com>
* env.c (parse_unsigned_long_1): Factor out of ...
(parse_unsigned_long): ... here.
(parse_int_1): Factor out of ...
(parse_int): ... here.
(parse_int_secure): New function.
(initialize_env): Use parse_int_secure for GOMP_DEBUG.
* secure_getenv.h: Factor out of ...
* plugin/plugin-hsa.c: ... here.
* testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c: New test.
---
libgomp/env.c | 44 +++++++++++++---
libgomp/plugin/plugin-hsa.c | 27 +---------
libgomp/secure_getenv.h | 60 ++++++++++++++++++++++
.../libgomp.oacc-c-c++-common/gomp-debug-env.c | 13 +++++
4 files changed, 111 insertions(+), 33 deletions(-)
diff --git a/libgomp/env.c b/libgomp/env.c
index ced752d..802c73b 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -26,6 +26,7 @@
/* This file defines the OpenMP internal control variables and arranges
for them to be initialized from environment variables at startup. */
+#define _GNU_SOURCE
#include "libgomp.h"
#include "gomp-constants.h"
#include <limits.h>
@@ -58,6 +59,8 @@
#endif
#endif /* LIBGOMP_OFFLOADED_ONLY */
+#include "secure_getenv.h"
+
struct gomp_task_icv gomp_global_icv = {
.nthreads_var = 1,
.thread_limit_var = UINT_MAX,
@@ -171,15 +174,17 @@ parse_schedule (void)
}
/* Parse an unsigned long environment variable. Return true if one was
- present and it was successfully parsed. */
+ present and it was successfully parsed. If SECURE, use secure_getenv to the
+ environment variable. */
static bool
-parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero)
+parse_unsigned_long_1 (const char *name, unsigned long *pvalue, bool allow_zero,
+ bool secure)
{
char *env, *end;
unsigned long value;
- env = getenv (name);
+ env = (secure ? secure_getenv (name) : getenv (name));
if (env == NULL)
return false;
@@ -206,14 +211,23 @@ parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero)
return false;
}
+/* As parse_unsigned_long_1, but always use getenv. */
+
+static bool
+parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero)
+{
+ return parse_unsigned_long_1 (name, pvalue, allow_zero, false);
+}
+
/* Parse a positive int environment variable. Return true if one was
- present and it was successfully parsed. */
+ present and it was successfully parsed. If SECURE, use secure_getenv to the
+ environment variable. */
static bool
-parse_int (const char *name, int *pvalue, bool allow_zero)
+parse_int_1 (const char *name, int *pvalue, bool allow_zero, bool secure)
{
unsigned long value;
- if (!parse_unsigned_long (name, &value, allow_zero))
+ if (!parse_unsigned_long_1 (name, &value, allow_zero, secure))
return false;
if (value > INT_MAX)
{
@@ -224,6 +238,22 @@ parse_int (const char *name, int *pvalue, bool allow_zero)
return true;
}
+/* As parse_int_1, but use getenv. */
+
+static bool
+parse_int (const char *name, int *pvalue, bool allow_zero)
+{
+ return parse_int_1 (name, pvalue, allow_zero, false);
+}
+
+/* As parse_int_1, but use getenv_secure. */
+
+static bool
+parse_int_secure (const char *name, int *pvalue, bool allow_zero)
+{
+ return parse_int_1 (name, pvalue, allow_zero, true);
+}
+
/* Parse an unsigned long list environment variable. Return true if one was
present and it was successfully parsed. */
@@ -1207,7 +1237,7 @@ initialize_env (void)
gomp_global_icv.thread_limit_var
= thread_limit_var > INT_MAX ? UINT_MAX : thread_limit_var;
}
- parse_int ("GOMP_DEBUG", &gomp_debug_var, true);
+ parse_int_secure ("GOMP_DEBUG", &gomp_debug_var, true);
#ifndef HAVE_SYNC_BUILTINS
gomp_mutex_init (&gomp_managed_threads_lock);
#endif
diff --git a/libgomp/plugin/plugin-hsa.c b/libgomp/plugin/plugin-hsa.c
index 90ca247..adb07ac 100644
--- a/libgomp/plugin/plugin-hsa.c
+++ b/libgomp/plugin/plugin-hsa.c
@@ -39,32 +39,7 @@
#include <dlfcn.h>
#include "libgomp-plugin.h"
#include "gomp-constants.h"
-
-/* Secure getenv() which returns NULL if running as SUID/SGID. */
-#ifndef HAVE_SECURE_GETENV
-#ifdef HAVE___SECURE_GETENV
-#define secure_getenv __secure_getenv
-#elif defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \
- && defined(HAVE_GETGID) && defined(HAVE_GETEGID)
-
-#include <unistd.h>
-
-/* Implementation of secure_getenv() for targets where it is not provided but
- we have at least means to test real and effective IDs. */
-
-static char *
-secure_getenv (const char *name)
-{
- if ((getuid () == geteuid ()) && (getgid () == getegid ()))
- return getenv (name);
- else
- return NULL;
-}
-
-#else
-#define secure_getenv getenv
-#endif
-#endif
+#include "secure-getenv.h"
/* As an HSA runtime is dlopened, following structure defines function
pointers utilized by the HSA plug-in. */
diff --git a/libgomp/secure_getenv.h b/libgomp/secure_getenv.h
new file mode 100644
index 0000000..1d03eb7
--- /dev/null
+++ b/libgomp/secure_getenv.h
@@ -0,0 +1,60 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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, or (at your option)
+any later version.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+#ifndef _SECURE_GETENV_H
+#define _SECURE_GETENV_H 1
+
+/* Secure getenv() which returns NULL if running as SUID/SGID. */
+#ifndef HAVE_SECURE_GETENV
+#if defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \
+ && defined(HAVE_GETGID) && defined(HAVE_GETEGID)
+
+#include <unistd.h>
+
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
+static char* weak_secure_getenv (const char*)
+ __attribute__((__weakref__("__secure_getenv")));
+#endif
+
+/* Implementation of secure_getenv() for targets where it is not provided but
+ we have at least means to test real and effective IDs. */
+
+static inline char *
+secure_getenv (const char *name)
+{
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
+ if (weak_secure_getenv)
+ return weak_secure_getenv (name);
+#endif
+
+ if ((getuid () == geteuid ()) && (getgid () == getegid ()))
+ return getenv (name);
+ else
+ return NULL;
+}
+#else
+#define secure_getenv getenv
+#endif
+#endif
+
+#endif /* _SECURE_GETENV_H. */
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c
new file mode 100644
index 0000000..3fc3503
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-set-target-env-var GOMP_DEBUG "1" } */
+
+/* Check that GOMP_DEBUG=1 triggers some output. */
+
+int
+main (void)
+{
+#pragma acc parallel
+ ;
+}
+
+/* { dg-output "GOACC_parallel_keyed" } */