Re: bitpos expansion patches summary

Jan Kratochvil
2012-09-04T15:03:51Z

Hi Siddhesh,

here is the last part of this first phase.

I will now check some new splint output with the various forgotten cases.

Thanks,
Jan

SAFE: (valprint.c:848): CMP: (size_t to ULONGEST) [(type)->length > sizeof(LONGEST)]
SAFE(float): (valprint.c:1024): VARINIT(len): (ULONGEST to UINT) [(type)->length]
SAFE: (valprint.c:1096): VARINIT(len): (ULONGEST to UINT) [(type)->length]
FIXED(Expand i. Callers could be expanded separately): (valprint.c:1607): CMP: (ULONGEST to UINT) [i < len]
- Callers should be expanded because of splint warnings there.
But for example ada-valprint.c:700 is not reported in this file at all, why?
FIXED(Expand rep1): (valprint.c:1631): CMP: (ULONGEST to UINT) [rep1 < len]
FIXED: (valprint.c:1666): CMP: (ULONGEST to UINT) [i < len]
SAFE: (valprint.c:1979): FUNC(C_alloca): (ULONGEST to size_t) [(type)->length]
SAFE: (valprint.c:1982): FUNC(make_wchar_iterator): (ULONGEST to size_t) [(type)->length]
SAFE: (valprint.c:1983): FUNC(make_wchar_iterator): (ULONGEST to size_t) [(type)->length]
SAFE: (valprint.c:2023): FUNC(print_wchar): (ULONGEST to int) [(type)->length]
SAFE: (valprint.c:2030): FUNC(print_wchar): (ULONGEST to int) [(type)->length]
SAFE(char width): (valprint.c:2071): VARINIT(width): (ULONGEST to int) [(type)->length]
SAFE: (valprint.c:2079): CMP: (int to ULONGEST) [length == -1]
- But as LENGTH is just a length of character the expansion
'unsigned int'->ULONGEST should be reverted.
SAFE: (valprint.c:2109): FUNC(make_wchar_iterator): (ULONGEST to size_t) [length * width]
SAFE: (valprint.c:2313): VARINIT(width): (ULONGEST to int) [(elttype)->length]
SAFE: (value.c:568): FUNC(memcmp): (LONGEST to size_t) [length]
ENSURED_SIZET: (value.c:697): FUNC(xzalloc): (ULONGEST to size_t) [(val->enclosing_type)->length]
SAFE: (value.c:949): FUNC(memcpy): (LONGEST to size_t) [length]
- The expansion of int dst_offset, src_offset and length could be just ssize_t (instead of LONGEST).
SAFE: (value.c:1047): FUNC(memcmp): (LONGEST to size_t) [len]
SAFE: (value.c:1402): FUNC(memcpy): (ULONGEST to size_t) [(value_enclosing_type(arg))->length]
SAFE: (value.c:1430): FUNC(memcpy): (ULONGEST to size_t) [(enc_type)->length]
SAFE: (value.c:1957): FUNC(memcpy): (ULONGEST to size_t) [(value_type(newval))->length]
SAFE: (value.c:2421): VARINIT(len): (ULONGEST to int) [(type)->length]
SAFE: (value.c:2477): ASSIGN: (ULONGEST to int) [ len = (type)->length]
ENSURED_SIZET: (value.c:2600): FUNC(xrealloc): (ULONGEST to size_t) [(new_encl_type)->length]
SAFE: (value.c:2647): CMP: (int to ULONGEST) [(type)->length <= (int)sizeof(LONGEST)]
SAFE: (value.c:2648): ASSIGN: (LONGEST to int) [ v->bitpos = bitpos % container_bitsize]
- The local variable container_bitsize does not need to be LONGEST.
bitfield cannot be too large, at most about 64 bits or so.
'type' there is the containing the of the bitfield (such as long long),
not the whole struct the bitfield is contained in.
There could be some ENSURE instead as some bogus DWARF could violate that.
SAFE: (value.c:2650): ASSIGN: (LONGEST to int) [ v->bitpos = bitpos % 8]
SAFE: (value.c:2816): ASSIGN: (LONGEST to int) [ bytes_read = ((bitpos % 8) + bitsize + 7) / 8]
SAFE: (value.c:2818): ASSIGN: (ULONGEST to int) [ bytes_read = (field_type)->length]
SAFE: (value.c:2833): ASSIGN: (LONGEST to int) [ lsbcount = (bytes_read * 8 - bitpos % 8 - bitsize)]
SAFE: (value.c:2835): ASSIGN: (LONGEST to int) [ lsbcount = (bitpos % 8)]
SAFE: (value.c:3028): ASSIGN: (ULONGEST to int) [ len = (type)->length]
SAFE: (value.c:3063): ASSIGN: (ULONGEST to int) [ len = (type)->length]
ENSURED_SIZET(allocate_value_lazy): (value.c:3144): FUNC(memcpy): (ULONGEST to size_t) [(type)->length]
- I do not see any need and any change of source code here, it is protected by ENSURE in allocate_value_contents.
ENSURED_SIZET: (value.c:3160): FUNC(memcpy): (ULONGEST to size_t) [(type)->length]
- Likewise.
ENSURED_SIZET: (value.c:3186): FUNC(memcpy): (ULONGEST to size_t) [(type)->length]
- Likewise.
UNRELATED(children needs to be expanded maybe later as arrays fix): (varobj.c:3153): ASSIGN: (ULONGEST to int) [ children = (type)->length / (target)->length]
- You already made some such patch, this issue should be addressed.
Currently the implementation cannot cope with high number of children
but that is a different problem, GDB would overflow host memory.
SAFE: (vax-tdep.c:132): FUNC(write_memory): (LONGEST to ssize_t) [len]
SAFE: (vax-tdep.c:227): FUNC(read_memory): (LONGEST to ssize_t) [len]
SAFE: (vax-tdep.c:239): FUNC(memcpy): (LONGEST to size_t) [len]
SAFE: (vax-tdep.c:244): FUNC(memcpy): (LONGEST to size_t) [len]
SAFE: (xstormy16-tdep.c:287): ASSIGN: (LONGEST to int) [ slacklen = typelen & 1]
UNSAFE_ALLOCA: (xstormy16-tdep.c:288): FUNC(C_alloca): (LONGEST to size_t) [typelen + slacklen]
- Yes, fix it, please.
ENSURED_SIZET: (xstormy16-tdep.c:289): FUNC(memcpy): (LONGEST to size_t) [typelen]
ENSURED_SIZET: (xstormy16-tdep.c:293): FUNC(write_memory): (LONGEST to ssize_t) [typelen + slacklen]
SAFE: (xtensa-tdep.c:1621): ASSIGN: (LONGEST to int) [ offset = 4 - len]
SAFE: (xtensa-tdep.c:1626): FUNC(regcache_raw_read_part): (LONGEST to int) [len]
SAFE: (xtensa-tdep.c:1671): ASSIGN: (LONGEST to int) [ offset = 4 - len]
SAFE: (xtensa-tdep.c:1676): FUNC(regcache_raw_write_part): (LONGEST to int) [len]
SAFE: (xtensa-tdep.c:1822): ASSIGN: (ULONGEST to int) [ info->align = (arg_type)->length]
SAFE: (xtensa-tdep.c:1856): ASSIGN: (LONGEST to int) [ info->u.regno = (gdbarch_tdep(gdbarch)->call_abi == CallAbiCall0Only ? (gdbarch_tdep(gdbarch)->a0_base + 2) : (gdbarch_tdep(gdbarch)->a0_base + 6)) + size / 4]
FIXED(Expand n): (xtensa-tdep.c:1886): VARINIT(n): (LONGEST to int) [info->length]
- info->length could be just ssize_t; +swap the initialization lines:
info->length = TYPE_LENGTH (arg_type);
info->contents = value_contents (arg);
Then also this n would be just ssize_t.
Just a nitpick.
SAFE: (xtensa-tdep.c:1897): FUNC(write_memory): (LONGEST to ssize_t) [info->length]
- And this warning would no longer exist.
FIXED(Expand n): (xtensa-tdep.c:1902): VARINIT(n): (LONGEST to int) [info->length]
- Likewise.

-------------------------
Warnings I haven't figured out:

LOC breakpoint.c:15874 (breakpoint.c:15873)
Function observer_attach_memory_changed expects arg 1 to be observer_memory_changed_ftype * gets [function (CORE_ADDR, LONGEST, bfd_byte *) returns void]: invalidate_bp_value_on_memory_change
*** SAFE
- Again LONGEST len could be just ssize_t.

LOC findcmd.c:184
Conditional clauses are not of same type: (val_bytes) (LONGEST), (sizeof(int64_t)) (size_t)
*** SAFE
- Missing some ENSURE.

LOC i386-nat.c:531 (i386-nat.c:553)
Conditional clauses are not of same type: (max_wp_len - 1) (int), len - 1 (LONGEST)
*** WATCHPOINT
- I see no problem there now. i386 does not support large HW watchpoints,
it just needs to reject gigantic (>4GB) LEN requests.

LOC memrange.c:77
Conditional clauses are not of same type: (ra->length) (LONGEST), ((rb->start - ra->start) + rb->length) (ULONGEST)
*** SAFE

LOC python/py-type.c:197
Clauses exit with arg referencing fresh storage in true branch, local storage in false branch
*** SAFE
- I do not see what does it reference, the line numbers are shifted somehow, I use:
2636a39d8bf9b24dce328e4f906e8710b52d2105

LOC rs6000-aix-tdep.c:284 (rs6000-aix-tdep.c:283)
Conditional clauses are not of same type: reg_size (int), len - argbytes (LONGEST)
*** SAFE

LOC s390-tdep.c:2520
Operands of || are non-boolean (int): is_integer_like(type) || is_pointer_like(type)
*** SAFE

LOC sh-tdep.c:1322
Function regcache_raw_read expects arg 3 to be gdb_byte * gets char *: (char *)valbuf + i
*** SAFE

LOC stack.c:540 (stack.c:536)
Assignment of arbitrary UINTegral type to LONGEST: current_offset = ((current_offset + arg_size + sizeof(int) - 1) & ~(sizeof(int) - 1))
*** SAFE