Skip to content
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.

Sign up for GitHub

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

[android][debugger] Implement support to debug after hot reload. #55722

Merged
merged 2 commits into from
Jul 15, 2021
Merged

[android][debugger] Implement support to debug after hot reload. #55722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ad167e
Implement support to debug after hot reload.
thaystg Jul 15, 2021
a95d832
Remove unused variable.
thaystg Jul 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/mono/mono/component/debugger-agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8449,7 +8449,7 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
ERROR_DECL (error);
MonoDebugMethodInfo *minfo;
char *source_file;
int i, j, n_il_offsets;
int i, j, n_il_offsets, n_il_offsets_original;
int *source_files;
GPtrArray *source_file_list;
MonoSymSeqPoint *sym_seq_points;
Expand All @@ -8472,7 +8472,8 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
break;
}

mono_debug_get_seq_points (minfo, &source_file, &source_file_list, &source_files, &sym_seq_points, &n_il_offsets);
mono_debug_get_seq_points (minfo, &source_file, &source_file_list, &source_files, NULL, &n_il_offsets_original);
mono_debug_get_seq_points (minfo, NULL, NULL, NULL, &sym_seq_points, &n_il_offsets);
buffer_add_int (buf, header->code_size);
if (CHECK_PROTOCOL_VERSION (2, 13)) {
buffer_add_int (buf, source_file_list->len);
Expand All @@ -8494,14 +8495,17 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
const char *srcfile = "";

if (source_files [i] != -1) {
MonoDebugSourceInfo *sinfo = (MonoDebugSourceInfo *)g_ptr_array_index (source_file_list, source_files [i]);
int idx = i;
if (i >= n_il_offsets_original)
idx = 0;
MonoDebugSourceInfo *sinfo = (MonoDebugSourceInfo *)g_ptr_array_index (source_file_list, source_files [idx]);
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]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can source_files [0] ever be -1 here? Also why is this ok?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

if (CHECK_PROTOCOL_VERSION (2, 19))
buffer_add_int (buf, sp->column);
if (CHECK_PROTOCOL_VERSION (2, 32)) {
Expand Down
41 changes: 22 additions & 19 deletions src/mono/mono/component/hot_reload.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "mono/utils/mono-lazy-init.h"
#include "mono/utils/mono-logger-internals.h"
#include "mono/utils/mono-path.h"
#include "mono/metadata/mono-debug.h"


#include <mono/component/hot_reload.h>
Expand Down Expand Up @@ -321,13 +322,21 @@ baseline_info_lookup (MonoImage *base_image)
static DeltaInfo*
delta_info_init (MonoImage *image_dmeta, MonoImage *image_base, BaselineInfo *base_info, uint32_t generation);

static void
free_ppdb_entry (gpointer key, gpointer val, gpointer user_data)
{
g_free (val);
}

static void
delta_info_destroy (DeltaInfo *dinfo)
{
if (dinfo->method_table_update)
g_hash_table_destroy (dinfo->method_table_update);
if (dinfo->method_ppdb_table_update)
if (dinfo->method_ppdb_table_update) {
g_hash_table_foreach (dinfo->method_ppdb_table_update, free_ppdb_entry, NULL);
g_hash_table_destroy (dinfo->method_ppdb_table_update);
}
g_free (dinfo);
}

Expand Down Expand Up @@ -1182,7 +1191,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, gconstpointer
}

static void
set_update_method (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, MonoImage *image_dmeta, DeltaInfo *delta_info, uint32_t token_index, const char* il_address, const char* pdb_address)
set_update_method (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, MonoImage *image_dmeta, DeltaInfo *delta_info, uint32_t token_index, const char* il_address, MonoDebugInformationEnc* pdb_address)
{
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "setting method 0x%08x in g=%d IL=%p", token_index, generation, (void*)il_address);
/* FIXME: this is a race if other threads are doing a lookup. */
Expand All @@ -1191,7 +1200,7 @@ set_update_method (MonoImage *image_base, BaselineInfo *base_info, uint32_t gene
g_hash_table_insert (delta_info->method_ppdb_table_update, GUINT_TO_POINTER (token_index), (gpointer) pdb_address);
}

static const char *
static MonoDebugInformationEnc *
hot_reload_get_method_debug_information (MonoImage *image_dppdb, int idx)
{
if (!image_dppdb)
Expand All @@ -1204,21 +1213,15 @@ hot_reload_get_method_debug_information (MonoImage *image_dppdb, int idx)
mono_metadata_decode_row (table_encmap, i, cols, MONO_ENCMAP_SIZE);
int map_token = cols [MONO_ENCMAP_TOKEN];
int token_table = mono_metadata_token_table (map_token);
if (token_table != MONO_TABLE_METHODBODY)
continue;
int token_index = mono_metadata_token_index (map_token);
if (token_index == idx)
{
guint32 cols [MONO_METHODBODY_SIZE];
MonoTableInfo *methodbody_table = &image_dppdb->tables [MONO_TABLE_METHODBODY];
mono_metadata_decode_row (methodbody_table, i, cols, MONO_METHODBODY_SIZE);
if (!cols [MONO_METHODBODY_SEQ_POINTS])
return NULL;

const char *ptr = mono_metadata_blob_heap (image_dppdb, cols [MONO_METHODBODY_SEQ_POINTS]);
return ptr;
if (token_table == MONO_TABLE_METHODBODY) {
int token_index = mono_metadata_token_index (map_token);
if (token_index == idx) {
MonoDebugInformationEnc *encDebugInfo = g_new0 (MonoDebugInformationEnc, 1);
encDebugInfo->idx = i;
encDebugInfo->image = image_dppdb;
return encDebugInfo;
}
}
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb encmap i=%d: token=0x%08x (table=%s)", i, map_token, mono_meta_table_name (token_table));
}
return NULL;
}
Expand Down Expand Up @@ -1304,7 +1307,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen
int rva = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_METHOD], mapped_token - 1, MONO_METHOD_RVA);
if (rva < dil_length) {
char *il_address = ((char *) dil_data) + rva;
const char *method_debug_information = hot_reload_get_method_debug_information (image_dppdb, token_index);
MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (image_dppdb, token_index);
set_update_method (image_base, base_info, generation, image_dmeta, delta_info, token_index, il_address, method_debug_information);
} else {
/* rva points probably into image_base IL stream. can this ever happen? */
Expand Down Expand Up @@ -1385,7 +1388,7 @@ hot_reload_apply_changes (MonoImage *image_base, gconstpointer dmeta_bytes, uint
MonoImage *image_dpdb = NULL;
if (dpdb_length > 0)
{
MonoImage *image_dpdb = image_open_dmeta_from_data (image_base, generation, dpdb_bytes_orig, dpdb_length);
image_dpdb = image_open_dmeta_from_data (image_base, generation, dpdb_bytes_orig, dpdb_length);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image string size: 0x%08x", image_dpdb->heap_strings.size);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image user string size: 0x%08x", image_dpdb->heap_us.size);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image blob heap addr: %p", image_dpdb->heap_blob.data);
Expand Down
71 changes: 49 additions & 22 deletions src/mono/mono/metadata/debug-mono-ppdb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,19 +543,29 @@ mono_ppdb_get_seq_points_internal (const char* ptr, MonoSymSeqPoint **seq_points

if (n_seq_points) {
*n_seq_points = sps->len;
g_assert (seq_points);
*seq_points = g_new (MonoSymSeqPoint, sps->len);
memcpy (*seq_points, sps->data, sps->len * sizeof (MonoSymSeqPoint));
if (seq_points) {
*seq_points = g_new (MonoSymSeqPoint, sps->len);
memcpy (*seq_points, sps->data, sps->len * sizeof (MonoSymSeqPoint));
}
}
int sps_len = sps->len;
g_array_free (sps, TRUE);
return sps_len;
}

void
mono_ppdb_get_seq_points_enc (const char* ptr, MonoSymSeqPoint **seq_points, int *n_seq_points)
gboolean
mono_ppdb_get_seq_points_enc (MonoImage *image, int idx, MonoSymSeqPoint **seq_points, int *n_seq_points)
{
guint32 cols [MONO_METHODBODY_SIZE];
MonoTableInfo *tables = image->tables;
MonoTableInfo *methodbody_table = &tables [MONO_TABLE_METHODBODY];
mono_metadata_decode_row (methodbody_table, idx, cols, MONO_METHODBODY_SIZE);
if (!cols [MONO_METHODBODY_SEQ_POINTS])
return FALSE;

const char *ptr = mono_metadata_blob_heap (image, cols [MONO_METHODBODY_SEQ_POINTS]);
mono_ppdb_get_seq_points_internal (ptr, seq_points, n_seq_points, 0, NULL, NULL, NULL, NULL, NULL, NULL, FALSE);
return TRUE;
}

void
Expand Down Expand Up @@ -619,27 +629,16 @@ mono_ppdb_get_seq_points (MonoDebugMethodInfo *minfo, char **source_file, GPtrAr

}

MonoDebugLocalsInfo*
mono_ppdb_lookup_locals (MonoDebugMethodInfo *minfo)
static MonoDebugLocalsInfo*
mono_ppdb_lookup_locals_internal (MonoImage *image, int method_idx, gboolean is_enc)
{
MonoPPDBFile *ppdb = minfo->handle->ppdb;
MonoImage *image = ppdb->image;
MonoDebugLocalsInfo *res;
MonoTableInfo *tables = image->tables;
MonoMethod *method = minfo->method;

guint32 cols [MONO_LOCALSCOPE_SIZE];
guint32 locals_cols [MONO_LOCALVARIABLE_SIZE];
int i, lindex, sindex, method_idx, start_scope_idx, scope_idx, locals_idx, locals_end_idx, nscopes;
MonoDebugLocalsInfo *res;
MonoMethodSignature *sig;

if (!method->token)
return NULL;

sig = mono_method_signature_internal (method);
if (!sig)
return NULL;

method_idx = mono_metadata_token_index (method->token);

int i, lindex, sindex, locals_idx, locals_end_idx, nscopes, start_scope_idx, scope_idx;

start_scope_idx = mono_metadata_localscope_from_methoddef (image, method_idx);

Expand Down Expand Up @@ -725,6 +724,34 @@ mono_ppdb_lookup_locals (MonoDebugMethodInfo *minfo)
return res;
}

MonoDebugLocalsInfo*
mono_ppdb_lookup_locals_enc (MonoImage *image, int method_idx)
{
return mono_ppdb_lookup_locals_internal (image, method_idx + 1, TRUE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe it would be better to store a 1-based offset in MonoDebugInformationEnc:idx.

}

MonoDebugLocalsInfo*
mono_ppdb_lookup_locals (MonoDebugMethodInfo *minfo)
{
MonoPPDBFile *ppdb = minfo->handle->ppdb;
MonoImage *image = ppdb->image;
MonoMethod *method = minfo->method;
int method_idx;
MonoMethodSignature *sig;

if (!method->token)
return NULL;

sig = mono_method_signature_internal (method);
if (!sig)
return NULL;

method_idx = mono_metadata_token_index (method->token);


return mono_ppdb_lookup_locals_internal (image, method_idx, FALSE);
}

/*
* We use this to pass context information to the row locator
*/
Expand Down
7 changes: 5 additions & 2 deletions src/mono/mono/metadata/debug-mono-ppdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ mono_ppdb_lookup_location (MonoDebugMethodInfo *minfo, uint32_t offset);
void
mono_ppdb_get_seq_points (MonoDebugMethodInfo *minfo, char **source_file, GPtrArray **source_file_list, int **source_files, MonoSymSeqPoint **seq_points, int *n_seq_points);

void
mono_ppdb_get_seq_points_enc (const char* ptr, MonoSymSeqPoint **seq_points, int *n_seq_points);
gboolean
mono_ppdb_get_seq_points_enc (MonoImage *image, int idx, MonoSymSeqPoint **seq_points, int *n_seq_points);

MonoDebugLocalsInfo*
mono_ppdb_lookup_locals (MonoDebugMethodInfo *minfo);

MonoDebugLocalsInfo*
mono_ppdb_lookup_locals_enc (MonoImage *image, int method_idx);

MonoDebugMethodAsyncInfo*
mono_ppdb_lookup_method_async_debug_info (MonoDebugMethodInfo *minfo);

Expand Down
21 changes: 16 additions & 5 deletions src/mono/mono/metadata/mono-debug.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,17 @@ mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb)
{
MonoDebugMethodInfo *minfo;
MonoDebugLocalsInfo *res;

MonoImage* img = m_class_get_image (method->klass);
if (img->has_updates) {
int idx = mono_metadata_token_index (method->token);
MonoDebugInformationEnc *mdie = (MonoDebugInformationEnc *) mono_metadata_update_get_updated_method_ppdb (img, idx);
if (mdie != NULL) {
res = mono_ppdb_lookup_locals_enc (mdie->image, mdie->idx);
if (res != NULL)
return res;
}
}

if (mono_debug_format == MONO_DEBUG_FORMAT_NONE)
return NULL;
Expand Down Expand Up @@ -1117,12 +1128,12 @@ void
mono_debug_get_seq_points (MonoDebugMethodInfo *minfo, char **source_file, GPtrArray **source_file_list, int **source_files, MonoSymSeqPoint **seq_points, int *n_seq_points)
{
MonoImage* img = m_class_get_image (minfo->method->klass);
if (img->has_updates) {
if (img->has_updates && !source_file_list) {
int idx = mono_metadata_token_index (minfo->method->token);
gpointer ptr = mono_metadata_update_get_updated_method_ppdb (img, idx);
if (ptr != NULL) {
mono_ppdb_get_seq_points_enc (ptr, seq_points, n_seq_points);
return;
MonoDebugInformationEnc *mdie = (MonoDebugInformationEnc *) mono_metadata_update_get_updated_method_ppdb (img, idx);
if (mdie != NULL) {
if (mono_ppdb_get_seq_points_enc (mdie->image, mdie->idx, seq_points, n_seq_points))
return;
}
}
if (minfo->handle->ppdb)
Expand Down
7 changes: 7 additions & 0 deletions src/mono/mono/metadata/mono-debug.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ typedef struct _MonoDebugMethodInfo MonoDebugMethodInfo;
typedef struct _MonoDebugLocalsInfo MonoDebugLocalsInfo;
typedef struct _MonoDebugMethodAsyncInfo MonoDebugMethodAsyncInfo;
typedef struct _MonoDebugSourceLocation MonoDebugSourceLocation;
typedef struct _MonoDebugInformationEnc MonoDebugInformationEnc;

typedef struct _MonoDebugList MonoDebugList;

Expand Down Expand Up @@ -110,6 +111,12 @@ struct _MonoDebugSourceLocation {
uint32_t il_offset;
};


struct _MonoDebugInformationEnc {
MonoImage *image;
int idx;
};

MONO_API mono_bool mono_debug_enabled (void);

/*
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/debugger/DebuggerTestSuite/BreakpointTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,11 @@ public async Task DebugHotReloadMethodAddBreakpoint()

var locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value<string>());
CheckNumber(locals, "a", 10);
pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 29, 12, "StaticMethod3");
pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 30, 12, "StaticMethod3");
locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value<string>());
CheckNumber(locals, "b", 15);

pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 29, 12, "StaticMethod3");
pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 30, 12, "StaticMethod3");
locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value<string>());
CheckBool(locals, "c", true);
}
Expand Down