[Qemu-devel] [PULL for-2.5] last minute tcg fix

[Qemu-devel] [PULL for-2.5] last minute tcg fix

[Richard Henderson]

Sent to me privately, for some reason, but absolutely correct
that it can occasionally cause problems.


r~


The following changes since commit 541abd10a01da56c5f16582cd32d67114ec22a5c:

  Update version for v2.5.0-rc1 release (2015-11-20 17:43:46 +0000)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20151123

for you to fetch changes up to 644da9b39e477caa80bab69d2847dfcb468f0d33:

  tcg: Fix highwater check (2015-11-23 13:16:05 +0100)

----------------------------------------------------------------
Last minute fix.

----------------------------------------------------------------
John Clarke (1):
      tcg: Fix highwater check

 tcg/tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[Qemu-devel] [PULL for-2.5] tcg: Fix highwater check

[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;
         }
     }
-- 
2.4.3

Re: [Qemu-devel] [PULL for-2.5] tcg: Fix highwater check

[Stefan Weil]

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

Re: [Qemu-devel] [PULL for-2.5] tcg: Fix highwater check

[Richard Henderson]

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~

Re: [Qemu-devel] [PULL for-2.5] last minute tcg fix

[Peter Maydell]

On 23 November 2015 at 12:45, Richard Henderson <rth@twiddle.net> wrote:
> Sent to me privately, for some reason, but absolutely correct
> that it can occasionally cause problems.
>
>
> r~
>
>
> The following changes since commit 541abd10a01da56c5f16582cd32d67114ec22a5c:
>
>   Update version for v2.5.0-rc1 release (2015-11-20 17:43:46 +0000)
>
> are available in the git repository at:
>
>   git://github.com/rth7680/qemu.git tags/pull-tcg-20151123
>
> for you to fetch changes up to 644da9b39e477caa80bab69d2847dfcb468f0d33:
>
>   tcg: Fix highwater check (2015-11-23 13:16:05 +0100)
>
> ----------------------------------------------------------------
> Last minute fix.
>
> ----------------------------------------------------------------
> John Clarke (1):
>       tcg: Fix highwater check

Applied, thanks.

-- PMM