Re: bitpos expansion patches summary

Jan Kratochvil
2012-08-12T17:57:59Z

Hi Siddhesh,

another part of review, it is pretty long...

Also I am marking every patch chunk with corresponding 'report' line as
otherwise I do not know how we can find the reasons each of the changes is
being committed.

(Some of it was already addressed, like ' - error: llp64'.)

Thanks,
Jan

RSAFE: (ada-lang.c:579): FUNC(memcpy): (ULONGEST to size_t) [(type)->length]
RSAFE: (ada-lang.c:670): FUNC(umax_of_size): (ULONGEST to int) [(t)->length]
RSAFE: (ada-lang.c:672): FUNC(max_of_size): (ULONGEST to int) [(t)->length]
RSAFE: (ada-lang.c:682): FUNC(min_of_size): (ULONGEST to int) [(t)->length]
RSAFE: (ada-lang.c:1622): RET: (ULONGEST to int) [8 * (ada_check_typedef((((type)->main_type->flds_bnds.fields[1]).type)))->length]
RSAFE: (ada-lang.c:1688): RET: (ULONGEST to int) [8 * ((((type)->main_type->flds_bnds.fields[0]).type))->length]
RSAFE: (ada-lang.c:1724): RET: (ULONGEST to int) [8 * ((((type)->main_type->flds_bnds.fields[2 * i + which - 2]).type))->length]

UNSURE(Expand bitsize?): (ada-lang.c:2072): ASSIGN: (LONGEST to UINT) [ (((new_type)->main_type->flds_bnds.fields[0]).bitsize) = *elt_bits]
- That is whether to extend:
/* Size of this field, in bits, or zero if not packed.
If non-zero in an array type, indicates the element size in
bits (used only in Ada at the moment).
For an unpacked field, the field's type's length
says how many bytes the field occupies. */
unsigned int bitsize : 28;
I do not know how to create such large bitfield with gcc, during such attempts
I get:
bitsize.c:7:3: error: bit-field ‘x’ has invalid type
Even if it is possible I would find OK to leave it for a different patchset.

RSAFE: (ada-lang.c:2185): FUNC(ada_value_primitive_packed_val): (LONGEST to int) [bit_pos % 8]

RSAFE: (ada-lang.c:2315): VARINIT(new_offset): (LONGEST to LONG) [offset]
- error: llp64

RSAFE: (ada-lang.c:2342): FUNC(memset): (ULONGEST to size_t) [(type)->length]
RSAFE: (ada-lang.c:2379): ASSIGN: (LONGEST to int) [ src = targ = 0]
RSAFE: (ada-lang.c:2445): ASSIGN: (LONGEST to int) [ accum_bits = 8 - src_offset]
RSAFE: (ada-lang.c:2455): CMP: (LONGEST to int) [chunk_size > n]
RSAFE: (ada-lang.c:2456): ASSIGN: (LONGEST to int) [ chunk_size = n]
RSAFE: (ada-lang.c:2470): ASSIGN: (LONGEST to UINT) [ accum = (UCHAR)*source >> src_offset]
RSAFE: (ada-lang.c:2472): ASSIGN: (LONGEST to int) [ accum_bits = 8 - src_offset]
RSAFE: (ada-lang.c:2480): CMP: (LONGEST to int) [chunk_size > n]
RSAFE: (ada-lang.c:2481): ASSIGN: (LONGEST to int) [ chunk_size = n]
RSAFE: (ada-lang.c:2544): FUNC(memcpy): (ULONGEST to size_t) [(type)->length]
RSAFE: (ada-lang.c:4093): FUNC(write_memory): (LONGEST to ssize_t) [len]
RSAFE: (ada-lang.c:4137): FUNC(memcpy): (ULONGEST to size_t) [(actual_type)->length]
RSAFE: (ada-lang.c:4162): FUNC(C_alloca): (ULONGEST to size_t) [len]
RSAFE: (ada-lang.c:6430): FUNC(ada_value_primitive_packed_val): (LONGEST to int) [bit_pos % 8]
RSAFE: (ada-lang.c:6488): ASSIGN: (LONGEST to int) [ *bit_offset_p = bit_pos % 8]
RSAFE: (ada-lang.c:8738): FUNC(memcmp): (ULONGEST to size_t) [(value_type(arg1))->length]
RSAFE: (ada-tasks.c:785): VARINIT(target_ptr_byte): (ULONGEST to int) [(data->known_tasks_element)->length]
RSAFE: (ada-tasks.c:812): VARINIT(target_ptr_byte): (ULONGEST to int) [(data->known_tasks_element)->length]
RSAFE: (ada-valprint.c:376): FUNC(ada_emit_char): (ULONGEST to int) [(type)->length]
FIXED(expand i, rep1, reps ULONGEST): (ada-valprint.c:483): CMP: (ULONGEST to UINT) [i < length]
FIXED(Likewise): (ada-valprint.c:501): CMP: (ULONGEST to UINT) [rep1 < length]
FIXED(Likewise): (ada-valprint.c:553): CMP: (ULONGEST to UINT) [i < length]
RSAFE: (ada-valprint.c:563): FUNC(printstr): (ULONGEST to int) [(type)->length]
- agreed, it is length of char
RSAFE: (ada-valprint.c:629): CMP: (UINT to LONGEST) [temp_len < options->print_max]
RSAFE: (ada-valprint.c:631): FUNC(char_at): (ULONGEST to int) [eltlen]
RSAFE: (ada-valprint.c:636): FUNC(printstr): (ULONGEST to int) [eltlen]
RSAFE: (ada-valprint.c:964): CMP: (size_t to ULONGEST) [((type)->main_type->target_type)->length != sizeof(char)]
RSAFE: (ada-valprint.c:1132): FUNC(ada_value_primitive_packed_val): (LONGEST to int) [bit_pos % 8]
ENSURED_SIZET: (alpha-tdep.c:402): ASSIGN: (LONGEST to int) [ accumulate_size = (accumulate_size + m_arg->len + 7) & ~7]
RSAFE: (alpha-tdep.c:432): CMP: (size_t to LONGEST) [offset + len <= sizeof((arg_reg_buffer))]
RSAFE: (alpha-tdep.c:434): FUNC(memcpy): (LONGEST to size_t) [len]
RSAFE: (alpha-tdep.c:448): FUNC(write_memory): (LONGEST to ssize_t) [len]
RSAFE: (alpha-tdep.c:630): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length]
RSAFE: (amd64-tdep.c:635): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length]
RSAFE: (amd64-tdep.c:698): FUNC(regcache_raw_read_part): (LONGEST to int) [((len) < (8) ? (len) : (8))]
RSAFE: (amd64-tdep.c:701): FUNC(regcache_raw_write_part): (LONGEST to int) [((len) < (8) ? (len) : (8))]
RSAFE: (amd64-tdep.c:824): FUNC(memcpy): (LONGEST to size_t) [((len) < (8) ? (len) : (8))]
RSAFE: (amd64-tdep.c:845): FUNC(write_memory): (LONGEST to ssize_t) [len]
RSAFE: (amd64-tdep.c:2777): FUNC(target_read_memory): (LONGEST to ssize_t) [len]
RSAFE: (amd64-windows-tdep.c:112): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length]
FIXED(Expand param to LONGEST): (amd64-windows-tdep.c:120): FUNC(regcache_raw_read_part): (LONGEST to int) [len]
FIXED(Likewise): (amd64-windows-tdep.c:122): FUNC(regcache_raw_write_part): (LONGEST to int) [len]
WPREVERTED(Only ok_for_watchpoint needs LONGEST): (arm-linux-nat.c:938): ASSIGN: (LONGEST to UINT) [ mask = (1 << len) - 1]
RSAFE: (arm-tdep.c:3292): RET: (ULONGEST to int) [(t)->length]
FIXED(arm_vfp_cprc_sub_candidate return LONGEST): (arm-tdep.c:3425): RET: (ULONGEST to int) [(t)->length / unitlen]
- this is correct but arm-tdep.c:3442 is wrong, I guess you did not rerun
splint after the changes to check for new reported regressions.
RSAFE: (arm-tdep.c:3695): FUNC(C_alloca): (LONGEST to size_t) [len]
RSAFE: (arm-tdep.c:3707): VARINIT(partial_len): (LONGEST to int) [len < 4 ? len : 4]
RSAFE: (auxv.c:80): VARINIT(ptr_size): (ULONGEST to size_t) [(ptr_type)->length]
REVERTED: (avr-tdep.c:932): FUNC(regcache_cooked_write): (LONGEST to int) [lsb_reg + i]
RSAFE: (avr-tdep.c:938): FUNC(regcache_cooked_read): (LONGEST to int) [lsb_reg + i]
ENSURED_SIZET: (avr-tdep.c:1184): FUNC(xmalloc): (LONGEST to size_t) [len]
- You do not, you have changed parameter len to ssize_t, you did ENSURED_SIZET on line 1298.
- But that ENSURED_SIZET is redundant there as the code already calls value_contents (arg).
ENSURED_SIZET: (avr-tdep.c:1185): ASSIGN: (LONGEST to int) [ si->len = len]
- Likewise.
ENSURED_SIZET: (avr-tdep.c:1187): FUNC(memcpy): (LONGEST to size_t) [len]
- Likewise.
FIXED(Expand j): (avr-tdep.c:1290): CMP: (LONGEST to int) [j < len]
RSAFE: (ax-gdb.c:490): FUNC(ax_ext): (ULONGEST to int) [(type)->length * 8]
RSAFE: (ax-gdb.c:500): VARINIT(bits): (ULONGEST to int) [(type)->length * 8]
FIXED(Expand N to LONGEST): (ax-gdb.c:516): FUNC(ax_trace_quick): (ULONGEST to int) [(type)->length]
RSAFE: (ax-gdb.c:1428): FUNC(): (LONGEST to int) [end - start]
SAFE: (bfin-tdep.c:538): FUNC(write_memory): (LONGEST to ssize_t) [container_len]
WPFIXED(Expanded bp_target_info.length): (breakpoint.c:2397): ASSIGN: (LONGEST to int) [ bl->target_info.length = bl->length]
SAFE(Hardware watchpoints should fit into int): (breakpoint.c:4624): FUNC(): (LONGEST to int) [loc->length]
- That's not true, s390x can set arbitrary hardware watchpoint memory range, see s390_fix_watch_points.
WPFIXED(Expand breakpoint_address_match_range len1 arg): (breakpoint.c:6611): FUNC(breakpoint_address_match_range): (LONGEST to int) [bl->length]
WPFIXED(Likewise): (breakpoint.c:9837): FUNC(breakpoint_address_match_range): (LONGEST to int) [bl->length]
RSAFE: (c-lang.c:291): ASSIGN: (ULONGEST to int) [ width = (element_type)->length]
RSAFE: (c-lang.c:417): FUNC(C_alloca): (ULONGEST to size_t) [(type)->length]
SAFE: (c-lang.c:419): ASSIGN: (ULONGEST to LONG) [ (output)->temp = ((type)->length)]
RSAFE: (c-lang.c:673): CMP: (ULONGEST to UINT) [(UINT)((&output)->next_free - (&output)->object_base) != (type)->length]
RSAFE: (coffread.c:2043): ASSIGN: (CORE_ADDR to LONGEST) [ ((list->field).loc.bitpos) = (ms->c_value)]
RSAFE: (corefile.c:346): FUNC(C_alloca): (ULONGEST to size_t) [(type)->length]
RSAFE: (corefile.c:348): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length]
FIXED(Expand i_offset): (cp-valprint.c:363): VARINIT(i_offset): (LONGEST to int) [offset + ((((type)->main_type->flds_bnds.fields[i]).loc.bitpos) + 0) / 8]
ENSURED_SIZET: (cp-valprint.c:561): FUNC(xmalloc): (ULONGEST to size_t) [(baseclass)->length]
ENSURED_SIZET: (cp-valprint.c:565): FUNC(target_read_memory): (ULONGEST to ssize_t) [(baseclass)->length]
ENSURED_SIZET: (cris-tdep.c:683): FUNC(xmalloc): (LONGEST to size_t) [len]
- You did not, but ensure_type_fits_sizet at the caller is redundant as
value_contents is already called there beforehand.
ENSURED_SIZET: (cris-tdep.c:684): ASSIGN: (LONGEST to int) [ si->len = len]
- Likewise.
ENSURED_SIZET: (cris-tdep.c:686): FUNC(memcpy): (LONGEST to size_t) [len]
- Likewise.
ENSURED_SIZET: (cris-tdep.c:899): FUNC(write_memory): (LONGEST to ssize_t) [len]
- Likewise.
SAFE: (doublest.c:816): CMP: (UINT to LONGEST) [len * 8 == gdbarch_long_double_format(gdbarch)[0]->totalsize]
RSAFE: (doublest.c:879): FUNC(memset): (ULONGEST to size_t) [(type)->length]
RSAFE: (doublest.c:905): FUNC(memset): (ULONGEST to size_t) [(to_type)->length]
RSAFE: (doublest.c:915): FUNC(memset): (ULONGEST to size_t) [(to_type)->length]
RSAFE: (doublest.c:916): FUNC(memcpy): (ULONGEST to size_t) [(((from_type)->length) < ((to_type)->length) ? ((from_type)->length) : ((to_type)->length))]
SAFE(size of DW_OP_GNU_const_type): (dwarf2expr.c:452): CMP: (int to ULONGEST) [(result)->length != size]
SAFE(Address size): (dwarf2expr.c:1050): CMP: (int to ULONGEST) [(type)->length != addr_size]
SAFE(Address size): (dwarf2expr.c:1055): FUNC(C_alloca): (ULONGEST to size_t) [(type)->length]
RSAFE: (dwarf2loc.c:1279): FUNC(memcpy): (ULONGEST to size_t) [(checked_type)->length]
SAFE: (dwarf2loc.c:1496): ASSIGN: (ULONGEST to UINT) [ source_offset_bits = source_offset % 8]
SAFE: (dwarf2loc.c:1498): ASSIGN: (ULONGEST to UINT) [ dest_avail = 8 - dest_offset_bits % 8]
SAFE: (dwarf2loc.c:1501): CMP: (ULONGEST to UINT) [dest_avail < bit_count]
SAFE: (dwarf2loc.c:1505): FUNC(insert_bits): (ULONGEST to UINT) [dest_offset_bits]
SAFE: (dwarf2loc.c:1524): FUNC(extract_bits): (ULONGEST to int) [bit_count]
SAFE: (dwarf2loc.c:1526): FUNC(insert_bits): (ULONGEST to UINT) [dest_offset_bits]
SAFE: (dwarf2loc.c:1526): FUNC(insert_bits): (ULONGEST to int) [bit_count]
SAFE: (dwarf2loc.c:1596): CMP: (ULONGEST to size_t) [buffer_size < this_size]
SAFE: (dwarf2loc.c:1598): ASSIGN: (ULONGEST to size_t) [ buffer_size = this_size]
- I do not see how this can be safe, some ENSURED_SIZET is needed here.
SAFE: (dwarf2loc.c:1610): VARINIT(reg_offset): (LONGEST to int) [source_offset]
SAFE: (dwarf2loc.c:1613): CMP: (int to ULONGEST) [this_size < register_size(arch, gdb_regnum)]
SAFE: (dwarf2loc.c:1616): ASSIGN: (ULONGEST to int) [ reg_offset = register_size(arch, gdb_regnum) - this_size]
SAFE: (dwarf2loc.c:1627): FUNC(get_frame_register_bytes): (ULONGEST to int) [this_size]
RSAFE: (dwarf2loc.c:1631): FUNC(memset): (ULONGEST to size_t) [this_size]
FIXED(Expand read_value_memory arg): (dwarf2loc.c:1648): FUNC(read_value_memory): (LONGEST to int) [offset]
SAFE(Caller ensures size_t): (dwarf2loc.c:1651): FUNC(read_value_memory): (ULONGEST to size_t) [this_size]
SAFE: (dwarf2loc.c:1656): VARINIT(n): (ULONGEST to size_t) [this_size]
SAFE: (dwarf2loc.c:1658): CMP: (LONGEST to size_t) [n > c->addr_size - source_offset]
SAFE: (dwarf2loc.c:1659): CMP: (LONGEST to int) [c->addr_size >= source_offset]
SAFE: (dwarf2loc.c:1659): ASSIGN: (LONGEST to size_t) [ n = (c->addr_size >= source_offset ? c->addr_size - source_offset : 0)]
SAFE: (dwarf2loc.c:1677): VARINIT(n): (ULONGEST to size_t) [this_size]
FIXED(Expand offset): (dwarf2loc.c:1748): CMP: (ULONGEST to LONG) [offset < type_len]
SAFE: (dwarf2loc.c:1787): CMP: (ULONGEST to size_t) [buffer_size < this_size]
SAFE: (dwarf2loc.c:1789): ASSIGN: (ULONGEST to size_t) [ buffer_size = this_size]
- I do not see how this can be safe, some ENSURED_SIZET is needed here.
SAFE: (dwarf2loc.c:1802): VARINIT(reg_offset): (LONGEST to int) [dest_offset]
SAFE: (dwarf2loc.c:1805): CMP: (int to ULONGEST) [this_size <= register_size(arch, gdb_regnum)]
SAFE: (dwarf2loc.c:1807): ASSIGN: (ULONGEST to int) [ reg_offset = register_size(arch, gdb_regnum) - this_size]
SAFE: (dwarf2loc.c:1816): FUNC(get_frame_register_bytes): (ULONGEST to int) [this_size]
SAFE: (dwarf2loc.c:1836): FUNC(put_frame_register_bytes): (ULONGEST to int) [this_size]
SAFE: (dwarf2loc.c:1860): FUNC(write_memory): (ULONGEST to ssize_t) [this_size]
FIXED(Expand this_size_bits): (dwarf2loc.c:1902): CMP: (size_t to LONGEST) [bit_offset >= this_size_bits]
FIXED(Expand this_size_bits): (dwarf2loc.c:2002): CMP: (size_t to LONGEST) [bit_offset >= this_size_bits]
- I do not see this_size_bits expanded, there is still: size_t this_size_bits = p->size;

--- bitpos3.patch-orig 2012-08-09 21:21:04.722849376 +0200
+++ bitpos3.patch 2012-08-12 19:51:17.978915788 +0200
@@ -651,6 +664,11 @@ index 5d3affa..de72c9c 100644
break;
}
+ ensure_type_fits_sizet (arg_type);
+xENSURED_SIZET: (alpha-tdep.c:402): ASSIGN: (LONGEST to int) [ accumulate_size = (accumulate_size + m_arg->len + 7) & ~7]
+
+Just move m_arg->contents = value_contents (arg); here and you can drop this
+call of ensure_type_fits_sizet.
+
m_arg->len = TYPE_LENGTH (arg_type);
m_arg->offset = accumulate_size;
accumulate_size = (accumulate_size + m_arg->len + 7) & ~7;
@@ -658,7 +676,14 @@ index 5d3affa..de72c9c 100644
+ /* Check for overflow. */
+ if (accumulate_size < 0)
+ error (_("Total size of arguments too large for host GDB memory."));
+xENSURED_SIZET: (alpha-tdep.c:402): ASSIGN: (LONGEST to int) [ accumulate_size = (accumulate_size + m_arg->len + 7) & ~7]
+
+If you make accumulate_size LONGEST you can drop this check. Then just verify
+you do not underflow SP in the line below:
+ sp -= accumulate_size;
+
+
+xENSURED_SIZET: (alpha-tdep.c:402): ASSIGN: (LONGEST to int) [ accumulate_size = (accumulate_size + m_arg->len + 7) & ~7]
m_arg->contents = value_contents (arg);
}