public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: Arthur Norman <acn1@cam.ac.uk>
To: Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>
Cc: cygwin@cygwin.com
Subject: Re: linker fails with multiple definitions after inline thread_local var within class
Date: Sat, 16 Nov 2019 03:35:00 -0000	[thread overview]
Message-ID: <alpine.WNT.2.00.1911151945330.112204@panamint> (raw)
In-Reply-To: <769583d7-b1d7-fbc3-15ec-d377163c9d7f@SystematicSw.ab.ca>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6259 bytes --]

> I notice that you are not independently compiling your source files and have no
> include guard in t.h.
> Could I suggest that you add the include guard to t.h and retest.
> If you still have an issue, try independently compiling your source files, then
> linking them as a separate step, to see if that works.
> You could also test reproducing the issue on another gcc platform, under a Unix
> VM, or a WSL Linux distro.

I attach a versiuon of the test with an include guard and with t1.cpp and 
t2.cpp compiled in separate invocations of g++ - I still see the multiple 
definition.

${1:-g++} -std=c++17 -I. -c t1.cpp
${1:-g++} -std=c++17 -I. -c t2.cpp
${1:-g++} -std=c++17 t1.o t2.o -o t
/usr/lib/gcc/x86_64-pc-cygwin/8.3.0/../../../../x86_64-pc-cygwin/bin/ld: 
t2.o:t2.cpp:(.text+0x86): multiple definition of `TLS init function for 
Data::valref'; t1.o:t1.cpp:(.text+0x4a): first defined here
collect2: error: ld returned 1 exit status
./t

As per my original posting before enquiring here I had tried on a 64-bit 
ubuntu machine, on a Macbook (where it is clang not g++, but I invoke the 
compiler as g++) and on a Raspberry pi.

The original code I had pain with was with include guards and all files 
compiled independently via make - the test casee I submitted had perhaps 
tried to hard to shorten and simplify.

I also tried both 32 and 64-bit mingw compilers under cygwin - making that 
easier is why the test script goes ${1:-g++} so I can override the 
compiler selection to be x86_64-w64-mingw32-g++ or the i686 variant. Those 
both show the multiple definition problem.


I had also spent some time trying to check in a file 
c++17-draft-standard.pdf and doing web research on rules regarding static 
thread_local fields in classes. There is plenty to alert me to the fact 
that I need to be vary sensitive to issues about exactly when initializers 
get invoked and in which order, and I spent some time trying to convince 
myself I had my mind clear on that - but anyway even if I was wrong on 
that count it would not cause multiple-definitions and linker failure. But 
I have tried reading up and thinking about initialization order too, but 
that is not the issue in this query!

I have also experimented with g++ options such as -ftls-model=... and 
-mtls-dialect and not observed any altering the gross behavior.

So at present the cygwin g++, i686-w32-mingw32-g++ and 
x86_64-w64-mingw32-g++ are the cases where this linker clash arises for me 
but no others, and I have failed to achieve a web-search explaining this 
as a valid limitation. But I am painfully aware that the C++ standard is 
complicated enough that I could have missed spottting a paragraph and that 
my web-searches may not have found the correct keywords to use!

> You would have to check the C++17 standard, gcc docs, and/or mailing list, or
> bugzilla, to see if using that feature in that manner is supported, or if there
> are issues, limitations, or restrictions on doing so.

My practical response to this clearly has to be to work around it - but it 
still seems proper to alert cygwin people to a potential issue on their 
platform! The relevant code in my real example already conditionalizes on 
whether the C++ compiletr being used does or does not claim to support 
inline variables and whether or not it is liable to be on top of Windows - 
I want it to build and run happily for others more or less regardless - 
but I am also fairly fussed about performance (which is a reason for C++ 
not Java, Python etc etc!).


> It may be more useful to ask about such language/build interaction issues first
> on forums such as StackOverflow and/or language mailing lists, before posting
> here, as Cygwin and gcc are implemented in C++, so most issues are likely to
> have been noticed and fixed, but language/build issues are not often discussed,
> as all support is from volunteers, and language issues are off topic.

Thank you - that is useful and I will try again directly with gcc-central. 
It looks as if even if this issue mainly hits on Cygwin that there are at 
least not a lot of others who have experienced it and felt like jumping 
in.

> There may be little response here unless someone has encountered the same issue
> using the same features.
>
> General points:
>
> Support by gcc of standards often lags; library feature support is dependent on
> libstdc++, newlib, and Cygwin winsup; and the Cygwin gcc release is a couple of
> versions behind the head/tip:
>
Accepted. But __cpp_inline_variables being defined is advertising support 
for same! And the gcc page you cite below declares it ok from gcc 7 
onwards, and indeed I had looked at the first one!

> https://gcc.gnu.org/projects/cxx-status.html#cxx17
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017
>


> Specifying the language standard -std=c++17 requires your sources to be strictly
> conforming, as it disables a lot of the GNU, POSIX, and Cygwin features and
> support that extend and relax the standards, sometimes with GNU rather than
> standard semantics, with issues, limitations, or restrictions, or may not yet be
> available.

-std=c++17 and -std=gnu++17 does not change things for me. Indeed my 
original project was using gnu++17 but to report it I dropped back to 
-std=c++11 and dropped the -Wall (which does not moan about anything).

> Features may not work, and may not issue a diagnostic, if that is not required,
> or just not yet implemented, perhaps dependent on support in binutils or other
> parts of the tool chain.
>
Agreed and accepted quite happily. And it is pretty clear that 
implementing thread-local on less that helpful platforms involved a deal 
of pain - the Windows native scheme used in a naive way may not help with 
destructors and linker support for you is "interestingly" different from 
that for Linux etc.

I think I hope for two things - the first is to see if my code violated 
some part of the C++ specification that I have not found and hence the 
tool-chain is entitled to reject it. I tried doing my homework first but 
the C++ specification is Big and Complicated! The second is that if this 
is an issue that it has been reported so that it can get addressed in due 
course.

Thank you!   Arthur

[-- Attachment #2: Type: APPLICATION/octet-stream, Size: 3008 bytes --]

#! /bin/bash
# When I run this on Ubuntu-x86_64, Macintosh(clang) or a Raspberry pi
# the code links and when it runs it just prints 999 - which is the behaviour
# that I expect. On both Cygwin and using x86_64-w64-mingw32-g++  (and i686)
# I get a linker diagnostic of the form
# ./tltest.sh
# /usr/lib/gcc/x86_64-pc-cygwin/8.3.0/../../../../x86_64-pc-cygwin/bin/ld:
#   /tmp/cczVTcZ1.o:t2.cpp:(.text+0x86): multiple definition of
#   `TLS init function for Data::valref';
#   /tmp/ccyLQlRb.o:t1.cpp:(.text+0x4a): first defined here
# collect2: error: ld returned 1 exit status
#
# I believe that given the specification of C++17 "inline" variables this
# is incorrect, but there are better experts who may be able to explain
# otherwise.
#
# When people raise issues here I often see other asking "Why are you doing
# that?". This is a cut-down version of my real code where rather than
# storing a reference to the thread_local Record that I am interested in
# in a simple array (tvec) at a fixed offset (1) I store and retrieve a
# referece to it using TlsAlloc(), TlsGetValue and TlsSetValue - those being
# the Microsoft API for thread-local access, and for my purposes my
# measurements suggest that using them gives me useful performance gains
# over the emutls code that g++ creates on the relevant platforms.
# And I am then (trying to) build a header-only library (for which t.h is
# the surrogate here) which can be included from several other compilation
# units but by virtue of "inline variables" its private (including thread-
# local) date can be defined within that header file  so that the files that
# #include the header-only library do not need to contain anything beyond
# uses of it. To illustrate this I have two source files which each include
# t.h but the bad behaviour does not need any other code in the first one!
# The second just contains a tiny main program that inspects data from the
# library data.

# Have I misunderstood C++ and so am I doing something wrong or is this
# a g++/Windows bug?

cat <<XXX > t.h
#ifndef t_included
#define t_included 1

#include <cstdint>

inline void *tvec[4];

class Record
{
public:
    int val = 999;
};

class Data_Ref
{   static inline thread_local Record val;
public:
    static Record* get()  // Get reference to Record via tvec[]
    {   return (Record*)tvec[1];
    }
    Data_Ref()            // Stash it in tvec[] for later use
    {   tvec[1] = (void *)&val;
    }
};

class Data
{   static inline thread_local Data_Ref valref;
public:
    static Record &get()
    {   return *valref.get(); // note that get() is static in Data_Ref
    }
};
#endif // t_included
XXX
cat <<XXX > t1.cpp
#include <iostream>
#include "t.h"  // First copy

XXX
cat <<XXX > t2.cpp
#include <iostream>
#include "t.h"  // Second copy

int main()
{   std::cout << Data::get().val << std::endl;
    return 0;
}
XXX

${1:-g++} -std=gnu++17 -I. -c t1.cpp
${1:-g++} -std=gnu++17 -I. -c t2.cpp
${1:-g++} -std=gnu++17 t1.o t2.o -o t
./t

# end of test script

[-- Attachment #3: Type: text/plain, Size: 219 bytes --]


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

  reply	other threads:[~2019-11-15 22:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 18:41 Arthur Norman
2019-11-14 23:45 ` Brian Inglis
2019-11-15  8:14   ` Arthur Norman
2019-11-15 22:07     ` Brian Inglis
2019-11-16  3:35       ` Arthur Norman [this message]
2019-11-16  3:54         ` Brian Inglis
2019-11-16 10:48           ` Brian Inglis
2019-11-17 21:05             ` Arthur Norman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.WNT.2.00.1911151945330.112204@panamint \
    --to=acn1@cam.ac.uk \
    --cc=Brian.Inglis@SystematicSw.ab.ca \
    --cc=cygwin@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).