Yodel Eldar <yodel.el...@gmail.com> writes:
> Currently, execlog searches for a space separator between the
> instruction mnemonic and operands, but some disassemblers, e.g. Alpha's,
> use a tab separator instead; this results in a null pointer being passed
> as the haystack in g_strstr during a subsequent register search, i.e.
> undefined behavior, because of a missing null check.
>
> This patch adds tab to the separator search and a null check on the
> result.
>
> Also, existing, affected pointers are changed to const.
>
> Lastly, a break statement was added to immediately terminate the
> register search when a user-requested register is found in the current
> instruction as a trivial optimization, because searching for the
> remaining requested registers is unnecessary once one is found.
>
> Signed-off-by: Yodel Eldar <yodel.el...@gmail.com>
> ---
> contrib/plugins/execlog.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> index d67d010761..08fc1f12d4 100644
> --- a/contrib/plugins/execlog.c
> +++ b/contrib/plugins/execlog.c
> @@ -232,12 +232,15 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
> qemu_plugin_tb *tb)
> */
> if (disas_assist && rmatches) {
> check_regs_next = false;
> - gchar *args = g_strstr_len(insn_disas, -1, " ");
> - for (int n = 0; n < all_reg_names->len; n++) {
> - gchar *reg = g_ptr_array_index(all_reg_names, n);
> - if (g_strrstr(args, reg)) {
> - check_regs_next = true;
> - skip = false;
> + const gchar *args = strpbrk(insn_disas, " \t");
We have a general preference for glib here, could we use g_strsplit_set?
Something like:
modified contrib/plugins/execlog.c
@@ -232,12 +232,14 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
*/
if (disas_assist && rmatches) {
check_regs_next = false;
- gchar *args = g_strstr_len(insn_disas, -1, " ");
- for (int n = 0; n < all_reg_names->len; n++) {
- gchar *reg = g_ptr_array_index(all_reg_names, n);
- if (g_strrstr(args, reg)) {
- check_regs_next = true;
- skip = false;
+ g_auto(GStrv) args = g_strsplit_set(insn_disas, " \t", 2);
+ if (args && args[1]) {
+ for (int n = 0; n < all_reg_names->len; n++) {
+ gchar *reg = g_ptr_array_index(all_reg_names, n);
+ if (g_strrstr(args[1], reg)) {
+ check_regs_next = true;
+ skip = false;
+ }
> + if (args) {
> + for (int n = 0; n < all_reg_names->len; n++) {
> + const gchar *reg = g_ptr_array_index(all_reg_names, n);
> + if (g_strrstr(args, reg)) {
> + check_regs_next = true;
> + skip = false;
> + break;
> + }
> }
> }
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
