diff mbox

[PULL,for-2.5] tcg: Fix highwater check

Message ID 1448282741-22897-2-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Nov. 23, 2015, 12:45 p.m. UTC
From: John Clarke <johnc@kirriwa.net>

A simple typo in the variable to use when comparing vs the highwater mark.
Reports are that qemu can in fact segfault occasionally due to this mistake.

Signed-off-by: John Clarke <johnc@kirriwa.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Weil Nov. 23, 2015, 1:16 p.m. UTC | #1
Am 23.11.2015 um 13:45 schrieb Richard Henderson:
> From: John Clarke <johnc@kirriwa.net>
> 
> A simple typo in the variable to use when comparing vs the highwater mark.
> Reports are that qemu can in fact segfault occasionally due to this mistake.
> 
> Signed-off-by: John Clarke <johnc@kirriwa.net>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 682af8a..b20ed19 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -2443,7 +2443,7 @@ int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
>             one operation beginning below the high water mark cannot overrun
>             the buffer completely.  Thus we can test for overflow after
>             generating code without having to check during generation.  */
> -        if (unlikely(s->code_gen_ptr > s->code_gen_highwater)) {
> +        if (unlikely((void *)s->code_ptr > s->code_gen_highwater)) {
>              return -1;
>          }
>      }
> 

Is a comparison of void pointers portable? Or would it be better
to cast both sides to uintptr_t? Or fix the declaration of
code_gen_highwater to use an uint8_t pointer and cast s->code_ptr
to that type? code_gen_highwater should be fixed anyway because
in translate-all a difference is calculated with it.

Stefan
Richard Henderson Nov. 23, 2015, 1:49 p.m. UTC | #2
On 11/23/2015 02:16 PM, Stefan Weil wrote:
> Am 23.11.2015 um 13:45 schrieb Richard Henderson:
>> From: John Clarke <johnc@kirriwa.net>
>>
>> A simple typo in the variable to use when comparing vs the highwater mark.
>> Reports are that qemu can in fact segfault occasionally due to this mistake.
>>
>> Signed-off-by: John Clarke <johnc@kirriwa.net>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>   tcg/tcg.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>> index 682af8a..b20ed19 100644
>> --- a/tcg/tcg.c
>> +++ b/tcg/tcg.c
>> @@ -2443,7 +2443,7 @@ int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
>>              one operation beginning below the high water mark cannot overrun
>>              the buffer completely.  Thus we can test for overflow after
>>              generating code without having to check during generation.  */
>> -        if (unlikely(s->code_gen_ptr > s->code_gen_highwater)) {
>> +        if (unlikely((void *)s->code_ptr > s->code_gen_highwater)) {
>>               return -1;
>>           }
>>       }
>>
>
> Is a comparison of void pointers portable?

Of course.  Particularly since these really are pointers into the same 
allocated object.  That's 100% ANSI C.

> code_gen_highwater should be fixed anyway because
> in translate-all a difference is calculated with it.

Yes, but we freely make use of this gcc extension in many places.


r~
diff mbox

Patch

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 682af8a..b20ed19 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2443,7 +2443,7 @@  int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
            one operation beginning below the high water mark cannot overrun
            the buffer completely.  Thus we can test for overflow after
            generating code without having to check during generation.  */
-        if (unlikely(s->code_gen_ptr > s->code_gen_highwater)) {
+        if (unlikely((void *)s->code_ptr > s->code_gen_highwater)) {
             return -1;
         }
     }