-
Notifications
You must be signed in to change notification settings - Fork 16
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
The S-function does not compile with Matlab R2018b #4
Comments
From what it seems, the entrypoint function of the mex is missing. More information at mexFunction. TL;DR There are already few bug reports I found, specifically: |
As the set_target_properties(${${prefix}_NAME}
PROPERTIES
LINK_FLAGS "${_previous_link_flags} /EXPORT:mexFunction") outside the |
This happens only on Windows or also on Linux? It applies also to 2019a ? |
Ok, 2019a is not out yet. : ) |
Yesterday on @aikolina setup (Ubuntu 16.04) we added: set_target_properties(Mex PROPERTIES COMPILE_FLAGS "-fvisibility=default") For sure this works, I am not yet sure if this is too drastic (though it's not a big issue since we've never hidden any symbol in this project). I am waiting @aikolina pc to investigate deeper. |
Further infos on the reason why the symbols are usually hidden:
|
Does this mean that the upstream |
If there was an equivalent to MSVC's However, it seems that the visibility of the |
If you have time, can you check inside the |
No my last quote is the reason why CMake hides by default all symbols but // R2018a
#ifdef _WIN32
# define DLL_EXPORT_SYM __declspec(dllexport)
# define SUPPORTS_PRAGMA_ONCE
#elif __GNUC__ >= 4
# define DLL_EXPORT_SYM __attribute__ ((visibility("default")))
# define SUPPORTS_PRAGMA_ONCE
#else
# define DLL_EXPORT_SYM
#endif
#ifdef DLL_EXPORT_SYM
# define MEXFUNCTION_LINKAGE EXTERN_C DLL_EXPORT_SYM
#else
# ifdef MW_NEEDS_VERSION_H
# include "version.h"
# define MEXFUNCTION_LINKAGE EXTERN_C DLL_EXPORT_SYM
# else
# define MEXFUNCTION_LINKAGE EXTERN_C
# endif
#endif // R2018b
#ifdef _MSC_VER
# define MWMEX_EXPORT_SYM __declspec(dllexport)
#elif __GNUC__ >= 4
# define MWMEX_EXPORT_SYM __attribute__ ((visibility("default")))
#else
# define MWMEX_EXPORT_SYM
#endif
#ifdef MW_NEEDS_VERSION_H
# define MEXFUNCTION_LINKAGE LIBMWMEX_API_EXTERN_C MWMEX_EXPORT_SYM
#else
# define MEXFUNCTION_LINKAGE LIBMWMEX_API_EXTERN_C
#endif |
Refer to the upstream CMake fix, particularly: if(NOT ${Matlab_VERSION_STRING} VERSION_LESS "9.5") # For 9.5 (R2018b) (and newer?)
target_compile_options(${${prefix}_NAME} PRIVATE "-fvisibility=default")
# This one is weird, it might be a bug in <mex.h> for R2018b. When compiling with
# -fvisibility=hidden, the symbol `mexFunction` cannot be exported. Reading the
# source code for <mex.h>, it seems that the preprocessor macro `MW_NEEDS_VERSION_H`
# needs to be defined for `__attribute__ ((visibility("default")))` to be added
# in front of the declaration of `mexFunction`. In previous versions of MATLAB this
# was not the case, there `DLL_EXPORT_SYM` needed to be defined.
# Adding `-fvisibility=hidden` to the `mex` command causes the build to fail.
# TODO: Check that this is still necessary in R2019a when it comes out.
endif() |
Why we don't simply define |
It would be cool to see which compiler commands are actually called by the |
Reading comments from the issues I linked, it does not seem the right way to fix this. If CMake went to this direction I would apply the same fix in order to match the behavior of recent CMake version (I guess, >= 3.13.0, not sure if it will be backported). |
Why?
Ok, this make sense even if I suspect the fix is sub-optimal. |
I am not sure if it is used in other places nor what can happen now or in the future by enabling it. I think that the solution of exposing more symbols knowing for sure that there is no symbols collision (knowing our code) is safer than enabling an option that can insert code that we do not control (and are not aware since we cannot test many Matlab versions). |
I see. I think the definitive fix is to check which kind of preprocessor defines the |
It looks that The only user that as of today has the R2018b version of Matlab is @aikolina as far as I know. |
I think we just need to run mex on any mex library (for example a one of the matlab examples, see the examples in https://it.mathworks.com/help/matlab/ref/mex.html), and check which flags are passed: we do not need to compile one of our specific projects. From my understanding, this problem is shared by all the projects that compile mex libraries (such as our YARP and iDynTree SWIG-based bindings) and so I am concerned in finding a solution that works fine for all our use cases (and I suspect at that point also CMake fix will need to be changed, if it turns out that |
If @aikolina can do run the |
Anyhow, I just realized that I had run an old copy of https://github.com/robotology/mex-wholebodymodel on a Matlab 2018b recently, and everything was working fine. Note that mex-wholebodymodel and also iDynTree YARP bindings actually use a vendored version of FindMatlab.cmake, that is probably different from the one that |
@traversaro Probably the vendored As soon as we will be back we should do the check on the definitions you were mentioning. |
Apparently this is not the case: https://github.com/robotology/mex-wholebodymodel/blob/master/cmake/FindMatlab.cmake#L909 , but it is possible that that code is not working as intended. |
Here is the map file provided in Matlab 2018a:
At least this is provided directly from Mathworks. From the MATLAB Central thread it seems that this is not the case for Windows. |
Indeed, we should check also what they are doing on macOS, as that is not reported in the thread.
The equivalent functionality in Windows is provided by the |
It is interesting that for some reason they document to export |
Next week I plan to release the first release. If you don't have anything against it, I would temporary go for the visibility fix that is quick and effective, and leave more in-depth investigations and tests to the release after. @traversaro |
Ok for me. |
This is a temporary fix of robotology#4. A more stable fix should be found.
This is a temporary fix of robotology#4. A more stable fix should be found.
PR #36 should fix this issue but let's keep it open to check if the situation can be improved further. |
I had a similar issue when compiling The error I was getting is the following:
@diegoferigo helped me to debug the problem, and it is currently fixed by commenting out |
Here some more details. @lrapetti configuration arrives at this line of the blockfactory/cmake/FindMatlab.cmake Line 1048 in a05e6d0
since his matlab is recognized as |
@diegoferigo @traversaro @lrapetti , I just got the same problem on MacOS Sierra with Matlab 2017b. commenting the mentioned line also worked. Any observed side effects? |
Not that I know. Since blockfactory has a vendored version of blockfactory/cmake/FindMatlab.cmake Lines 1045 to 1049 in a05e6d0
and changing the check on the Matlab version. However, it will be just a workaround since this problem might appear also on other Matlab versions and we cannot test more than the ones that we own. What do you think @traversaro? |
I would fix the blockfactory vendored |
If I understood correctly, @Giulero has Ubuntu 18.04 and Matlab 2019a and he is able to run the WB-Toolbox without problems. Probably the workaround in #4 (comment) is necessary just for macOS (or at least it is not necessary on Linux)? |
MathWorks never replied to my comment in #4 (comment) . Given how this answer system works, it may be worth to try to ask the same question in a new question, instead of just commenting an old answered question. |
Maybe they fixed it in 2019a? I still didn't update, they might have changed piece of code reported above in #4 (comment). |
@Giulero (or anyone that has 2019a) can you check what the |
When compiling the
Mex
target, the compilation fails with the following error:This happens only on Matlab R2018b. The same commit builds fine on (at least) R2018a.
The text was updated successfully, but these errors were encountered: