On 10/21/25 12:23 PM, Peter Maydell wrote:
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.
I don't mind doing both changes. I'll see if things are still working as
expected after doing them and send a fix.
And thanks for the analysis and heads up! I saw the Coverity Scan email in
my inbox today and haven't opened it yet. Seems like I'm on the hook for
CID 161401 and CID 1641393.
Daniel
+ (reg2 && strcasecmp(reg2, name) == 0)) {
+ *val = vals[i];
+ return true;
+ }
+ }
+
+ return false;
+}
thanks
-- PMM