Samsung / netcoredbg

NetCoreDbg is a managed code debugger with MI interface for CoreCLR.
MIT License
743 stars 98 forks source link

Can't be built on Ubuntu 23.10 #147

Closed MakiWolf closed 6 months ago

MakiWolf commented 7 months ago

Can't be built on Ubuntu 23.10 with cmake 3.27.4 and dotnet 8.

In file included from /home/maki/Documents/netcore/netcoredbg/src/debugger/breakpoints_exception.cpp:5:
In file included from /home/maki/Documents/netcore/netcoredbg/src/debugger/breakpoints_exception.h:10:
/home/maki/Documents/netcore/netcoredbg/src/interfaces/types.h:196:5: error: no type named 'uintptr_t' in namespace 'std'; did you mean simply 'uintptr_t'?
    std::uintptr_t addr; // exposed for MI and CLI protocols
    ^~~~~~~~~~~~~~
    uintptr_t
/usr/include/stdint.h:90:27: note: 'uintptr_t' declared here
typedef unsigned long int       uintptr_t;
                                ^
1 error generated.
make[2]: *** [src/CMakeFiles/netcoredbg.dir/build.make:134: src/CMakeFiles/netcoredbg.dir/debugger/breakpoints_exception.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:229: src/CMakeFiles/netcoredbg.dir/all] Error 2
make: *** [Makefile:156: all] Error 2
dotnet --info
.NET SDK:
 Version:           8.0.100
 Commit:            57efcf1350
 Workload version:  8.0.100-manifests.2d90560f

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  23.10
 OS Platform: Linux
 RID:         ubuntu.23.10-x64
 Base Path:   /usr/lib/dotnet/sdk/8.0.100/

.NET workloads installed:
 Workload version: 8.0.100-manifests.2d90560f
There are no installed workloads to display.

Host:
  Version:      8.0.0
  Architecture: x64
  Commit:       5535e31a71

.NET SDKs installed:
  8.0.100 [/usr/lib/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.0 [/usr/lib/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.0 [/usr/lib/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
maki@maki-1-2:~/Documents/netcore/netcoredbg/build$ cmake --version
cmake version 3.27.4

CMake suite maintained and supported by Kitware (kitware.com/cmake).
maki@maki-1-2:~/Documents/netcore/netcoredbg/build$ 
viewizard commented 7 months ago

Please provide your clang version. Looks like some changes in STL headers were made.

MakiWolf commented 7 months ago

clang --version Ubuntu clang version 16.0.6 (15) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin

vindicatorr commented 7 months ago

Using the same version as OP.

I see usage was switched from uint64_t a couple of months ago, and I'm seeing std::uintptr_t usage seems nearly non-existent to me: https://github.com/fmtlib/fmt/issues/1197 https://youtrack.jetbrains.com/issue/RSCPP-34814

I don't know if we could get away with just changing those references to uintptr_t or uint64_t.

Finding reference of uintptr_t in headers with the namespace std:

$ find /usr/include/c++/13.2.1/ -type f -exec sh -c 'if (grep -q "namespace std" $0); then if (grep "uintptr_t" $0); then echo $0; fi; fi' {} \;
  /// atomic_uintptr_t
  typedef atomic<uintptr_t>          atomic_uintptr_t;
/usr/include/c++/13.2.1/atomic
#include <stdint.h>     // uintptr_t
  const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
        _GLIBCXX_DEBUG_ASSERT((uintptr_t)__ptr % _Align == 0);
/usr/include/c++/13.2.1/bits/align.h
      { __glibcxx_assert(((uintptr_t)_M_ptr % required_alignment) == 0); }
      { __glibcxx_assert(((uintptr_t)_M_ptr % required_alignment) == 0); }
      { __glibcxx_assert(((uintptr_t)_M_ptr % required_alignment) == 0); }
      { __glibcxx_assert(((uintptr_t)_M_ptr % required_alignment) == 0); }
/usr/include/c++/13.2.1/bits/atomic_base.h
      constexpr uintptr_t __ct = 16;
      auto __key = (uintptr_t(__addr) >> 2) % __ct;
/usr/include/c++/13.2.1/bits/atomic_wait.h
      { return _Type(reinterpret_cast<uintptr_t>(_M_impl.get()) & 0x3); }
/usr/include/c++/13.2.1/bits/fs_path.h
      : _M_val(reinterpret_cast<uintptr_t>(__c._M_pi))
        auto __x = reinterpret_cast<uintptr_t>(__c._M_pi);
      mutable __atomic_base<uintptr_t> _M_val{0};
      static constexpr uintptr_t _S_lock_bit{1};
/usr/include/c++/13.2.1/bits/shared_ptr_atomic.h
  using ::uintptr_t;
  using uintptr_t = __UINTPTR_TYPE__;
/usr/include/c++/13.2.1/cstdint
          return reinterpret_cast<_Tp*>(reinterpret_cast<uintptr_t>(this)
          _M_diff = reinterpret_cast<uintptr_t>(__arg) 
                    - reinterpret_cast<uintptr_t>(this);
      { return (reinterpret_cast<uintptr_t>(this->get())
        < reinterpret_cast<uintptr_t>(__rarg.get())); }
      { return (reinterpret_cast<uintptr_t>(this->get())
        == reinterpret_cast<uintptr_t>(__rarg.get())); }
      typedef __UINTPTR_TYPE__ uintptr_t;
      uintptr_t _M_diff;
            (reinterpret_cast<uintptr_t>(this) + _M_diff);
          _M_diff = reinterpret_cast<uintptr_t>(__arg) 
                    - reinterpret_cast<uintptr_t>(this);
      { return (reinterpret_cast<uintptr_t>(this->get())
        < reinterpret_cast<uintptr_t>(__rarg.get())); }
      { return (reinterpret_cast<uintptr_t>(this->get())
        == reinterpret_cast<uintptr_t>(__rarg.get())); }
      typedef __UINTPTR_TYPE__ uintptr_t;
      uintptr_t _M_diff;
/usr/include/c++/13.2.1/ext/pointer.h
    using uintptr_t = __UINTPTR_TYPE__;
    using native_handle_type = uintptr_t;
      auto __cb = [](void* __data, uintptr_t, const char* __filename,
        auto __cb2 = [](void* __data, uintptr_t, const char* __symname,
            uintptr_t, uintptr_t) {
      using uintptr_t = __UINTPTR_TYPE__;
      -> int (*) (void*, uintptr_t)
      auto __cb = +[](void* __data, uintptr_t __pc) {
        __cb = [](void* __data, uintptr_t __pc) {
/usr/include/c++/13.2.1/stacktrace
  using ::uintptr_t;
/usr/include/c++/13.2.1/tr1/cstdint

It would seem I could build it just fine using uintptr_t alone (no std:: namespace):

$ <pathTo>/bin/netcoredbg --buildinfo
.NET Core debugger 3.0.0-7 (3.0.0-7)

Build info:
      Build type:  Release
      Build date:  Nov 18 2023 17:31:21
      Target OS:   Linux
      Target arch: x64
      Hostname:    computerName

NetcoreDBG VCS info:  aafa6f3
CoreCLR VCS info:     6a4e500

Now to see if it borks on me from normal usage.

viewizard commented 7 months ago

Looks like std::uintptr_t was never part of standard and uintptr_t should be used instead. Will test this at work with clang16 and CI tests.

vindicatorr commented 7 months ago

If that's the case, I'm wondering how this was ever committed to a remote repository if it wouldn't pass locally if it was never part of standard.

viewizard commented 7 months ago

https://en.cppreference.com/w/cpp/header/cstdint Have uintptr_t but mention about std::uintptr_t in UINTPTR_MAX maximum value of std::uintptr_t.

Looks like define std::uintptr_t was clang developers decision, and now it was silently removed. I didn't found any mention about this in clang release related notices.

viewizard commented 7 months ago

Hmm... looks like I was wrong and uintptr_t is part of STL from C99/C++11. We compile c++ code with "-std=c++11" option. Have no idea why clang16 don't see std::uintptr_t now and propose uintptr_t instead, probably cstdint header was removed from STL headers we are using and at this point compiller don't see std::uintptr_t declaration...

If I am correct, this issue could be fixed with #include <cstdint> line added to src/interfaces/types.h.

vindicatorr commented 7 months ago

I did notice #ifdef _GLIBCXX_USE_C99_STDINT_TR1 in /usr/include/c++/13.2.1/cstdint which does lead to using ::uintptr_t;, but $ clang -dM -E -x c /dev/null | grep USE_C doesn't show any result.

I'm currently working through trying to get the latest vscode-csharp to output the vsix, so I'll try to remember to try the #include later. I had to update my build process for each main/master element I manually bring into csharp.

vindicatorr commented 7 months ago

Yup, adding cstdint did it. Everything's back up and running again.

viewizard commented 6 months ago

Fixed in upstream.