Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
Re: summing up: the kicad issue
To:
tech@openbsd.org
Date:
Sun, 9 Mar 2025 08:51:28 +0100

Download raw body.

Thread
On Fri Mar 07, 2025 at 09:23:21PM +0100, Marc Espie wrote:

Thanks Marc for the summing up.

> - kicad doesn't run correctly on either FreeBSD or OpenBSD after the last
> update (made necessary because of the python update)

> - sthen@ has suggested we "unbreak" kicad and make it non executable, because
> pkg_add -u would be annoyed.

Unfortunately, the best solution for the release.

> - the basic problem is a difference between libstdc++ and libcxx.  libcxx
> has a more pedantic approach to solving dynamic_cast.
> - the gory details lie in /usr/src/gnu/llvm/libcxxabi/src/private_typeinfo.cxx
> look up the _LIBCXXABI_FORGIVING_DYNAMIC_CAST define
> 
> IMO there is a problem with that define, which encompasses two completely
> distinct behaviors:
> -> emitting a diagnostic for problematic typeinfo information
> -> using a simple-minded (strcmp based!) work-around strategy that's (mostly)
> compatible with libstdc++.

> 
> There are several actions that could be persued related to that issue.
> 
> - thanks to the heroic sleuthing of rsadowski, we know that the problem lies
> there. Unfortunately, enabling forgiving dynamic casts yields the ambiguous
> error near line 701:
>     dynamic_cast error 2: One or more of the following type_info's
>     has hidden visibility or is defined in more than one translation
>     unit. They should all have public visibility

What I really don't understand at all and drives me crazy is:

If I patch the following, it the dynamic_cast works (but I would have to
find countless patches and I have no idea how to find them).

Index: include/settings/color_settings.h
--- include/settings/color_settings.h.orig
+++ include/settings/color_settings.h
@@ -48,7 +48,7 @@ using KIGFX::COLOR4D;
  * Each application (eeschema, gerbview, pcbnew) can have a different active color scheme selected.
  * The "child applications" (library editors) inherit from either eeschema or pcbnew.
  */
-class COLOR_SETTINGS : public JSON_SETTINGS
+class __attribute__((visibility("default"))) COLOR_SETTINGS : public JSON_SETTINGS
 {
 public:
     explicit COLOR_SETTINGS( const wxString& aFilename = wxT( "user" ),
@@ -109,7 +109,7 @@ class COLOR_SETTINGS : public JSON_SETTINGS (private)
     std::unordered_map<int, COLOR4D> m_defaultColors;
 };
 
-class COLOR_MAP_PARAM : public PARAM_BASE
+class __attribute__((visibility("default"))) COLOR_MAP_PARAM : public PARAM_BASE
 {
 public:
     COLOR_MAP_PARAM( const std::string& aJsonPath, int aMapKey, COLOR4D aDefault,


BUT but when I set (-fvisibility=default) globally , it doesn't work.

 # Global setting: exports are explicit
-set( CMAKE_CXX_VISIBILITY_PRESET "hidden" )
-set( CMAKE_VISIBILITY_INLINES_HIDDEN ON )
+set( CMAKE_CXX_VISIBILITY_PRESET "default" )
+set( CMAKE_VISIBILITY_INLINES_HIDDEN OFF )

That doesn't go into my head

> looking further, those typeinfos are hidden, AND they may be multiply
> defined (thanks to the pcb_kicad code)
> 
> Making those diagnostics non optional seems like the right thing to do
> in OpenBSD (I understand that they're optional because windows doesn't
> have syslog, but we have numerous examples of non-kosher stuff that
> leaves traces in syslog already), independently of the work-around.
> 
> - explaining properly to the kicad people that the problem is related
> to libcxx and not libstdc++

I don't know if they will accept that. They argue that it works on MacOS
and MacOS uses clang. And yes MacOS also uses libc++ and libc++abi.
I have tested it on my Mac.

The same behaviour as with us, but the test code in the ticket works for
us and on the latest MacOS (tested by me):
https://github.com/llvm/llvm-project/issues/72464

> 
> - evaluating the idea of enabling that compatibility mode
> There is an olden post initiated by Akim Demaille in 2015 on stackoverflow:
> https://stackoverflow.com/questions/19496643/using-clang-fvisibility-hidden-and-typeinfo-and-type-erasure
> 
> This is the best write-up I could find on this exact problem. The end answer 
> "hack", unfortunately, doesn't quite work (emulating dynamic_cast is
> complicated).
> 
> Evaluating the pros-and-cons: I think that we should enable forgiving dynamic
> casts for now. I understand the pedantic attitude of the llvm people, and yes,
> frankly this problem ought to be fixed eventually, but for now, I think there
> is a very low probability that local typeinfos will match exactly (especially
> with most C++ developers using namespaces and removing collisions)
> 
> - one further venue to investigate, looking at the comments 
> in private_typeinfo.cpp, is that both functions doing the typeinfo traversal,
> up and down, are obfuscated optimized versions of a much simpler algorithm.
> 
> I wonder what would happen if we just used the unoptimised version (and
> analyzed it further to AT LEAST store a better diagnostic in info, just so
> we know for sure whether it's a visibility problem or a multiple definition
> problem).
> 
> Lastly one last problem got me stumped: is there a reasonably simple way to
> tell clang to change the visibility of a class's typeinfo (or make it weak
> to work-around the multiple definition issue).
> 
> This could use some fresh eyes, and it's definitely an annoying issue that
> waaay too many people have lost a lot of time on so far !!!
> 

I wasted so much compile time and brain cycles on it. Here is a list of
what I remember doing:

- Visibility: Set -fvisibility=default
- Symbol Handling: Removed .hidden symbols using llvm-objcopy from
  kiface (libs). The idea was to get rid of double hidden symbols.
- Linking and CMake: Cleaned up duplicate linking in CMake files.
  Used PRIVATE and PUBLIC in target_link_libraries properly.
- Replaced dynamic_cast with typeid.
        Tested -DLIBCXXABI_FORGIVING_DYNAMIC_CAST=ON without success.
- I renamed _XXX.kiface to libXXX.kiface.so with the hope that there is
  something here that reacts to lib*so*.

The only thing that works, as mentioned by Marc, is the following:
$ cd /usr/src && patch -p1 -E < ~/patches/libcxxabi.diff
$ cd gnu/lib/libcxxabi && make clean && make && doas make install

diff --git a/gnu/lib/libcxxabi/Makefile b/gnu/lib/libcxxabi/Makefile
index 81673e86a2b..692ab7cc72e 100644
--- a/gnu/lib/libcxxabi/Makefile
+++ b/gnu/lib/libcxxabi/Makefile
@@ -54,6 +54,7 @@ CPPFLAGS+=	-Wall -I${SHDRDIR} -I${HDRDIR} -I${UHDRDIR} \
 CPPFLAGS+=	-D_LIBUNWIND_IS_NATIVE_ONLY
 CPPFLAGS+=	-D_LIBCXXABI_BUILDING_LIBRARY
 CPPFLAGS+=	-D_LIBCPP_BUILDING_LIBRARY
+CPPFLAGS+=	-D_LIBCXXABI_FORGIVING_DYNAMIC_CAST
 CPPFLAGS+=	-DNDEBUG
 CFLAGS+=	-nostdlib -funwind-tables
 CXXFLAGS+=	-nostdlib -nostdinc++ -funwind-tables