Rework debugging interface to allow coexisting for multiple frameworks. #3818
Conversation
runtime/src/debug/cpp/KDebug.cpp
Outdated
| return sizeof(debugBuffer); | ||
| } | ||
|
|
||
| // Auxilary function which can be called by developer/debugger to inspect an object. | ||
| RUNTIME_USED KInt Konan_DebugObjectToUtf8Array(KRef obj, char* buffer, KInt bufferSize) { | ||
| RUNTIME_USED RUNTIME_WEAK KInt Konan_DebugObjectToUtf8Array(KRef obj, char* buffer, KInt bufferSize) { |
There was a problem hiding this comment.
Debugger still uses this API, so now it allows an object of one runtime to leak into other runtime.
There was a problem hiding this comment.
Now it's up to bindings if use legacy API or rely upon the new API. So this is runtime part of the mechanism.
There was a problem hiding this comment.
Legacy API turns out to be unsafe.
Why not make it safe by implementing public API through function pointers from "extended info"?
runtime/src/debug/cpp/KDebug.cpp
Outdated
|
|
||
| // Auxilary function which can be called by developer/debugger to inspect an object. | ||
| RUNTIME_USED RUNTIME_WEAK int Konan_DebugObjectToUtf8Array(KRef obj, char* buffer, int bufferSize) { | ||
| auto* impl = getImpl<int (*)(KRef, char*, int)>(obj, DO_DebugObjectToUtf8Array); |
There was a problem hiding this comment.
Switched all to explicit int32_t.
runtime/src/debug/cpp/KDebug.cpp
Outdated
| } | ||
|
|
||
| RUNTIME_USED RUNTIME_WEAK | ||
| void* Konan_DebugGetOperation(KRef obj, /* Konan_DebugOperation */ int32_t operation) { |
There was a problem hiding this comment.
Is this public debug API actually required?
There was a problem hiding this comment.
Mostly for the completness of the debug API, but indeed, we may not expose it and enum.
runtime/src/debug/cpp/KDebug.cpp
Outdated
| } | ||
|
|
||
| // IMPORTANT: when updating, fix `debugOperationsSize` in RTTIGenerator.kt. | ||
| RUNTIME_USED RUNTIME_WEAK |
There was a problem hiding this comment.
Why are these attributes required?
There was a problem hiding this comment.
So that we could use them from Kotlin side and not conflict on dups.
There was a problem hiding this comment.
What do you mean by "Kotlin side"? Generated code?
There are plenty of runtime functions that aren't RUNTIME_USED RUNTIME_WEAK and can be properly used from generated code without conflicts.
There was a problem hiding this comment.
OK, if needed we can easily add it.
| private val debugOperations: ConstValue by lazy { | ||
| if (debugRuntimeOrNull != null) { | ||
| val external = LLVMGetNamedGlobal(debugRuntimeOrNull, "Konan_debugOperationsList")!! | ||
| val local = LLVMAddGlobal(context.llvmModule, LLVMTypeOf(external),"Konan_debugOperationsList")!! |
There was a problem hiding this comment.
LLVMTypeOf(external) is not the type of the global itself, it is the type of the pointer to it.
As a result, Konan_debugOperationsList is declared here as external global [14 x i8*]*, while originally defined as global [14 x i8*].
This is also likely the cause of "LLVMGetArrayLength() doesn't work as expected" below.
There was a problem hiding this comment.
Type of the global added here is still wrong.
There was a problem hiding this comment.
Not sure if it's important, but fixed.
| @@ -395,6 +395,30 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { | |||
| private fun mapRuntimeType(type: LLVMTypeRef): Int = | |||
| runtimeTypeMap[type] ?: throw Error("Unmapped type: ${llvmtype2string(type)}") | |||
|
|
|||
| private val debugRuntimeOrNull: LLVMModuleRef? by lazy { | |||
| context.config.runtimeNativeLibraries.singleOrNull { it.endsWith("debug.bc")}?.let { | |||
| parseBitcodeFile(it) | |||
There was a problem hiding this comment.
Should be disposed.
There was a problem hiding this comment.
Alternatively we can avoid loading this module at all.
debugOperationsSize could be stored in a runtime-defined struct containing debug operations array (or a pointer to it) too.
Then it would be enough to have a single pointer in ExtendedTypeInfo (which can also be considered an optimization).
There was a problem hiding this comment.
Prefer disposing solution.
| @@ -105,6 +105,9 @@ internal class RTTIGeneratorVisitor(context: Context) : IrElementVisitorVoid { | |||
| } | |||
| } | |||
|
|
|||
| fun dispose() { | |||
| generator.dispose() | |||
There was a problem hiding this comment.
It's not enough since there are more instances of RTTIGenerator.
No description provided.