8000 Base your stack tracing / "symbolization" on Boost stack_trace · Issue #146 · doctest/doctest · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Base your stack tracing / "symbolization" on Boost stack_trace #146

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

Open
eyalroz opened this issue Aug 24, 2018 · 15 comments
Open

Base your stack tracing / "symbolization" on Boost stack_trace #146

eyalroz opened this issue Aug 24, 2018 · 15 comments
Labels
category/reporting Issues related to reporting functionality type/feature-proposal

Comments

@eyalroz
Copy link
eyalroz commented Aug 24, 2018

The roadmap includes the following item:

  • symbolizer - for a stack trace - when an assertion fails - and it's in a user function with some deep callstack away from the current test case - how to know the exact code path that lead to the failing assert.

Facebook's folly is by now pretty old. Instead, you should base this work on Antony Polukhin's excellent and extensive work for the Boost Stack Trace library (now officially released as part of Boost, and with documentation, too).

@eyalroz eyalroz changed the title [For 2.3] Base your stack tracing / "symbolization" on Boost stack_trace [For v2.3] Base your stack tracing / "symbolization" on Boost stack_trace Aug 24, 2018
@onqtam
Copy link
Member
onqtam commented Aug 24, 2018

Good to know - I'll most likely use that!

@onqtam onqtam changed the title [For v2.3] Base your stack tracing / "symbolization" on Boost stack_trace Base your stack tracing / "symbolization" on Boost stack_trace Dec 1, 2018
@eyalroz
Copy link
Author
eyalroz commented Dec 8, 2021

I should mention that last year I created a de-boostified version of libstacktrace.

@cdeln
Copy link
Contributor
cdeln commented Aug 15, 2024

@eyalroz Do you think this is still relevant?

Personally, whenever I have a bug I just drop into a debugger and figure things out that way.

@eyalroz
Copy link
Author
eyalroz commented Aug 15, 2024

@cdeln : This is a testing framework. Its most common mode of operation is running all the way through and collecting errors and information about where/how/when/why they occurred. A stack trace is important information in that regard.

If one is working in C++23, there's std::stracktrace - which is based on Polukhin's boost library. If one is using an earlier C++ standard version... well, some compilers might still offer some non-standard way to access their stack trace support. As for my library - I have not kept it up-to-date I'm afraid. It should still work, but I can't commit to "backing it up" with support - I am too swamped with other work, I'm sorry.

@cdeln
Copy link
Contributor
cdeln commented Aug 16, 2024

I'm well aware that it's a testing framework ;)

I believe this is only useful when the user code throws an exception or have a failing assertion right?
I think it might be easy to setup stacktracing manually in your own main, here is a draft

#define DOCTEST_CONFIG_IMPLEMENT
#include <doctest/doctest.h>

int f() {
    return 0;
}

int run() {
    REQUIRE(f());
    return 0;
}

int main(int argc, char** argv) {
    doctest::Context context;
    context.applyCommandLine(argc, argv);
    context.setAsDefaultForAssertsOutOfTestCases();
    context.setAssertHandler([](const doctest::AssertData& d){
      std::cerr << "Catched error:"
                << " line = " << d.m_line
                << " expr = " << d.m_expr
                << " file = " << d.m_file
                << std::endl;
      // You could do the stacktrace here?
    });
    return run();
}

I still don't see why this is helpful in a test case though, which nails down the exact point of failure for you already.

This is my typical workflow. Compile this test with g++ -g

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>

int f() {
    return 0;
}

TEST_CASE("my-test") {
    REQUIRE(f());
}

and run the executable with gdb ./a.out. This prints

[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
main.cpp:8:
TEST CASE:  my-test

main.cpp:9: FATAL ERROR: REQUIRE( f() ) is NOT correct!
  values: REQUIRE( 0 )


Program received signal SIGTRAP, Trace/breakpoint trap.
0x0000555555570850 in DOCTEST_ANON_FUNC_14 () at main.cpp:9
9	    REQUIRE(f());

and breaks right at the test failure.
If I want I can get the stacktrace with bt (which I don't think is very informative, it just shows you the doctest stacktrace)

(gdb) bt
#0  0x0000555555570850 in DOCTEST_ANON_FUNC_14 () at main2.cpp:9
#1  0x000055555556fc20 in doctest::Context::run (this=0x7fffffffdd50) at /usr/include/doctest/doctest.h:7007
#2  0x00005555555706c9 in main (argc=1, argv=0x7fffffffde88) at /usr/include/doctest/doctest.h:7085

@onqtam
Copy link
Member
onqtam commented Aug 16, 2024

its good to have as much information as possible when running on CI and especially if the failure is hard to reproduce IMHO

@cdeln
Copy link
Contributor
cdeln commented Aug 16, 2024

I agree, CI can be extremely difficult to debug and might be a good use case for the feature.
However, I am trying to bone how this feature is supposed to behave even, because I don't understand it yet.

If there are simple workarounds, isn't that preferable to implementing it as part of this library?
Continuing on the gdb approach, you can do this gdb --batch -ex run -ex bt a.out to get a stack trace non-interactively. This is the output I get

This GDB supports auto-downloading debuginfo from the following URLs:
  <https://debuginfod.ubuntu.com>
Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
main.cpp:9:
TEST CASE:  my-test

main.cpp:10: FATAL ERROR: REQUIRE( f() ) is NOT correct!
  values: REQUIRE( 0 )


Program received signal SIGTRAP, Trace/breakpoint trap.
0x00005555555706f8 in DOCTEST_ANON_FUNC_14 () at main.cpp:10
10	    REQUIRE(f());
#0  0x00005555555706f8 in DOCTEST_ANON_FUNC_14 () at main.cpp:10
#1  0x000055555556fac8 in doctest::Context::run (this=0x7fffffffdd50) at /usr/include/doctest/doctest.h:7007
#2  0x0000555555570571 in main (argc=1, argv=0x7fffffffde88) at /usr/include/doctest/doctest.h:7085

Again, I think the stacktrace is useless for my example, if you can provide a good example then that would help me understand the feature request better.

@eyalroz
Copy link
Author
eyalroz commented Aug 16, 2024

I believe this is only useful when the user code throws an exception or have a failing assertion right?

No, also with CHECK()s and WARN()s where the user can't really collect the stack trace themselves (unless they collect the stacktrace regardless of the failure, which is possible, but why would we want that?

I think it might be easy to setup stacktracing manually in your own main, here is a draft

...      // You could do the stacktrace here?

I suppose - but why should users of DOCTEST have to set up their own assertion handlers to be able to see stack traces printed? I think that's not reasonable.

I still don't see why this is helpful in a test case though, which nails down the exact point of failure for you already.

Ah, but it doesn't:

void bar() {
   // do stuff
   ASSERT(whatever)
}

TEST_CASE("my-test") {
   auto a = do_foo();
   bar(a);
   auto b = do_baz();
   bar(b);
}

if you only tell me an assertion failed at line 3, I may not know which call it is to bar().


Bottom line: Remember that this issue is not about whether or not to introduce stack trace support - that had already been added to the roadmap; this one is about what base the implementation on.

@cdeln
Copy link
Contributor
cdeln commented Aug 16, 2024

This is exactly what I mean, this feature only makes sense when there is an assertion in the user code as you just described (that is, in the bar function).

Haven't thought about CHECK and WARN, but that would not be possible to do with gdb as I described in my workaround.
So your idea is that every time a check fails then a stack trace is printed to stdout/stderr?
Personally, if I use CHECK/WARN in my production code I wouldn't just want random printing to stdout/stderr. I would probably want to configure some error handler to handle it. The default of just printing the error without stacktrace seems like a good default to me.

It's not in the current and updated roadmap (I understand it might have been 6 years ago): #600 .

I see the value of this feature, not arguing on that, I'm just trying to understand how this should work and possible implementation paths. We might now have to make it part of doctest, but could be possible to do as a plugin integration thingie.... that would imply less maintenance burden on doctest, and allow users to use different ways to do stacktracing.
Using std::stacktrace seems like a good alternative, but then it's just for latest and greatest, it would be a pity to exclude anyone using older versions the standard.
Either way, to implement the feature we need to understand what it should do and discuss it properly, it's no small feature request.

@cdeln cdeln added type/feature-proposal category/reporting Issues related to reporting functionality and removed enhancement labels Aug 21, 2024
@mitchgrout
Copy link

Regardless of whether this is added, I'd argue against introducing third-party dependencies into doctest, unless they are entirely optional. The main appeal of doctest for me is the ability to quickly drop it into a project without much hassle. If suddenly it now depends on boost, or folly, or whichever library is selected to provide backtrace support, then I'm no longer able to use it.

This feature was noted in the original doc/markdown/roadmap.md, which has since been removed in favour of the community roadmap. I think it is worth re-considering what value this would add, compared to the costs of introducing and maintaining it.

@eyalroz
Copy link
Author
eyalroz commented Aug 24, 2024

@mitchgrout : Certainly agree that no dependence should be mandatory; either an opportunistic dependence on a third-party library / standard library in C++23, or a "vendored" stack trace library as part of doctest.

@cdeln:

So your idea is that every time a check fails then a stack trace is printed to stdout/stderr?

Well, the user should be able to choose whether or not stack traces are printed, but yes. The motivation is that you might not know where exactly the check failed just by the location of the check - if the function is called from many places. In fact, you might have a check_results() function which gets run from everywhere. If that has a CHECK or ASSERT - you have to print a stack trace, or at least some lines from it, to know what's failing.

@cdeln
Copy link
Contributor
cdeln commented Aug 24, 2024

@mitchgrout Good points

@eyalroz I start to grok why this is a good feature

I suggest to split the feature into two steps

  1. Ensure it is possible to configure this through in your own main
  2. Optionally enable a default stack trace reporter for C++23 and above

What do you think?

@eyalroz
Copy link
Author
eyalroz commented Aug 24, 2024

@cdeln : If you start with (1.), there will be almost no work to be done for (2.) , I think. But perhaps we could start with configurable stack tracing based on C++23, and have the boost-for-before-C++23 as a second bug.

@cdeln
Copy link
Contributor
cdeln commented Aug 24, 2024

Exactly! The art of breaking down problems into easy sub-problems ;)
I think 2) will not be trivial though, because 1) does not burden the library on ensuring compiler compatibility or reporting format, which needs to be tested and decided upon in 2). 1) would allow for any stacktrace implementation, such as the one written by yourself, 2) would always be std::* based.

@eyalroz
Copy link
Author
eyalroz commented Aug 24, 2024

Since the implementations in Boost and the standard library are slightly different, to allow "any" stracktrace implementation one would need to abstract. But - eventually it will be up to whoever implements to decide what's easier for them to do first and I'm just speculating.

Anyway, remember that this particular issue is not about the feature itself - it was part of the roadmap already.

Bottom line: I am ok with any splitting / renaming / rearranging of stack-trace-related issues - go right ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/reporting Issues related to reporting functionality type/feature-proposal
Projects
None yet
Development

No branches or pull requests

4 participants
0