Re: [PATCH v2] gdb: Fix numerical field extraction for target description "flags"

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-07-23 8:38 a.m., Shahab Vahedi via Gdb-patches wrote:
> From: Shahab Vahedi <shahab@synopsys.com>
> 
> v2 (This section will be removed when checking the patch in):
> 1. There are no lines in the commit message starting with "---".
> 2. Joined 2 lines together that now fit under character limits.
> 3. Added the unit-test "test_print_flags" as proposed by Simon.
> 
> The "val_print_type_code_flags ()" function is responsible for
> extraction of fields for "flags" data type.  These data types are
> used when describing a custom register type in a target description
> XML.  The logic used for the extraction though is not sound:
> 
>     unsigned field_len = TYPE_FIELD_BITSIZE (type, field);
>     ULONGEST field_val
>       = val >> (TYPE_FIELD_BITPOS (type, field) - field_len + 1);
> 
> TYPE_FIELD_BITSIZE: The bit length of the field to be extracted.
> TYPE_FIELD_BITPOS:  The starting position of the field; 0 is LSB.
> val:                The register value.
> 
> Imagine you have a field that starts at position 1 and its length
> is 4 bits.  According to the third line of the code snippet the
> shifting right would become "val >> -2", or "val >> 0xfff...fe"
> to be precise.  That will result in a "field_val" of 0.
> 
> The correct extraction should be:
> 
>     ULONGEST field_val = val >> TYPE_FIELD_BITPOS (type, field);
> 
> The rest of the algorithm that masks out the higher bits is OK.
> 
> Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>

LGTM, thanks.

Simon