Turn Debugger.Log, Debugger.Launch, and Delegate.BindToMethodName to QCalls #48211
Conversation
ghost
added
the
|
Tagging subscribers to this area: @tommcdon Issue Details
Some changes to add the BEGIN_QCALL/END_QCALL macros.
Uses same helper as Debugger.Log
|
|
I haven't ported the other call (ValidateObject), but wanted to get a feel if this is what we need to do. Will open the 3.1 PR after this. I am currently testing this locally. |
It may be useful to make this change in master first, and then port it to servicing. |
hoyosjs
force-pushed
the
juhoyosa/qcall-change5
branch
from
239d98a to
feab66b
Compare
src/coreclr/src/vm/comdelegate.cpp
Outdated
There was a problem hiding this comment.
You still have to protect the GC references like before. Number of the calls below triggers GC - it is not ok to have unprotected GC reference in a local while calling GC triggering method.
The easiest way to do this is to keep the gc struct and use GCPROTECT_BEGIN/END macro to protect it.
It may be possible to avoid GCPROTECT_BEGIN/END and instead re-fetch the value each time from ObjectHandleOnStack. It would be more involved change.
There was a problem hiding this comment.
I think this shows that we should do the change in dotnet/runtime first so that subtle GC holes (if there are any) will show up as intermittent crashes in CI. The servicing CI runs much less often and so these bugs would take a lot longer to surface.
There was a problem hiding this comment.
I will do this on master first, I just can't turn this into draft, so I continued working here first.
hoyosjs
force-pushed
the
juhoyosa/qcall-change5
branch
from
feab66b to
f647808
Compare
|
@jkotas yeah. I am closing this one. |
Some changes to add the BEGIN_QCALL/END_QCALL macros.
Uses same helper as Debugger.Log, so convert too.