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 build issue on macOS (aarch64, gcc) and use debugtrap wherever possible #25

Conversation

MrAnno
Copy link

@MrAnno MrAnno commented Feb 1, 2022

This PR makes debugbreak prefer __builtin_debugtrap() wherever it's available (clang).

The code snippet I've used is @jwakely's work, I've added a signoff to my commit message to mark the original author.

The patch fixes a build issue too on macOS (Apple silicon) when a real GCC compiler is used:

warning: implicit declaration of function '__builtin_debugtrap';

Depends on #22
Fixes #24


I'm testing this change on the following architectures (with clang + gcc):

  • x86-64
    • gcc/clang: works
  • ARM mode, 32-bit (Raspberry Pi 1 armv6l)
    • gcc/clang: works (stepping in GDB does NOT work)
  • Thumb mode, 32-bit (Cirrus CI)
    • gcc/clang: works (stepping in GDB does NOT work)
  • AArch64, ARMv8 (Raspberry Pi 4)
    • gcc/clang: works (stepping in GDB does NOT work)
  • Apple silicon (macOS M1 machine):
    • gcc: works (stepping in LLDB does NOT work)
    • clang: works

Unfortunately, I won't be able to test the change on riscv and powerpc.

jwakely and others added 2 commits January 4, 2022 13:29
Also fix typos in macro names.
This also fixes a build issue on macOS (Apple silicon) when a real
GCC compiler is used:
> warning: implicit declaration of function '__builtin_debugtrap';

Signed-off-by: Jonathan Wakely <[email protected]>
Signed-off-by: László Várady <[email protected]>
@MrAnno MrAnno force-pushed the fix-apple-silicon-gcc-and-use-debugtrap-where-possible branch from 826e258 to 83bf7e9 Compare February 1, 2022 23:32
@jwakely
Copy link

jwakely commented Feb 1, 2022

I'll see if I can test it on powerpc64.

MrAnno added a commit to MrAnno/Criterion that referenced this pull request Feb 6, 2022
Currently, this bump contains unmerged changes:
scottt/debugbreak#25
@MrAnno
Copy link
Author

MrAnno commented Feb 6, 2022

I've completed my tests on the platforms that were available to me.
It seems nothing got worse with this change, just slightly better, for example, on macOS.

@scottt What do you think? Is this something you would consider integrating into the master branch?

@jwakely
Copy link

jwakely commented Feb 6, 2022

on POWER8 running CentOS 7.9:

$ test/fib ; test/trap ; test/break ; test/break-c++
Trace/breakpoint trap (core dumped)
Trace/breakpoint trap (core dumped)
Trace/breakpoint trap (core dumped)
Trace/breakpoint trap (core dumped)

@MrAnno
Copy link
Author

MrAnno commented Feb 7, 2022

@jwakely Thank you.

MrAnno added a commit to MrAnno/Criterion that referenced this pull request Feb 12, 2022
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
Copy link
Author

MrAnno commented Mar 24, 2022

@scottt Could you have a look at this?

@MrAnno
Copy link
Author

MrAnno commented Jul 3, 2022

We had to start using a fork of debugbreak due to this build issue on macOS.

@MrAnno
Copy link
Author

MrAnno commented Oct 22, 2022

@scottt Do you have any comments on this?

@MrAnno MrAnno closed this Dec 3, 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.

__builtin_debugtrap detection is wrong
2 participants