Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/hotblocks.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 02bc5078bdd..09b0932275c 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -34,6 +34,7 @@ static guint64 limit = 20;
  */
 typedef struct {
     uint64_t start_addr;
+    uint64_t hash;
     struct qemu_plugin_scoreboard *exec_count;
     int trans_count;
     unsigned long insns;
@@ -91,7 +92,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
 
 static void plugin_init(void)
 {
-    hotblocks = g_hash_table_new(NULL, g_direct_equal);
+    hotblocks = g_hash_table_new(g_int64_hash, g_int64_equal);
 }
 
 static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
@@ -114,16 +115,17 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
     uint64_t hash = pc ^ insns;
 
     g_mutex_lock(&lock);
-    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) hash);
+    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, &hash);
     if (cnt) {
         cnt->trans_count++;
     } else {
         cnt = g_new0(ExecCount, 1);
         cnt->start_addr = pc;
+        cnt->hash = hash;
         cnt->trans_count = 1;
         cnt->insns = insns;
         cnt->exec_count = qemu_plugin_scoreboard_new(sizeof(uint64_t));
-        g_hash_table_insert(hotblocks, (gpointer) hash, (gpointer) cnt);
+        g_hash_table_insert(hotblocks, &cnt->hash, cnt);
     }
 
     g_mutex_unlock(&lock);
-- 
2.39.5

On 12/16/24 19:07, Pierrick Bouvier wrote:
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   contrib/plugins/hotblocks.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
> index 02bc5078bdd..09b0932275c 100644
> --- a/contrib/plugins/hotblocks.c
> +++ b/contrib/plugins/hotblocks.c
> @@ -34,6 +34,7 @@ static guint64 limit = 20;
>    */
>   typedef struct {
>       uint64_t start_addr;
> +    uint64_t hash;
>       struct qemu_plugin_scoreboard *exec_count;
>       int trans_count;
>       unsigned long insns;
> @@ -91,7 +92,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>   
>   static void plugin_init(void)
>   {
> -    hotblocks = g_hash_table_new(NULL, g_direct_equal);
> +    hotblocks = g_hash_table_new(g_int64_hash, g_int64_equal);
>   }

I think this one should be using a proper comparison function: compare both pc and insns, 
the inputs to the simplistic hash function:

>       uint64_t hash = pc ^ insns;

An existing bug with the plugin, obviously.


r~

On 12/17/24 07:34, Richard Henderson wrote:
> On 12/16/24 19:07, Pierrick Bouvier wrote:
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    contrib/plugins/hotblocks.c | 8 +++++---
>>    1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
>> index 02bc5078bdd..09b0932275c 100644
>> --- a/contrib/plugins/hotblocks.c
>> +++ b/contrib/plugins/hotblocks.c
>> @@ -34,6 +34,7 @@ static guint64 limit = 20;
>>     */
>>    typedef struct {
>>        uint64_t start_addr;
>> +    uint64_t hash;
>>        struct qemu_plugin_scoreboard *exec_count;
>>        int trans_count;
>>        unsigned long insns;
>> @@ -91,7 +92,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>>    
>>    static void plugin_init(void)
>>    {
>> -    hotblocks = g_hash_table_new(NULL, g_direct_equal);
>> +    hotblocks = g_hash_table_new(g_int64_hash, g_int64_equal);
>>    }
> 
> I think this one should be using a proper comparison function: compare both pc and insns,
> the inputs to the simplistic hash function:
> 

I will replace the equal function, and the hash one as well, so we don't 
have to store hash in the entry.

>>        uint64_t hash = pc ^ insns;
> 
> An existing bug with the plugin, obviously.
> 
> 
> r~