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

Implement support for breaking into debugger under Linux #585

Merged
merged 3 commits into from
Jan 15, 2017

Conversation

vadz
Copy link
Contributor

@vadz vadz commented Feb 5, 2016

This makes -b command line option to work under Linux.

Only tested under amd64, but should work on any architectures, in theory.

vadz added 3 commits February 5, 2016 15:02
Use Linux-specific /proc/$PID/status file to check whether we're being
debugged and a generic raise(SIGTRAP) to actually break into the debugger.
Don't duplicate Catch::isDebuggerActive() check many times, do it just once
in CATCH_BREAK_INTO_DEBUGGER() definition and use a separate CATCH_TRAP()
macro for the really platform-dependent part.
This is more convenient than using the generic portable raise(SIGTRAP) as it
avoids having another stack frame in the debugger when the break happens.
// directly at the location of the failing check instead of breaking inside
// raise() called from it, i.e. one stack frame below.
#if defined(__GNUC__) && (defined(__i386) || defined(__x86_64))
#define CATCH_TRAP() asm volatile ("int $3")
Copy link

Choose a reason for hiding this comment

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

How about using __builtin_trap() available on both gcc and clang to avoid the architecture dependence ? Though it will not specifically raise SIGTRAP.

EDIT: On gcc 5.3.0 and clang 3.7.1, it raises SIGILL. It is kind of confusing to see "Illegal instruction" for an expected break. Thus I think your solution is better. 👍

@caryoscelus
Copy link

Any reason this isn't merged yet?

@horenmar
Copy link
Member

horenmar commented Jan 8, 2017

Since there is interest in this functionality (#372 is very similar), Ill try to take a look soon.

@horenmar
Copy link
Member

horenmar commented Jan 15, 2017

This PR seems to bring in about 13% runtime penalty for failing tests, even if -b is not specified. (Its about 200% with -b specified and no debugger attached).

Since masses of failing tests are not the usual use case, this probably is not a problem, but it seems to suggest that there is something wrong with how the debugger check is gated if -b is not specified.

@vadz
Copy link
Contributor Author

vadz commented Jan 15, 2017

This is really strange, I admit I haven't tested the performance implications of this, but I just don't see how could it affect the execution time so much without -b. How exactly can I reproduce this, i.e. what methodology did you use for benchmarking?

FWIW with -b the code could be optimized by caching the result of isDebuggerActive() at the price of not detecting debugger being attached (or, even more rarely, detached) while the test is executing.

Thanks for looking at this!

@horenmar
Copy link
Member

I've made branch for performance improvements, dev-performance. A very simple benchmark is there, with about a 1M failing assertions, so I ran it against this.

Anyway, the penalty isn't that bad, but it could bear looking into later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants