Skip to content

Conversation

@MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented Jan 31, 2022

  • --debug now uses the official macOS debugserver by default (if it's on PATH)
  • --debug will fall back to the idle method in case no debugger can be found

Depends on Snaipe/BoxFort#36
Fixes #420

@MrAnno
Copy link
Collaborator Author

MrAnno commented Jan 31, 2022

I've also added a new section to the documentation about debugging on macOS.

@MrAnno
Copy link
Collaborator Author

MrAnno commented Feb 1, 2022

@Snaipe debugbreak is currently broken on Apple silicon, but their development branch is 1 commit ahead of the previous release, containing a fix for our problem:

https://github.com/scottt/debugbreak/commits/master

Can I bump the submodule to point to the mentioned commit?

@MrAnno
Copy link
Collaborator Author

MrAnno commented Feb 1, 2022

It seems there is an issue with the mentioned debugbreak commit, it won't work when compiling with GCC on macOS:
scottt/debugbreak#24

A bugfix proposal is already provided in the above issue.

@Snaipe
Copy link
Owner

Snaipe commented Feb 1, 2022

I don't mind using non-tagged commits, be it there or on forks. To be fair we could probably vendor it ourselves here too; we already do this with valgrind.

@MrAnno MrAnno marked this pull request as draft February 1, 2022 21:05
@MrAnno MrAnno changed the title Add macOS "debugserver" support and fallback debug method Fix debugging on macOS and add fallback debug method Feb 1, 2022
@MrAnno
Copy link
Collaborator Author

MrAnno commented Feb 2, 2022

Let's see what happens with scottt/debugbreak#25. I'll try to "vendor" the change after waiting a few days.

@MrAnno MrAnno force-pushed the fallback-idledebug-and-macos-debugserver branch from d501d5a to 2f0450a Compare February 6, 2022 19:20
@Snaipe
Copy link
Owner

Snaipe commented Feb 7, 2022

LGTM; could you add a ChangeLog entry for the fix?

Support macOS "debugserver" and fallback "idle" debug method
…ound

This is done only when no specific debugger type is specified by the user.
The following contribution fixes a gcc compilation issue on macOS:
scottt/debugbreak#25

This commit replaces the upstream repo with a fork until the above PR gets
integrated into the upstream master.
@MrAnno MrAnno force-pushed the fallback-idledebug-and-macos-debugserver branch from 2f0450a to af870a1 Compare February 12, 2022 15:13
@MrAnno MrAnno marked this pull request as ready for review February 12, 2022 15:15
Copy link
Owner

@Snaipe Snaipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, two nits on the changelog but other than that feel free to merge once these are fixed

@MrAnno
Copy link
Collaborator Author

MrAnno commented Feb 12, 2022

I really don't like doing this, but I temporarly replaced debugbreak with a fork. The forked repo contains 2 small patches on top of the upstream's master, which fixes a compilation issue on macOS with a real gcc compiler.

Those 2 patches have been contributed back to upstream (scottt/debugbreak#25), hopefully, they will be integrated soon.

I think fully bundling debugbreak is not worth it currently, as there are plenty of improvement possibilities on ARM devices, and it would be easier to follow those future changes with submodules.

@MrAnno MrAnno force-pushed the fallback-idledebug-and-macos-debugserver branch from af870a1 to 1cc3911 Compare February 12, 2022 15:25
@Snaipe
Copy link
Owner

Snaipe commented Feb 12, 2022

Using a fork is fine. Criterion had to use a special fork of nanomsg for years with some changes to make it fork-safe until BoxFort was introduced, it can live with a fork of debugbreak for a while.

@MrAnno
Copy link
Collaborator Author

MrAnno commented Feb 12, 2022

Thank you.

@MrAnno MrAnno merged commit 1cc3911 into Snaipe:master Feb 12, 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

Successfully merging this pull request may close these issues.

2 participants