On Fri, 3 Oct 2025 at 04:29, <[email protected]> wrote:
>
> From: Daniel Henrique Barboza <[email protected]>
>
> The MonitorDef API is related to two HMP monitor commands: 'p' and 'x':
>
> (qemu) help p
> print|p /fmt expr -- print expression value (use $reg for CPU register access)
> (qemu) help x
> x /fmt addr -- virtual memory dump starting at 'addr'
>
> For x86, one of the few targets that implements it, it is possible to
> print the PC register value with $pc and use the PC value in the 'x'
> command as well.
>
> Those 2 commands are hooked into get_monitor_def(), called by
> exp_unary() in hmp.c. The function tries to fetch a reg value in two
> ways: by reading them directly via a target_monitor_defs array or using
> a target_get_monitor_def() helper. In RISC-V we have *A LOT* of
> registers and this number will keep getting bigger, so we're opting out
> of an array declaration.
>
> We're able to retrieve all regs but vregs because the API only fits an
> uint64_t and vregs have 'vlen' size that are bigger than that.
Hi; Coverity complains about these functions
(CID 161401, CID 1641393):
> +static bool reg_is_ulong_integer(CPURISCVState *env, const char *name,
> + target_ulong *val, bool is_gprh)
> +{
> + const char * const *reg_names;
> + target_ulong *vals;
> +
> + if (is_gprh) {
> + reg_names = riscv_int_regnamesh;
> + vals = env->gprh;
> + } else {
> + reg_names = riscv_int_regnames;
> + vals = env->gpr;
> + }
> +
> + for (int i = 0; i < 32; i++) {
> + g_autofree char *reg_name = g_strdup(reg_names[i]);
> + char *reg1 = strtok(reg_name, "/");
> + char *reg2 = strtok(NULL, "/");
> +
> + if (strcasecmp(reg1, name) == 0 ||
Coverity complains because we call strtok() to get reg1,
and that might return NULL, but strcasecmp() wants a
non-NULL pointer.
Similarly with the use of strtok() in reg_is_u64_fpu().
We could fix this with an assert(reg1) since the
names are compile-time fixed and it would be an error
for the string to be empty.
But taking a step back, strtok() isn't thread safe.
Maybe we should use g_strsplit() instead ?
More speculatively, do we need to care about locale here?
strcasecmp() does a locale-aware comparison, which is
probably not what we want. (Notoriously Turkish locales
don't have the upper/lowercase mapping you would expect
for "i" and "I".) glib has a g_ascii_strcasecmp() which
might be helpful here.
> + (reg2 && strcasecmp(reg2, name) == 0)) {
> + *val = vals[i];
> + return true;
> + }
> + }
> +
> + return false;
> +}
thanks
-- PMM
