Library linkage failures with gcc when user compiles with a different -std=c++ level

Issue #181 resolved
Dan Bonachea created an issue

I've discovered that even very simple UPC++ programs will often fail to link if UPC++ is installed using the default -std=c++11 (for gcc) and then the application compilation raises this to -std=c++17.

Below is a demonstration on cori using example/prog-guide/compute-pi.cpp and PrgEnv-gnu/6.0.4. I've seen the same gcc failure on dirac and for many other example programs. I have not yet seen the problem for clang.

This is potentially a big problem because our current install strategy is to install for the lowest common denominator language compliance setting, and allow applications to individually raise the compliance level as needed to match the app's C++ requirements.

{cori-hsw ~/UPC/upcxx/example/prog-guide} module load upcxx/nightly
## Loaded 'PrgEnv-gnu-6.0.4-7.3.0-2018.11.26' based on currently loaded modules.
## The selected build is for target CPU "haswell", and thus if
## you change craype-{haswell,mic-knl} modules then you should
## 'module unload upcxx' and reload to get the correct upcxx module.
##
## WARNING: nightly snapshots typically live for only 48 hours and
## are not intended for general use due to potential instability.

{cori-hsw ~/UPC/upcxx/example/prog-guide} module list
Currently Loaded Modulefiles:
  1) modules/3.2.10.6                                 10) git/2.15.1                                       19) job/2.2.3-6.0.7.0_44.1__g6c4e934.ari
  2) nsg/1.2.0                                        11) gcc/7.3.0                                        20) dvs/2.7_2.2.113-6.0.7.1_7.1__g1bbc03e
  3) craype-haswell                                   12) cray-libsci/18.03.1                              21) alps/6.6.43-6.0.7.0_26.4__ga796da3.ari
  4) craype-network-aries                             13) udreg/2.3.2-6.0.7.0_33.18__g5196236.ari          22) rca/2.2.18-6.0.7.0_33.3__g2aa4f39.ari
  5) craype/2.5.14                                    14) ugni/6.0.14.0-6.0.7.0_23.1__gea11d3d.ari         23) atp/2.1.1
  6) cray-mpich/7.7.0                                 15) pmi/5.0.13                                       24) PrgEnv-gnu/6.0.4
  7) altd/2.0                                         16) dmapp/7.1.1-6.0.7.0_34.3__g5a674e0.ari           25) upcxx/nightly
  8) darshan/3.1.4                                    17) gni-headers/5.0.12.0-6.0.7.0_24.1__g3b1768f.ari
  9) bupc/2.26.0                                      18) xpmem/2.2.15-6.0.7.1_5.8__g7549d06.ari

{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx --version
UPC++ version 20180901 upcxx-2018.3.3-159-g57a9814
Copyright (c) 2018, The Regents of the University of California,
through Lawrence Berkeley National Laboratory.
http://upcxx.lbl.gov

g++ (GCC) 7.3.0 20180125 (Cray Inc.)
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx -g -std=c++11 compute-pi.cpp 
{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx -g -std=c++14 compute-pi.cpp 
{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx -g -std=c++17 compute-pi.cpp 
/usr/common/ftg/upcxx/nightly/hsw/gnu/PrgEnv-gnu-6.0.4-7.3.0-2018.11.26/upcxx.debug.gasnet_seq.aries/lib/libupcxx.a(d7828db60e09cbf1149e4d9fe53335ad17c53a32.core.cpp.o):(.rodata+0x10): multiple definition of `upcxx::detail::future_header_nil::the_nil'
/tmp/ccROZIis.o:(.rodata._ZN5upcxx6detail17future_header_nil7the_nilE[_ZN5upcxx6detail17future_header_nil7the_nilE]+0x0): first defined here
/usr/bin/ld: link errors found, deleting executable `a.out'
collect2: error: ld returned 1 exit status

Comments (8)

  1. Dan Bonachea reporter

    The problem appears to be that since C++17, static constexpr implies inline, which then collides with the explicit definition in src/future/core.cpp.

    However simply adding an explicit inline keyword to core.hpp breaks library build (with g++ 7.3.0 in -std=c++11 mode), because that feature was not added until C++17. Raising the library build level to -std=c++17 allows it to accept the inline keyword here, but that breaks our desire to build the library with the C++11 "least common denominator".

    Deleting the definition in core.cpp fixes the problem for {library/c++11 and app/c++17}, where this variable is implicitly inline, but leads to an undefined symbol linker error for {library/c++11 and app/c++11}.

    Some relevant discussion

    C++17 [depr.static_constexpr] adds some deprecated usage that appears to be intended to help handle this problem, but either I can't figure out how to apply it here or maybe it only applies when all TU use the same language level.

    @jdbachan really needs to look at this to determine if we can continue to deploy one set of library/headers that will work for any higher language level.

  2. Benjamin Driscoll

    Would it be possible to check #if __cplusplus < 201703L in src/future/core.cpp and redefine only if that is the case?

  3. john bachan

    With src/future/core.cpp compiled under c++11, that TU emits a linker symbol for the_nil that is marked as the singular definition.

    With app.cpp compiled under c++17, as it includes our future/core.hpp header, it will interpret the constexpr as an inline variable definition and emit a possibly redundant linker symbol definition. The linker knows to collapse all such redundant definitions found in TU's at link time.

    The problem is that the linker is refusing to collapse redundant definitions with the singular definition from the c++11 TU.

    The best fix, which preserves constexpr and thus the optimization time knowledge it grants (which affects reference counting logic against a provably nil future), is to define the_nil such that all language levels will emit redundant symbols. The trick is to bury it under a dummy template variable which is always instantiated to the same value.

    // old way, language disagree on how to emit symbols
    struct foo { constexpr int bar = 1; };
    // usage
    foo::bar
    
    // new way, always a redundant symbol
    template<typename>
    struct foo { constexpr int bar = 1; };
    //usage
    foo<void>::bar
    

    Fix forthcoming.

  4. john bachan

    Fixes issue 181 to allow building UPC++ as C++11 yet be successfully linked against TUs that have interpreted our headers as C++17.

    nobs will now sniff out -std=... flags in the CC,CXX variables and won't append its own desired language level if present.

    → <<cset 29e5396482a4>>

  5. Dan Bonachea reporter

    Thanks @john bachan : I've confirmed the change fixes the originally reported problem on gcc-9.2.0/Linux and will conduct some additional testing.

  6. Log in to comment