public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Don't check for __secure_getenv
@ 2017-04-27 13:15 Janne Blomqvist
  2017-04-27 13:24 ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Janne Blomqvist @ 2017-04-27 13:15 UTC (permalink / raw)
  To: fortran, gcc-patches; +Cc: Janne Blomqvist

Glibc 2.17 made __secure_getenv an officially supported function, and
renamed it secure_getenv. The libgfortran configure has checked for
both of these, per
https://sourceware.org/glibc/wiki/Tips_and_Tricks/secure_getenv.

Unfortunately, while the dynamical library (libc.so) retains the
__secure_getenv symbol for backwards compatibility, the static library
(libc.a) does not. This means that a libgfortran.a compiled against an
older glibc will not work if one tries to link against a newer
libc.a. This creates problems for providing gfortran binary
distributions that work on as many target systems as possible.

Thus, drop the support for __secure_getenv. This implies that when
libgfortran is compiled against an older glibc it instead uses the
fallback secure_getenv in libgfortran/runtime/environ.c instead of
glibc __secure_getenv.

Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
6, 5?

2017-04-27  Janne Blomqvist  <jb@gcc.gnu.org>

	* configure.ac: Don't check for presence of __secure_getenv.
	* libgfortran.h: HAVE_SECURE_GETENV: Don't check
	HAVE___SECURE_GETENV.
	* Makefile.in: Regenerated.
	* config.h.in: Regenerated.
	* configure: Regenerated.
---
 libgfortran/Makefile.in   | 5 +++--
 libgfortran/config.h.in   | 3 ---
 libgfortran/configure     | 7 ++-----
 libgfortran/configure.ac  | 2 +-
 libgfortran/libgfortran.h | 4 +---
 5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/libgfortran/Makefile.in b/libgfortran/Makefile.in
index 05b183d..4914a6f 100644
--- a/libgfortran/Makefile.in
+++ b/libgfortran/Makefile.in
@@ -137,8 +137,9 @@ am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
 	$(top_srcdir)/../ltversion.m4 $(top_srcdir)/../lt~obsolete.m4 \
 	$(top_srcdir)/acinclude.m4 $(top_srcdir)/../config/acx.m4 \
 	$(top_srcdir)/../config/no-executables.m4 \
-	$(top_srcdir)/../config/math.m4 $(top_srcdir)/../libtool.m4 \
-	$(top_srcdir)/configure.ac
+	$(top_srcdir)/../config/math.m4 \
+	$(top_srcdir)/../config/ax_check_define.m4 \
+	$(top_srcdir)/../libtool.m4 $(top_srcdir)/configure.ac
 am__configure_deps = $(am__aclocal_m4_deps) $(CONFIGURE_DEPENDENCIES) \
 	$(ACLOCAL_M4)
 am__CONFIG_DISTCLEAN_FILES = config.status config.cache config.log \
diff --git a/libgfortran/config.h.in b/libgfortran/config.h.in
index b762d099..45e349d 100644
--- a/libgfortran/config.h.in
+++ b/libgfortran/config.h.in
@@ -837,9 +837,6 @@
 /* Define to 1 if you have the `ynl' function. */
 #undef HAVE_YNL
 
-/* Define to 1 if you have the `__secure_getenv' function. */
-#undef HAVE___SECURE_GETENV
-
 /* Define to the sub-directory in which libtool stores uninstalled libraries.
    */
 #undef LT_OBJDIR
diff --git a/libgfortran/configure b/libgfortran/configure
index 81238fc..dd99620 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -2598,7 +2598,6 @@ as_fn_append ac_func_list " geteuid"
 as_fn_append ac_func_list " umask"
 as_fn_append ac_func_list " getegid"
 as_fn_append ac_func_list " secure_getenv"
-as_fn_append ac_func_list " __secure_getenv"
 as_fn_append ac_func_list " mkostemp"
 as_fn_append ac_func_list " strnlen"
 as_fn_append ac_func_list " strndup"
@@ -12421,7 +12420,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12424 "configure"
+#line 12423 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12527,7 +12526,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12530 "configure"
+#line 12529 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -16685,8 +16684,6 @@ done
 
 
 
-
-
 fi
 
 # Check strerror_r, cannot be above as versions with two and three arguments exist
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 37b12d2..fe47fcd 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -310,7 +310,7 @@ else
    gettimeofday stat fstat lstat getpwuid vsnprintf dup \
    getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \
    getgid getpid getuid geteuid umask getegid \
-   secure_getenv __secure_getenv mkostemp strnlen strndup newlocale \
+   secure_getenv mkostemp strnlen strndup newlocale \
    freelocale uselocale strerror_l)
 fi
 
diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h
index cfa4fcf..9d9d117 100644
--- a/libgfortran/libgfortran.h
+++ b/libgfortran/libgfortran.h
@@ -808,9 +808,7 @@ internal_proto(get_unformatted_convert);
 
 /* 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_GETUID) && defined(HAVE_GETEUID) \
+#if defined(HAVE_GETUID) && defined(HAVE_GETEUID) \
   && defined(HAVE_GETGID) && defined(HAVE_GETEGID)
 #define FALLBACK_SECURE_GETENV
 extern char *secure_getenv (const char *);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't check for __secure_getenv
  2017-04-27 13:15 [PATCH] Don't check for __secure_getenv Janne Blomqvist
@ 2017-04-27 13:24 ` Jakub Jelinek
  2017-04-27 19:27   ` [PATCH] Don't assume __secure_getenv is available Janne Blomqvist
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2017-04-27 13:24 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: fortran, gcc-patches

On Thu, Apr 27, 2017 at 03:50:19PM +0300, Janne Blomqvist wrote:
> Glibc 2.17 made __secure_getenv an officially supported function, and
> renamed it secure_getenv. The libgfortran configure has checked for
> both of these, per
> https://sourceware.org/glibc/wiki/Tips_and_Tricks/secure_getenv.
> 
> Unfortunately, while the dynamical library (libc.so) retains the
> __secure_getenv symbol for backwards compatibility, the static library
> (libc.a) does not. This means that a libgfortran.a compiled against an
> older glibc will not work if one tries to link against a newer
> libc.a. This creates problems for providing gfortran binary
> distributions that work on as many target systems as possible.

I don't think it is a good idea, if __secure_getenv is available, better use
it, because it can do stuff that the fallback one can't (figuring out
priviledge elevatation through AT_* values in aux vector).

So, if anything, I'd suggest if !HAVE_SECURE_GETENV and HAVE___SECURE_GETENV
to use weakref for __secure_getenv and call it if non-NULL, otherwise
use the fallback implementation.

> Thus, drop the support for __secure_getenv. This implies that when
> libgfortran is compiled against an older glibc it instead uses the
> fallback secure_getenv in libgfortran/runtime/environ.c instead of
> glibc __secure_getenv.
> 
> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
> 6, 5?
> 
> 2017-04-27  Janne Blomqvist  <jb@gcc.gnu.org>
> 
> 	* configure.ac: Don't check for presence of __secure_getenv.
> 	* libgfortran.h: HAVE_SECURE_GETENV: Don't check
> 	HAVE___SECURE_GETENV.
> 	* Makefile.in: Regenerated.
> 	* config.h.in: Regenerated.
> 	* configure: Regenerated.

	Jakub

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Don't assume __secure_getenv is available
  2017-04-27 13:24 ` Jakub Jelinek
@ 2017-04-27 19:27   ` Janne Blomqvist
  2017-04-27 19:33     ` Janne Blomqvist
  2017-05-12  7:24     ` Thomas Schwinge
  0 siblings, 2 replies; 10+ messages in thread
From: Janne Blomqvist @ 2017-04-27 19:27 UTC (permalink / raw)
  To: fortran, gcc-patches; +Cc: Janne Blomqvist

Glibc 2.17 made __secure_getenv an officially supported function, and
renamed it secure_getenv. The libgfortran configure has checked for
both of these, per
https://sourceware.org/glibc/wiki/Tips_and_Tricks/secure_getenv.

Unfortunately, while the dynamical library (libc.so) retains the
__secure_getenv symbol for backwards compatibility, the static library
(libc.a) does not. This means that a libgfortran.a compiled against an
older glibc will not work if one tries to link against a newer
libc.a. This creates problems for providing gfortran binary
distributions that work on as many target systems as possible.

Thus, retain the support for __secure_getenv but call it only via a
weak reference.

Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
6, 5?

2017-04-27  Janne Blomqvist  <jb@gcc.gnu.org>

	* libgfortran.h: HAVE_SECURE_GETENV: Don't check
	HAVE___SECURE_GETENV.
	* environ/runtime.c (secure_getenv): Use __secure_getenv via a
        weak reference.
	* Makefile.in: Regenerated.
---
 libgfortran/Makefile.in       |  5 +++--
 libgfortran/libgfortran.h     |  4 +---
 libgfortran/runtime/environ.c | 11 +++++++++++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/libgfortran/Makefile.in b/libgfortran/Makefile.in
index 05b183d..4914a6f 100644
--- a/libgfortran/Makefile.in
+++ b/libgfortran/Makefile.in
@@ -137,8 +137,9 @@ am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
 	$(top_srcdir)/../ltversion.m4 $(top_srcdir)/../lt~obsolete.m4 \
 	$(top_srcdir)/acinclude.m4 $(top_srcdir)/../config/acx.m4 \
 	$(top_srcdir)/../config/no-executables.m4 \
-	$(top_srcdir)/../config/math.m4 $(top_srcdir)/../libtool.m4 \
-	$(top_srcdir)/configure.ac
+	$(top_srcdir)/../config/math.m4 \
+	$(top_srcdir)/../config/ax_check_define.m4 \
+	$(top_srcdir)/../libtool.m4 $(top_srcdir)/configure.ac
 am__configure_deps = $(am__aclocal_m4_deps) $(CONFIGURE_DEPENDENCIES) \
 	$(ACLOCAL_M4)
 am__CONFIG_DISTCLEAN_FILES = config.status config.cache config.log \
diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h
index cfa4fcf..9d9d117 100644
--- a/libgfortran/libgfortran.h
+++ b/libgfortran/libgfortran.h
@@ -808,9 +808,7 @@ internal_proto(get_unformatted_convert);
 
 /* 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_GETUID) && defined(HAVE_GETEUID) \
+#if defined(HAVE_GETUID) && defined(HAVE_GETEUID) \
   && defined(HAVE_GETGID) && defined(HAVE_GETEGID)
 #define FALLBACK_SECURE_GETENV
 extern char *secure_getenv (const char *);
diff --git a/libgfortran/runtime/environ.c b/libgfortran/runtime/environ.c
index bf02188..969dcdf 100644
--- a/libgfortran/runtime/environ.c
+++ b/libgfortran/runtime/environ.c
@@ -37,9 +37,20 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    provided. */
 
 #ifdef FALLBACK_SECURE_GETENV
+
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
+static char* weak_secure_getenv (const char*)
+  __attribute__((__weakref__("__secure_gettime")));
+#endif
+
 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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-04-27 19:27   ` [PATCH] Don't assume __secure_getenv is available Janne Blomqvist
@ 2017-04-27 19:33     ` Janne Blomqvist
  2017-05-08  6:37       ` Janne Blomqvist via gcc-patches
  2017-05-12  7:24     ` Thomas Schwinge
  1 sibling, 1 reply; 10+ messages in thread
From: Janne Blomqvist @ 2017-04-27 19:33 UTC (permalink / raw)
  To: Fortran List, GCC Patches

On Thu, Apr 27, 2017 at 9:50 PM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
[snip]

And on top of that patch this simple typo fix:

diff --git a/libgfortran/runtime/environ.c b/libgfortran/runtime/environ.c
index 969dcdf..f488e87 100644
--- a/libgfortran/runtime/environ.c
+++ b/libgfortran/runtime/environ.c
@@ -46,7 +46,7 @@ static char* weak_secure_getenv (const char*)
 char *
 secure_getenv (const char *name)
 {
-#if SUPPORTS_WEAKREF && defined(HAVE__SECURE_GETENV)
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
   if (weak_secure_getenv)
     return weak_secure_getenv (name);
 #endif



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-04-27 19:33     ` Janne Blomqvist
@ 2017-05-08  6:37       ` Janne Blomqvist via gcc-patches
  2017-05-08 17:37         ` Jerry DeLisle
  0 siblings, 1 reply; 10+ messages in thread
From: Janne Blomqvist via gcc-patches @ 2017-05-08  6:37 UTC (permalink / raw)
  To: Fortran List, GCC Patches

PING

On Thu, Apr 27, 2017 at 9:55 PM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> On Thu, Apr 27, 2017 at 9:50 PM, Janne Blomqvist
> <blomqvist.janne@gmail.com> wrote:
> [snip]
>
> And on top of that patch this simple typo fix:
>
> diff --git a/libgfortran/runtime/environ.c b/libgfortran/runtime/environ.c
> index 969dcdf..f488e87 100644
> --- a/libgfortran/runtime/environ.c
> +++ b/libgfortran/runtime/environ.c
> @@ -46,7 +46,7 @@ static char* weak_secure_getenv (const char*)
>  char *
>  secure_getenv (const char *name)
>  {
> -#if SUPPORTS_WEAKREF && defined(HAVE__SECURE_GETENV)
> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
>    if (weak_secure_getenv)
>      return weak_secure_getenv (name);
>  #endif
>
>
>
> --
> Janne Blomqvist



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-05-08  6:37       ` Janne Blomqvist via gcc-patches
@ 2017-05-08 17:37         ` Jerry DeLisle
  0 siblings, 0 replies; 10+ messages in thread
From: Jerry DeLisle @ 2017-05-08 17:37 UTC (permalink / raw)
  To: Janne Blomqvist, Fortran List, GCC Patches

On 05/07/2017 11:37 PM, Janne Blomqvist via fortran wrote:
> PING
> 
> On Thu, Apr 27, 2017 at 9:55 PM, Janne Blomqvist
> <blomqvist.janne@gmail.com> wrote:
>> On Thu, Apr 27, 2017 at 9:50 PM, Janne Blomqvist
>> <blomqvist.janne@gmail.com> wrote:
>> [snip]
>>
>> And on top of that patch this simple typo fix:
>>
>> diff --git a/libgfortran/runtime/environ.c b/libgfortran/runtime/environ.c
>> index 969dcdf..f488e87 100644
>> --- a/libgfortran/runtime/environ.c
>> +++ b/libgfortran/runtime/environ.c
>> @@ -46,7 +46,7 @@ static char* weak_secure_getenv (const char*)
>>  char *
>>  secure_getenv (const char *name)
>>  {
>> -#if SUPPORTS_WEAKREF && defined(HAVE__SECURE_GETENV)
>> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
>>    if (weak_secure_getenv)
>>      return weak_secure_getenv (name);
>>  #endif
>>
>>
>>
>> --
>> Janne Blomqvist
> 
> 
> 

Looks Ok to me. I thought you might be waiting for Jakub to reply.

Thanks,

Jerry

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-04-27 19:27   ` [PATCH] Don't assume __secure_getenv is available Janne Blomqvist
  2017-04-27 19:33     ` Janne Blomqvist
@ 2017-05-12  7:24     ` Thomas Schwinge
  2017-05-12  7:32       ` Janne Blomqvist
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Schwinge @ 2017-05-12  7:24 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: fortran, gcc-patches, Jerry DeLisle

Hi!

On Thu, 27 Apr 2017 21:50:51 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> [...], retain the support for __secure_getenv but call it only via a
> weak reference.
> 
> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
> 6, 5?

Hmm, how has this been tested?  Because:

> --- a/libgfortran/runtime/environ.c
> +++ b/libgfortran/runtime/environ.c

>  #ifdef FALLBACK_SECURE_GETENV
> +
> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
> +static char* weak_secure_getenv (const char*)
> +  __attribute__((__weakref__("__secure_gettime")));
> +#endif

"gettime" vs. "getenv"?  ;-)


Grüße
 Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-05-12  7:24     ` Thomas Schwinge
@ 2017-05-12  7:32       ` Janne Blomqvist
  2017-05-12  8:14         ` Thomas Schwinge
  0 siblings, 1 reply; 10+ messages in thread
From: Janne Blomqvist @ 2017-05-12  7:32 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: Fortran List, GCC Patches, Jerry DeLisle

On Fri, May 12, 2017 at 10:23 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Hi!
>
> On Thu, 27 Apr 2017 21:50:51 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> [...], retain the support for __secure_getenv but call it only via a
>> weak reference.
>>
>> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
>> 6, 5?
>
> Hmm, how has this been tested?  Because:
>
>> --- a/libgfortran/runtime/environ.c
>> +++ b/libgfortran/runtime/environ.c
>
>>  #ifdef FALLBACK_SECURE_GETENV
>> +
>> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
>> +static char* weak_secure_getenv (const char*)
>> +  __attribute__((__weakref__("__secure_gettime")));
>> +#endif
>
> "gettime" vs. "getenv"?  ;-)

Oops. I'm not at my gcc development box now, please consider a patch
to fix this pre-approved.

As for testing, I regtested, but my gcc development machine has glibc
2.23 which has secure_getenv so it doesn't exercise the fallback
path..



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-05-12  7:32       ` Janne Blomqvist
@ 2017-05-12  8:14         ` Thomas Schwinge
  2017-05-12  8:16           ` Janne Blomqvist
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Schwinge @ 2017-05-12  8:14 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches, Jerry DeLisle

Hi!

On Fri, 12 May 2017 10:26:59 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> On Fri, May 12, 2017 at 10:23 AM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
> > On Thu, 27 Apr 2017 21:50:51 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> >> [...], retain the support for __secure_getenv but call it only via a
> >> weak reference.
> >>
> >> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
> >> 6, 5?
> >
> > Hmm, how has this been tested?  Because:
> >
> >> --- a/libgfortran/runtime/environ.c
> >> +++ b/libgfortran/runtime/environ.c
> >
> >>  #ifdef FALLBACK_SECURE_GETENV
> >> +
> >> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
> >> +static char* weak_secure_getenv (const char*)
> >> +  __attribute__((__weakref__("__secure_gettime")));
> >> +#endif
> >
> > "gettime" vs. "getenv"?  ;-)
> 
> Oops. I'm not at my gcc development box now, please consider a patch
> to fix this pre-approved.
> 
> As for testing, I regtested, but my gcc development machine has glibc
> 2.23 which has secure_getenv so it doesn't exercise the fallback
> path..

Then, that clearly isn't an appropriate testing methodology?  What I do
in such cases is manually induce the expected environment (for example,
here I'd probably try hacking out "HAVE_SECURE_GETENV" and
"HAVE___SECURE_GETENV" from the generated "config.h" or even
"libgfortran/configure"), and then manually run something that is
expected to behave differently in an environment relevant to
"secure_getenv" -- I doubt that such a thing is being (or, can really be)
included in the GCC testsuite?

Still untested -- ;-) -- but including another typo fix, committed to
trunk in r247952:

commit bc9457364b4b9a847c91e35a0aa5fc3b73df53a0
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri May 12 07:56:41 2017 +0000

    Typo fixes for "Don't assume __secure_getenv is available"
    
            libgfortran/
            * runtime/environ.c (weak_secure_getenv): Fix "__secure_gettime"
            vs. "__secure_getenv" typo.
            (secure_getenv): Fix "HAVE__SECURE_GETENV"
            vs. "HAVE___SECURE_GETENV" typo.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@247952 138bc75d-0d04-0410-961f-82ee72b054a4
---
 libgfortran/ChangeLog         | 7 +++++++
 libgfortran/runtime/environ.c | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git libgfortran/ChangeLog libgfortran/ChangeLog
index 337daaf..6b7da0a 100644
--- libgfortran/ChangeLog
+++ libgfortran/ChangeLog
@@ -1,3 +1,10 @@
+2017-05-12  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* runtime/environ.c (weak_secure_getenv): Fix "__secure_gettime"
+	vs. "__secure_getenv" typo.
+	(secure_getenv): Fix "HAVE__SECURE_GETENV"
+	vs. "HAVE___SECURE_GETENV" typo.
+
 2017-05-11  Janne Blomqvist  <jb@gcc.gnu.org>
 
 	* libgfortran.h: HAVE_SECURE_GETENV: Don't check
diff --git libgfortran/runtime/environ.c libgfortran/runtime/environ.c
index 969dcdf..f0a593e 100644
--- libgfortran/runtime/environ.c
+++ libgfortran/runtime/environ.c
@@ -40,13 +40,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
 static char* weak_secure_getenv (const char*)
-  __attribute__((__weakref__("__secure_gettime")));
+  __attribute__((__weakref__("__secure_getenv")));
 #endif
 
 char *
 secure_getenv (const char *name)
 {
-#if SUPPORTS_WEAKREF && defined(HAVE__SECURE_GETENV)
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
   if (weak_secure_getenv)
     return weak_secure_getenv (name);
 #endif


Grüße
 Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Don't assume __secure_getenv is available
  2017-05-12  8:14         ` Thomas Schwinge
@ 2017-05-12  8:16           ` Janne Blomqvist
  0 siblings, 0 replies; 10+ messages in thread
From: Janne Blomqvist @ 2017-05-12  8:16 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: Fortran List, GCC Patches, Jerry DeLisle

On Fri, May 12, 2017 at 11:02 AM, Thomas Schwinge
<thomas@codesourcery.com> wrote:
> Hi!
>
> On Fri, 12 May 2017 10:26:59 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> On Fri, May 12, 2017 at 10:23 AM, Thomas Schwinge
>> <thomas@codesourcery.com> wrote:
>> > On Thu, 27 Apr 2017 21:50:51 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> >> [...], retain the support for __secure_getenv but call it only via a
>> >> weak reference.
>> >>
>> >> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
>> >> 6, 5?
>> >
>> > Hmm, how has this been tested?  Because:
>> >
>> >> --- a/libgfortran/runtime/environ.c
>> >> +++ b/libgfortran/runtime/environ.c
>> >
>> >>  #ifdef FALLBACK_SECURE_GETENV
>> >> +
>> >> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
>> >> +static char* weak_secure_getenv (const char*)
>> >> +  __attribute__((__weakref__("__secure_gettime")));
>> >> +#endif
>> >
>> > "gettime" vs. "getenv"?  ;-)
>>
>> Oops. I'm not at my gcc development box now, please consider a patch
>> to fix this pre-approved.
>>
>> As for testing, I regtested, but my gcc development machine has glibc
>> 2.23 which has secure_getenv so it doesn't exercise the fallback
>> path..
>
> Then, that clearly isn't an appropriate testing methodology?  What I do
> in such cases is manually induce the expected environment (for example,
> here I'd probably try hacking out "HAVE_SECURE_GETENV" and
> "HAVE___SECURE_GETENV" from the generated "config.h" or even
> "libgfortran/configure"), and then manually run something that is
> expected to behave differently in an environment relevant to
> "secure_getenv"

Yes, I've done something like that in the past to test different
branches of #ifdeffy code. Here it was simple enough that I thought
"what could possibly go wrong?"; Plenty, apparently! ;)

> Still untested -- ;-) -- but including another typo fix, committed to
> trunk in r247952:

Thanks. I had actually fixed the HAVE___SECURE_GETENV typo myself in
my git tree, but then I committed the wrong patch to svn. Aagh..

Moar coffee needed...



-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-12  8:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 13:15 [PATCH] Don't check for __secure_getenv Janne Blomqvist
2017-04-27 13:24 ` Jakub Jelinek
2017-04-27 19:27   ` [PATCH] Don't assume __secure_getenv is available Janne Blomqvist
2017-04-27 19:33     ` Janne Blomqvist
2017-05-08  6:37       ` Janne Blomqvist via gcc-patches
2017-05-08 17:37         ` Jerry DeLisle
2017-05-12  7:24     ` Thomas Schwinge
2017-05-12  7:32       ` Janne Blomqvist
2017-05-12  8:14         ` Thomas Schwinge
2017-05-12  8:16           ` Janne Blomqvist

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).