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

Fix cross-compiling "int $3" build error #380

Closed
wants to merge 1 commit into from

Conversation

GravisZro
Copy link
Contributor

@GravisZro GravisZro commented May 21, 2024

libmve/mveasm.cpp was using a macro to invoke int $3 instead of using the portable Int3() macro in pserror.h.

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

Replace non-portable macro int3 with the portable macro Int3() from pserror.h.
Fixes #311

Related Issues

Screenshots (if applicable)

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

I've cross-compiled a the libmve module but not the rest of the code because I don't have a port of SDL2 for that.

`libmve/mveasm.cpp` was using a macro to invoke `int $3` instead of using the
portable `Int3()` macro in `pserror.h`.
@JeodC
Copy link
Collaborator

JeodC commented May 21, 2024

Potentially dumb question, why not use debugbreak() as suggested in the issue? I also recommend tagging winterheart for review once this is prepared since the new libmve may not use asm--and therefore this change not needed in the long run.

@GravisZro
Copy link
Contributor Author

Potentially dumb question, why not use debugbreak() as suggested in the issue?

The Int3() macro is a bit more sophisticated for debugging but it does actually end up calling the debug_break function.

I also recommend tagging winterheart for review

As far as I can tell, I don't have the ability to do that.

@JeodC JeodC requested a review from winterheart May 21, 2024 16:39
@GravisZro GravisZro marked this pull request as ready for review May 21, 2024 16:59
@JeodC
Copy link
Collaborator

JeodC commented May 21, 2024

Potentially dumb question, why not use debugbreak() as suggested in the issue?

The Int3() macro is a bit more sophisticated for debugging but it does actually end up calling the debug_break function.

I also recommend tagging winterheart for review

As far as I can tell, I don't have the ability to do that.

Done. Over in #289, libmve.asm is completely gone. I'm not sure how close we are to merging that though.

@pzychotic
Copy link
Contributor

Uhm, I also changed that file in #354 for Windows x64 to compile, but opted for the debug_break() route. I wasn't aware, we have an issue for that.
If the Int3() solution is preferable, I should revert my change in favor of this.

@JeodC
Copy link
Collaborator

JeodC commented May 21, 2024

Uhm, I also changed that file in #354 for Windows x64 to compile, but opted for the debug_break() route. I wasn't aware, we have an issue for that. If the Int3() solution is preferable, I should revert my change in favor of this.

Int3() is a simple macro that's strategically placed in the codebase to quickly identify issues that should only occur in rare circumstances. From my understanding, debug_break() should be preferred when using debug builds, but Int3() should be used in release builds for easier user-friendly reporting.

Of course, since Int3() just calls debug_break() anyway, I don't see why we shouldn't just use Int3().

@JeodC
Copy link
Collaborator

JeodC commented May 21, 2024

Actually, at the moment Int3() is rather unhelpful in comparison. It's defined at pserror.h where it calls debug_break() if it's available (which it usually will be). If not, then it just does the int3 messagebox. The mprintf statement, however, is the user-friendly part I was referring to.
https://github.com/DescentDevelopers/Descent3/blob/main/lib/pserror.h#L222-L229

There are tons of mprintf statements scattered throughout the codebase. It used to output to a monochrome debug console:
https://github.com/DescentDevelopers/Descent3/blob/main/ddebug/mono.h#L72
https://github.com/DescentDevelopers/Descent3/blob/main/ddebug/winmono.cpp#L370-L410

We have a logging module, DLOGGER, but I don't know to what extent it takes advantage of existing debug messages (in windows anyway).

Edit: It doesn't work for windows.

Reference #157 and 782d527

@pzychotic
Copy link
Contributor

pzychotic commented May 21, 2024

-DLOGGER works in Windows too, after we fixed it in #256 (b591154).
It outputs to the VS output window

OutputDebugString(Mono_buffer);
(or any other debugger supporting it) and to a log file
Debug_LogWrite(Mono_buffer);
when activated with -logfile.

@JeodC
Copy link
Collaborator

JeodC commented May 21, 2024

-DLOGGER works in Windows too, after we fixed it in #256 (b591154).

It outputs to the VS output window

OutputDebugString(Mono_buffer);
(or any other debugger supporting it) and to a log file
Debug_LogWrite(Mono_buffer);
when activated with -logfile.

That explains it. I expected it to output to terminal when I ran the exe with it. We were going to have a rough writeup of how to use logging back when we had spdlog, but it never came about and -logfile slipped my mind.

@JeodC JeodC added the bugfix Pull request fixes one or more bugs label May 22, 2024
@Lgt2x
Copy link
Member

Lgt2x commented May 24, 2024

mveasm.cpp will be removed by #289, there is not point in fixing the current version of libmve

@Lgt2x Lgt2x closed this May 24, 2024
@GravisZro
Copy link
Contributor Author

GravisZro commented May 30, 2024

mveasm.cpp will be removed by #289, there is not point in fixing the current version of libmve

I don't understand why you closed this PR. I'm trying to resolve #311. If nothing else, it will fix the build problem for ARM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request fixes one or more bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Build Failure]: fix "bad instruction 'int $3'" error on the Raspberry Pi 4 32 bit
4 participants