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

Print stacktraces using std::stacktrace if available #4337

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

strager
Copy link

@strager strager commented Aug 10, 2023

Reviewers: See issue #4336 for some design considerations. This PR chooses approach B. The first commit changes the design, and the second commit adds the std::stacktrace feature. If this should be split into two separate pull requests (one commit per PR), let me know.

If std::stacktrace is available (currently only GCC (opt-in feature) and MSVC), use it for printing stack traces.

If both Abseil and std::stacktrace are available, prefer std::stacktrace.

Advantages of std::stacktrace over Abseil:

  • Includes file and line information
  • Includes DLL information
  • Better stack trace quality on Windows

Disadvantages of std::stacktrace compared to Abseil:

  • Runs more slowly on Windows
  • Includes information which might not be relevant such as byte offsets
  • Printed lines are longer thus harder to skim

Before (Windows using Abseil):

[ RUN      ] FooTest.Xyz
[snip]\googletest-filter-unittest_.cc(49): error: Failed
Expected failure.
Stack trace:
  00007FF69EF90E8D: testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,void>
  00007FF69EF90AC3: testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
  00007FF69EF5C01B: testing::Test::Run
  00007FF69EF5CCC5: testing::TestInfo::Run
  00007FF69EF5D6EC: testing::TestSuite::Run
... Google Test internal frames ...

[  FAILED  ] FooTest.Xyz (21 ms)

After (Windows using MSVC STL's std::stacktrace):

[ RUN      ] FooTest.Xyz
[snip]\googletest-filter-unittest_.cc(49): error: Failed
Expected failure.
Stack trace:
  00007FF6770283A9: googletest_filter_unittest_!`anonymous namespace'::FooTest_Xyz_Test::TestBody+0x89 [snip]\googletest-filter-unittest_.cc:49
  00007FF677080E8D: googletest_filter_unittest_!testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,void>+0x1D [snip]\gtest.cc:2609
  00007FF677080AC3: googletest_filter_unittest_!testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>+0x73 [snip]\gtest.cc:2652
  00007FF67704C01B: googletest_filter_unittest_!testing::Test::Run+0xAB [snip]\gtest.cc:2698
... Google Test internal frames ...

[  FAILED  ] FooTest.Xyz (127 ms)

…tter

The OsStackTraceGetter class does different things depending on whether
GTEST_HAS_ABSL is set. I want to add another implementation of
OsStackTraceGetter, and I think it's ugly if my new implementation
litters OsStackTraceGetter with more #if-s.

Split OsStackTraceGetter into two different classes:

* AbslStackTraceGetter, which is only defined if GTEST_HAS_ABSL is set.
  This class behaves the same as OsStackTraceGetter currently behaves.
* NoopStackTraceGetter, which is always defined. This class behaves the
  same as OsStackTraceGetter currently behaves if GTEST_HAS_ABSL is not
  set.

After this refactor, it is easier to add new OsStackTraceGetter
backend classes.
If std::stacktrace is available (currently only GCC (opt-in feature) and
MSVC), use it for printing stack traces.

If both Abseil and std::stacktrace are available, prefer
std::stacktrace.

Advantages of std::stacktrace over Abseil:

* Includes file and line information
* Includes DLL information
* Better stack trace quality on Windows

Disadvantages of std::stacktrace compared to Abseil:

* Runs more slowly on Windows
* Includes information which might not be relevant such as byte offsets
* Printed lines are longer thus harder to skim

Before (Windows using Abseil):

    [ RUN      ] FooTest.Xyz
    [snip]\googletest-filter-unittest_.cc(49): error: Failed
    Expected failure.
    Stack trace:
      00007FF69EF90E8D: testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,void>
      00007FF69EF90AC3: testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
      00007FF69EF5C01B: testing::Test::Run
      00007FF69EF5CCC5: testing::TestInfo::Run
      00007FF69EF5D6EC: testing::TestSuite::Run
    ... Google Test internal frames ...

    [  FAILED  ] FooTest.Xyz (21 ms)

After (Windows using MSVC STL's std::stacktrace):

    [ RUN      ] FooTest.Xyz
    [snip]\googletest-filter-unittest_.cc(49): error: Failed
    Expected failure.
    Stack trace:
      00007FF6770283A9: googletest_filter_unittest_!`anonymous namespace'::FooTest_Xyz_Test::TestBody+0x89 [snip]\googletest-filter-unittest_.cc:49
      00007FF677080E8D: googletest_filter_unittest_!testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,void>+0x1D [snip]\gtest.cc:2609
      00007FF677080AC3: googletest_filter_unittest_!testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>+0x73 [snip]\gtest.cc:2652
      00007FF67704C01B: googletest_filter_unittest_!testing::Test::Run+0xAB [snip]\gtest.cc:2698
    ... Google Test internal frames ...

    [  FAILED  ] FooTest.Xyz (127 ms)
@snnn
Copy link

snnn commented Aug 15, 2023

Recently, we found a problem that: after gtest started taking dependency on abseil, the "::testing::InitGoogleTest" function will call SymInitialize Windows API indirectly. However, the API can only be called once per process. For example,

#include <iostream>
#include <Windows.h>
#include <DbgHelp.h>

int main()
{
    HANDLE process = GetCurrentProcess();
    SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_UNDNAME);
    if (!SymInitialize(process, nullptr, true)) {
        return -1;
    }
    if (!SymInitialize(process, nullptr, true)) {
        return -1;
    }
    return 0;
}

The second SymInitialize call would definitely fail. So, if the library we are testing also has its own backtrace implementation, the extra "SymInitialize" function call would cause trouble. I wonder how it is handled in VC++ runtime's "<stacktrace>" library.

@derekmauro derekmauro self-assigned this Aug 16, 2023
@snnn
Copy link

snnn commented Aug 22, 2023

I think std::stacktrace has two benefits compared to the existing one:

  1. You don't need to worry about initialization/cleanup. The C++ runtime should handle it.
  2. It dynamically loads dbghelp.dll. So the dependency is optional.

@zrno
Copy link

zrno commented Feb 26, 2024

@derekmauro any plans to merge this?

@yuslepukhin
Copy link

Recently, we found a problem that: after gtest started taking dependency on abseil, the "::testing::InitGoogleTest" function will call SymInitialize Windows API indirectly. However, the API can only be called once per process. For example,

#include <iostream>
#include <Windows.h>
#include <DbgHelp.h>

int main()
{
    HANDLE process = GetCurrentProcess();
    SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_UNDNAME);
    if (!SymInitialize(process, nullptr, true)) {
        return -1;
    }
    if (!SymInitialize(process, nullptr, true)) {
        return -1;
    }
    return 0;
}

The second SymInitialize call would definitely fail. So, if the library we are testing also has its own backtrace implementation, the extra "SymInitialize" function call would cause trouble. I wonder how it is handled in VC++ runtime's "" library.

Is this getting some attention? All that is need to be done is not to use GetCurrentProcess()

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.

None yet

5 participants