Bug 47802 - [4.6 Regression] libgfortran/intrinsics/ctime.c:75:3: error: too few arguments to function 'ctime_r'
Summary: [4.6 Regression] libgfortran/intrinsics/ctime.c:75:3: error: too few argument...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.6.0
: P4 normal
Target Milestone: 4.6.0
Assignee: Janne Blomqvist
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords: build
Depends on:
Blocks:
 
Reported: 2011-02-18 16:35 UTC by John David Anglin
Modified: 2011-03-10 16:58 UTC (History)
4 users (show)

See Also:
Host: hppa1.1-hp-hpux10.20
Target: hppa1.1-hp-hpux10.20
Build: hppa1.1-hp-hpux10.20
Known to work:
Known to fail:
Last reconfirmed: 2011-02-22 16:21:09


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John David Anglin 2011-02-18 16:35:12 UTC
libtool: compile:  /xxx/gnu/gcc/objdir/./gcc/xgcc -B/xxx/gnu/gcc/objdir/./gcc/ -
B/opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/bin/ -B/opt/gnu/gcc/gcc-4.6/hppa1.1-h
p-hpux10.20/lib/ -isystem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/include -isy
stem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/sys-include -DHAVE_CONFIG_H -I. -
I../../../../gcc/libgfortran -iquote../../../../gcc/libgfortran/io -I../../../..
/gcc/libgfortran/../gcc -I../../../../gcc/libgfortran/../gcc/config -I../../../.
./gcc/libgfortran/../libquadmath -I../../.././gcc -D_GNU_SOURCE -std=gnu99 -Wall
 -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite
-strings -fcx-fortran-rules -g -O2 -threads -MT ctime.lo -MD -MP -MF .deps/ctime
.Tpo -c ../../../../gcc/libgfortran/intrinsics/ctime.c  -fPIC -DPIC -o .libs/cti
me.o
../../../../gcc/libgfortran/intrinsics/ctime.c: In function 'fdate':
../../../../gcc/libgfortran/intrinsics/ctime.c:75:3: error: too few arguments to
 function 'ctime_r'
/usr/include/sys/time.h:157:13: note: declared here
...

It seems HP-UX 10.20 has a different declaration for ctime_r:

      int ctime_r(const time_t *timer, char *buffer, int buflen);
Comment 1 Jeffrey A. Law 2011-02-18 19:53:35 UTC
Is there no way to get a posix compliant ctime?  Alternatively, we'll need autoconf magic to detect the extra arg.  I know at one time it was relatively common, so autoconf magic might be around somewhere.  Assuming it is you just have to do something like


#if defined (oddballctime)
  *date = ctime_r (&now, cbuf, CSZ);
#else
  *date = ctime_r (&now, cbuf);
#endif
Comment 2 dave 2011-02-18 20:56:54 UTC
> Is there no way to get a posix compliant ctime?  Alternatively, we'll need
> autoconf magic to detect the extra arg.  I know at one time it was relatively
> common, so autoconf magic might be around somewhere.  Assuming it is you just
> have to do something like
> 
> 
> #if defined (oddballctime)
>   *date = ctime_r (&now, cbuf, CSZ);
> #else
>   *date = ctime_r (&now, cbuf);
> #endif

Using ctime_r is a bit of a can of worms.  The GNU autoconf manual recommends
not using ctime_r unless the inputs are known to be within certain limits.

It seems HP-UX and Solaris have both forms.  In the HP-UX 11 case,
which form is used at compilation time depends on _PTHREADS_DRAFT4:

#          ifndef _PTHREADS_DRAFT4
             extern char *ctime_r(const __time_t *, char *);
#          else /* _PTHREADS_DRAFT4 */
	     extern int ctime_r(const __time_t *, char *, int);
#          endif /* _PTHREADS_DRAFT4 */

Potential autoconf test:

# ctime_r --
#
# There are two versions of ctime_r, one of which takes a buffer length as a
# third argument, and one which only takes two arguments.  (There is also a
# difference in return values, but we handle that in the code itself.)
AC_CHECK_FUNCS(ctime_r)
if test "$ac_cv_func_ctime_r" = "yes"; then
AC_CACHE_CHECK([for 2 or 3 argument version of ctime_r], db_cv_ctime_r_3arg, [
AC_TRY_LINK([
#include <time.h>], [
	ctime_r(NULL, NULL, 100);
],  [db_cv_ctime_r_3arg="3-argument"], [db_cv_ctime_r_3arg="2-argument"])])
fi
if test "$db_cv_ctime_r_3arg" = "3-argument"; then
	AC_DEFINE(HAVE_CTIME_R_3ARG)
	AH_TEMPLATE(HAVE_CTIME_R_3ARG,
	    [Define to 1 if ctime_r takes a buffer length as a third argument.])
fi

Dave
Comment 3 Janne Blomqvist 2011-02-19 16:57:18 UTC
(In reply to comment #2)
> > Is there no way to get a posix compliant ctime?  Alternatively, we'll need
> > autoconf magic to detect the extra arg.  I know at one time it was relatively
> > common, so autoconf magic might be around somewhere.  Assuming it is you just
> > have to do something like
> > 
> > 
> > #if defined (oddballctime)
> >   *date = ctime_r (&now, cbuf, CSZ);
> > #else
> >   *date = ctime_r (&now, cbuf);
> > #endif
> 
> Using ctime_r is a bit of a can of worms.  The GNU autoconf manual recommends
> not using ctime_r unless the inputs are known to be within certain limits.

Well, since the usage here is for the Fortran intrinsics CTIME and FDATE which are defined as interfaces to ctime(), unless you're volunteering to fix all the Fortran code out there using these intrinsics, there's not much we can do.

gfortran at least makes sure to call ctime_r() with a buffer of >= 26 bytes which is what POSIX requires. If some platform overflows this buffer, a bug report to the platform libc maintainers seems to be the correct approach.

> It seems HP-UX and Solaris have both forms.

For Solaris we had some similar problems with _r() functions, yes. This was solved by 

AC_USE_SYSTEM_EXTENSIONS

(from ../config/extensions.m4) which sets _POSIX_PTHREAD_SEMANTICS on Solaris, which makes it expose the POSIX standard interfaces rather than some pre-standard version.

>  In the HP-UX 11 case,
> which form is used at compilation time depends on _PTHREADS_DRAFT4:
> 
> #          ifndef _PTHREADS_DRAFT4
>              extern char *ctime_r(const __time_t *, char *);
> #          else /* _PTHREADS_DRAFT4 */
>          extern int ctime_r(const __time_t *, char *, int);
> #          endif /* _PTHREADS_DRAFT4 */

Yes, so HP-UX 11 seems to do the right thing by default. So the problem is HP-UX 10, which only provides the 3-arg form and not the standard one, right?

> Potential autoconf test:
> 
> # ctime_r --
> #
> # There are two versions of ctime_r, one of which takes a buffer length as a
> # third argument, and one which only takes two arguments.  (There is also a
> # difference in return values, but we handle that in the code itself.)
> AC_CHECK_FUNCS(ctime_r)
> if test "$ac_cv_func_ctime_r" = "yes"; then
> AC_CACHE_CHECK([for 2 or 3 argument version of ctime_r], db_cv_ctime_r_3arg, [
> AC_TRY_LINK([
> #include <time.h>], [
>     ctime_r(NULL, NULL, 100);
> ],  [db_cv_ctime_r_3arg="3-argument"], [db_cv_ctime_r_3arg="2-argument"])])
> fi
> if test "$db_cv_ctime_r_3arg" = "3-argument"; then
>     AC_DEFINE(HAVE_CTIME_R_3ARG)
>     AH_TEMPLATE(HAVE_CTIME_R_3ARG,
>         [Define to 1 if ctime_r takes a buffer length as a third argument.])
> fi

Thanks, that looks like a doable approach. Unless somebody else gets there first, I'll try to find some time to do this next week.
Comment 4 John David Anglin 2011-02-19 17:23:08 UTC
Also, we have another problem:

libtool: compile:  /xxx/gnu/gcc/objdir/./gcc/xgcc -B/xxx/gnu/gcc/objdir/./gcc/ -
B/opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/bin/ -B/opt/gnu/gcc/gcc-4.6/hppa1.1-h
p-hpux10.20/lib/ -isystem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/include -isystem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/sys-include -DHAVE_CONFIG_H -I. -
I../../../../gcc/libgfortran -iquote../../../../gcc/libgfortran/io -I../../../..
/gcc/libgfortran/../gcc -I../../../../gcc/libgfortran/../gcc/config -I../../../../gcc/libgfortran/../libquadmath -I../../.././gcc -D_GNU_SOURCE -std=gnu99 -Wall
 -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite
-strings -fcx-fortran-rules -g -O2 -threads -MT getlog.lo -MD -MP -MF .deps/getl
og.Tpo -c ../../../../gcc/libgfortran/intrinsics/getlog.c  -fPIC -DPIC -o .libs/
getlog.o
../../../../gcc/libgfortran/intrinsics/getlog.c: In function '_gfortran_getlog':
../../../../gcc/libgfortran/intrinsics/getlog.c:89:3: error: too many arguments to function 'getpwuid_r'
/usr/include/pwd.h:37:17: note: declared here

The HP-UX 10.20 version of 'getpwuid_r' doesn't have last argument of POSIX
version.

_REENTRANT needs to be defined to obtain header declarations for reentrant
functions.
Comment 5 John David Anglin 2011-02-19 17:30:27 UTC
> Yes, so HP-UX 11 seems to do the right thing by default. So the problem is
> HP-UX 10, which only provides the 3-arg form and not the standard one, right?

I believe HP changed to the POSIX interfaces in HP-UX 10.30.  Prior to that,
only the 3-arg form is provided and not the standard one.

The library build if I hack ctime.c and getlog.c, so these are the only
build issues.
Comment 6 Jeffrey A. Law 2011-02-21 16:56:18 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/18/11 13:56, dave at hiauly1 dot hia.nrc.ca wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47802
> 
> --- Comment #2 from dave at hiauly1 dot hia.nrc.ca 2011-02-18 20:56:54 UTC ---
>> Is there no way to get a posix compliant ctime?  Alternatively, we'll need
>> autoconf magic to detect the extra arg.  I know at one time it was relatively
>> common, so autoconf magic might be around somewhere.  Assuming it is you just
>> have to do something like
>>
>>
>> #if defined (oddballctime)
>>   *date = ctime_r (&now, cbuf, CSZ);
>> #else
>>   *date = ctime_r (&now, cbuf);
>> #endif
> 
> Using ctime_r is a bit of a can of worms.  The GNU autoconf manual recommends
> not using ctime_r unless the inputs are known to be within certain limits.
Correct.  The problem is some implementations can trigger buffer
overflows for bad input.  Certain implementations pass in a buffer size
parameter to deal with that problem, others (glibc) presumably do some
checking before dumping results into the user supplied buffer to make
sure they don't exceed the 26 bytes or whatever the minimum size of hte
buffer is supposed to be.

Even for checking versions like glibc, if the wrong sized buffer is
passed in, then it'll probably break.

I guess the question we need to ask is how important are these routines
and should we be issuing warnings when they are used, much like is done
with gets.  If we don't need them, I'd much prefer to see them go away
as they're a rats nest of security issues.

Just a quick glance at the code in libgfortran/ctime.c and I'm pretty
sure it's vulnerable to a buffer overflow attack.


Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNYpkoAAoJEBRtltQi2kC7HwcH/3IEgG2sh265kwu9kKQQ87gf
um1qKykJo4/Ph3W4UF7q1G26mw5luemVE6ga4+4nEzpivH0hzgsxWADDPXjQzq26
tqUXwh0nKi5665O1rcW88EZpej5J0MDLtUBTQXv1DipQWDBa/YjDqrmO4IRkw+MK
QlkgPvCqosS1wvlbVJ9xKpTn2XY8tVTPdLlAMI3iBbbtDcsWMdKxaG5mpnhh8P4i
HkVepfpRr5RtpuVN3SJ6AWhqR0PQgS1e2PB2WbbY8bvNy5ev1GggJZj/3j101jza
/QseJ16lj3CqOMHCppHOhXGL8bxMFW17AWv/hL74+gTn9rZCH/JUjOQ+YzRgs0A=
=SpA9
-----END PGP SIGNATURE-----
Comment 7 Jakub Jelinek 2011-02-21 17:41:20 UTC
Well, we don't want to use ctime because it is not thread-safe.
glibc ctime_r implementation should be safe if the passed buffer is at least 26 bytes long, it calls internally asctime, which is:
/* Like asctime, but write result to the user supplied buffer.  The
   buffer is only guaranteed to be 26 bytes in length.  */
char *
__asctime_r (const struct tm *tp, char *buf)
{
  return asctime_internal (tp, buf, 26);
}
and asctime_internal uses the passed buflen as second argument to snprintf.
Comment 8 Tobias Burnus 2011-02-21 18:08:13 UTC
(In reply to comment #6)
> Certain implementations pass in a buffer size
> parameter to deal with that problem, others (glibc) presumably do some
> checking before dumping results into the user supplied buffer to make
> sure they don't exceed the 26 bytes or whatever the minimum size of hte
> buffer is supposed to be.

Baring implementation bugs I would claim that all two-argument ctime_r implementations should work with 26 byte arguments as POSIX has been defined as such:

"The ctime_r() function shall convert the calendar time pointed to by clock to local time in exactly the same form as ctime() and put the string into the array pointed to by buf (which shall be at least 26 bytes in size) and return buf.
Unlike ctime(), the thread-safe version ctime_r() is not required to set tzname."
http://pubs.opengroup.org/onlinepubs/009695399/functions/ctime.html

The definition goes back to POSIX (SUSv2) of February 1997 - and IEEE Std 1003.1c-1995. One should assume that within the last 15 years they vendors managed to get the 26-byte buffer issue correct ...

(How to deal with a three-argument version is a separate question; different vendors have probably different arguments, though buflen could be a more common choice.)


> I guess the question we need to ask is how important are these routines

From he non-normative part of POSIX:

"The ctime_r() function is thread-safe and shall return values in a user-supplied buffer instead of possibly using a static data area that may be overwritten by each call."
Comment 9 Jeffrey A. Law 2011-02-21 18:49:02 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/21/11 10:41, jakub at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47802
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-02-21 17:41:20 UTC ---
> Well, we don't want to use ctime because it is not thread-safe.
Right.

> glibc ctime_r implementation should be safe if the passed buffer is at least 26
> bytes long, it calls internally asctime, which is:
I'm aware that glibc's variant is safe from bogus input causing a buffer
overrun.  The problem is not every vendor's implementation is safe with
regards to buffer overruns due to bogus input.

Furthermore, I don't think any of the implementations are safe if the
user supplied buffer is less than 26 bytes.  So if an idiot programmer
called ctime_r with too small a buffer, then we've got a buffer overrun
and a vector for a security attack.

jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNYrOUAAoJEBRtltQi2kC7KPMH/25knyvBbLrN5lHbuBHJ9sh3
eGFMuym9/5yXRn/oAesxoPA/PqakfULGUgecF7168H+N+ECoHhn53D/clY5ea7Ti
6yuLb0a2rFMtZpn+BxB4JFzW3hdDXKjj8nIZiT5PuZX7yjLfIYlQZiVBpVG0IpfU
wGGFXHUnGM1j4YDB0tStZnzU+4/rkXml2pmjBzApjGGDrMRXarrrCD4cEffBGZOc
xnVLfcarKQ/wnltrEs3PCogG8zwpu4Gp6jJLnZDYNF4Rk8K4RhsvmeRzFND0n0ZM
3w9dBEQXF3AqmrWVBX08krgXornXN1n7zwj3bZdM6o6jH6iW5NY4vsyx4SRtZ7Q=
=JcEq
-----END PGP SIGNATURE-----
Comment 10 Jeffrey A. Law 2011-02-21 18:51:09 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/21/11 11:09, burnus at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47802
> 
> --- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-02-21 18:08:13 UTC ---
> (In reply to comment #6)
>> Certain implementations pass in a buffer size
>> parameter to deal with that problem, others (glibc) presumably do some
>> checking before dumping results into the user supplied buffer to make
>> sure they don't exceed the 26 bytes or whatever the minimum size of hte
>> buffer is supposed to be.
> 
> Baring implementation bugs I would claim that all two-argument ctime_r
> implementations should work with 26 byte arguments as POSIX has been defined as
> such:
> 
> "The ctime_r() function shall convert the calendar time pointed to by clock to
> local time in exactly the same form as ctime() and put the string into the
> array pointed to by buf (which shall be at least 26 bytes in size) and return
> buf.
> Unlike ctime(), the thread-safe version ctime_r() is not required to set
> tzname."
> http://pubs.opengroup.org/onlinepubs/009695399/functions/ctime.html
The problem is some vendors mucked up their implementations and don't
stay within the 26 byte limit for bogus input values.  And just because
their implementations are correct *today* doesn't mean that they're
correct on the systems which might be running this code.  There's people
still running very old systems out there.

Furthermore, no implementation I'm aware of can catch teh case where the
user supplied buffer is less than 26 bytes.

Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNYrQNAAoJEBRtltQi2kC7f28IAJdrRtgUiAewCofhz6jWNcZN
DRBjZLybLfNx0sHX6czLql/1q1MNmJdtc0Vwmp6VnGHkoZtepY+HRwyu/6Y/5nAi
cLaNHSkeOAKwR+JElzOqczRKxli/YBzYtgcTAJFD2nNTB0gK5h53hhR/Pup2JLmC
PSddl3cDUdYdl9KRydRJpU0Z8hOC03fd70MMIxO//H12HTzpHXDsjCA8PrZTcY0l
hEwyKRgw81zwB0+LHt0E14v9XhMm9t3U81FCngo1W/EKtEqAsGFHaMiZXl+ums0H
9LSIE53e10Tq5R6V4LpJhOa3Tpk7G/hbGraojdbq6w6unt/7nfVoZB4G4+yq9J8=
=cpqO
-----END PGP SIGNATURE-----
Comment 11 Jakub Jelinek 2011-02-21 18:58:27 UTC
In libgfortran it is not the user, but libgfortran implementation, so it makes sure it always passes buffer of at least 26 bytes.  If there are OSes where we can't trust ctime_r, we could either blacklist them (or whitelist the known good ones), or on some of them try to supply larger buffer (is there any implementation that would overflow say 256 or 1024 bytes)?
Comment 12 dave 2011-02-21 19:38:33 UTC
On Mon, 21 Feb 2011, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47802
> 
> --- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-02-21 18:58:27 UTC ---
> In libgfortran it is not the user, but libgfortran implementation, so it makes
> sure it always passes buffer of at least 26 bytes.  If there are OSes where we
> can't trust ctime_r, we could either blacklist them (or whitelist the known
> good ones), or on some of them try to supply larger buffer (is there any
> implementation that would overflow say 256 or 1024 bytes)?

The gnulib thread that led to the autoconf recommendation suggests using
strftime.  localtime_r is thread safe.

Besides the buffer overflow issues, asctime_r and ctime_r do not honor
i18n, and incorrect values are produced for some input values.  For example,
some Solaris versions silently generate incorrect values for timestamps
before 1900.

See:
http://www.mail-archive.com/bug-gnulib@gnu.org/msg02248.html

Dave
Comment 13 Janne Blomqvist 2011-02-22 08:37:13 UTC
FWIW here's a glibc PR wrt overflowing the 26 byte limit:

http://sourceware.org/bugzilla/show_bug.cgi?id=1460

This was fixed in 2005, which is a while ago, but not long enough so it's completely unimaginable that someone would want to run gfortran on such a platform. For this particular glibc issue, a workaround is to make sure the buffer is at least 33 bytes.

In any case, I'm thinking that Dave's suggestion to use strftime could work. Also, in addition to the autoconf manual recommending to avoid ctime/ctime_r, POSIX 2008 also marks those as obsolescent and recommends strftime instead. strftime appears to have well defined behavior for all kinds of time input and string lengths. And, using it would obviously also solve the problem of non-standard ctime_r implementations. strftime needs localtime_r, but we have already used that one without problems for quite a while (in the implementation of the DATE_AND_TIME intrinsic) so it shouldn't cause any further portability problems.

The caveat is that if using a non-default locale the output is different. In practice, (lib)gfortran never calls setlocale so unless the application explicitly calls it a gfortran program runs in the "C" locale, in which case a suitable choice of format string guarantees identical output as ctime(). And, if the applications does call setlocale, outputting a localized string could be seen as a feature.

(In reply to comment #6)
> I guess the question we need to ask is how important are these routines
> and should we be issuing warnings when they are used, much like is done
> with gets.  If we don't need them, I'd much prefer to see them go away
> as they're a rats nest of security issues.

Standard Fortran, as of Fortran 2008, supports 3 time related intrinsics; DATE_AND_TIME, SYSTEM_CLOCK, and CPU_TIME. Every other time intrinsic in gfortran is due to legacy g77 support, or some other widely used extension. If one compiles with one of the standards conformance options (-std=fxxxx) these other intrinsics are not available. Personally, I'd be happy to get rid of all the nonstandard time intrinsics, but some of our users might disagree..
Comment 14 Jakub Jelinek 2011-02-22 09:26:50 UTC
Well, if you don't want to localize, you can as well do it yourself, call localtime_r and then simple:
char result[26];
int n = snprintf (result, sizeof (result), "%3.3s %3.3s%3d %.2d:%.2d:%.2d %d\n",
                  "SunMonTueWedThuFriSat" + tm->tm_wday * 3,
                  "JanFebMarAprMayJunJulAugSepOctNovDec" + tm->tm_mon * 3,
                  tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec, 1900 + tm->tm_year);
if ((size_t) n >= sizeof (result))
  result[0] = '\0'; /* The result didn't fit into 26 bytes.  */ 
(or use larger buffer, perhaps you can just snprintf into the caller provided buffer instead and depending on return value from snprintf just memset to ' ' the rest of the string if there are still any bytes left in it.
Comment 15 Janne Blomqvist 2011-02-22 16:21:09 UTC
Patch: http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01453.html
Comment 16 Janne Blomqvist 2011-02-22 20:24:13 UTC
Patch which should hopefully fix the getpwuid_r issue on HP-UX 10.2: http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01478.html
Comment 17 dave 2011-02-23 14:28:38 UTC
On Tue, 22 Feb 2011, jb at gcc dot gnu.org wrote:

> Patch: http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01453.html

Patch resolves ctime.c build.  However, _REENTRANT is not defined,
so there are various warnings regarding the declaration of reentrant
functions such as localtime_r:

libtool: compile:  /xxx/gnu/gcc/objdir/./gcc/xgcc -B/xxx/gnu/gcc/objdir/./gcc/ -B/opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/bin/ -B/opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/lib/ -isystem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/include -isystem /opt/gnu/gcc/gcc-4.6/hppa1.1-hp-hpux10.20/sys-include -DHAVE_CONFIG_H -I. -I../../../gcc/libgfortran -iquote../../../gcc/libgfortran/io -I../../../gcc/libgfortran/../gcc -I../../../gcc/libgfortran/../gcc/config -I../../../gcc/libgfortran/../libquadmath -I../.././gcc -D_GNU_SOURCE -std=gnu99 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -fcx-fortran-rules -g -O2 -MT ctime.lo -MD -MP -MF .deps/ctime.Tpo -c ../../../gcc/libgfortran/intrinsics/ctime.c  -fPIC -DPIC -o .libs/ctime.o
../../../gcc/libgfortran/intrinsics/ctime.c: In function 'fctime':
../../../gcc/libgfortran/intrinsics/ctime.c:43:10: warning: implicit declaration of function 'localtime_r' [-Wimplicit-function-declaration]
../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization makes pointer from integer without a cast [enabled by default]

Dave
Comment 18 dave 2011-02-23 14:30:15 UTC
On Tue, 22 Feb 2011, jb at gcc dot gnu.org wrote:

> Patch which should hopefully fix the getpwuid_r issue on HP-UX 10.2:
> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01478.html

Patch fixes the getpwuid_r build issue on HP-UX 10.2.

Dave
Comment 19 Tobias Burnus 2011-02-23 18:24:20 UTC
(In reply to comment #17)
> Patch resolves ctime.c build.  However, _REENTRANT is not defined,
> so there are various warnings regarding the declaration of reentrant
> functions such as localtime_r:

As there localtime_r is also used in intrinsics/date_and_time.c, I would assume that one sees the same message there.

I am also not quite sure that _REENTRANT is the correct solution. At least for Solaris, one should use _POSIX_C_SOURCE and not _REENTRANT to get the POSIX version, cf. http://download.oracle.com/docs/cd/E19455-01/806-5257/compile-4/

For HP-UX, I could not find anything (cf. also http://docs.hp.com/en/B2355-90683/ctime.3C.html). However, IBM has a statement regarding HP-UX, which is similar to Solaris's above: http://publib.boulder.ibm.com/infocenter/db2luw/v8/topic/com.ibm.db2.udb.doc/ad/t0007651.htm
Comment 20 dave 2011-02-23 20:15:34 UTC
> As there localtime_r is also used in intrinsics/date_and_time.c, I would assume
> that one sees the same message there.

Yes.  I see them for all _r uses.

> I am also not quite sure that _REENTRANT is the correct solution. At least for
> Solaris, one should use _POSIX_C_SOURCE and not _REENTRANT to get the POSIX
> version, cf. http://download.oracle.com/docs/cd/E19455-01/806-5257/compile-4/

I looked at the headers.  On HP-UX 11, _REENTRANT is defined by
<sys/stdsyms.h> if _POSIX_C_SOURCE is defined and _REENTRANT is not
defined.  However, this does not occur on HP-UX 10.  The system
headers never define _REENTRANT.

The default _HPUX_SOURCE is equivalent to -D_POSIX_C_SOURCE=199506L
on HP-UX 11.  Defining _REENTRANT also includes the same POSIX sources.

The PA backend defines _REENTRANT if -threads is specified on HP-UX 10
and if -pthread is specified on HP-UX 11.  -threads also defines
_DCE_THREADS which we may not want for the single thread model.

libgfortran.sl is built twice on HP-UX 10, once for the single thread
model and once for the dce thread model.  It's the single thread build
that's the problem.

> For HP-UX, I could not find anything (cf. also
> http://docs.hp.com/en/B2355-90683/ctime.3C.html). However, IBM has a statement
> regarding HP-UX, which is similar to Solaris's above:
> http://publib.boulder.ibm.com/infocenter/db2luw/v8/topic/com.ibm.db2.udb.doc/ad/t0007651.htm

All the reentrant functions are in libc.

Dave
Comment 21 Janne Blomqvist 2011-02-24 11:34:18 UTC
(In reply to comment #20)
> libgfortran.sl is built twice on HP-UX 10, once for the single thread
> model and once for the dce thread model.  It's the single thread build
> that's the problem.

Why is this necessary? Isn't it possible to use the dce thread library from a single threaded application? 

I suppose one way to fix this would be to somehow make the autoconf checks for the _r functions fail if the prototypes are not found?
Comment 22 Janne Blomqvist 2011-02-24 14:51:21 UTC
Author: jb
Date: Thu Feb 24 14:51:17 2011
New Revision: 170471

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170471
Log:
PR 47802 Test for POSIX getpwuid_r

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/config.h.in
    trunk/libgfortran/configure
    trunk/libgfortran/configure.ac
    trunk/libgfortran/intrinsics/getlog.c
Comment 23 dave 2011-02-24 14:55:52 UTC
On Thu, 24 Feb 2011, jb at gcc dot gnu.org wrote:

> > libgfortran.sl is built twice on HP-UX 10, once for the single thread
> > model and once for the dce thread model.  It's the single thread build
> > that's the problem.
> 
> Why is this necessary? Isn't it possible to use the dce thread library from a
> single threaded application? 

I believe that it's primarily a performance issue.  Threaded applications
are linked with -lcma -lc_r.  Non threaded applications are linked with -lc.

If only the dce thread model was used, there probably would be issues with
the GTHREADS interface due to the lack of weak symbols.  libc.sl doesn't
have dce thread stubs.  So, all applications would have to be linked as 
threaded applications.

> I suppose one way to fix this would be to somehow make the autoconf checks for
> the _r functions fail if the prototypes are not found?

I'm wondering if the backend should always define _REENTRANT on HP-UX 10.
I'd have to check that all the *_r functions are in libc.  For libraries
that don't explicitly need thread support, it would be good if they could
be linked into both threaded and non threaded applications.

Dave
Comment 24 Janne Blomqvist 2011-02-24 21:51:41 UTC
Author: jb
Date: Thu Feb 24 21:51:39 2011
New Revision: 170478

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170478
Log:
PR 47802 Use strftime for CTIME and FDATE intrinsics

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/config.h.in
    trunk/libgfortran/configure
    trunk/libgfortran/configure.ac
    trunk/libgfortran/intrinsics/ctime.c
    trunk/libgfortran/intrinsics/date_and_time.c
    trunk/libgfortran/intrinsics/time_1.h
Comment 25 Tobias Burnus 2011-02-25 10:57:32 UTC
I think the build bug is now FIXED; thus:

Please shout loudly if there you still encounter a build failure!


TO BE DONE: The HP-UX (et al.?) compile warning regarding the _r functions and _REENTRANT, cf. comment 20 and comment 23.
Comment 26 dave 2011-02-26 13:59:50 UTC
On Fri, 25 Feb 2011, burnus at gcc dot gnu.org wrote:

> I think the build bug is now FIXED; thus:
> 
> Please shout loudly if there you still encounter a build failure!

Building...  Should know today but I'm on vacation in Costa Rica.

> TO BE DONE: The HP-UX (et al.?) compile warning regarding the _r functions and
> _REENTRANT, cf. comment 20 and comment 23.

Testing a fix.  I'm 95% certain that _REENTRANT should be defined on
HP-UX 10 even when not linking with -lcma (dce thread library).  This
should provide a build environment closer to HP-UX 11 POSIX environment.

We should also always link threaded applications with -lc as libc was
enhanced to add reentrant support in HP-UX 10.  -lc_r remained for
HP-UX 9 compatibility.

What is unclear is what happens to the locking needed for some
reentrant routines.  I'm thinking it should just work as it does on
HP-UX 11 when not linking with -lpthread ("single" thread).  There
is code in libc to do this locking (_p_mutex_lock/_p_mutex_unlock)
but it is hard to tell what it does because it makes a couple of
indirect calls.  Couldn't find any documentation on this.

Dave
Comment 27 Jakub Jelinek 2011-03-02 13:51:36 UTC
This isn't a bootstrap failure any longer from what I can understand, any Fortran isn't release critical.
Comment 28 Jeffrey A. Law 2011-03-02 13:58:19 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/02/11 06:51, jakub at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47802
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>            Priority|P2                          |P4
> 
> --- Comment #27 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-02 13:51:36 UTC ---
> This isn't a bootstrap failure any longer from what I can understand, any
> Fortran isn't release critical.
And the PA isn't (or shouldn't be) a primary platform :-)

jeff

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNbkzwAAoJEBRtltQi2kC70GMH/0ZIq10zhwuOjnMBJaUPMZVV
Bq8Jsu7PqFFE4IJ0rQUpFEcNNmKkPlrINHK3eZNafKTmacqwSfVCZd1worUdvHpH
JRr86cOxHKF2diwih4NpGgx4JtcfeGEDlA5I++3iTRifz5H/nQfrBil7BVI4ocpi
wBcd6mCQa25LxfJePxxR6Az0fDcxxHPnHuQt/ve40v1zugDpk8TE3kjxLXfoclCg
DZIqNZ84D7UGwPuGo/+VYNi07u1RO/MxBStSBFJvCDatAIQ8kzKLP60SVUGfSHyK
auckyBMr5+LQWmJyh50UJyxtYn7c/aLnUzQfQ3aNUNdlaJT1mubgXCgFhzTGw4A=
=pWH0
-----END PGP SIGNATURE-----
Comment 29 Jakub Jelinek 2011-03-02 14:01:20 UTC
hppa2.0w-hp-hpux11.11 is listed as secondary platform, though not sure how narrowly we consider that (if hppa2.0w-hp-hpux10.* is considered also secondary, or just hpux11...).
Comment 30 dave 2011-03-03 13:56:25 UTC
On Fri, 25 Feb 2011, burnus at gcc dot gnu.org wrote:

> Please shout loudly if there you still encounter a build failure!
> 
> 
> TO BE DONE: The HP-UX (et al.?) compile warning regarding the _r functions and
> _REENTRANT, cf. comment 20 and comment 23.

In testing fix for above, I see:

../../../gcc/libgfortran/intrinsics/ctime.c: In function 'strctime':
../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization makes pointer from integer without a cast [enabled by default]

Unfortunately, localtime_r has a different proto:

int localtime_r(const time_t *timer, struct tm *result);

Dave
Comment 31 Janne Blomqvist 2011-03-04 17:37:23 UTC
Author: jb
Date: Fri Mar  4 17:37:11 2011
New Revision: 170679

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170679
Log:
PR 47802 Update doc for CTIME and FDATE intrinsics

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/intrinsic.texi
Comment 32 Janne Blomqvist 2011-03-04 17:52:43 UTC
Author: jb
Date: Fri Mar  4 17:52:10 2011
New Revision: 170680

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170680
Log:
PR 47802 Hack to work around draft POSIX localtime_r

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/intrinsics/ctime.c
Comment 33 Janne Blomqvist 2011-03-04 19:07:53 UTC
Author: jb
Date: Fri Mar  4 19:07:49 2011
New Revision: 170683

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170683
Log:
PR 47802 Use builtins to check localtime_r return type

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/intrinsics/ctime.c
Comment 34 Janne Blomqvist 2011-03-04 19:15:34 UTC
(In reply to comment #30)
> On Fri, 25 Feb 2011, burnus at gcc dot gnu.org wrote:
> 
> > Please shout loudly if there you still encounter a build failure!
> > 
> > 
> > TO BE DONE: The HP-UX (et al.?) compile warning regarding the _r functions and
> > _REENTRANT, cf. comment 20 and comment 23.
> 
> In testing fix for above, I see:
> 
> ../../../gcc/libgfortran/intrinsics/ctime.c: In function 'strctime':
> ../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization
> makes pointer from integer without a cast [enabled by default]
> 
> Unfortunately, localtime_r has a different proto:
> 
> int localtime_r(const time_t *timer, struct tm *result);
> 
> Dave

Thanks to some __builtin tricks by Jakub, handling the different return types should now work. However, one more possible issue: In http://docs.hp.com/en/947/d8.html it says the old DCE threads prototype is 

int localtime_r ( struct tm *result, time_t *clock );

which has the arguments the other way around than what you say above. For now, the code assumes the order you provided; If this turns out to be wrong, we need to do yet one more fix.. :(
Comment 35 dave 2011-03-05 17:06:23 UTC
> > In testing fix for above, I see:
> > 
> > ../../../gcc/libgfortran/intrinsics/ctime.c: In function 'strctime':
> > ../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization
> > makes pointer from integer without a cast [enabled by default]
> > 
> > Unfortunately, localtime_r has a different proto:
> > 
> > int localtime_r(const time_t *timer, struct tm *result);
> > 
> > Dave
> 
> Thanks to some __builtin tricks by Jakub, handling the different return types
> should now work. However, one more possible issue: In
> http://docs.hp.com/en/947/d8.html it says the old DCE threads prototype is 
> 
> int localtime_r ( struct tm *result, time_t *clock );

I believe the above document is wrong.  Both the HP-UX 10.20 manpage and
header show the definition that I quoted.  Same for the _PTHREADS_DRAFT4
definition in HP-UX 11.X

Dave
Comment 36 Janne Blomqvist 2011-03-09 07:47:43 UTC
(In reply to comment #35)
> > > In testing fix for above, I see:
> > > 
> > > ../../../gcc/libgfortran/intrinsics/ctime.c: In function 'strctime':
> > > ../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization
> > > makes pointer from integer without a cast [enabled by default]
> > > 
> > > Unfortunately, localtime_r has a different proto:
> > > 
> > > int localtime_r(const time_t *timer, struct tm *result);
> > > 
> > > Dave
> > 
> > Thanks to some __builtin tricks by Jakub, handling the different return types
> > should now work. However, one more possible issue: In
> > http://docs.hp.com/en/947/d8.html it says the old DCE threads prototype is 
> > 
> > int localtime_r ( struct tm *result, time_t *clock );
> 
> I believe the above document is wrong.  Both the HP-UX 10.20 manpage and
> header show the definition that I quoted.  Same for the _PTHREADS_DRAFT4
> definition in HP-UX 11.X
> 
> Dave

Thanks for checking. I did some more googling, and I found some other documentation for DCE/POSIX draft localtime_r that seems to confirm this. 

Closing as fixed.
Comment 37 Tobias Burnus 2011-03-09 13:41:57 UTC
(In reply to comment #30)
> On Fri, 25 Feb 2011, burnus at gcc dot gnu.org wrote:
> > TO BE DONE: The HP-UX (et al.?) compile warning regarding the _r functions and
> > _REENTRANT, cf. comment 20 and comment 23.
> 
> In testing fix for above, I see:
> ../../../gcc/libgfortran/intrinsics/ctime.c:43:20: warning: initialization
> makes pointer from integer without a cast [enabled by default]
> Unfortunately, localtime_r has a different proto:

While the latter is fixed, I think the _REENTRANT issue isn't. Or is it?

If it it not fixed, I think we should have (a different) PR open to track that issue. Dave, you wrote you were testing a patch for _REENTRANT ...
Comment 38 dave 2011-03-10 16:58:38 UTC
> While the latter is fixed, I think the _REENTRANT issue isn't. Or is it?
> 
> If it it not fixed, I think we should have (a different) PR open to track that
> issue. Dave, you wrote you were testing a patch for _REENTRANT ...

Correct, I have a patch which I have been testing.  While it seems to
work, there is one issue that I don't particularly like.

`-threads' is a multilib option handled completely by the gcc driver.
It is not passed to the standard option processing, so I wasn't able
to find a way to check for `-threads' in TARGET_OS_CPP_BUILTINS.
Currently, I define  _REENTRANT whenever we define _HPUX_SOURCE in
TARGET_OS_CPP_BUILTINS.  It is also defined by CPP_SPEC when -threads
is specified.  As a result, there are some situations where _REENTRANT
is effectively defined twice.

This could be avoided if _REENTRANT was only defined in CPP_SPEC but
it is difficult to duplicate the option handling in TARGET_OS_CPP_BUILTINS.

Since I haven't seen any actual problems with the current patch, I think
I will commit it tonight.

Dave