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

Platforms without any exceptions support #36

Closed
glebm opened this issue Jul 2, 2022 · 10 comments
Closed

Platforms without any exceptions support #36

glebm opened this issue Jul 2, 2022 · 10 comments

Comments

@glebm
Copy link
Contributor

glebm commented Jul 2, 2022

I'm trying to port DevilutionX to the original XBox NXDK

This platform does not support exceptions at all, which results in:

lld: error: undefined symbol: ___std_terminate
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(int `public: __thiscall std::exception::exception(class exception::dtor$2 const &)'::`1'::dtor$2)

Not sure what the best way to resolve this is.

@realnc
Copy link
Owner

realnc commented Jul 2, 2022

There's no std::terminate()? That's weird. So if you call that directly, you get the same error?

@glebm
Copy link
Contributor Author

glebm commented Jul 3, 2022

Yeah, there is no exceptions support at all and it looks like std::terminate is part of the exceptions support.

We'd need to call std::exit or std::abort instead.

@realnc
Copy link
Owner

realnc commented Jul 3, 2022

Does the obvious workaround of simply providing an implementation of std::terminate work?

@glebm
Copy link
Contributor Author

glebm commented Jul 3, 2022

Defining anything in the std namespace is UB IIRC.
It would probably need to be compiler-specific (___std_terminate) and while it may work, it doesn't seem like a good workaround.

I think the solution in #33 is a better option here.
I can imagine in many cases sound is the only thing threads are used for (e.g. this is the case for DevilutionX single player), so failing to create a mutex is not necessarily a fatal error.

Alternatively, there could be a CMake option to replace that one throw with std::exit.

glebm added a commit to glebm/devilutionX that referenced this issue Jul 3, 2022
Disabled for now because SDL_audiolib fails to link
because NXDK does not support exception at all (and so does not have
std::terminate): realnc/SDL_audiolib#36

SDL_audiolib only has one line of code that uses exceptions
in the entire codebase so hopefully this can be fixed.
glebm added a commit to glebm/devilutionX that referenced this issue Jul 3, 2022
Disabled for now because SDL_audiolib fails to link
because NXDK does not support exception at all (and so does not have
std::terminate): realnc/SDL_audiolib#36

SDL_audiolib only has one line of code that uses exceptions
in the entire codebase so hopefully this can be fixed.
glebm added a commit to glebm/devilutionX that referenced this issue Jul 4, 2022
Disabled for now because SDL_audiolib fails to link
because NXDK does not support exception at all (and so does not have
std::terminate): realnc/SDL_audiolib#36

SDL_audiolib only has one line of code that uses exceptions
in the entire codebase so hopefully this can be fixed.
glebm added a commit to glebm/devilutionX that referenced this issue Jul 4, 2022
Disabled for now because SDL_audiolib fails to link
because NXDK does not support exception at all (and so does not have
std::terminate): realnc/SDL_audiolib#36

SDL_audiolib only has one line of code that uses exceptions
in the entire codebase so hopefully this can be fixed.
@realnc
Copy link
Owner

realnc commented Jul 4, 2022

Defining anything in the std namespace is UB IIRC. It would probably need to be compiler-specific (___std_terminate) and while it may work, it doesn't seem like a good workaround.

It seems like an excellent workaround, until fixed in the SDK itself, at which point you should get a link error and can then remove it again. I looked at https://github.com/XboxDev/nxdk/issues and nobody has reported the issue yet though.

Defining things in std may be UB, but at this point you're implementing your own std missing function that should be there to begin with.

glebm added a commit to glebm/devilutionX that referenced this issue Jul 5, 2022
Disabled for now because SDL_audiolib fails to link
because NXDK does not support exception at all (and so does not have
std::terminate): realnc/SDL_audiolib#36

SDL_audiolib only has one line of code that uses exceptions
in the entire codebase so hopefully this can be fixed.
@glebm
Copy link
Contributor Author

glebm commented Jul 5, 2022

I tried the workaround but it doesn't work because there are some other undefined symbols as well:

lld: error: undefined symbol: __CxxThrowException@8
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(public: __thiscall SdlMutex::SdlMutex(void))

lld: error: undefined symbol: ___std_terminate
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(int `public: __thiscall std::exception::exception(class exception::dtor$2 const &)'::`1'::dtor$2)

lld: error: undefined symbol: ___CxxFrameHandler3
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(___ehhandler$??0exception@std@@QAE@ABV01@@Z)

Defining std::terminate leads to duplicate symbol definition.

I looked at XboxDev/nxdk/issues and nobody has reported the issue yet though.

There is this issue XboxDev/nxdk#463

@glebm
Copy link
Contributor Author

glebm commented Jul 5, 2022

Putting aside the lack of a workaround, does this really need to be an exception?

You said in #33 (comment):

Locking is not optional. If a lock cannot be acquired, the application should shut down since something is seriously broken. The exception that is thrown simply gives applications the opportunity to do a more graceful shutdown. It's not a condition that can just be logged and then have execution resumed.

However, multi-threading is not a critical feature for a lot of the applications.

DevilutionX can run perfectly well on a single thread in single player (just without audio).

I don't think SDL_audiolib should be deciding that for the application.

Can you please reconsider merging #33 instead?

@realnc
Copy link
Owner

realnc commented Jul 6, 2022

Does the improve-exceptions branch work? 8d11c63 is the relevant commit.

@glebm
Copy link
Contributor Author

glebm commented Jul 6, 2022

Hmm, it doesn't work with that branch

-- Performing Test HAVE_EXCEPTIONS - Failed
lld: error: undefined symbol: ___std_terminate
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj)

lld: error: undefined symbol: ___CxxFrameHandler3
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(___ehhandler$??0SdlMutex@@QAE@XZ)
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(___ehhandler$?lock@SdlMutex@@QAEXXZ)
>>> referenced by libSDL_audiolib.lib(SdlMutex.cpp.obj):(___ehhandler$?unlock@SdlMutex@@QAEXXZ)

Perhaps set_source_files_properties should not be called if the exceptions test fails.

@glebm
Copy link
Contributor Author

glebm commented Jul 6, 2022

With #37 it links!

@glebm glebm closed this as completed Jul 6, 2022
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

No branches or pull requests

2 participants