Re: [Qemu-devel] [RFC PATCH 05/11] decodetree: Force Python to print unsigned values

[Philippe Mathieu-Daudé]

On Mon, Nov 12, 2018 at 6:03 PM Eduardo Habkost <address@hidden> wrote:
> On Mon, Nov 12, 2018 at 12:36:16AM +0100, Philippe Mathieu-Daudé wrote:
> > Python internal representation is signed, so unsigned values
> > bigger than 31-bit are interpreted as signed (and printed with
> > a '-' signed).
> > Mask out to force unsigned values.
>
> I don't understand this commit description. Python surely
> supports integers larger than 2^31, and its internal
> representation shouldn't matter at all:
>
>   >>> 
> '0x{0:08x}'.format(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
>   
> '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'
>
> Can you explain how the code ends up with a negative value in the
> `self.thismask` or `b` variables?  If `self.subs` contains
> negative values, this is likely to break other parts of the code.

I guess I misunderstood the error, thus the description is invalid.

>
> >
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> > TODO: display error encountered:
> >
> >    case 0x-1:
> >        ....
>
> How can I reproduce it?

$ scripts/decodetree.py /dev/null

    switch (insn & 0x-0000001) {
    }

main() ->
  build_tree(patterns, 0, outermask = 0) ->
       innermask = ~outermask # = -1
      Tree(fullmask, innermask = -1) ->
        __init__(self, fm, tm = -1) ->
          self.thismask = tm # -1
  t.output_code(4, False, 0, 0) ->
    sh = is_contiguous(self.thismask) # -1
        output(ind, 'switch (', str_switch(self.thismask), ') {\n')

        with:

            def str_switch(b):
                return 'insn & 0x{0:08x}'.format(b)

So the fix is rather:

-- >8 --
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
@@ -916,7 +916,7 @@ class Tree:

def build_tree(pats, outerbits, outermask):
     # Find the intersection of all remaining fixedmask.
-    innermask = ~outermask
+    innermask = ~outermask & insnmask
     for i in pats:
         innermask &= i.fixedmask
 ---