typeinfo for PlException gets undefined_symbol error when loading with load_foreign_library #60

Closed pizza-pal closed 1 year ago

Since the PlExceptionBase class was added (circa v9.1.10), I get an undefined_symbol error for the typeinfo for PlException when I try to load my shared library with load_foreign_library. Before that, I could load the libraries and use the foreign predicates in Prolog (there was another issue with the string allocation functions in used with the PL_Blob_t write callback, which is why I upgraded to the latest version).

The undefined_symbol thing is usually something to do with the vtable and abstract base classes and the like; or how the objects are being linked (or both haha).

Previously, I was just using the "SWI-cpp2.h" file from my /usr/local includes as an include, but I didn't link with anything (would there even be anything to link to?) I just compiled my libraries using cmake --build and I didn't use swipl-ld. Should I be using swipl-ld? I saw that it's used in the examples. This is my cmake file: https://git.sr.ht/~sam_russell/swipl-sfml2/tree/main/item/CMakeLists.txt

I tried adding the "-rdynamic" link option to my CMakeLists.txt but that did not fix anything.

I looked PlException in the source and I saw that there were a couple TODOs associated with refactoring this and some other classes, so maybe that has something to do with it. I tried making a few haphazard code changes to see what would happen if I implemented some virtual members (also tried changing some to pure virtual) but most of these attempts wouldn't compile had other runtime errors.

I'm going to try to identify the problem in a more minimal example since my project has a lot of stuff in it, including Blob_t structs and callbacks that might not be right.

Do you have an #include <SWI-cpp2.cpp> in one of your .cpp files? -- the header has been split into a .h and a .cpp file, because there was too much bloat from putting all the code in the .h file, resulting in everything being inlined.

If this isn't the problem, then the c++ line that gets output from your cmake probably is the most useful thing for me in helping you figure things out.

Including SWI-cpp2.cpp in my .cpp files made it work. Thanks!

Now I can load the library, but now I'm back to the original error I was dealing with, when I try to run my test predicate, I get a symbol lookup error for undefined symbol PL_mark_string_buffers.

Not sure what's going on there, it looks like that one should be included from SWI-cpp2.h. It doesn't affect all of my foreign predicates, so I'll just have to take a closer look at what I might be doing wrong with this blob a bit later.

If you have any suggestion about how to improve the documentation to emphasize the need for #include <SWI-cpp2.cpp>, please tell me (I am not a technical writer, nor do I have access to one who will work for $0/hr).

I looked in the source but couldn't find the TODOs for refactor PlException that you mentioned ... perhaps these have been done? - at least in the latest "dev" version (which I suggest using, if you can).

PL_mark_string_buffers is defined in SWI-Prolog.h, so I don't understand how it's undefined. It's used all over the place, for many functions that return a std::string (SWI-cpp2.h uses std::string wherever possible, to avoid "dangling char*" bugs; and creating a string uses the PL_STRINGS_MARK()/PL_STRINGS_RELEASE() macros, which are wrapped in the PlStringBuffers class).

If you have any suggestion about how to improve the documentation

Improving the docs only helps a little in most cases :cry: Can we find a way to either make this work by default or get a clear error message using some cpp magic?

For example include cpp2.cpp at the end of cpp2.h unless some macro is defined before including cpp2.h? Note that most of the C++ wrappers are a single file only.

The problem is that SWI-cpp2.h needs to be included everywhere it's used but SWI-cpp2.cpp must be included only once (if it's included multiple times, the linker will complain about duplicates, I think). Alternatively, SWI-cpp2.cpp can be #included in its own file, or compiled separately to a .a file (I don't know how to do this with cmake). This is because we don't provide a .a or .so file, because we don't want to specify which C++ compiler is being used.

The original SWI-cpp2.h had everything inline, so the problem didn't arise. We can make a SWI-cpp2i.h that behaves the way the original SWI-cpp.h did, with everything inline (and, I think, code bloat):

#define _SWI_CPP2_CPP_inline inline
#include <SWI-cpp2.h>
#include <SWI-cpp2.cpp>

(the _SWI_CPP2_CPP_inline macro is already used in SWI-cpp2.cpp but I haven't tested it)

The problem is that SWI-cpp2.h needs to be included everywhere it's used but SWI-cpp2.cpp must be included only once (if it's included multiple times, the linker will complain about duplicates, I think).

I know that. In practice though, as I claimed, extensions packages typically define only a single cpp file that contains the wrapper. So, this file must, if I understand correctly, do

#include <SWI-cpp2.h>
#include <SWI-cpp2.cpp>

....

As a side node, did you verify that inlining or not makes a significant difference? C(++) compiler are allowed to ignore inline (and typically have some attribute or other extension to force inlining of particular functions).

That aside, SWI-cpp2.h could by default include SWI-cpp2.cpp, where we must be careful that including SWI-cpp2.cpp, twice is a no-op, so we don't break the normal case. If you have a project with multiple C++ files to define the wrapper you define some macro like _SWI_CPP2_CPP_inline that prevents including SWI-cpp2.cpp.

Note that in several somewhat larger projects I simply include additional .c (.cpp) files in the main one, so I can keep everything static and thus don't spill any symbols. If you don't do that you have to use pretty unique names or fiddle around with non-portable symbol visibility extensions as you only want the (un)install function to be visible . I only do the latter for really big extensions.

I can test the size of the .o files with gcc 13.2.0, but not other compilers (Clang, Windows, etc.). So, even if I get a definitive answer for one, there's no guarantee that other compilers won't do things differently. Do you now what the most commonly used compilers are? I think I could put in conditional compilations that do things differently, according to the compiler.

You make a good point about a single file being the most common case (packages/swipl-win is an exception). I'll investigate your proposed solution.

Do you now what the most commonly used compilers are?

clang on Apple and gcc otherwise. I'd certainly not go for different approaches using different compilers. That makes things even more complicated. If the important compilers get things the way I like and others don't get it completely wrong I tend to rely on such features. Wasting a few bytes on some compilers is fine IMO.

It appears that inline-ing adds about 10% code size to test_cpp.cpp's executable, which has a lot of calls to SWI-Prolog (a typical foreign function would have fewer, I think), so the inline-ing solution seems best because it avoids confusion. (This is with g++ 13.2.0; I assume Clang is similar). I'll make a PR, but it might not happen until the end of this month.

OK, so this is interesting; the PL_mark_strings undefined_symbol error is only an issue under these conditions:

Calling a predicate that takes a Prolog string as an argument (such as the render_window_create(W, VideoMode, "My Window Title") example given previously), and that argument is converted to a std::string using PlTerm.as_string.
I'm using the top level query shell in Emacs, the one you get by calling M-x prolog-consult-buffer. If I consult my file using just swipl at the command line, everything works as expected.

I have to say, I'm pretty perplexed by this -- maybe Emacs prolog-mode compiles Prolog files differently than what you get if you consult them from the swipl command?

The TODOs I mentioned are at line 1000 in SWI-cpp2.h:

/ TODO: PlException should be pure virtual, with 2 implementations:
//       - the same as below
//       - PlExceptionFailImpl - for error(resource_error(_))

It looks like this was actually referring to PlExceptionBase. This was a bit of a red herring for me because I was getting an undefined_symbol error that I've gotten a lot in my own C++ programs because I didn't have all the pure virtual functions implemented for a subclass.

As for the documentation, I obviously didn't read this part in "2.5.2 Summary of files" closely enough, because it says it right there in the second sentence: "SWI-cpp2.cpp Contains the implementations of some methods and functions. It must be compiled as-is or included in the foreign predicate's source file."

That said, when I skimmed over this section, I wasn't sure of to whom the guidance was meant to be directed. For some reason I'd assumed that this information was meant for people working on development of the SWI-cpp package rather than authors of foreign predicates. It might help to specify in the documentation which parts are more relevant to people writing C++ code to use within Prolog vs. people embedding Prolog in C++ programs vs. people working on development of the package.

I think that the TODO about a "pure virtual" is obsolete; I've changed how things like error(resource_error(...)) are generated (they used to be classes; they're now functions; but user code doesn't need to change).

As for the PL_mark_strings() error ... is it possible that when the foreign file is compiled under Emacs prolog-mode is picking up a different swipl (and associated files) than the one you're using when you build it (using swipl-ld or its equivalent under cmake)? But that doesn't make sense, because PL_mark_strings would be in the libswipl.so file ... if you remove the code that uses PlTerm::as_string(), do you get a different link error (I don't know if the loader complains about all the missing functions or just the first one)?

Maybe the SWI-cpp2 documentation needs a "quick start" section with two subsections: "Calling Prolog from C++" and "Implementing a foreign predicate using C++". I agree that part of the documentation is aimed at whoever comes after me in maintaining it, I can try to separate it out.

About code bloat ... I tried a different source file and the resulting .o file was smaller with inlining. I surmise that this is because g++ generates code only when the inlined code is used, which is why I didn't get some warning messages (Microsoft C++ seems to be different). A proper solution would be to generate a .a file, but that seems to be tricky to set up for the multiple compilers and OSes, so reverting to the original inlined code seems reasonable (and I'll define macros so that the SWI-cpp2.cpp file can be compiled separately).

Reverted to previous behavior (no need for #include <SWI-cpp2.cpp> by PR https://github.com/SWI-Prolog/packages-cpp/pull/63.

I was able to resolve the Emacs top level shell undefined_symbol issue by upgrading from Emacs 27.x to 29.x. I got the idea from the sweep docs: https://eshelyaron.com/sweep.html#Getting-Started I guess maybe Emacs was doing something funny with the foreign binary? Still not sure what the actual problem was, but it's fixed now.

It is (probably) related to visibility rules between multiple dynamically loaded shared objects.

SWI-Prolog / packages-cpp

typeinfo for PlException gets undefined_symbol error when loading with load_foreign_library #60