wxSimplebook causes link conflict (Issue #22805)
Mark Roszko
This is a linker bug introduced in 3.2.0, wxSimplebook was derived from wxNavigationEnabled<wxBookCtrlBase> instead of wxBookCtrlBase.
But per the comment above it
// NB: This class doesn't use DLL export declaration as it's fully inline.
The problem is other classes like wxAuiNotebook use wxNavigationEnabled
class WXDLLIMPEXP_AUI wxAuiNotebook : public wxNavigationEnabled<wxBookCtrlBase>
thus the wx DLLs end up with their own templated of wxNavigationEnabled exported.
But because wxSimplebook is an inline class, your application will also generate its own wxNavigationEnabled expansion.
The result is a conflict if you use wxSimplebook and one of the other wx book controls.
E:\kicad\kicad\build\x64-Release\kicad\wxmsw32u_aui.lib(wxmsw32u_aui_vc.dll) : error LNK2005: "public: __cdecl wxNavigationEnabled<class wxBookCtrlBase>::wxNavigationEnabled<class wxBookCtrlBase>(void)" (??0?$wxNavigationEnabled@VwxBookCtrlBase@@@@QEAA@XZ) already defined in panel_display_options_base.cpp.obj
E:\kicad\kicad\build\x64-Release\kicad\wxmsw32u_aui.lib(wxmsw32u_aui_vc.dll) : error LNK2005: "public: virtual __cdecl wxNavigationEnabled<class wxBookCtrlBase>::~wxNavigationEnabled<class wxBookCtrlBase>(void)" (??1?$wxNavigationEnabled@VwxBookCtrlBase@@@@UEAA@XZ) already defined in panel_display_options_base.cpp.obj
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
Does exporting wxSimplebook solves the problem for you ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks for reporting this, it's clearly a problem, but I'm not so sure it's wxSimplebook's fault -- wouldn't the same thing happen if you derived your own class from e.g. wxNavigationEnabled<wxWindow>?
This class is pretty weird in general because it exports its individual members instead of (in spite of?) not being exported itself. Unfortunately 8738110 doesn't provide much explanation, but perhaps we should use WXDLLIMPEXP_INLINE_CORE for the ctor and dtor too? Or maybe we should make the entire class DLL-exported and explicitly instantiate it for all the classes it's used with inside wx?
In any case we clearly need some good tests for this as this isn't the first time this gets broken, judging from the history of the code.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
wouldn't the same thing happen if you derived your own class from e.g. wxNavigationEnabled<wxWindow>?
Yes, the same would happen. However, the hack here is if you #include say, "splitter.h" which already uses wxNavigationEnabled<wxWindow> as the parent for wxSplitterWindow being exported. Since the child class is exported, that's enough hinting that to the compiler that an implementation exists elsewhere.
wxSimplebook can't be fixed because it's an inline class so the compiler/
I actually had a similar issue with wxScrolledCanvas in kicad 2 years ago because it's only a typedef to a templated class. I had to add #include "wx/grid.h" includes to give the hint that's its implemented elsewhere.
I'm not sure what the cleanest fix would be but it would require declaring template specialization in a header somewhere as extern.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I've tried reproducing this bug but couldn't: after adding
#pragma comment(lib, "wxmsw33u_aui")
wxSimplebook sb;
wxAuiNotebook ab;to the minimal sample (and the corresponding includes at the top), I can still link without problems using
nmake -f makefile.vc SHARED=1 BUILD=release
What am I missing here?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
I'm not 100% sure what the link behavior breakage is at the moment, it might have to do with kicad building multiple object libraries under cmake which get linked into DLLs and that's where it blows up.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I'd like to fix this in 3.2.2 if possible, but it would be great to have a way to reproduce this in order to do it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
Yea It's been killing me to get around to this.
I think the issue comes up when you use wxSimplebook in a static module and that gets pulled in by linker again into an executable or shared library that also used wxSimplebook directly within themselves. The result I believe is both link objects contain wxSimplebook that isn't tied to a singular extern.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I think the solution might be as simple as creating some wxNavigationEnabledBookCtrlBase simply inheriting from wxNavigationEnabled<wxBookCtrlBase> and doing nothing else, but having a non-inline ctor and/or dtor and being DLL-exported and then derive wxSimplebook and all the other classes from that class instead.
AFAICS this should force instantiating this class only once inside the DLL and so solve the problem. Unfortunately I am not sure if this is really going to be ABI-compatible (in fact I'm almost sure it's not), but we could at least fix this in master like this.
But, again, I can't even test this myself, so it'd be great if you could please do it (I could make the PR for you to test if this would help).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
Well, I sort of have a repro ....with some other things broken I'm trying to sort out so that it's not a false repro.
The big problem for me is we don't usually use msbuild at all, we roll cmake with vcpkg...which makes a few things easier for me. So this is taking some time for me to relearn msbuild :D
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Sorry, not sure why do you need MSBuild? If you can just give/show me the minimum setup needed to see it, I can build myself. Or, as I wrote above, if can at least test my proposed fix, it could also be enough.
TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
Well, kicad does not make a good repro case unless you have say....a Ryzen 5900X or higher in processor, if you want the build time to be less than 12 hours (all our dependencies together are absolute monsters) :D
Here, I finally wrestled it to work under a standard msbuild project within the wxwidgets source, dump the wxsimplebooklink project to the samples folder.
Use the DLL Debug, x64 config.
wxsimplebook link error repro.zip
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks, I can confirm that my proposed fix fixes your test case (I still get some warnings for PANEL_DUMMY_BASE ctor and dtor but I don't think it's due to any problems in wx), so I've created #23185 with these changes.
Please let me know if you can test this with KiCad and if you still have this problem with it. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
Please let me know if you can test this with KiCad and if you still have this problem with it. TIA!
Unfortunately because of the way we build kicad usually with vcpkg, it's a little difficult for small things to patch.
I have confidence your change fixes it, since this purely has to do with the child templated expanded combination of wxNavigationEnabled being generated outside of wx since wxSimplebook lacked any export declaration. Now that you just threw it into a dummy base class that is exported, it'll be fine.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
OK, I'll merge this into master soon. I'm afraid I don't see how can we fix it without breaking the ABI in 3.2, so we'll have to leave it there.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #22805 as completed via dbbf509.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Roszko
OK, I'll merge this into master soon. I'm afraid I don't see how can we fix it without breaking the ABI in 3.2, so we'll have to leave it there.
Well, a dumb idea is throw a #include wxListbook in the simplebook header. This way MSVC should see the exported wxListbook which also includes the same wxNavigationEnabled as a hint
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Well, a dumb idea is throw a #include wxListbook in the simplebook header. This way MSVC should see the exported wxListbook which also includes the same wxNavigationEnabled as a hint that the template expansion is being exported
If this actually fixes the problem, we should indeed do this, thanks for the idea!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.