Commit 6829cc6
authored
Fix Debugger timeout on 32 bit devices. (#9880)
Fixes: #9573
Context: 8e1c0e6
A customer reports that they are unable to attach a debugger to
32-bit Android apps:
> am start -a "android.intent.action.MAIN" -c "android.intent.category.LAUNCHER" -n "com.companyname.exemploandroid/crc64358cda76bdc6f75f.MainActivity"
> Starting: Intent { act=android.intent.action.MAIN cat=[android.intent.category.LAUNCHER] cmp=com.companyname.exemploandroid/crc64358cda76bdc6f75f.MainActivity }
…
[monodroid-debug] Trying to initialize the debugger with options: --debugger-agent=transport=dt_socket,loglevel=0,address=127.0.0.1:8805,server=y,embedding=1,timeout=8
…
[mono] Listening on 127.0.0.1:8805 (timeout=8 ms)...
[mono] debugger-agent: Timed out waiting to connect.
Of particular note from the above log messages is the `timeout=8`
value, which is considerably lower than it needs to be, which is a
[value in *milliseconds*][0]. This low value is responsible for the
subsequent `debugger-agent: Timed out` log message.
The `timeout=` value was introduced in commit 8e1c0e6, which used
`%d` to convert the `timeout_time` value; [for context][1]:
struct RuntimeOptions {
int64_t timeout_time = 0;
// …
};
// …
char *debug_arg = utils.monodroid_strdup_printf (
"--debugger-agent=transport=dt_socket,loglevel=%d,address=%s:%d,%sembedding=1,timeout=%d",
loglevel,
options.host,
options.sdb_port,
options.server ? "server=y," : "",
options.timeout_time
);
`options` is a `RuntimeOptions`, and `options.timeout_time` is thus a
`int64_t`, but it's being `printf`d via `%d`.
`%d` is supposed to be an *int*; from [**printf**(3)][2]:
> **d**, **i**
> The *int* argument is converted to signed decimal notation.
which means using `%d` for an `int64_t` will only work properly on
ILP64 targets. Android, notably, is an ILP32 (32-bit) or
LP64 (64-bit) target, *never* ILP64; it's a wonder this worked
*anywhere*, with any degree of reliability.
The fix is to realize that it doesn't even make sense to be
forwarding `RuntimeOptions::timeout_time` in this manner in the first
place: `RuntimeOptions::timeout_time` is compared against
`time(NULL)`, i.e. it's a [Unix time value][3] (*seconds* since
1970-01-01 UTC), not a value in milliseconds at all!
Replacing the use of `options.timeout_time` with 30000 allows for
a value that is reasonable for the target domain, *and* works
properly and consistently on both ILP32 (32-bit) and LP64 (64-bit)
Android targets.
[0]: https://github.com/dotnet/runtime/blob/9022f3a9b63b56234726606bc5547378b2d08f6b/src/mono/mono/component/debugger-agent.c#L581
[1]: https://github.com/dotnet/android/blob/8e1c0e6e2f4c41c9a24904bb1cea943357d78ac4/src/monodroid/jni/monodroid-glue-internal.hh#L98-L106
[2]: https://linux.die.net/man/3/printf
[3]: https://en.wikipedia.org/wiki/Unix_time1 parent a449dc9 commit 6829cc6
1 file changed
+2
-3
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
524 | 524 | | |
525 | 525 | | |
526 | 526 | | |
527 | | - | |
| 527 | + | |
528 | 528 | | |
529 | 529 | | |
530 | 530 | | |
531 | | - | |
532 | | - | |
| 531 | + | |
533 | 532 | | |
534 | 533 | | |
535 | 534 | | |
| |||
0 commit comments