-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[mono][debugger] Implementing mscordbi to support iCorDebug on mono runtime #47639
Changes from 36 commits
46acad4
086f051
7baa9d3
c6cd6e8
c5ce818
b3629ff
d78b7a2
130866a
759501d
74524ab
2dcaeb4
12127b9
45dce7b
217dc72
91e7497
d01a9ed
be8f56d
11a62cb
ac1735f
d3cc3b4
2ff0e68
a1cb8e6
9a98304
e3b74f4
b7de3c8
6c3c0a2
8e54659
cc38142
9fdaaa2
6e53c1a
a9d86ef
785d53c
f1de772
5939ca5
8a6f3e9
768b5e9
cdcbe2d
7cc409c
abc2aa3
208d222
5363bce
24bccdb
bafe8ee
5682529
8627c1a
0248812
10389fd
3ebfdd8
c5b4d4c
02349c3
c02a256
a16ce8e
94d957c
ae5f701
2aae463
b5de06d
a8f0318
25f24bc
6032aa3
8be3bfd
946f27a
df1ac59
1886964
5082ae4
eb47443
88f5021
8278121
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,18 @@ endif() | |
|
||
project(mono) | ||
|
||
set(CMAKE_C_FLAGS_CHECKED "") | ||
set(CMAKE_CXX_FLAGS_CHECKED "") | ||
set(CMAKE_EXE_LINKER_FLAGS_CHECKED "") | ||
set(CMAKE_SHARED_LINKER_FLAGS_CHECKED "") | ||
|
||
set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "") | ||
set(CMAKE_SHARED_LINKER_FLAGS_RELEASE "") | ||
set(CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO "") | ||
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "") | ||
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "") | ||
set(CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I would like a comment here what this is for (CoreCLR's checked build, I guess?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we really need to set these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunatelly I don't remember, I will remove and wait for CI. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @akoeplinger, I removed, now do you think it's okay to merge? |
||
|
||
include(GNUInstallDirs) | ||
include(CheckIncludeFile) | ||
include(CheckFunctionExists) | ||
|
@@ -699,6 +711,9 @@ endif() | |
### End of OS specific checks | ||
|
||
add_subdirectory(mono) | ||
if (NOT CMAKE_CROSSCOMPILING AND NOT TARGET_IOS AND NOT TARGET_ANDROID AND NOT TARGET_BROWSER AND NOT HOST_MACCATALYST) | ||
add_subdirectory(dbi) | ||
endif() | ||
|
||
configure_file(cmake/config.h.in config.h) | ||
configure_file(cmake/eglib-config.h.cmake.in mono/eglib/eglib-config.h) # TODO: eglib-config.h is not needed, we're using hardcoded eglib-config.hw |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
project(mscordbi) | ||
|
||
set(CMAKE_INCLUDE_CURRENT_DIR ON) | ||
|
||
set(CMAKE_EXPORT_COMPILE_COMMANDS ON) | ||
set(CMAKE_CXX_STANDARD 11) | ||
set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
|
||
set(CLR_DIR ${PROJECT_SOURCE_DIR}/../../coreclr) | ||
set(VM_DIR ${PROJECT_SOURCE_DIR}/../../coreclr/vm) | ||
set(CMAKE_OSX_ARCHITECTURES ${CMAKE_SYSTEM_PROCESSOR}) | ||
set(CMAKE_EXE_LINKER_FLAGS_CHECKED "") | ||
set(CMAKE_SHARED_LINKER_FLAGS_CHECKED "") | ||
set(CLR_CMAKE_HOST_ARCH ${CMAKE_GENERATOR_PLATFORM}) | ||
set(FEATURE_EVENT_TRACE 0) | ||
|
||
if(HOST_WIN32) | ||
if(HOST_X86) | ||
set(CLR_CMAKE_HOST_ARCH x86) | ||
elseif(HOST_ARM64) | ||
set(CLR_CMAKE_HOST_ARCH arm64) | ||
elseif(HOST_ARM) | ||
set(CLR_CMAKE_HOST_ARCH arm) | ||
elseif(HOST_AMD64) | ||
set(CLR_CMAKE_HOST_ARCH x64) | ||
endif() | ||
endif() | ||
|
||
add_definitions(-DDBI_COMPONENT_MONO) | ||
|
||
include_directories( | ||
${CMAKE_CURRENT_SOURCE_DIR}/../.. | ||
${PROJECT_SOURCE_DIR}/../ | ||
${PROJECT_SOURCE_DIR}/../dbi | ||
${PROJECT_SOURCE_DIR}/../dbi/socket-dbi | ||
${PROJECT_SOURCE_DIR}/../../coreclr/md/enc | ||
${PROJECT_SOURCE_DIR}/../../coreclr/inc | ||
${PROJECT_SOURCE_DIR}/../../coreclr/md/inc | ||
${PROJECT_SOURCE_DIR}/../../coreclr/md/compiler) | ||
|
||
set(mscorbi_sources_base | ||
cordb.cpp | ||
cordb.h | ||
cordb-appdomain.cpp | ||
cordb-appdomain.h | ||
cordb-assembly.cpp | ||
cordb-assembly.h | ||
cordb-blocking-obj.cpp | ||
cordb-blocking-obj.h | ||
cordb-breakpoint.cpp | ||
cordb-breakpoint.h | ||
cordb-chain.cpp | ||
cordb-chain.h | ||
cordb-class.cpp | ||
cordb-class.h | ||
cordb-code.cpp | ||
cordb-code.h | ||
cordb-eval.cpp | ||
cordb-eval.h | ||
cordb-frame.cpp | ||
cordb-frame.h | ||
cordb-function.cpp | ||
cordb-function.h | ||
cordb-process.cpp | ||
cordb-process.h | ||
cordb-register.cpp | ||
cordb-register.h | ||
cordb-stepper.cpp | ||
cordb-stepper.h | ||
cordb-thread.cpp | ||
cordb-thread.h | ||
cordb-type.cpp | ||
cordb-type.h | ||
cordb-value.cpp | ||
cordb-value.h | ||
) | ||
|
||
|
||
if(HOST_DARWIN) | ||
set(OS_LIBS "-framework CoreFoundation" "-framework Foundation") | ||
elseif(HOST_LINUX) | ||
set(OS_LIBS pthread m dl) | ||
elseif(HOST_WIN32) | ||
set(OS_LIBS bcrypt.lib Mswsock.lib ws2_32.lib psapi.lib version.lib advapi32.lib winmm.lib kernel32.lib) | ||
endif() | ||
|
||
addprefix(mscorbi_sources ../dbi/ "${mscorbi_sources_base}") | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/socket-dbi) | ||
|
||
include(${PROJECT_SOURCE_DIR}/../../../eng/native/configuretools.cmake) | ||
include(${PROJECT_SOURCE_DIR}/../../../eng/native/configurepaths.cmake) | ||
include(${PROJECT_SOURCE_DIR}/../../../eng/native/configureplatform.cmake) | ||
include(${PROJECT_SOURCE_DIR}/../../../eng/native/configurecompiler.cmake) | ||
|
||
if (CLR_CMAKE_HOST_UNIX) | ||
include_directories("${PROJECT_SOURCE_DIR}/../../coreclr/pal/inc") | ||
include_directories("${PROJECT_SOURCE_DIR}/../../coreclr/pal/inc/rt") | ||
include_directories("${PROJECT_SOURCE_DIR}/../../coreclr/pal/src/safecrt") | ||
|
||
append("-Wno-missing-prototypes -Wno-pointer-arith -Wno-macro-redefined" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/pal pal) | ||
|
||
include_directories("../../coreclr/pal/inc/rt/cpp") | ||
add_compile_options(-nostdinc) | ||
endif (CLR_CMAKE_HOST_UNIX) | ||
|
||
include_directories("../../coreclr/pal/prebuilt/inc") | ||
include_directories("../../coreclr/nativeresources") | ||
|
||
if (CLR_CMAKE_HOST_UNIX) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/nativeresources nativeresources) | ||
endif() | ||
|
||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/md/runtime md/runtime) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/md/compiler md/compiler) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Has there been any discussion in the past or a plan in place about these cross-runtime build links? I'm not sure if this is intended as a temporary step or something more permanent. The main example that I am aware of for native code shared between mono and coreclr is @lateralusX EventPipe work. I could imagine other components following the same trajectory, hopefully with vastly less labor involved assuming we are just moving them to another directory and not attempting to rewrite them from C++ to C. From my POV the main concern is setting expectations with developers about what pieces of code are shared cross-runtime so that everyone knows what they need to keep in mind when making changes or running tests. Moving directories isn't the only way to accomplish that, but it appears to be the pattern we've adopted so far. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion this would be permanent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a permanent dependency. I think it would be good to componentize this part of CoreCLR. I'm not sure whether moving this to src/native is worth it. It's still more closely tied to CoreCLR than to Mono. (Mono itself doesn't use it. Just mono's cordb). Maybe moving some files around under src/coreclr/md to more clearly define what is a separable component here makes sense. Certainly we should teach our CI to run debugger tests on mono (or at least build Mono's cordb when those files change). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a strong opinion as most of the developers I work with day to day rarely make changes in these areas of the code. Adding @davidwrighton or @jkotas who might be better positioned to represent devs that touch that code more frequently in case they have any concerns. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I expect that we will eventually end up in a place where majority of the unmanaged code is shared between CoreCLR and Mono, similar how vast majority of managed code is shared between CoreCLR and Mono today. I think it is good idea to be moving the shared pieces under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lambdageek, if we decide to move, should move files in this PR or in another one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My personal preference is to separate re-organization PRs from PRs that add functionality. Also this PR is already pretty big. I would do it in a follow-up PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that it is a good idea to do the move in separate PR. |
||
|
||
include(${PROJECT_SOURCE_DIR}/../../coreclr/clrdefinitions.cmake) | ||
include_directories(${CMAKE_CURRENT_BINARY_DIR}/../) | ||
include_directories(${CMAKE_CURRENT_BINARY_DIR}/../inc/) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/md/enc md/enc) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/utilcode utilcode) | ||
if (CLR_CMAKE_HOST_UNIX) | ||
add_subdirectory(${PROJECT_SOURCE_DIR}/../../coreclr/palrt palrt) | ||
append("-Wno-strict-prototypes -Wno-deprecated -Wno-pointer-arith" CMAKE_C_FLAGS CMAKE_CXX_FLAGS) | ||
endif (CLR_CMAKE_HOST_UNIX) | ||
|
||
add_library(mscordbi SHARED "${mscorbi_sources};${PROJECT_SOURCE_DIR}/../mono/mini/debugger-protocol.c;${PROJECT_SOURCE_DIR}/../../coreclr/pal/prebuilt/idl/xcordebug_i.cpp;${PROJECT_SOURCE_DIR}/../../coreclr/pal/prebuilt/idl/cordebug_i.cpp") | ||
|
||
#SET(CMAKE_C_COMPILER ${CMAKE_CXX_COMPILER}) | ||
|
||
set_source_files_properties(${PROJECT_SOURCE_DIR}/../mono/mini/debugger-protocol.c PROPERTIES LANGUAGE CXX) | ||
|
||
set(COREDBI_LIBRARIES | ||
utilcodestaticnohost | ||
mdruntime-dbi | ||
mdcompiler-dbi | ||
mdruntimerw-dbi | ||
socket-dbi | ||
${OS_LIBS} | ||
) | ||
|
||
if(CLR_CMAKE_HOST_UNIX) | ||
list(APPEND COREDBI_LIBRARIES | ||
coreclrpal | ||
palrt | ||
nativeresourcestring | ||
) | ||
endif() | ||
|
||
target_link_libraries(mscordbi ${COREDBI_LIBRARIES} ) | ||
install(TARGETS mscordbi DESTINATION lib) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akoeplinger @ViktorHofer is this ok?
The intention is to build a piece of CoreCLR for Mono's new cordb library - Is it the case that during the mono build we use slightly different system names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this happening? the values we use in src/mono/CMakeLists.txt are upper-cased (
ARM64
andX86
) so I'm not sure this is really what you want?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if you can see the errors in this commit: a1cb8e6
These were the CI failures without this change:
Build Linux arm Debug AllSubsets_Mono
Build Linux arm64 Debug AllSubsets_Mono_LLVMJIT
Build Linux arm64 Release AllSubsets_Mono_LLVMAOT
Mono Product Build Linux arm64 debug
Mono Product Build Linux arm64 release
Mono llvmaot Product Build Linux arm64 release