[Bug libfortran/51646] New: Make libgfortran compile on Android - without S_IREAD

[Bug libfortran/51646] New: Make libgfortran compile on Android - without S_IREAD

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

             Bug #: 51646
           Summary: Make libgfortran compile on Android - without S_IREAD
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: build
          Severity: normal
          Priority: P3
         Component: libfortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: burnus@gcc.gnu.org
                CC: jb@gcc.gnu.org


Created attachment 26156
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26156
Draft patch

Janne: What do you think of the attached patch?

http://specificimpulses.blogspot.com/2011/01/my-android-speaks-fortran-yours-can-too.html
shows that gfortran can be build for on Android (= host + target).

However, Android's NDK does not support S_IREAD and S_IWRITE but only S_IRUSR
and S_IWUSR.

In the Linux man page I find:
       UNIX  V7 (and later systems) had S_IREAD, S_IWRITE, S_IEXEC, where POSIX
       prescribes the synonyms S_IRUSR, S_IWUSR, S_IXUSR.

While POSIX (IEEE Std 1003.1, 2003) just lists the latter:
       S_IRUSR
              Read permission, owner.
       S_IWUSR
              Write permission, owner.

Expected: If no S_IREAD/S_IWRITE is available, use S_IRUSR/S_IWUSR. (Or rather
the other way round, given that only the latter is standard.)

S_IREAD/S_IWRITE is
- checked for in LIBGFOR_CHECK_UNLINK_OPEN_FILE  (libgfortran/acinclude.m4)
- used in io/unix.c's id_from_fd

 * * *

In the blog, the existence of other issues is mentioned and it links to a hack:
http://aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch
(Google cache:
http://webcache.googleusercontent.com/search?q=cache:khmjfgpFiAIJ:aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch+aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch
)

However, the patch/hack does not tell me much: It disable configure checking
(as_fn_exit returns 0 ("echo $1") instead of "exit $1") and in
intrinsics/date_and_time.c the "#ifndef" has been changed into an "#ifdef" for
HAVE_LOCALTIME_R and for HAVE_GMTIME_R.

Without config.log, one can probably not deduce what went wrong.

[Bug libfortran/51646] Make libgfortran compile on Android - without S_IREAD

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

Janne Blomqvist <jb at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-12-21
     Ever Confirmed|0                           |1

--- Comment #1 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-12-21 10:10:10 UTC ---
(In reply to comment #0)
> Created attachment 26156 [details]
> Draft patch
> 
> Janne: What do you think of the attached patch?
> 
> http://specificimpulses.blogspot.com/2011/01/my-android-speaks-fortran-yours-can-too.html
> shows that gfortran can be build for on Android (= host + target).
> 
> However, Android's NDK does not support S_IREAD and S_IWRITE but only S_IRUSR
> and S_IWUSR.
> 
> In the Linux man page I find:
>        UNIX  V7 (and later systems) had S_IREAD, S_IWRITE, S_IEXEC, where POSIX
>        prescribes the synonyms S_IRUSR, S_IWUSR, S_IXUSR.
> 
> While POSIX (IEEE Std 1003.1, 2003) just lists the latter:
>        S_IRUSR
>               Read permission, owner.
>        S_IWUSR
>               Write permission, owner.
> 
> Expected: If no S_IREAD/S_IWRITE is available, use S_IRUSR/S_IWUSR. (Or rather
> the other way round, given that only the latter is standard.)
> 
> S_IREAD/S_IWRITE is
> - checked for in LIBGFOR_CHECK_UNLINK_OPEN_FILE  (libgfortran/acinclude.m4)
> - used in io/unix.c's id_from_fd
> 
>  * * *

Good catch! I suppose it's possible that all targets we support have the POSIX
flags, but since we're in stage 3 I think it's fine to play it safe and do the
ifdef dance.

So your patch per se is OK, however you have manage to introduce quite a few
typos ;-)

Since I copy-paste the patch here, I'll put my comments within "<<<    >>>" 
blocks:

Index: acinclude.m4
===================================================================
--- acinclude.m4    (revision 182565)
+++ acinclude.m4    (working copy)
@@ -115,11 +115,19 @@ AC_DEFUN([LIBGFOR_CHECK_UNLINK_OPEN_FILE], [
 #include <unistd.h>
 #include <sys/stat.h>

+#if !defined(S_IRUSR) && defined(S_IREAD)
+#define S_IRUSR S_IREAD
+#endif
+
+#if !defined(S_IWUSR) && defined(S_IWRITE)
+#define S_IWGRP S_IWRITE

<<< Should be S_IWUSR >>>

+#endif
+
 int main ()
 {
   int fd;

-  fd = open ("testfile", O_RDWR | O_CREAT, S_IWRITE | S_IREAD);
+  fd = open ("testfile", O_RDWR | O_CREAT, S_IWUSR | S_IRUSR);
   if (fd <= 0)
     return 0;
   if (unlink ("testfile") == -1)
@@ -127,7 +135,7 @@ int main ()
   write (fd, "This is a test\n", 15);
   close (fd);

-  if (open ("testfile", O_RDONLY, S_IWRITE | S_IREAD) == -1 && errno ==
ENOENT)
+  if (open ("testfile", O_RDONLY, S_IUSR | S_IUSR) == -1 && errno == ENOENT)

<<<
There's two bugs here:

1. You have S_IUSR twice, presumably you mean S_IRUSR | S_IWUSR.

2. The mode argument is ignored if O_CREAT is not or'ed into the flags
argument, so the open call could just as well be open ("testfile", O_RDONLY)
>>>

     return 0;
   else
     return 1;
Index: io/unix.c
===================================================================
--- io/unix.c    (revision 182565)
+++ io/unix.c    (working copy)
@@ -113,6 +113,14 @@ id_from_fd (const int fd)
 #define PATH_MAX 1024
 #endif

+#if !defined(S_IRUSR) && defined(S_IREAD)
+#define S_IRUSR S_IREAD
+#endif
+
+#if !defined(S_IWUSR) && defined(S_IWRITE)
+#define S_IWGRP S_IWRITE

<<< Same here, should be S_IWUSR. >>>

+#endif
+
 /* These flags aren't defined on all targets (mingw32), so provide them
    here.  */
 #ifndef S_IRGRP
@@ -1112,9 +1120,9 @@ tempfile (st_parameter_open *opp)

 #if defined(HAVE_CRLF) && defined(O_BINARY)
       fd = open (template, O_RDWR | O_CREAT | O_EXCL | O_BINARY,
-         S_IREAD | S_IWRITE);
+         S_IUSR | S_IWUSR);

<<< Should be S_IRUSR | S_IWUSR  (R is missing) >>>

 #else
-      fd = open (template, O_RDWR | O_CREAT | O_EXCL, S_IREAD | S_IWRITE);
+      fd = open (template, O_RDWR | O_CREAT | O_EXCL, S_IUSR | S_IWUSR);

<<< Should be S_IRUSR | S_IWUSR  (R is missing) >>>

 #endif
     }
   while (fd == -1 && errno == EEXIST);




> In the blog, the existence of other issues is mentioned and it links to a hack:
> http://aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch
> (Google cache:
> http://webcache.googleusercontent.com/search?q=cache:khmjfgpFiAIJ:aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch+aeromonkey.homeip.net/public/ugly_gfortran_hacks.patch
> )
> 
> However, the patch/hack does not tell me much: It disable configure checking
> (as_fn_exit returns 0 ("echo $1") instead of "exit $1") and in
> intrinsics/date_and_time.c the "#ifndef" has been changed into an "#ifdef" for
> HAVE_LOCALTIME_R and for HAVE_GMTIME_R.
> 
> Without config.log, one can probably not deduce what went wrong.

Agreed, that patch is hacky to the point of being useless for us. Lets not
worry about that.

[Bug libfortran/51646] Make libgfortran compile on Android - without S_IREAD

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

--- Comment #2 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-12-21 10:21:32 UTC ---
(In reply to comment #1)
> Good catch! I suppose it's possible that all targets we support have the POSIX
> flags, but since we're in stage 3 I think it's fine to play it safe and do the
> ifdef dance.

Actually, in unix.c:1243 we unconditionally use S_IRUSR and S_IRGRP, which
means that all targets we support do have those flags. So on second thought I
think the ifdef dance is not necessary.

[Bug libfortran/51646] Make libgfortran compile on Android - without S_IREAD

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

--- Comment #3 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-12-22 20:44:44 UTC ---
Author: jb
Date: Thu Dec 22 20:44:32 2011
New Revision: 182638

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182638
Log:
PR 51646 Use POSIX mode flags in open() argument.

2011-12-22  Janne Blomqvist  <jb@gcc.gnu.org>
    Tobias Burnus  <burnus@net-b.de>

    PR libfortran/51646
    * acinclude.m4 (LIBGFOR_CHECK_UNLINK_OPEN_FILE): Use POSIX mode
    flags, omit mode argument when flags argument does not have
    O_CREAT.
    * io/unix.c (tempfile): Use POSIX mode flags.
    * configure: Regenerate.


Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/acinclude.m4
    trunk/libgfortran/configure
    trunk/libgfortran/io/unix.c

[Bug libfortran/51646] Make libgfortran compile on Android

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

Janne Blomqvist <jb at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING
            Summary|Make libgfortran compile on |Make libgfortran compile on
                   |Android - without S_IREAD   |Android
           Severity|normal                      |enhancement

--- Comment #4 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-12-22 20:50:26 UTC ---
Mode flags patch committed to trunk. Leaving the PR open as an enhancement in
case the reason behind the hack-patch comes to light.

[Bug libfortran/51646] Make libgfortran compile on Android

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

--- Comment #5 from Janne Blomqvist <jb at gcc dot gnu.org> 2012-04-18 08:31:47 UTC ---
Another build problem at

http://stackoverflow.com/questions/10202966/android-ndk-fortran-build-of-lapack-problems-with-unresolved-sincos

[Bug libfortran/51646] Make libgfortran compile on Android

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-04-18 12:36:18 UTC ---
(In reply to comment #5)
> Another build problem at
> http://stackoverflow.com/questions/10202966/android-ndk-fortran-build-of-lapack-problems-with-unresolved-sincos

Judging from the line given at the stackoverflow posting, the problem is
related to:
  COMPLEX_ASSIGN (v, cosf (b), sinf (b));

Thus, the middle-end should convert this into a sincosf - but only if it is
available. Given that the compilation is for arm-linux-androideabi, I assume
that gcc/config/linux.h applies, which has:

/* Whether we have sincos that follows the GNU extension.  */
#undef TARGET_HAS_SINCOS
#define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC)

For Android/Bionic, one finds:
   /bionic/libc/docs/CHANGES.TXT
   ...
   Differences between Android 2.3 and Android 2.2:
   ...
   - <math.h>: Added sincos(), sincosf() and sincosl() (GLibc compatibility).

Cf. http://source-android.frandroid.com/bionic/libc/docs/CHANGES.TXT


Hence, there are two options:
a) Using a newer Bionic
b) Manually patch gcc/config/linux.h to simply claim that no sincos is
available.

The latter is what is also suggested at
https://bugs.launchpad.net/linaro-android/+bug/908125

[Bug libfortran/51646] Make libgfortran compile on Android

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51646

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-04-18 12:54:20 UTC ---
Given that the problem of comment 0 is fixed and comment 5 is also not a bug -
especially not a libgfortran bug, I close now the PR as FIXED.

Regarding comment 0: The current blog does no longer list any libgfortran
patches, cf.
http://specificimpulses.blogspot.de/2011/10/android-fortran-step-by-step-part-2.html

Regarding comment 5: See comment 6.