[android][debugger] Implement support to debug after hot reload. #55722
Conversation
|
Tagging subscribers to this area: @thaystg Issue Details
|
There was a problem hiding this comment.
lgtm, although I'm not sure about source_files[0].
Also @thaystg did you try multiple updates (ie, change a method, then apply the update, then change it again and apply update again)? This code looks like it will work for that case, but I just wanted to ask if it's something you looked at.
| srcfile = sinfo->source_file; | ||
| } | ||
| PRINT_DEBUG_MSG (10, "IL%x -> %s:%d %d %d %d\n", sp->il_offset, srcfile, sp->line, sp->column, sp->end_line, sp->end_column); | ||
| buffer_add_int (buf, sp->il_offset); | ||
| buffer_add_int (buf, sp->line); | ||
| if (CHECK_PROTOCOL_VERSION (2, 13)) | ||
| buffer_add_int (buf, source_files [i]); | ||
| buffer_add_int (buf, i >= n_il_offsets_original ? source_files [0] : source_files [i]); |
There was a problem hiding this comment.
can source_files [0] ever be -1 here? Also why is this ok?
There was a problem hiding this comment.
This is because as I understand on EnC pdb we don't have information about source code.
And we can have more il_offsets then the original pdb, like when you add more lines, so I get the source of the first breakpoint.
There was a problem hiding this comment.
So source_files[] is an array of indexes of the source documents for a particular method? And each source_files[x] says where sequence point number x comes from? So this is saying "if we got additional sequence points beyond the original ones, assume they're from the same document as the first original sequence point?
I guess that makes sense (at least until we have to deal with partial C# methods - although I'm not sure if those have sequence points in multiple files).
What if the original method body didn't have any sequence points? Can that happen? What if the method was completely empty at the start? I guess there's still a ret in there - and maybe we have a sequence point?
Will debugger-libs be ok if we send back source_files[0] and it is -1?
|
AzDO and GitHub are out of sync - the test results for a95d832 are here: https://dev.azure.com/dnceng/public/_build/results?buildId=1239524&view=results |
Yes, I tested for multiple updates, I will add a video with it working here! :) |
| MonoDebugLocalsInfo* | ||
| mono_ppdb_lookup_locals_enc (MonoImage *image, int method_idx) | ||
| { | ||
| return mono_ppdb_lookup_locals_internal (image, method_idx + 1, TRUE); |
There was a problem hiding this comment.
nit: maybe it would be better to store a 1-based offset in MonoDebugInformationEnc:idx.
Related to this PR:
mono/debugger-libs#344