bombela/backward-cpp

Why do we have a dependency on msvcr90 if compiling with MINGW?

cowo78 opened this issue · 18 comments

I see from 146e2ed that msvcr90 is brought in as a dependency when compiling with MINGW.

Could it be clarified what are the dependencies that we have from msvcr?

Why is msvcr90 (Visual Studio 2008) hardcoded? Shouldn't we link to whatever is available on the system?

in my opinion this is a mistake,

I use MSYS2 clang 15.0.5 and MINGW is True
I remove lines

	if(MINGW)
	    set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>>" CACHE STRING "Mingw MSVC runtime import library")
	    list(APPEND _BACKWARD_LIBRARIES ${MINGW_MSVCR_LIBRARY})
	endif()

then:
[ctest] 100% tests passed, 0 tests failed out of 5

The pull request #234 states:

msvcr90 was needed because backward-cpp uses _set_abort_behavior.

It is used there:

_set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT);

Maybe MinGW didn't implements this before? Or it behaves differently? Clearly it seems to compile fine for you.

I'm programming in C++ very little, I'm not an expert(I've used Delphi all my life) , but MinGW is broad concept.
It is included in it:

  • MinGW (I don't think anyone uses it anymore)
  • Cygwin
  • Mingw-w64
  • MSYS2-mingw32
  • MSYS2-mingw64
  • MSYS2-clang32
  • MSYS2-clang64
  • other ?

In MSYS2-mingw64 and MSYS2-clang64 _set_abort_behavior exists and msvcr90 is unnecessary

As far as I understand _set_abort_behaviour is available in MSYS import libraries that in turn refer to native DLLs (i.e. ucrtbase, msvcr90 ...). Such import libraries are distributed inside mingw-*-crt*.

In our case the problem is that I don't see why it's necessary to hardcode a specific redistributable dependency. The same should be obtained by finding whatever is available on the system.

As far as I know there's no import library for debug MSVC libraries, so I would suggest to change set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>:d>" CACHE STRING "Mingw MSVC runtime import library") to find_library(MINGW_MSVCR_LIBRARY ucrtbase msvcr140 msvcr120 msvcr110 msvcr100 msvcr90 REQUIRED DOC "Mingw MSVC runtime import library").

I can provide a PR for that if you see the point.

In MSYS2-mingw64 and MSYS2-clang64 _set_abort_behavior exists and msvcr90 is unnecessary

AFAIK import libraries exist but the target EXE is dynamically linked to msvcr* or ucrtbase where the symbol is really defined.

madebr commented

I see the missing __imp__set_abort_behavior symbol error again in current master when I build for mingw with -DBUILD_SHARED_LIBS=ON.
The following fixes it for me:

--- a/BackwardConfig.cmake
+++ b/BackwardConfig.cmake
@@ -215,10 +215,9 @@ if(WIN32)
                if(SUPPORT_WINDOWS_DEBUG_INFO)
                        set(CMAKE_EXE_LINKER_FLAGS "-Wl,--pdb= ")
                        add_compile_options(-gcodeview)
-               else()
-                       set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>:d>" CACHE STRING "Mingw MSVC runtime import library")
-                       list(APPEND _BACKWARD_LIBRARIES ${MINGW_MSVCR_LIBRARY})
                endif()
+               set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>:d>" CACHE STRING "Mingw MSVC runtime import library")
+               list(APPEND _BACKWARD_LIBRARIES ${MINGW_MSVCR_LIBRARY})
        endif() 
 endif()
 

@madebr
which mingw ?

could you send link to your mingw compiler, version and CMakeCache.txt from build ?

on my msys2 when I turn on BACKWARD_SHARED=YES then I must add -lDbghelp

I will look at the problem globally

madebr commented

When I build with -DBACKWARD_SHARED=YES, I additionally get a __imp_SymGetModuleBase64 missing symbol.
Looks like the order of libraries is wrong: -ldbghelp must be after backward.cpp.obj, not before.

Here's my CMakeCache.txt, configured on top of current main (without any patch applied).

I'm using a mingw toolchain, provided by my Linux distribution (Fedora 38).
It provides a gcc toolchain with version 12.2.1, using mingw64 10.0.0.

I will look at the problem globally

It might be interesting to link with MSVC using -nodefaultlib (while debugging, don't push this upstream). This will force you to explicitly add every library as a link argument (including the c, c++ and windows libraries).

In my opinion better solution,
compatible with Linux and msys2

if(WIN32)
    list(APPEND _BACKWARD_LIBRARIES dbghelp psapi)
    if(MINGW)
        if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
            # mingw on Linux platform
            set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>:d>" CACHE STRING "Mingw MSVC runtime import library")
            list(APPEND _BACKWARD_LIBRARIES ${MINGW_MSVCR_LIBRARY})
        else()
            # mingw on msys2
            include(CheckCXXCompilerFlag)
            check_cxx_compiler_flag(-gcodeview SUPPORT_WINDOWS_DEBUG_INFO)  
            if(SUPPORT_WINDOWS_DEBUG_INFO)
                set(CMAKE_EXE_LINKER_FLAGS "-Wl,--pdb= ")
                add_compile_options(-gcodeview)
            endif()
        endif()
    endif() 
endif()
madebr commented

Or do a check_c_source_compiles test on all mingw/win32 platforms?

Also, the --codeview test should run on all mingw toolchains.
I don't see why the __imp__set_abort_behavior symbol issue and the -gcodeview check are mutually exclusive.

cowo78 commented

Please note also #307 regarding the usage of fixed msvcr90. Better use whatever is available on the system IMO.

cowo78 commented

As of 51f0700 there are no unresolved symbols when building with ninja/-DBACKWARD_SHARED=YES/gcc 12.2

cowo78 commented

Or do a check_c_source_compiles test on all mingw/win32 platforms?

Also, the --codeview test should run on all mingw toolchains. I don't see why the __imp__set_abort_behavior symbol issue and the -gcodeview check are mutually exclusive.

_set_abort_behavior sets if we want to show a windows error report message or print some text (on the console if any I guess).
-gcodeview produces debug info in CodeView format (compatible with visual c++).

I don't understand why we should use -gcodeview option. If we build on windows with MINGW we will not be able to use the binary with MSVC anyway so I don't understand why we should produce a MSVC debug info. I see this was introduced in af4436f by @mariuszmaximus so maybe you want to shed some light?

@cowo78 various compilers are included in the word "mingw" (on Windows and Linux) , I am using msys2 and clang , and only CodeView format enable beautiful stack trace
CodeView also has some drawbacks, in vscode I must use cppvsdbg :D
Please tell us what compiler you use, preferably provide a download link

cowo78 commented

@cowo78 various compilers are included in the word "mingw" (on Windows and Linux) , I am using msys2 and clang , and only CodeView format enable beautiful stack trace CodeView also has some drawbacks, in vscode I must use cppvsdbg :D Please tell us what compiler you use, preferably provide a download link

I am using GCC 12.2.0/mingw64/msys2; didn't know that CodeView provided better looking traces.
I understand that -gcodeview should be enabled on GCC/CLANG for windows target if possible, regardless of the host. Correct?

In my opinion backward-cpp doesn't work in msys2:mingw64
not exists function _set_abort_behavior
and not exists BACKTRACE_SYMBOL

STACK_DETAILS_AUTO_DETECT = ON

msys2:mingw64

BACKWARD_DEFINITIONS:
BACKWARD_HAS_UNWIND=1;
BACKWARD_HAS_LIBUNWIND=0;
BACKWARD_HAS_BACKTRACE=0;
BACKWARD_HAS_BACKTRACE_SYMBOL=0;
BACKWARD_HAS_DW=0;
BACKWARD_HAS_BFD=1;
BACKWARD_HAS_DWARF=0
-- running test case: smalltrace
Stack trace (most recent call last):
#11   Object "", at 0x7ffe8f8b257d, in BaseThreadInitThunk
Attempt to access invalid address.

#10   Object "", at 0x7ff6d3ec1406, in  ?? 
Attempt to access invalid address.

#9    Object "", at 0x7ff6d3ec12ee, in  ?? 
Attempt to access invalid address.

#8    Object "", at 0x7ff6d3ec1b3b, in  ?? 
Attempt to access invalid address.

#7    Object "", at 0x7ff6d3ec178b, in  ?? 
Attempt to access invalid address.

#6    Object "", at 0x7ff6d3ec89d6, in  ??
Attempt to access invalid address.

#5    Object "", at 0x7ff6d3ec1624, in  ?? 
Attempt to access invalid address.

#4    Object "", at 0x7ff6d3ec15e0, in  ??
Attempt to access invalid address.

msys2:clang64

BACKWARD_DEFINITIONS:
BACKWARD_HAS_UNWIND=1;
BACKWARD_HAS_LIBUNWIND=0;
BACKWARD_HAS_BACKTRACE=0;
BACKWARD_HAS_BACKTRACE_SYMBOL=1;
BACKWARD_HAS_DW=0;
BACKWARD_HAS_BFD=0;
BACKWARD_HAS_DWARF=0
running test case: smalltrace
Stack trace (most recent call last):
#11   Object "", at 00007FFE8F8B257D, in BaseThreadInitThunk
#10   Object "", at 00007FF70DE81366, in mainCRTStartup
#9    Object "", at 00007FF70DE81315, in WinMainCRTStartup
#8    Source "D:\mariusz\pulpit\gity\_kasuj\backward-cpp_2024\test\_test_main.cpp", line 203, in main
        200:       if (strcasecmp(argv[2], test.name) == 0) {
        201:         run_test(test, false);
        202:
      > 203:         return 0;
        204:       }
        205:     }
        206:     return -1;
#7    Source "D:\mariusz\pulpit\gity\_kasuj\backward-cpp_2024\test\_test_main.cpp", line 77, in run_test
         75: bool run_test(TestBase &test, bool use_child_process = true) {
         76:   if (!use_child_process) {
      >  77:     exit(static_cast<int>(test.run()));
         78:   }
         79:
         80:   printf("-- running test case: %s\n", test.name);
#6    Source "D:\mariusz\pulpit\gity\_kasuj\backward-cpp_2024\test\test.hpp", line 92, in test::TestBase::run
         90:   TestStatus run() {
         91:     try {
      >  92:       do_test();
         93:       return SUCCESS;
         94:     } catch (const AssertFailedError &e) {
         95:       printf("!! %s\n", e.what());
#5    Source "D:\mariusz\pulpit\gity\_kasuj\backward-cpp_2024\test\stacktrace.cpp", line 54, in TEST_smalltrace::do_test

-gcodeview only works on msys2:clang !!!

@cowo78 in my opinion backward-cpp use many libraries (libdw,libbfd,libdwarf etc...)
default is STACK_DETAILS_AUTO_DETECT = ON
on my platform I don't have any extra libs
then backward-cpp use StackWalk64 for walk and we need codeview for beautiful stack trace

codeview is broken in GCC :( msys2/MINGW-packages#17599