tci: Remove invalid assertions

tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
and cannot be used with ARRAY_SIZE.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 tcg/tci/tcg-target.inc.c | 2 --
 1 file changed, 2 deletions(-)

On 02/02/2017 01:56 PM, Stefan Weil wrote:
> tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
> and cannot be used with ARRAY_SIZE.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  tcg/tci/tcg-target.inc.c | 2 --
>  1 file changed, 2 deletions(-)

mst posted an alternative patch:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00551.html

Am 02.02.2017 um 21:00 schrieb Eric Blake:
> On 02/02/2017 01:56 PM, Stefan Weil wrote:
>> tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
>> and cannot be used with ARRAY_SIZE.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  tcg/tci/tcg-target.inc.c | 2 --
>>  1 file changed, 2 deletions(-)
>
> mst posted an alternative patch:
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00551.html


Yes, I noticed that, too. It's not obvious that this new
assertion will be correct, and none of the other targets
has that kind of assertion. Only two targets use an
assertion which detects NULL pointers, but NULL pointers
will result in an abort anyway.

Do you think that there are reasons why TCI should use
the assertion suggested by Michael?

Stefan

On 02/02/2017 02:10 PM, Stefan Weil wrote:
> Am 02.02.2017 um 21:00 schrieb Eric Blake:
>> On 02/02/2017 01:56 PM, Stefan Weil wrote:
>>> tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
>>> and cannot be used with ARRAY_SIZE.
>>>

>> mst posted an alternative patch:
>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00551.html
> 
> 
> Yes, I noticed that, too. It's not obvious that this new
> assertion will be correct, and none of the other targets
> has that kind of assertion. Only two targets use an
> assertion which detects NULL pointers, but NULL pointers
> will result in an abort anyway.
> 
> Do you think that there are reasons why TCI should use
> the assertion suggested by Michael?

I don't have any strong opinions on which patch is better, so much as
just pointing out that alternative proposals exist so that we have good
threading in making the decision on which one to go with.

On Thu, Feb 02, 2017 at 09:10:26PM +0100, Stefan Weil wrote:
> Am 02.02.2017 um 21:00 schrieb Eric Blake:
> > On 02/02/2017 01:56 PM, Stefan Weil wrote:
> > > tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
> > > and cannot be used with ARRAY_SIZE.
> > > 
> > > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > > ---
> > >  tcg/tci/tcg-target.inc.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > 
> > mst posted an alternative patch:
> > https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00551.html
> 
> 
> Yes, I noticed that, too. It's not obvious that this new
> assertion will be correct, and none of the other targets
> has that kind of assertion. Only two targets use an
> assertion which detects NULL pointers, but NULL pointers
> will result in an abort anyway.
> 
> Do you think that there are reasons why TCI should use
> the assertion suggested by Michael?
> 
> Stefan

You know what this code does and I don't, not really.
I just did a monkey patch guessing at what was intended
(value is used as an array index, so we do a bounds check).

I sent the patch before I saw yours simply to fix the build in a way
that's as unintrusive as possible: args[0] seemed to come from guest so
I thought it might be prudent to do a bounds check.

So feel free to ignore mine. Here's an ack for yours

Acked-by: Michael S. Tsirkin <mst@redhat.com>

On 2 February 2017 at 19:56, Stefan Weil <sw@weilnetz.de> wrote:
> tb_jmp_insn_offset and tb_jmp_reset_offset are pointers
> and cannot be used with ARRAY_SIZE.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  tcg/tci/tcg-target.inc.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/tcg/tci/tcg-target.inc.c b/tcg/tci/tcg-target.inc.c
> index 26ee9b1664..b6a15569f8 100644
> --- a/tcg/tci/tcg-target.inc.c
> +++ b/tcg/tci/tcg-target.inc.c
> @@ -566,7 +566,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>      case INDEX_op_goto_tb:
>          if (s->tb_jmp_insn_offset) {
>              /* Direct jump method. */
> -            tcg_debug_assert(args[0] < ARRAY_SIZE(s->tb_jmp_insn_offset));
>              /* Align for atomic patching and thread safety */
>              s->code_ptr = QEMU_ALIGN_PTR_UP(s->code_ptr, 4);
>              s->tb_jmp_insn_offset[args[0]] = tcg_current_code_size(s);
> @@ -575,7 +574,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>              /* Indirect jump method. */
>              TODO();
>          }
> -        tcg_debug_assert(args[0] < ARRAY_SIZE(s->tb_jmp_reset_offset));
>          s->tb_jmp_reset_offset[args[0]] = tcg_current_code_size(s);
>          break;
>      case INDEX_op_br:
> --
> 2.11.0

Applied to master as a buildfix; thanks.

-- PMM