Problems w/ ___errno on Solaris #112

PHHargrove · 2015-03-02T22:42:42Z

With clang-upc on Solaris/x86 and gcc or cc (Sun/Oracle compiler) as the back-end compiler, I see failures on upc_io_test.upc:

[benchmarks/upc_io_test] FAILED: UPC-To-C Translation or Link error     (NEW)
cd /shared/upcnightly-32/cupc2c/work/dbg/upc-tests/benchmarks
/export/home/phargrov/upcnightly-32/cupc2c/runtime/inst/bin/upcc -Ww,-Wno-duplicate-decl-specifier -Ww,-Werror=pointer-arith -g  -network=smp  -o upc_io_test upc_io_test.upc
upcc: error compiling translated C code:
upc_io_test.upc: In function 'doit':
upc_io_test.upc:200:13: warning: implicit declaration of function '___errno'
upc_io_test.upc:200:13: warning: nested extern declaration of '___errno' 
upc_io_test.upc:200:14: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:205:175: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:205:198: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:207:211: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:207:234: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:207:179: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:207:202: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:207:26: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:208:229: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:208:252: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:208:181: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:208:204: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:208:26: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:228: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:251: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:223: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:246: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:34: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:218: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:241: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:351: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:374: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:197: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:220: error: invalid type argument of unary '*' (have 'int')
upc_io_test.upc:232:26: error: invalid type argument of unary '*' (have 'int')
[... and LOTS more...]

The problem stems from the following in /usr/include/errno.h:

#if defined(_REENTRANT) || defined(_TS_ERRNO) || _POSIX_C_SOURCE - 0 >= 199506L
extern int *___errno();
#define errno (*(___errno()))
#else
extern int errno;
/* ANSI C++ requires that errno be a macro */
#if __cplusplus >= 199711L
#define errno errno
#endif
#endif  /* defined(_REENTRANT) || defined(_TS_ERRNO) */

It appears that clang (and thus clang-upc2c) defines _REENTRANT on Solaris, leading to incompatible differences between the first and second pre-process with respect tothe handling of errno.

This is odd because clang does not appear to unconditionally pre-define _REENTRANT on Linux, Darwin, FreeBSD, NetBSD or OpenBSD. Additionally, neither the vendor compiler nor gcc on Solaris are predefining it. So, this might simply by a side-effect of something that is already a "bug".

The text was updated successfully, but these errors were encountered:

PHHargrove · 2015-03-02T23:08:31Z

This issue is (of course) not present with "-pthreads" since then _REENTRANT is defined consistently.

Looking deeper, the problem is only with "-network=smp" or "udp", but not "ibv".
This is true only because ibv-conduit is adding -D_REENTRANT to the backend compiler flags.
Similarly passing -Wc,-D_REENTRANT works for any network.

While sticking with -network=smp, compiling with -U_REENTRANT or -Wp,-U_REENTRANT do not work. Since passing -U_REENTRANT to the pre-process step (using -Wp,) did not work, I am uncertain as to how we might fix this issue properly. Clang-upc2c appears to be defining this despite a command-line argument to UNdefine it.

For the record, compiling the following confirms that it is only _REENTRANT that is leading to the errno definition:

#if defined(_TS_ERRNO) || _POSIX_C_SOURCE - 0 >= 199506L
  #error Not the fault of _REENTRANT
#endif

swatanabe · 2015-03-03T17:05:07Z

AMDG

On 03/02/2015 03:42 PM, Paul H. Hargrove wrote:

With clang-upc on Solaris/x86 and gcc or _clang_ as the back-end compiler,

It appears that _clang_ (and thus clang-upc2c) defines _REENTRANT on Solaris, leading to incompatible differences between the first and second pre-process with respect tothe handling of errno.

This really doesn't make sense to me. Does clang really
#define _REENTRANT or is it only clang-upc2c? If clang
itself #defines _REENTRANT, then I would expect the
test to pass with clang as the backend.

In Christ,
Steven Watanabe

PHHargrove · 2015-03-03T18:22:06Z

Steven,

First, I need to make a correction:
The test is OK with clang as the backend; I should have said that the failure was with gcc or _cc_ (Sun/Oracle compiler). The tests is fine with clang as the backend. Sorry for the confusion I caused.

Does clang really #define _REENTRANT or is it only clang-upc2c?

it is clang:

$ /shared/llvm-upc-32/bin/clang -dM -E -x c /dev/null | grep _REENTRANT
#define _REENTRANT 1

nenadv · 2015-03-04T22:10:19Z

Looking at the code, I see lots of:

    if (Opts.POSIXThreads)
      Builder.defineMacro("_REENTRANT");

All of them are like this, except for Solaris:

// Solaris target
template<typename Target>
class SolarisTargetInfo : public OSTargetInfo<Target> {
protected:
[...]
    Builder.defineMacro("__EXTENSIONS__");
    Builder.defineMacro("_REENTRANT");

nenadv · 2015-03-04T22:23:04Z

More on this here http://newsgroups.derkeiler.com/Archive/Comp/comp.programming.threads/2006-09/msg00018.html

And this post for gcc suggests that _REENTRANT is needed for Solaris thread?
https://gcc.gnu.org/ml/gcc/2001-06/msg01589.html

Note that GCC has this for Solaris:

https://gcc.gnu.org/ml/gcc/2001-06/msg01589.html

nenadv · 2015-03-04T23:26:26Z

And GCC also condition _REENTRANT on -pthreads

#define CPP_SUBTARGET_SPEC "\
%{pthreads|pthread:-D_REENTRANT -D_PTHREADS}"

swatanabe · 2015-03-06T04:33:29Z

AMDG

On 03/03/2015 11:22 AM, Paul H. Hargrove wrote:

Steven,

First, I need to make a correction:
The test is OK with clang as the backend; I should have said that the failure was with gcc or _cc_ (Sun/Oracle compiler). The tests is fine with clang as the backend. Sorry for the confusion I caused.

Thank you. That makes a lot more sense.
It seems that Nenad has diagnosed the problem.
I suppose we should take this to cfe-dev to
find out if there's a good reason for them
defining _REENTRANT unconditionally.

In Christ,
Steven watanabe

nenadv · 2015-03-06T19:00:19Z

Paul, can you try the following patch on Solaris. This is the Clang repository.

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp
index 075f905..2571d4f 100644
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -561,7 +561,8 @@ protected:
     Builder.defineMacro("_LARGEFILE_SOURCE");
     Builder.defineMacro("_LARGEFILE64_SOURCE");
     Builder.defineMacro("__EXTENSIONS__");
-    Builder.defineMacro("_REENTRANT");
+    if (Opts.POSIXThreads)
+      Builder.defineMacro("_REENTRANT");
   }
 public:
   SolarisTargetInfo(const llvm::Triple &Triple) : OSTargetInfo<Target>(Triple) {

PHHargrove · 2015-03-06T20:12:06Z

Paul, can you try the following patch on Solaris. This is the Clang repository.

Sure, it will take about 1.5hr to build.
I will probably report after 2pm.

FYI: this is on pcp-j-19 and pcp-j-20 located "behind" n2001.
You (Nenad) have accounts on both.

-Paul

PHHargrove · 2015-03-06T22:22:39Z

Nenad,

The patch did as it was intended, and now _REENTRANT is no longer defined unconditionally.

However, I find the original failure is still present.
It appears that #include <upc_io.> results in defining _REENTRANT.
I am investigating and assume this is the "fault" of UPCR.

-Paul

PHHargrove · 2015-03-06T22:48:38Z

OK, this is not as simple as expected.
There is no longer a defn of _REENTRANT, and upc_io.h is only indirectly responsible.
There is something in (or below) stdlib.h that is defining _POSIX_C_SOURCE to a value that leads to the thread-safe defn of errno and this is happening only on the first pre-process (w/ clang) but not on the second pre-process (with gcc or cc).

$ cat -n test.c
     1  #if defined _REENTRANT
     2    #warning _REENTRANT is defined before stdlib.h
     3  #endif
     4  #if defined _TS_ERRNO
     5    #warning _TS_ERRNOR is defined before stdlib.h
     6  #endif
     7  #if _POSIX_C_SOURCE - 0 >= 199506L
     8    #warning _POSIX_C_SOURCE >= 199506L before stdlib.h
     9  #endif
    10  #include <stdlib.h>
    11  #if defined _REENTRANT
    12    #warning _REENTRANT is defined after stdlib.h
    13  #endif
    14  #if defined _TS_ERRNO
    15    #warning _TS_ERRNOR is defined after stdlib.h
    16  #endif
    17  #if _POSIX_C_SOURCE - 0 >= 199506L
    18    #warning _POSIX_C_SOURCE >= 199506L after stdlib.h
    19  #endif
    20  
    21  #include <errno.h>
    22  #ifdef errno
    23  #warning Errno is now a macro
    24  #endif

$ clang -c test.c
test.c:18:4: warning: _POSIX_C_SOURCE >= 199506L after stdlib.h [-W#warnings]
  #warning _POSIX_C_SOURCE >= 199506L after stdlib.h
   ^
test.c:23:2: warning: Errno is now a macro [-W#warnings]
#warning Errno is now a macro
 ^
2 warnings generated.

$ clang -Wno-#warnings -dM -E test.c | grep _POSIX_C_SOURCE
#define _POSIX_C_SOURCE 200112L

The /usr/include/sys/feature_tests.h header appears to be the most likely spot for the definition - I am looking now.

PHHargrove · 2015-03-06T23:21:30Z

OK, the difference is _XOPEN_SOURCE defined by clang, but not by gcc or cc. The following comment in clang (a few lines before Nenad's patch for _REENTRANT) explains the motivation for the definition, though I am inclined to disagree with what is says since gcc -std=c99 and cc -xc99=all do NOT define _XOPEN_SOURCE at all (see test at end of this comment).

    // Solaris headers require _XOPEN_SOURCE to be set to 600 for C99 and
    // newer, but to 500 for everything else.  feature_test.h has a check to
    // ensure that you are not using C99 with an old version of X/Open or C89
    // with a new version.
    if (Opts.C99 || Opts.C11)
      Builder.defineMacro("_XOPEN_SOURCE", "600");
    else
      Builder.defineMacro("_XOPEN_SOURCE", "500");

The defn _XOPEN_SOURCE==600 leads to /usr/include/sys/feature_test.h defining _POSIX_C_SOURCE to 200112L, which in turn leads to the definition of the thread-safe errno.
However, since gcc and cc do not define _XOPEN_SOURCE, the back-end pre-process does not follow the chain of definitions that leads to the thread-safe errno.

So, passing -U_XOPEN_SOURCE (to clang or clang-upc2c) is sufficient to work-around this problem.
Of course so is -D_XOPEN_SOURCE=600 for the same reason -D_REENTRANT works - either one forces the backend compiler down the same path in errno.h as taken by the clang front end.

This interesting chain of defines also reveals why -U_REENTRANT was not sufficient in previous testing. One must remove both definitions for the first and second pre-process environments to be sufficiently similar.

Since we don't know if it is safe to modify SolarisTargetInfo::getOSDefines(), I don't see what is "the right path" at this point. If we learn that upstream will not change/remove these definitions then we are stuck with the possibility that we must add logic to undef _REENTRANT and _XOPEN_SOURCE. However, if we start down that path, then we end up with a subset UPCR's gcc_as_cc.pl script which finds all the -D, -U and -isystem flags needed to make gcc (or clang in this case) act exactly like the back-end preprocessor. However, that runs the risk of activating compiler-specific extensions (pragmas, non-Gnu asm, etc.) that would choke clang's parser.

Here is my test for definitions of _XOPEN_SOURCE:

$ cat -n test2.c
     1  #if _XOPEN_SOURCE - 600 == 0
     2    #warning _XOPEN_SOURCE 600
     3  #elif _XOPEN_SOURCE - 500 == 0
     4    #warning _XOPEN_SOURCE 500
     5  #elif defined _XOPEN_SOURCE
     6    #warning _XOPEN_SOURCE unknown
     7  #else
     8    #warning _XOPEN_SOURCE not defined
     9  #endif

$ clang -c test2.c 
test2.c:2:4: warning: _XOPEN_SOURCE 600 [-W#warnings]
  #warning _XOPEN_SOURCE 600
   ^
1 warning generated.

$ gcc -c -std=c99 test2.c
test2.c:8:4: warning: #warning _XOPEN_SOURCE not defined

$ cc -c -xc99=all test2.c
"test2.c", line 8: #warning: _XOPEN_SOURCE not defined
"test2.c", line 9: warning: empty translation unit

nenadv · 2015-03-09T18:28:19Z

I checked GCC and this seems to be defined only for C++

/* Names to predefine in the preprocessor for this target machine.  */
#define TARGET_SUB_OS_CPP_BUILTINS()
#define TARGET_OS_CPP_BUILTINS()                        \
    do {                                                \
        builtin_define_std ("unix");                    \
        builtin_define_std ("sun");                     \
        builtin_define ("__svr4__");                    \
        builtin_define ("__SVR4");                      \
        builtin_assert ("system=unix");                 \
        builtin_assert ("system=svr4");                 \
        /* For C++ we need to add some additional macro \
           definitions required by the C++ standard     \
           library.  */                                 \
        if (c_dialect_cxx ())                           \
          {                                             \
            builtin_define ("__STDC_VERSION__=199901L");\
            builtin_define ("_XOPEN_SOURCE=600");       \
            builtin_define ("_LARGEFILE_SOURCE=1");     \
            builtin_define ("_LARGEFILE64_SOURCE=1");   \
            builtin_define ("__EXTENSIONS__");          \
          }                                             \
        TARGET_SUB_OS_CPP_BUILTINS();                   \
    } while (0)

I think we should follow gcc in defining macros in Clang-UPC.

Paul, if you can do additional patch:

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp
index 075f905..3ff0833 100644
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -548,20 +548,22 @@ protected:
     Builder.defineMacro("__ELF__");
     Builder.defineMacro("__svr4__");
     Builder.defineMacro("__SVR4");
-    // Solaris headers require _XOPEN_SOURCE to be set to 600 for C99 and
-    // newer, but to 500 for everything else.  feature_test.h has a check to
-    // ensure that you are not using C99 with an old version of X/Open or C89
-    // with a new version.  
-    if (Opts.C99 || Opts.C11)
-      Builder.defineMacro("_XOPEN_SOURCE", "600");
-    else
-      Builder.defineMacro("_XOPEN_SOURCE", "500");
-    if (Opts.CPlusPlus)
+    if (Opts.CPlusPlus) {
+      // Solaris headers require _XOPEN_SOURCE to be set to 600 for C99 and
+      // newer, but to 500 for everything else.  feature_test.h has a check to
+      // ensure that you are not using C99 with an old version of X/Open or C89
+      // with a new version.  
+      if (Opts.C99 || Opts.C11)
+        Builder.defineMacro("_XOPEN_SOURCE", "600");
+      else
+        Builder.defineMacro("_XOPEN_SOURCE", "500");
       Builder.defineMacro("__C99FEATURES__");
+    }
     Builder.defineMacro("_LARGEFILE_SOURCE");
     Builder.defineMacro("_LARGEFILE64_SOURCE");
     Builder.defineMacro("__EXTENSIONS__");
-    Builder.defineMacro("_REENTRANT");
+    if (Opts.POSIXThreads)
+      Builder.defineMacro("_REENTRANT");
   }

Paul, if you can please update the master, otherwise I'll do it, unless there are objections.

PHHargrove · 2015-03-09T19:09:24Z

Nenad,

Your patch is on my TODO list.
However, our CVS server died on Sunday AM and resurecting it is a top priority at the moment,

-Paul

PHHargrove · 2015-03-09T19:52:10Z

Nenad,

For the record the gcc code you quoted also shows the following three as c++-only:

            builtin_define ("_LARGEFILE_SOURCE=1");     \
            builtin_define ("_LARGEFILE64_SOURCE=1");   \
            builtin_define ("__EXTENSIONS__");          \

PHHargrove · 2015-03-10T07:47:02Z

Nenad,

The patch to only define _XOPEN_SOURCE for C++ appears to have worked to resolve the problem.
Since I am not working from a git checkout on the Solaris system, I've not committed to master.

I would suggest you also conditionalize the three defines I mentioned in the previous comment to make clang match gcc as closely as possible (even for tokens that haven't yet caused problems).

-Paul

PHHargrove self-assigned this Mar 2, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problems w/ ___errno on Solaris #112

Problems w/ ___errno on Solaris #112

PHHargrove commented Mar 2, 2015

PHHargrove commented Mar 2, 2015

swatanabe commented Mar 3, 2015

PHHargrove commented Mar 3, 2015

nenadv commented Mar 4, 2015

nenadv commented Mar 4, 2015

nenadv commented Mar 4, 2015

swatanabe commented Mar 6, 2015

nenadv commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

nenadv commented Mar 9, 2015

PHHargrove commented Mar 9, 2015

PHHargrove commented Mar 9, 2015

PHHargrove commented Mar 10, 2015

Problems w/ ___errno on Solaris #112

Problems w/ ___errno on Solaris #112

Comments

PHHargrove commented Mar 2, 2015

PHHargrove commented Mar 2, 2015

swatanabe commented Mar 3, 2015

PHHargrove commented Mar 3, 2015

nenadv commented Mar 4, 2015

nenadv commented Mar 4, 2015

nenadv commented Mar 4, 2015

swatanabe commented Mar 6, 2015

nenadv commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

PHHargrove commented Mar 6, 2015

nenadv commented Mar 9, 2015

PHHargrove commented Mar 9, 2015

PHHargrove commented Mar 9, 2015

PHHargrove commented Mar 10, 2015