Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link failures (pthreads) on OpenBSD #78

Closed
PHHargrove opened this issue Jun 8, 2015 · 7 comments
Closed

Link failures (pthreads) on OpenBSD #78

PHHargrove opened this issue Jun 8, 2015 · 7 comments

Comments

@PHHargrove
Copy link
Collaborator

All clang-upc compilations on OpenBSD are now failing with:

/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0xc): In function `__upc_omp_check':
: undefined reference to `pthread_self'
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x1df6): In function `main':
: undefined reference to `pthread_self'
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x1ed7): In function `main':
: undefined reference to `pthread_self'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
gmake: *** [RandomAccess_UPC] Error 1

This clang-upc build was configured via autoconf, in case that matters.

-Paul

@PHHargrove
Copy link
Collaborator Author

I should have mentioned that the problem appeared between test runs on May 26 (with no problem) and May 31 (all tests failed to link). I didn't have any OpenBSD+clang-upc tests between those two dates.

@nenadv
Copy link

nenadv commented Jun 8, 2015

It seems that -lpthread is missing on the linker line. I am not sure why linux does not require it.

I also noticed the following linker warning on openbsd:

/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x181): In function `__upc_fatal':
: warning: vsprintf() is often misused, please use vsnprintf()
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x18a6): In function `main':
: warning: strcpy() is almost always misused, please use strlcpy()
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0xc1): In function `__upc_fatal':
: warning: sprintf() is often misused, please use snprintf()
-bash-4.3$ /home/phargrov/upcnightly/llvm-upc/bin/clang-upc -o t t.upc -lpthread   
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x181): In function `__upc_fatal':
: warning: vsprintf() is often misused, please use vsnprintf()
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0x18a6): In function `main':
: warning: strcpy() is almost always misused, please use strlcpy()
/home/phargrov/upcnightly/llvm-upc/bin/../lib/libupc.a(upc_main.o)(.text+0xc1): In function `__upc_fatal':
: warning: sprintf() is often misused, please use snprintf()

@PHHargrove
Copy link
Collaborator Author

Nenad,

OpenBSD says to use "-pthread" in CFLAGS and LDFLAGS, and so do FreeBSD and NetBSD.
You are not expected to pass "-D_REENTRANT" to CC nor "-lpthread" to LD.
Brief testing shows clang accepts "-pthread" on those three BSD platforms AND on Linux and Solaris.
So, I think that is the option you should be using.

OpenBSD in particular does NOT recommend "-lpthreads" because their system has gone through multiple (at least 2, IIRC) distinct implementations of POSIX threads and libpthread.{a,so} might not be the right one.

The linker warnings are a "feature" on security-centric OpenBSD.

The harness normally filters these away. They only show in the nightly test output because in the event an error or non-filtered warning appears we emit the unfiltered stdout and stderr.

The warnings are generated once per symbol which means if you fix the warnings above you will just change the warning to reference the next instance of each of those functions. Starting down that path is a looong journey, complicated by the fact that the recommended "strlcpy()" family of replacements for "strncpy()" and related functions are not present on most platforms.

-Paul

PHHargrove added a commit to PHHargrove/clang-upc that referenced this issue Jun 8, 2015
PHHargrove added a commit to PHHargrove/clang-upc that referenced this issue Jun 8, 2015
@nenadv
Copy link

nenadv commented Jun 9, 2015

-pthread on the command line should be ok. But ultimately this will end
up with -lpthread on the 'ld' line. Right? We already selectively add
pthreads for the portals runtime and we also need it if OMP checks are
enabled.

On 6/8/15 2:17 PM, Paul H. Hargrove wrote:

Nenad,

OpenBSD says to use "-pthread" in CFLAGS and LDFLAGS, and so do FreeBSD
and NetBSD.
You are not expected to pass "-D_REENTRANT" to CC nor "-lpthread" to LD.
Brief testing shows clang accepts "-pthread" on the platforms, AND on
Linux and Solaris.
So, I think that is the option you should be using.

OpenBSD in particular does NOT recommend "-lpthreads" because their
system has gone through multiple (at least 2, IIRC) distinct
implementations of POSIX threads and libpthread.{a,so} might not be the
right one.

The linker warnings are a "feature" on security-centric OpenBSD.

The harness normally filters these away. They only show in the nightly
test output because in the event an error or non-filtered warning
appears we emit the unfiltered stdout and stderr.

The warnings are generated once per symbol which means if you fix the
warnings above you will just change the warning to reference the next
instance of each of those functions. Starting down that path is a looong
journey, complicated by the fact that the recommended "strlcpy()" family
of replacements for "strncpy()" and related functions are not present on
most platforms.

-Paul


Reply to this email directly or view it on GitHub
#78 (comment).

@PHHargrove
Copy link
Collaborator Author

Nenad asks:

-pthread on the command line should be ok. But ultimately this will end up with -lpthread on the 'ld' line. Right?

That is correct today.
In the past -pthread could have linked librthread ("r" in place of "p"), and in the future it may link something else.

-Paul

PHHargrove added a commit to PHHargrove/clang-upc that referenced this issue Jun 9, 2015
This commit corrects the reported alignment for the struct PTS on
a target with 32-bit pointers.  It should be the same as a 32-bit
integer, not a 64-bit one as was the case previously.

This did not show on x86_64, where the ILP32 ABI (x86) requires only
4-byte alignment for both 32- and 64-bit integers.  However on PPC64
both the LP64 and ILP32 ABIs specify 8-byte alignment of 8-byte
integers, leading to a mismatch between clang-upc and the C code in
UPCR.
@PHHargrove
Copy link
Collaborator Author

Just for completeness, here is the relevant goop from lib/Basic/Targets.cpp and lib/Driver/Tools.cpp.
They translate -pthread into -D_REENTRANT and -lpthreads (or -lpthread_p), respectively.

class OpenBSDTargetInfo : public OSTargetInfo<Target> {
protected:
  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
                    MacroBuilder &Builder) const override {
    // OpenBSD defines; list based off of gcc output

    Builder.defineMacro("__OpenBSD__");
    DefineStd(Builder, "unix", Opts);
    Builder.defineMacro("__ELF__");
    if (Opts.POSIXThreads)
      Builder.defineMacro("_REENTRANT");
  }
[...]
void openbsd::Link::ConstructJob(
[...]
    if (Args.hasArg(options::OPT_pthread)) {
      if (!Args.hasArg(options::OPT_shared) &&
          Args.hasArg(options::OPT_pg))
         CmdArgs.push_back("-lpthread_p");
      else
         CmdArgs.push_back("-lpthread");
    }
[...]

However, on NetBSD the preprocessor defines _POSIX_THREADS instead of _REENTRANT.
That is the sort of platform-specific thing use of -pthread is meant to hide.

So, I feel that the right behavior would be for the driver to assert the "-pthread" option for (at least) preprocess and link stages when the language is UPC, rather than assuming -D_REENTRANT and -lpthread. Of course I say that without having any idea how one would achieve this.

PHHargrove added a commit to PHHargrove/clang-upc that referenced this issue Jun 9, 2015
This commit corrects the reported alignment for the struct PTS on
a target with 32-bit pointers.  It should be the same as a 32-bit
integer, not a 64-bit one as was the case previously.

This did not show on x86_64, where the ILP32 ABI (x86) requires only
4-byte alignment for both 32- and 64-bit integers.  However on PPC64
both the LP64 and ILP32 ABIs specify 8-byte alignment of 8-byte
integers, leading to a mismatch between clang-upc and the C code in
UPCR.
PHHargrove added a commit that referenced this issue Jun 12, 2015
This commit corrects the reported alignment for the struct PTS on
a target with 32-bit pointers.  It should be the same as a 32-bit
integer, not a 64-bit one as was the case previously.

This did not show on x86_64, where the ILP32 ABI (x86) requires only
4-byte alignment for both 32- and 64-bit integers.  However on PPC64
both the LP64 and ILP32 ABIs specify 8-byte alignment of 8-byte
integers, leading to a mismatch between clang-upc and the C code in
UPCR.
@nenadv
Copy link

nenadv commented Jun 15, 2015

Fixed by cbec6de

@nenadv nenadv closed this as completed Jun 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants