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

Multiple definitions of symbol PC when statically linking readline and termcap into a program #8105

Closed
intvsteve opened this issue Mar 10, 2021 · 23 comments · Fixed by #15012

Comments

@intvsteve
Copy link

This used to work when using the package mingw-w64-i686-readline-7.0.005-1-any.pkg.tar.xz.

Now, when upgrading to the latest (mingw-w64-i686-readline-8.0.004-2-any.pkg.tar.zst), linking both of these libraries results in an error from ld:

D:/msys64/mingw32/lib\libtermcap.a(termcap.o):(.bss+0x8): multiple definition of `PC'; D:/msys64/mingw32/lib\libreadline.a(terminal.o):(.bss+0x5c): first defined here
collect2.exe: error: ld returned 1 exit status
@Biswa96
Copy link
Member

Biswa96 commented Mar 10, 2021

Can you provide a simple code to reproduce your issue?

@revelator
Copy link
Contributor

does it pr chance also link to one of the curses libraries ? PC is defined in them as well.

@intvsteve
Copy link
Author

intvsteve commented Mar 11, 2021

The code I'm building does not appear to link to curses.

The code I'm building is the jzintv emulator. I have access to a slightly more recent version, but this aspect has not changed and the code here will likely reproduce the issue as well.

Some caveats

  • use the 32-bit msys environment as only the working build for Windows is currently the 32-bit version
  • you will need SDL1 and / or SDL2 on the system, depending on which build you invoke (issue is same for both)
  • example build: make -f Makefile.w32_sdl2 in the src directory

I think most of what is needed for the 32-bit build can be installed via:

pacman -S base-devel
pacman -S gcc
pacman -S mingw-w64-i686-gcc
pacman -S mingw-w64-i686-SDL
pacman -S mingw-w64-i686-SDL2
pacman -S mingw-w64-i686-readline

I had to reconstruct my msys2 when pacman stopped working completely. However, I could downgrade to the 32-bit version of the mingw-w64-i686-readline package, as mentioned, and get things to link. I initially filed this issue against the readline package.

UPDATE: I have confirmed the build failure with the above linked sources. Downgrading to my older package (mingw-w64-i686-readline-7.0.005-1-any.pkg.tar.xz) avoids the linker error.

@intvsteve
Copy link
Author

@Biswa96 Unfortunately, the sample code I provided is not "simple". I have not created a "toy" program to reproduce.

@revelator
Copy link
Contributor

revelator commented Mar 11, 2021

hmm could have happened if say one of the libraries you link to include say ncurses as we only have a static version of that it might have exported the PC symbol to said library but it is hard to say.

can you try to build it with CFLAGS+=" -fcommon" CXXFLAGS+=" -fcommon" as i noticed this causes some packages that have previously linked without this error to suddenly throw a lot of multiple defines

EDIT: damn i had a lot to "say" here xD

@intvsteve
Copy link
Author

Sadly, same exact error.

Everything's* statically linked, so it'd have to be something that gloms ncurses on, which I see no clear evidence for.

Here's a representative source compile w/ flags:

gcc -std=gnu11 -o jzintv.o -O3 -fomit-frame-pointer -msse2 -fcommon -ffunction-sections -flto -Wall -W -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-qual -Werror -I. -I.. -DDIRECT_INTV2PC -DUSE_SDL=2 -DJZINTV_VERSION_MAJOR=0x01 -DJZINTV_VERSION_MINOR=0x00 -DJZINTV_SVN_REV=2140 -DJZINTV_SVN_DTY=1 -c jzintv.c

Here's the link (all the .o files excluded for brevity) so you can see what's being fed to link:

g++ -std=c++14 -U__STRICT_ANSI__ -fvisibility=hidden -o ...<object files omitted for brevity> -O3 -fomit-frame-pointer -msse2 -ffunction-sections -flto -Wall -W -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-qual -Werror -I. -I.. -DDIRECT_INTV2PC -DUSE_SDL=2 -DJZINTV_VERSION_MAJOR=0x01 -DJZINTV_VERSION_MINOR=0x00 -DJZINTV_SVN_REV=2140 -DJZINTV_SVN_DTY=0 -ID:/msys64/mingw32/include/SDL2 -Dmain=SDL_main -L../lib -static-libgcc -static-libstdc++ -static -LD:/msys64/mingw32/lib -lmingw32 -lSDL2main -lSDL2 -mconsole -lmingw32 -ldinput8 -lshell32 -lsetupapi -ladvapi32 -luuid -lversion -loleaut32 -lole32 -limm32 -lwinmm -lgdi32 -luser32 -lm -mconsole -Wl,--no-undefined -pipe -Wl,--dynamicbase,--nxcompat,--no-seh -lreadline -ltermcap

*the exception being the SDL library

@revelator
Copy link
Contributor

Hmm at this point it might be prudent to just use LDFLAGS+=" -Wl,--allow-multiple-definition" untill we figure out where this symbol is comming from.

@intvsteve
Copy link
Author

intvsteve commented Mar 12, 2021

Thanks, @revelator . I can chat w/ the code owner, but am more likely to use one of the other two workarounds available to me for now (the older build, or compiling out the usage of readline).

I did some basic snooping and found the termcap.h header has the extern char PC right out there for the world to see. So it would seem that termcap is not pulling any surprises here.

For readline that's not the case. PC does not appear in any of its headers. Based on the error, I'd expect a non-static, non-extern declaration to show up in a terminal.c file in the source for readline.

I'm not familiar w/ how msys2 packages are built, but it seems most likely to be a change in how readline was built from the 8.0 sources vs. the 7.0 sources.

FWIW, I found this repo and downloaded the 8.0 sources and checked terminal.c and found this:

#if !defined (__linux__) && !defined (NCURSES_VERSION)
#  if defined (__EMX__) || defined (NEED_EXTERN_PC)
extern 
#  endif /* __EMX__ || NEED_EXTERN_PC */
char PC, *BC, *UP;
#endif /* !__linux__ && !NCURSES_VERSION */

To me, it would seem it's entirely dependent upon how readline was built. The code for this particular bit seems identical in the 7.0 source from here - again indicating it seems to be a compile-time issue for readline.

In fact, if I were to throw out a wild guess... Did the definition of NEED_EXTERN_PC change?

@revelator
Copy link
Contributor

termcap is kind of a minimal implementation of the curses library's terminal capabilities so that is not so odd :) and by itself it should pose no problem unless the exports are wrong as you also point out readline might be the culprit, the funny thing is that readline can use both curses or termcap. The one from us does not use ncurses but it does use termcap
readline-symbols

@intvsteve
Copy link
Author

Indeed it does! It may well be that for the DLL builds, this isn't an issue, but that in static library builds it is. IIRC it may take a little more effort to export a global from a DLL build, but it's all too easy for static libraries.

@revelator
Copy link
Contributor

one experiment would be to try building readline without either ncurses or termcap :) it is entirely possible and depends only if you need the terminal capabilities in your program. If you dont then it should be an easy fix, though in the long run it would be best to find out why this happens so we dont run headlong into the same problem again.

@revelator
Copy link
Contributor

revelator commented Mar 12, 2021

static builds are nice when they work but as you noticed there are some pitfalls like the one here and also knowing exactly which system libraries need to be linked to (static builds are newer really truely static anyway as they link to several system dll's but the compiler only links to a small number of system libraries by default so sometimes we do get these "not so fun" problems), i work with an offshot of the compilers here using TDM's patches which by default links to the static gcc runtimes so it does give me some perspective.

@revelator
Copy link
Contributor

dll builds load the symbols dynamically from the dll while static builds have the symbols linked into the application at runtime, so if say the symbol PC is not delared with the correct export for a static library then we get these kind of problems :). This is also the reason why static builds are sometimes several magnitudes bigger than the dll builds as all the symbols are linked into the resulting application. This was actually one of the major problems with TDM gcc as by default it linked in winpthreads statically but winpthreads also has problems with static exports so it caused me a huge headache trying to figure out why some stuff worked while several other projects failed miserably. the symbol in question is actually allmost correct for a static build but termcap has wrong exports for a dll build -> extern char PC; should have been __declspec(dllexport) char PC; and the static one should just be char PC:

@intvsteve
Copy link
Author

Hehe yeah.... at a long-time prior employer I did a lot of work on a large multi-platform codebase with a pretty sophisticated (and bespoke) build toolchain. Exporting globals from Windows DLLs is almost a "dark art" involving secret incantations and rare reagents.

The fun part is the general difference in practices across platforms. Generally it seems the default *NIX approach is (or was) to statically link - it avoids "DLL Hell" and all sorts things, at the cost of larger executables and challenges to patch. E.g. if a security hole is found in termcap any app statically linked to it needs a rebuild to get the patch.

Windows tends to favor shared library linkages, inviting DLL Hell but getting smaller individual app sizes, easier patches outside every app needing to rebuild.

Mac apps seem to try to split the middle, but can't decide on .framework, .dylib, .so, or whatever lol.

So when you have portable code across compilers, platforms, and target linkages, things get pretty ugly pretty fast when you have to herd the cats. There's not a universal standard way to deal with all those combinations of sources and targets AFAIK.

Been a while since those things were the daily grind though! Maybe things are changing.

@revelator
Copy link
Contributor

its becoming more standardized that is for sure :) gcc has had a lot of hardening lately so things which would normally build no problem no longer does. one such is fortran which added more strictness at about version 8 i think gcc itself also became more strict and now errors out on things that would previously just had been warnings. personally i like clang though im a bit partial with rust (a bytecode compiler dependant on a bytecode compiler just using a different syntax as if things were not advanced enough as is huh...).

@revelator
Copy link
Contributor

btw i just noticed -Wl,--dynamicbase,--nxcompat,--no-seh if using binutils-2.36 these flags are no longer nessesary as it now defaults to having aslr and dep on. the --no-seh flag might still be needed though.

I guess msys2 needs an update ;)

@flinco
Copy link

flinco commented Aug 24, 2021

I confirm the issue.

This is what I get :-(

cc -s -static xxxxxx.o xxxxxx-local.o xxxxxx-xml.o xxxxxx_utils.o vg_xxxxxx_support.o vg_xxxxxx_utils.o km_xxxxxx_template.o -o xxxxxx.exe -LC:/msys64/mingw64/lib -lxml2 -LC:/msys64/mingw64/lib -lz -LC:/msys64/mingw64/lib -lws2_32 -lssl -LC:/msys64/mingw64/lib -lws2_32 -lgdi32 -lcrypt32 -lcrypto -lws2_32 -lgdi32 -lcrypt32 -lintl -lws2_32 -lole32 -lwinmm -lshlwapi -lm -LC:/msys64/mingw64/lib -lpcre -lgthread-2.0 -pthread -lintl -lintl -lws2_32 -lole32 -lwinmm -lshlwapi -pthread -lm -LC:/msys64/mingw64/lib -lpcre -lgobject-2.0 -lglib-2.0 -lws2_32 -lole32 -lwinmm -lshlwapi -pthread -lm -LC:/msys64/mingw64/lib -LC:/msys64/mingw64/lib/../lib -lffi -lreadline -LC:/msys64/mingw64/lib -ltermcap -lregex -ltre -pipe -lintl -liconv -llzma -lpcre -lole32 -lws2_32 -lz -ldl -lm C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/mingw64/lib\libtermcap.a(termcap.o):(.bss+0x8): multiple definition of 'PC'; C:/msys64/mingw64/lib\libreadline.a(terminal.o):terminal.c:(.bss+0xa8): first defined here collect2.exe: error: ld returned 1 exit status

@raedrizqie
Copy link
Contributor

why not linking readline with ncurses instead?

@revelator
Copy link
Contributor

at some point i think it did but it was reverted because of some other problems, try it out and see if everything works as expected :)

@brisingraerowing
Copy link
Contributor

I got around this when building some statically linked utility programs by rebuilding Termcap with the PC variable renamed to TERMCAP_PC (there's 4 places, 2 in termcap.h and 2 in termcap.c). It doesn't seem to be used anywhere else, so it's probably safe.

@mcuee
Copy link

mcuee commented Oct 20, 2022

I got around this when building some statically linked utility programs by rebuilding Termcap with the PC variable renamed to TERMCAP_PC (there's 4 places, 2 in termcap.h and 2 in termcap.c). It doesn't seem to be used anywhere else, so it's probably safe.

This work well. Thanks.

mcuee added a commit to mcuee/avrdude that referenced this issue Oct 24, 2022
We have to use dynamic link because static Link with readline will hit into MSYS2 issue here.
msys2/MINGW-packages#8105
@mcuee
Copy link

mcuee commented Oct 24, 2022

Hopefully this issue can be fixed so that avrdude can use readline properly with MSYS2 mingw32/64.

@brisingraerowing
Copy link
Contributor

I'm working on my static utilities again, and unfortunately this is not fixed. I still get multiple definition errors refering to the PC variable when linking readline with anything (found when building PCRE2). This also still breaks when something links both readline and NCurses (like gettext does).

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

Successfully merging a pull request may close this issue.

7 participants