| Summary: | Incorrect extraction of integer fields in target description "flags" data type | ||
|---|---|---|---|
| Product: | gdb | Reporter: | Shahab <shahab.vahedi> |
| Component: | gdb | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | simark |
| Priority: | P2 | ||
| Version: | HEAD | ||
| Target Milestone: | 11.1 | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
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:
------------------ 8< ------------------
unsigned field_len = TYPE_FIELD_BITSIZE (type, field);
ULONGEST field_val
= val >> (TYPE_FIELD_BITPOS (type, field) - field_len + 1);
------------------ >8 ------------------
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:
------------------ 8< ------------------
ULONGEST field_val = val >> TYPE_FIELD_BITPOS (type, field);
------------------ >8 ------------------
The rest of the algorithm that masks out the higher bits is OK.
The gdb-11-branch branch has been updated by Shahab Vahedi <shahab@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=70417f28b5b6d01e130fe506bf25e1a5c104573b commit 70417f28b5b6d01e130fe506bf25e1a5c104573b Author: Shahab Vahedi <shahab@synopsys.com> Date: Mon Jul 26 14:51:35 2021 +0200 gdb: Fix numerical field extraction for target description "flags" 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. gdb/ChangeLog: 2021-07-26 Shahab Vahedi <shahab@synopsys.com> Simon Marchi <simon.marchi@efficios.com> PR gdb/28103 * valprint.c (val_print_type_code_flags): Merely shift the VAL to the right to get rid of the lower bits. (test_print_flags): New. (_initialize_valprint): Invoke the "test_print_flags" as a unit-test. Co-Authored-By: Simon Marchi <simon.marchi@efficios.com> This PR was solved on master with this commit: | commit c9bd98593b785d9bf5f39c7aa74ed0226a23b830 | From: Shahab Vahedi <shahab@synopsys.com> | Date: Fri, 16 Jul 2021 16:49:15 +0200 | Subject: gdb: Fix numerical field extraction for target description "flags" The patch above was also pushed to gdb-11-branch as: | commit 70417f28b5b6d01e130fe506bf25e1a5c104573b | From: Shahab Vahedi <shahab@synopsys.com> | Date: Mon, 26 Jul 2021 14:51:35 +0200 | Subject: gdb: Fix numerical field extraction for target description "flags" |
Affected versions are HEAD, 11, 10, ... This is a more elaborated bug report of 21184. That report actually mentions two bugs. This report is only about one of those. ============================================================ Assume there is an XML target description as follows: ---------------- [ target description xml ] ---------------- <?xml version="1.0"?> <!DOCTYPE feature SYSTEM "gdb-target.dtd"> <feature name="org.gnu.gdb.arc.fpu"> <flags id="flag_type" size="4"> <field name="version" start="0" end="7" type="int" /> <field name="g" start="8" end="8" type="bool"/> <field name="u" start="9" end="9" type="bool"/> <field name="b" start="10" end="10" type="bool"/> <field name="" start="11" end="31" /> </flags> <reg name="f_reg" bitsize="32" type="flag_type"/> </feature> ------------------------------------------------------------ This corresponds to an "f_reg" register like below: ---------------- [ f_reg register format ] ----------------- ,-----------------.---.---.---.---------. | dont_care_field | b | u | g | version | `-----------------^---^---^---^---------' bit: 31 ... 11 10 9 8 7 ... 0 ------------------------------------------------------------ When the reported value of "f_reg" is 0x1337: ------- [ f_reg instance with 0x1337 as its value ] -------- 0 x 1 3 3 7 0001 0011 0011 0111 bug version ------------------------------------------------------------ GDB prints incorrect information regarding the "version": --------------------------- [gdb] -------------------------- (gdb) info reg $f_reg f_reg 0x1337 [ version=0 g u ] ------------------------------------------------------------ Here, while "$f_reg" holds the correct value (0x1337), the "version" field is 0 instead of 55. "b", "u", and "g" fields are inferred correctly though.