Re: [PATCH v2] plugins/stoptrigger: TCG plugin to stop execution under conditions

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

Hello Simon,

On 7/12/24 00:53, Simon Hamelin wrote:
> 
> 
> On 7/11/24 12:03, Alex Bennée wrote:
>>> +static void exit_emulation(int return_code)
>>> +{
>>> +    exit(return_code);
>>> +}
>>> +
>>> +static void exit_icount_reached(unsigned int cpu_index, void *udata)
>>> +{
>>> +    qemu_plugin_outs("icount reached, exiting\n");
>>> +    exit_emulation(icount_exit_code);
>>> +}
>>> +
>>> +static void exit_address_reached(unsigned int cpu_index, void *udata)
>>> +{
>>> +    uint64_t insn_vaddr = GPOINTER_TO_UINT(udata);
>>> +    g_mutex_lock(&addrs_ht_lock);
>>> +    int exit_code = GPOINTER_TO_INT(
>>> +        g_hash_table_lookup(addrs_ht, GUINT_TO_POINTER(insn_vaddr)));
>>> +    g_mutex_unlock(&addrs_ht_lock);
>>> +    char *msg = g_strdup_printf("0x%" PRIx64 " reached, exiting\n",
>>> insn_vaddr);
>>
>> Dont intermix variable declarations, put them at the top of the block.
>>
>>> +    qemu_plugin_outs(msg);
>>> +    exit_emulation(exit_code);
>>> +}
>>
>> How about something like:
>>
>>     static void exit_emulation(int return_code, char *message)
>>     {
>>         qemu_plugin_outs(message);
>>         g_free(message);
>>         exit(return_code);
>>     }
>>
>>     static void exit_icount_reached(unsigned int cpu_index, void *udata)
>>     {
>>         uint64_t insn_vaddr = GPOINTER_TO_UINT(udata);
>>         char *msg = g_strdup_printf("icount reached at 0x%" PRIx64 ", exiting\n", insn_vaddr);
>>
>>         exit_emulation(icount_exit_code, msg);
>>     }
>>
>>     static void exit_address_reached(unsigned int cpu_index, void *udata)
>>     {
>>         uint64_t insn_vaddr = GPOINTER_TO_UINT(udata);
>>         char *msg = g_strdup_printf("0x%" PRIx64 " reached, exiting\n", insn_vaddr);
>>         int exit_code;
>>
>>         g_mutex_lock(&addrs_ht_lock);
>>         exit_code = GPOINTER_TO_INT(
>>             g_hash_table_lookup(addrs_ht, GUINT_TO_POINTER(insn_vaddr)));
>>         g_mutex_unlock(&addrs_ht_lock);
>>
>>         exit_emulation(exit_code, msg);
>>     }
>>
>>
> 
> Looks good to me, will definitly put that in the next patch !
> 
>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>>> +                                           const qemu_info_t *info, int argc,
>>> +                                           char **argv)
>>> +{
>>> +    addrs_ht = g_hash_table_new(NULL, g_direct_equal);
>>> +
>>> +    insn_count_sb = qemu_plugin_scoreboard_new(sizeof(InstructionsCount));
>>> +    insn_count = qemu_plugin_scoreboard_u64_in_struct(
>>> +        insn_count_sb, InstructionsCount, insn_count);
>>> +
>>> +    for (int i = 0; i < argc; i++) {
>>> +        char *opt = argv[i];
>>> +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
>>> +        if (g_strcmp0(tokens[0], "icount") == 0) {
>>> +            g_auto(GStrv) icount_tokens = g_strsplit(tokens[1], ":", 2);
>>> +            icount = g_ascii_strtoull(icount_tokens[0], NULL, 0);
>>
>>
>>
>>> +            if (icount < 1 || g_strrstr(icount_tokens[0], "-") !=
>>> NULL) {
>>
>> I don't think strstoull would even parse something with - in it so I
>> would just do:
>>
>>     if (icount == 0) {
>>        /* fail */
>>     }
>>
> 
> According to the GLib documentation: "Note that input with a leading
> minus sign (-) is accepted, and will return the negation of the parsed
> number, unless that would overflow a guint64". So i guess we need to
> keep this check.
> 
>>> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
>>> index f7d7b9e3a4..954623f9bf 100644
>>> --- a/docs/devel/tcg-plugins.rst
>>> +++ b/docs/devel/tcg-plugins.rst
>>> @@ -642,6 +642,28 @@ The plugin has a number of arguments, all of them are optional:
>>>      configuration arguments implies ``l2=on``.
>>>      (default: N = 2097152 (2MB), B = 64, A = 16)
>>>    
>>> +- contrib/plugins/stoptrigger.c
>>> +
>>> +The stoptrigger plugin allows to setup triggers to stop emulation.
>>> +It can be used for research purposes to launch some code and precisely stop it
>>> +and understand where its execution flow went.
>>> +
>>> +Two types of triggers can be configured: a count of instructions to stop at,
>>> +or an address to stop at. Multiple triggers can be set at once.
>>> +
>>> +By default, QEMU will exit with return code 0. A custom return code can be
>>> +configured for each trigger using ``:CODE`` syntax.
>>> +
>>> +For example, to stop at the 20-th instruction with return code 41, at address
>>> +0xd4 with return code 0 or at address 0xd8 with return code 42::
>>> +
>>> +  $ qemu-system-aarch64 $(QEMU_ARGS) \
>>> +    -plugin ./contrib/plugins/libstoptrigger.so,icount=20:41,addr=0xd4,addr=0xd8:42 -d plugin
>>> +
>>> +The plugin will log the reason of exit, for example::
>>> +
>>> +  0xd4 reached, exiting
>>> +
>>>    Plugin API
>>>    ==========
>>
>> Otherwise it looks good to me. Unless you want to tackle additional exit
>> modes?
>>
>> What is your current use case for this?
>>
> 
> I'm currently using this plugin to determine where my programm stop
> after a given number of instructions executed.
> 

Could you share a bit more information on the final goal, if possible?
Is that used for fuzzing binaries, security analysis, or other things?