Splint report and scripts for gdb (was: Expand bitpos and type.length)

Splint report and scripts for gdb (was: Expand bitpos and type.length)

[Siddhesh Poyarekar]

Hi Jan,

I'm breaking this out into a new thread since I'm finding it difficult
to search for responses in the old thread due to the huge amount of
nesting. The old thread is here:

http://sourceware.org/ml/gdb-patches/2012-06/msg00635.html

I started trying to reduce the splint report further, again with a
newer approach. We're essentially looking for splint warnings that are
present after applying the bitpos patch, including those that were
newly generated and were relevant to the bitpos change, so the old
output sections (The '-' lines) are unnecessary in the report.

Based on this, I did the following:

* Modified splint-locdiff script to remove all unnecessary lines
  like 'finished checking', etc. The change is small enough that one
  can see the changes straight away.

* Removed all the '-' lines, so that what we get is only the report of
  warnings generated after bitpos.

After all the sanitization, what we have left is a very clean pattern,
which is:

LOC (file:num)
  The warning for this LOC

I used this to evaluate if the assignments, function parameters, etc.
for each of those warnings were size-compatible, i.e. they won't result
in truncation, and passed those. I don't look at sign preservation
across the patch any more because it is easier to do that from the
patch rather than the splint output.

Along with this, I have tried to make the output of the final script as
compact as possible without losing readability. The result is a final
output of 666 lines. I have uploaded the tarball here:

http://siddhesh.fedorapeople.org/splint-bitpos.tar.xz

These are the exact same files that you had uploaded earlier, so you
should be able to git-init a repository from your files and then
overwrite them with my copy to get a difference.

Can you please review the script changes?

Thanks,
Siddhesh

Re: Splint report and scripts for gdb (was: Expand bitpos and type.length)

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 2191 bytes --]

On Thu, 28 Jun 2012 15:18:14 +0200, Siddhesh Poyarekar wrote:
> After all the sanitization, what we have left is a very clean pattern,
> which is:
> 
> LOC (file:num)
>   The warning for this LOC

I have read it etc. and I agree.

[...]
> http://siddhesh.fedorapeople.org/splint-bitpos.tar.xz
> 
> These are the exact same files that you had uploaded earlier, so you
> should be able to git-init a repository from your files and then
> overwrite them with my copy to get a difference.
> 
> Can you please review the script changes?

You have made '^ LOC' and '^   LOC' equal but they are not by the changes:
-1 while $f=~s{^ LOC .*\n(?: .*?\n)*?(.LOC )}{$1}gm;
+1 while $f=~s{^\s+LOC .*\n(?: .*?\n)*?((.\s*LOC )|(.Finished checking))}{$1}gm;
+# Finally, remove the extra spaces from the beginning of the LOC lines.
+$f=~s{^\s+(LOC.*\n)}{$1}gm;

'^ LOC' always starts a new splint record while '^   LOC' (more spaces) is
only supplementary information for that record.  See this single original
record:
ada-lang.c:6510: Fresh storage type not released before return
   ada-lang.c:6478: Fresh storage type created

Translated into:
LOC ada-lang.c:6492
 Fresh storage type not released before return
   LOC ada-lang.c:6460
 Fresh storage type created

While you now consider it as two different records.  But this should not miss
any record, it just adds more records to process - although you discard these
in splint-siddhesh-process-locdiff by rules like so it does not matter:
        if (/Previous definition of \w+: \w+/) {
Not sure if it was all intentional or not.

        # If the TYPE_LENGTH expression is a builtin_type then we're safe.
        $_ = $expr;
        return 1 if (/builtin_type[^ ]+->length/);
 - I would not call it safe, it can be a part of an arbitrary large expression.
   But the 6 cases where it applies are OK.

        if (/^ (Right)|(Left) operand of \W+ /) {
 - it did not discard any important line but the regex is dangerously wrong, it
   does:
        if (/^ Right/ || /Left operand of \W+ /) {


Otherwise I have made comments to your .report file.  I believe we should
follow now the 'compact' form instead.  It is a lot of work as I see.


Thanks,
Jan

[-- Attachment #2: splint-bitpos.locdiff.report.diff --]
[-- Type: text/plain, Size: 29973 bytes --]

--- splint-bitpos.locdiff.report-orig	2012-06-28 14:27:29.000000000 +0200
+++ splint-bitpos.locdiff.report	2012-07-01 21:34:05.919249908 +0200
@@ -3,10 +3,15 @@
 @@ -1,1255255 +1,1255562 @@
  LOC ada-lang.c:579 (ada-lang.c:578)
 - Function memcpy expects arg 3 to be size_t gets UINT: (type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - it is OK, anything in memory fits in size_t.  The problem is in allocate_value_contents:
+ value.c:695
+   xzalloc (TYPE_LENGTH (val->enclosing_type));
+ which is listed here later.
+ "iterative" is a good idea but that is too complicated change outside of the scope of this patch.
 
  LOC ada-lang.c:670 (ada-lang.c:669)
 - Function umax_of_size expects arg 1 to be int gets UINT: (t)->length
 + Function umax_of_size expects arg 1 to be int gets ULONGEST: (t)->length
 *** SAFE: Type passed to function are primitive.
@@ -58,10 +63,11 @@
 
  LOC ada-lang.c:2342 (ada-lang.c:2339)
 - Function memset expects arg 3 to be size_t gets UINT: (type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - the problem is again in allocate_value_contents, not here.
 
 +LOC ada-lang.c:2379
 + Assignment of LONGEST to int: src = targ = 0
 *** SAFE.
 
@@ -107,20 +113,23 @@
   Right operand of << may be negative (int): 1 << chunk_size
  LOC ada-lang.c:2545 (ada-lang.c:2542)
 - Function memcpy expects arg 3 to be size_t gets UINT: (type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - the problem is again in allocate_value_contents, not here.
 
  LOC ada-lang.c:4094 (ada-lang.c:4091)
 - Function write_memory expects arg 3 to be ssize_t gets int: len
 + Function write_memory expects arg 3 to be ssize_t gets LONGEST: len
 *** LIMITED.
+ - the problem is again in allocate_value_contents, not here.
 
  LOC ada-lang.c:4138 (ada-lang.c:4135)
 - Function memcpy expects arg 3 to be size_t gets UINT: (actual_type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (actual_type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - the problem is again in allocate_value_contents, not here.
 
  LOC ada-lang.c:4163 (ada-lang.c:4160)
 - Function C_alloca expects arg 1 to be size_t gets UINT: len
 + Function C_alloca expects arg 1 to be size_t gets ULONGEST: len
 *** SAFE: value_pointer only accepts address types.
@@ -170,54 +179,68 @@
 
  LOC ada-lang.c:8739 (ada-lang.c:8735)
 - Function memcmp expects arg 3 to be size_t gets UINT: (value_type(arg1))->length
 + Function memcmp expects arg 3 to be size_t gets ULONGEST: (value_type(arg1))->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - the problem is again in allocate_value_contents, not here.
 
  LOC ada-lang.c:8794 (ada-lang.c:8790)
 - Function ada_index_struct_field expects arg 1 to be int gets LONGEST: index
 -LOC ada-lang.c:8790
   New fresh storage (type struct type *) passed as implicitly temp (not released): value_type(lhs)
  LOC ada-tasks.c:785
 - Variable target_ptr_byte initialized to type UINT, expects int: (data->known_tasks_element)->length
 + Variable target_ptr_byte initialized to type ULONGEST, expects int: (data->known_tasks_element)->length
 *** SAFE: known_tasks_element is usually a pointer.
+ - it seems read_known_tasks_array's extract_typed_address will assert-crash if it is not a pointer.
+   It does not necessarily needs to be a pointer, ada_tasks_inferior_data_sniffer can read anything
+   from inferior symbol tables.  But non-pointer types already do not make sense there.
 
  LOC ada-tasks.c:812
 - Variable target_ptr_byte initialized to type UINT, expects int: (data->known_tasks_element)->length
 + Variable target_ptr_byte initialized to type ULONGEST, expects int: (data->known_tasks_element)->length
 *** SAFE: known_tasks_element is usually a pointer.
+ - likewise
 
  LOC ada-valprint.c:376
 - Function ada_emit_char expects arg 5 to be int gets UINT: (type)->length
 + Function ada_emit_char expects arg 5 to be int gets ULONGEST: (type)->length
 *** SAFE: type is a char.
 
  ada-valprint.c: (in function ada_print_scalar)
  LOC ada-valprint.c:483 (ada-valprint.c:491)
 + Operands of < have incompatible types (UINT, ULONGEST): i < length
 *** FIX: Make i, rep1, reps ULONGEST.
+ - AI: Yes, I agree to fix what you describe.
+ - additionally: options->print_max is always at most UINT_MAX.
+   One may ask more for options->print_max being ULONGEST but that also does
+   not make sense, such text output would be just too long.
+   No need to fix that part.
 
 +LOC ada-valprint.c:491
   Test expression for if not boolean, type int: quit_flag
  LOC ada-valprint.c:501 (ada-valprint.c:502)
 - Function char_at expects arg 2 to be int gets UINT: rep1
 + Operands of < have incompatible types (UINT, ULONGEST): rep1 < length
 *** FIX: Make i, rep1, reps ULONGEST.
+ - Likewise above.
 
  LOC ada-valprint.c:502 (ada-valprint.c:503)
 - Function char_at expects arg 2 to be int gets UINT: i
 + Function char_at expects arg 2 to be LONGEST gets UINT: rep1
 *** SAFE.
+ - I will be non-issue after the change above.
 
 +LOC ada-valprint.c:503
 + Function char_at expects arg 2 to be LONGEST gets UINT: i
 *** SAFE.
+ - I will be non-issue after the change above.
 
  LOC ada-valprint.c:553
 + Operands of < have incompatible types (UINT, ULONGEST): i < length
 *** FIX: Make i, rep1, reps ULONGEST.
+ - I will be non-issue after the change above.
 
 +LOC ada-valprint.c:553
   Left operand of || is non-boolean (int): force_ellipses || i < length
  ada-valprint.c: (in function ada_printstr)
  LOC ada-valprint.c:563
@@ -252,10 +275,13 @@
 
   Null storage passed as non-null param: ada_value_primitive_packed_val (NULL, ...)
 +LOC alpha-tdep.c:402
 + Assignment of LONGEST to int: accumulate_size = (accumulate_size + m_arg->len + 7) & ~7
 *** FIX: Expand required_arg_regs, accumulate_size, m_arg->offset and offset
+ - AI: Yes, I agree.
+ - AI: But m_arg->len should be only ssize_t, as it depends on contents which
+   is in host memory.  Similar issue to allocate_value_contents.
 
  LOC alpha-tdep.c:432
 - Operands of <= have incompatible types (int, size_t): offset + len <= sizeof((arg_reg_buffer))
 + Operands of <= have incompatible types (LONGEST, size_t): offset + len <= sizeof((arg_reg_buffer))
 *** SAFE.
@@ -267,10 +293,11 @@
 
  LOC alpha-tdep.c:448
 - Function write_memory expects arg 3 to be ssize_t gets int: len
 + Function write_memory expects arg 3 to be ssize_t gets LONGEST: len
 *** LIMITED.
+ - This is non-issue after the last AI above.
 
  LOC alpha-tdep.c:534 (alpha-tdep.c:537)
 + Function store_unsigned_integer expects arg 2 to be int gets LONGEST: length
 *** SAFE.
 
@@ -320,36 +347,53 @@
 *** LIMITED.
 
 +LOC amd64-windows-tdep.c:119
 + Function regcache_raw_read_part expects arg 4 to be int gets LONGEST: len
 *** UNSURE: Should be safe since regcache_raw_read* reads from the *register* cache
+ - AI: regcache_raw_read_part and regcache_raw_write_part (and their callee
+   regcache_xfer_part) should get the parameter upgraded to LONGEST.  This way
+   gdb_assert in regcache_xfer_part will catch possibly too large LEN.
+   Currently GDB could return invalid value without failing that assertion.
+   Sure nothing such serious.
 
  LOC amd64-windows-tdep.c:121 (amd64-windows-tdep.c:77)
 + Function regcache_raw_write_part expects arg 4 to be int gets LONGEST: len
 *** UNSURE: Should be safe since regcache_raw_read* reads from the *register* cache
+ - AI: Likewise.
 
 +LOC amd64-windows-tdep.c:77
   Parameter gdbarch not used
  LOC arm-linux-nat.c:932
 - Right operand of << may be negative (int): 1 << len
 + Right operand of << may be negative (LONGEST): 1 << len
 *** UNSURE: maybe len should not be expanded at all.
+ - I would say keep it that way.  Or even put there gdb_assert (len < sizeof (int) * 8);
+   Here arm_linux_region_ok_for_hw_watchpoint (implementation of
+   to_region_ok_for_hw_watchpoint, therefore target_region_ok_for_hw_watchpoint)
+   already ensures LEN cannot be too large so I agree LEN here could be just
+   int.  OTOH if to_region_ok_for_hw_watchpoint gets the int type we would need
+   to re-check all other files installing to_region_ok_for_hw_watchpoint
+   implementation they protect the size early enougn.  We could no longer
+   assert-check any LEN in to_insert_watchpoint of other implementations
+   afterwards.
 
  LOC arm-linux-nat.c:932
 - Assignment of int to UINT: mask = (1 << len) - 1
 + Assignment of LONGEST to UINT: mask = (1 << len) - 1
 *** UNSURE: maybe len should not be expanded at all.
+ - Likewise.
 
  LOC arm-tdep.c:3289
 - Return value type UINT does not match declared type int: (t)->length
 + Return value type ULONGEST does not match declared type int: (t)->length
 *** SAFE: Types are primitive.
 
  LOC arm-tdep.c:3422
 - Return value type UINT does not match declared type int: (t)->length / unitlen
 + Return value type ULONGEST does not match declared type int: (t)->length / unitlen
 *** FIX: Make arm_vfp_cprc_sub_candidate return LONGEST.
+ - AI: Yes.
 
  LOC arm-tdep.c:3689 (arm-tdep.c:3690)
 + Function extract_unsigned_integer expects arg 2 to be int gets LONGEST: len
 *** SAFE.
 
@@ -378,10 +422,12 @@
  avr-tdep.c: (in function avr_pointer_to_address)
  LOC avr-tdep.c:932 (avr-tdep.c:937)
 - Operands of < have incompatible types (int, UINT): i < (valtype)->length
 + Function regcache_cooked_write expects arg 2 to be int gets LONGEST: lsb_reg + i
 *** SAFE: type is neither struct nor array.
+ - AI: Moreover for value > 8 it would get gdb_assert_not_reached ("unexpected type length");
+   Therefore simplify it, 'i' can be just 'int'.
 
 +LOC avr-tdep.c:937
 + Operands of < have incompatible types (LONGEST, ULONGEST): i < (valtype)->length
 *** SAFE: type is neither struct nor array.
 
@@ -393,14 +439,22 @@
   Return value (type enum register_status) ignored: regcache_cooked_...
  LOC avr-tdep.c:1184 (avr-tdep.c:1183)
 - Function xmalloc expects arg 1 to be size_t gets int: len
 + Function xmalloc expects arg 1 to be size_t gets LONGEST: len
 *** LIMITED.
+ - AI: It is limited at the caller, not here.  It may be dangerous if new code
+   starts using this function - it has LONGEST parameter but it cannot handle
+   LONGEST.  Just make LEN ssize_t.
+   The same applies to arm-tdep.c push_stack_item and cris-tdep.c push_stack_item.
 
  LOC avr-tdep.c:1185
 + Assignment of LONGEST to int: si->len = len
 *** FIX: Expand stack_item->len.
+ - AI: Yes and no.  Function already gets CONTENTS in host memory, so the LEN
+   size should be ssize_t, not LONGEST.
+   (I understand we are not strict in this regard and in many LONGEST cases we
+   could probably rather use ssize_t.)
 
 +LOC avr-tdep.c:1186
   Implicitly only storage si->prev (type struct stack_item *) not released before assignment: si->prev = prev
  LOC avr-tdep.c:1187 (avr-tdep.c:1186)
 - Function memcpy expects arg 3 to be size_t gets int: len
@@ -414,10 +468,11 @@
 *** SAFE.
 
 +LOC avr-tdep.c:1290
 + Operands of < have incompatible types (int, LONGEST): j < len
 *** FIX: Expand j.
+ - AI: Yes; in fact both LEN and J can be just ssize_t when we need value_contents anyway.
 
 +LOC avr-tdep.c:1292
 + Right operand of >> may be negative (LONGEST): val >> (8 * (len - j - 1))
 *** SAFE.
 
@@ -434,10 +489,12 @@
 
  LOC ax-gdb.c:512 (ax-gdb.c:511)
 - Function ax_trace_quick expects arg 2 to be int gets UINT: (type)->length
 + Function ax_trace_quick expects arg 2 to be int gets ULONGEST: (type)->length
 *** SAFE: Primitive types.
+ - AI: While I agree I would prefer making ax_trace_quick's N to be LONGEST.
+   To catch some more complicated internal issues.  Not important.
 
 -LOC ax-gdb.c:597
 - Function gen_offset expects arg 2 to be int gets LONGEST: frame_offset
  ax-gdb.c: (in function gen_frame_locals_address)
 -LOC ax-gdb.c:612
@@ -448,10 +505,11 @@
  ax-gdb.c: (in function gen_var_ref)
  ax-gdb.c: (in function gen_ptradd)
  LOC ax-gdb.c:1424 (ax-gdb.c:1427)
 + Function  expects arg 2 to be int gets LONGEST: end - start
 *** SAFE: bitfield ref, so end - start should fit into int.
+ - OK, there is even internal_error above.
 
 +LOC ax-gdb.c:1428
   Implicitly only storage value->type (type struct type *) not released before assignment: value->type = type
  ax-gdb.c: (in function gen_primitive_field)
  LOC bfin-tdep.c:538
@@ -460,23 +518,26 @@
 *** LIMITED.
 
  LOC breakpoint.c:2173 (breakpoint.c:2179)
 + Assignment of LONGEST to int: bl->target_info.length = bl->length
 *** FIX: Expand struct bp_target_info.length.
+ - AI: Yes.
 
 +LOC breakpoint.c:2180
   Test expression for if not boolean, type int: is_breakpoint(bl->owner)
  LOC breakpoint.c:4396 (breakpoint.c:4393)
 + Function  expects arg 4 to be int gets LONGEST: loc->length
 *** FIX: Expand to_watchpoint_addr_within_range arg 4.
+ - AI: Yes.
 
 +LOC breakpoint.c:4394
   Test expression for if not boolean, type int: (*&current_target.to_watchpoint_addr_within_range)(&current_target, addr, loc->address, loc->length)
  LOC breakpoint.c:6374 (breakpoint.c:6371)
 - Operands of && are non-boolean (int): bl->length && breakpoint_address_match_range(bl->pspace->aspace, bl->address, bl->length, aspace, addr)
 + Function breakpoint_address_match_range expects arg 3 to be int gets LONGEST: bl->length
 *** FIX: Expand breakpoint_address_match_range arg 3.
+ - AI: Yes.
 
 +LOC breakpoint.c:6372
 + Operands of && are non-booleans (LONGEST, int): bl->length && breakpoint_address_match_range(bl->pspace->aspace, bl->address, bl->length, aspace, addr)
 *** SAFE.
 
@@ -484,21 +545,24 @@
   Return value (type int) ignored: create_breakpoin...
 +breakpoint.c: (in function breakpoint_hit_ranged_breakpoint)
 +LOC breakpoint.c:9594
 + Function breakpoint_address_match_range expects arg 3 to be int gets LONGEST: bl->length
 *** FIX: Expand breakpoint_address_match_range arg 3.
+ - AI: Yes, already above.
 
  breakpoint.c: (in function resources_needed_ranged_breakpoint)
  LOC breakpoint.c:15660 (breakpoint.c:15659)
 - Function observer_attach_memory_changed expects arg 1 to be observer_memory_changed_ftype * gets [function (CORE_ADDR, int, bfd_byte *) returns void]: invalidate_bp_value_on_memory_change
 + 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: Generated code from doc/observer.texi.
+ - Yes, splint got confused somehow.
 
  LOC c-lang.c:205
 - Variable width initialized to type UINT, expects int: (type)->length
 + Variable width initialized to type ULONGEST, expects int: (type)->length
 *** SAFE: width is unused.
+ - AI: But then width must be removed as it contains invalid number now.
 
  LOC c-lang.c:292
 - Assignment of UINT to int: width = (element_type)->length
 + Assignment of ULONGEST to int: width = (element_type)->length
 *** SAFE: element_type is char-like.
@@ -549,22 +613,34 @@
 *** SAFE: Those paths throw exceptions.
 
  LOC cp-valprint.c:363 (cp-valprint.c:366)
 + Variable i_offset initialized to type LONGEST, expects int: offset + ((((type)->main_type->flds_bnds.fields[i]).loc.bitpos) + 0) / 8
 *** FIX: expand i_offset.
+ - AI: Yes.
 
 +LOC cp-valprint.c:366
   Test expression for if not boolean, type int: valprint_check_validity(stream, i_type, i_offset, val)
  LOC cp-valprint.c:560 (cp-valprint.c:559)
 - Function C_alloca expects arg 1 to be size_t gets UINT: (baseclass)->length
 + Function C_alloca expects arg 1 to be size_t gets ULONGEST: (baseclass)->length
 *** LATERFIX: alloca with large size may be unsafe.
+ - AI: Better to do a real fix.
+   gdb_byte *buf;
+   struct cleanup *back_to;
+   if ((size_t) TYPE_LENGTH (baseclass) != TYPE_LENGTH (baseclass))
+     error (_("Target object too large for host GDB memory."));
+   buf = xmalloc (TYPE_LENGTH (baseclass));
+   back_to = make_cleanup (xfree, buf);
+   [...]
+   do_cleanups (back_to);
+   The same should be done in allocate_value_contents.
 
  LOC cp-valprint.c:563 (cp-valprint.c:562)
 - Function target_read_memory expects arg 3 to be ssize_t gets UINT: (baseclass)->length
 + Function target_read_memory expects arg 3 to be ssize_t gets ULONGEST: (baseclass)->length
 *** LIMITED.
+ - See above.
 
  LOC cris-tdep.c:683
 - Function xmalloc expects arg 1 to be size_t gets int: len
 + Function xmalloc expects arg 1 to be size_t gets LONGEST: len
  LOC cris-tdep.c:684 (cris-tdep.c:685)
@@ -573,10 +649,11 @@
   Implicitly only storage si->prev (type struct stack_item *) not released before assignment: si->prev = prev
  LOC cris-tdep.c:686
 - Function memcpy expects arg 3 to be size_t gets int: len
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - AI: See push_stack_item note above, it is the same function.
 
  cris-tdep.c: (in function cris_frame_unwind_cache)
  LOC cris-tdep.c:899
 - Function write_memory expects arg 3 to be ssize_t gets int: len
 + Function write_memory expects arg 3 to be ssize_t gets LONGEST: len
@@ -599,25 +676,29 @@
 + Operands of == have incompatible types (LONGEST, UINT): len * 8 == gdbarch_long_double_format(gdbarch)[0]->totalsize
  LOC doublest.c:857
 - Function memset expects arg 3 to be size_t gets UINT: (type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed.  Caller already gave host GDB memory ADDR so TYPE has to fit in host memory.
 
  LOC doublest.c:883
 - Function memset expects arg 3 to be size_t gets UINT: (to_type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (to_type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC doublest.c:893
 - Function memset expects arg 3 to be size_t gets UINT: (to_type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (to_type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC doublest.c:894
 - Function memcpy expects arg 3 to be size_t gets UINT: (((from_type)->length) < ((to_type)->length) ? ((from_type)->length) : ((to_type)->length))
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (((from_type)->length) < ((to_type)->length) ? ((from_type)->length) : ((to_type)->length))
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC dwarf2expr.c:452
 - Operands of != have incompatible types (UINT, int): (result)->length != size
 + Operands of != have incompatible types (ULONGEST, int): (result)->length != size
  LOC dwarf2expr.c:1050
@@ -628,10 +709,11 @@
 + Function C_alloca expects arg 1 to be size_t gets ULONGEST: (type)->length
  LOC dwarf2loc.c:1279
 - Function memcpy expects arg 3 to be size_t gets UINT: (checked_type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (checked_type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed, it is reponsibility of value_contents_raw.
 
  LOC dwarf2loc.c:1496 (dwarf2loc.c:1503)
 + Assignment of ULONGEST to UINT: source_offset_bits = source_offset % 8
 +LOC dwarf2loc.c:1498
 + Assignment of ULONGEST to UINT: dest_avail = 8 - dest_offset_bits % 8
@@ -676,10 +758,11 @@
 - Function get_frame_register_bytes expects arg 4 to be int gets size_t: this_size
 + Function get_frame_register_bytes expects arg 4 to be int gets ULONGEST: this_size
  LOC dwarf2loc.c:1631 (dwarf2loc.c:1633)
 + Function memset expects arg 3 to be size_t gets ULONGEST: this_size
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed, it is reponsibility of code acquiring the buffer, such as value_contents_raw.
 
 +LOC dwarf2loc.c:1633
   Test expression for if not boolean, type int: optim
  LOC dwarf2loc.c:1636
 - Function mark_value_bytes_unavailable expects arg 2 to be int gets LONGEST: offset
@@ -765,10 +848,11 @@
 -LOC dwarf2loc.c:2255
   Fresh storage objfile_gdbarch not released before scope exit
  LOC dwarf2loc.c:2257
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: n
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
 +LOC dwarf2loc.c:2258
   Fresh storage value not released before scope exit
  LOC dwarf2loc.c:2277 (dwarf2loc.c:2276)
 - Operands of > have incompatible types (size_t, UINT): n > (type)->length
@@ -779,10 +863,11 @@
  dwarf2read.c: (in function dwarf2_section_buffer_overflow_complaint)
  LOC dwarf2read.c:6117
 - Function memcpy expects arg 3 to be size_t gets UINT: (type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC dwarf2read.c:7763 (dwarf2read.c:7764)
 + Assignment of size_t to ULONGEST: dlbaton->size = ((attr)->u.blk)->size
 +LOC dwarf2read.c:7764
   Implicitly only storage dlbaton->per_cu (type struct dwarf2_per_cu_data *) not released before assignment: dlbaton->per_cu = cu->per_cu
@@ -912,20 +997,23 @@
 + Variable ptr_size initialized to type ULONGEST, expects size_t: (ptr_type)->length
  LOC eval.c:446 (eval.c:445)
 - Function memcpy expects arg 3 to be size_t gets UINT: (value_type(val))->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (value_type(val))->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:489 (eval.c:488)
 - Function memcpy expects arg 3 to be size_t gets int: element_size
 + Function memcpy expects arg 3 to be size_t gets LONGEST: element_size
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:498 (eval.c:497)
 - Function memcpy expects arg 3 to be size_t gets int: element_size
 + Function memcpy expects arg 3 to be size_t gets LONGEST: element_size
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:611 (eval.c:610)
 - Operands of > have incompatible types (UINT, int): (type1)->length * 8 > gdbarch_double_bit(gdbarch)
 + Operands of > have incompatible types (ULONGEST, int): (type1)->length * 8 > gdbarch_double_bit(gdbarch)
  LOC eval.c:612 (eval.c:611)
@@ -939,25 +1027,29 @@
 + Operands of > have incompatible types (ULONGEST, int): result_len > gdbarch_long_bit(gdbarch) / 8
  LOC eval.c:979 (eval.c:978)
 - Function memset expects arg 3 to be size_t gets UINT: (type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:998 (eval.c:997)
 - Function memset expects arg 3 to be size_t gets UINT: (expect_type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (expect_type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:1029 (eval.c:1028)
 - Function memcpy expects arg 3 to be size_t gets int: element_size
 + Function memcpy expects arg 3 to be size_t gets LONGEST: element_size
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC eval.c:1052 (eval.c:1051)
 - Function memset expects arg 3 to be size_t gets UINT: (type)->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed like above.
 
  LOC f-valprint.c:85
   Return value type LONGEST does not match declared type int: ((((((type))->main_type->flds_bnds.fields[0]).type))->main_type->flds_bnds.bounds->high)
 -f-valprint.c: (in function f77_get_dynamic_length_of_aggregate)
 -LOC f-valprint.c:115
@@ -988,10 +1080,16 @@
 + Conditional clauses are not of same type: (val_bytes) (LONGEST), (sizeof(int64_t)) (size_t)
  LOC findcmd.c:216
 - Function memcpy expects arg 3 to be size_t gets int: val_bytes
 + Function memcpy expects arg 3 to be size_t gets LONGEST: val_bytes
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - AI: This patch did not make it non-compliant but one should fix instead xrealloc above.
+   But the code is already buggy anyway, filed as: 
+   There should be something like: http://sourceware.org/bugzilla/show_bug.cgi?id=14321
+   if ((size_t) (pattern_buf_size * 2) != pattern_buf_size)
+     error (_("Target object too large for host GDB memory."));
+   But there will not be * 2 after fix of that bug.
 
  findvar.c: (in function signed_pointer_to_address)
  findvar.c: (in function address_to_signed_pointer)
  LOC findvar.c:468
 + Function store_signed_integer expects arg 2 to be int gets LONGEST: len
@@ -999,20 +1097,23 @@
   New fresh storage (type gdb_byte *) passed as implicitly temp (not released): value_contents_raw(v)
  LOC findvar.c:493
 - Function memcpy expects arg 3 to be size_t gets int: len
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed.  It is responsibility of value_contents_raw.
 
  LOC frame.c:872
 - Function memcpy expects arg 3 to be size_t gets UINT: (value_type(value))->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (value_type(value))->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed.  It is responsibility of value_contents_all and the caller.
 
  LOC frame.c:874
 - Function memset expects arg 3 to be size_t gets UINT: (value_type(value))->length
 + Function memset expects arg 3 to be size_t gets ULONGEST: (value_type(value))->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Again.
 
  LOC frv-tdep.c:1129 (frv-tdep.c:1135)
 + Function store_unsigned_integer expects arg 2 to be int gets LONGEST: len
 +LOC frv-tdep.c:1135
   Passed storage &regval not completely defined: regcache_cooked_read_unsigned (..., &regval)
@@ -1022,10 +1123,11 @@
   Function extract_unsigned_integer expects arg 1 to be gdb_byte * gets char *: val
  LOC frv-tdep.c:1337
 - Function memcpy expects arg 3 to be size_t gets int: len
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - No, there is 'if (len <= 4)' above.
 
  frv-tdep.c: (in function frv_return_value)
  LOC gdbtypes.c:804
 - Operands of > have incompatible types (UINT, size_t): (type)->length > sizeof(LONGEST)
 + Operands of > have incompatible types (ULONGEST, size_t): (type)->length > sizeof(LONGEST)
@@ -1075,15 +1177,17 @@
 + Function C_alloca expects arg 1 to be size_t gets LONGEST: padded_len
  LOC h8300-tdep.c:677
 - Function memset expects arg 3 to be size_t gets int: padded_len
 + Function memset expects arg 3 to be size_t gets LONGEST: padded_len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - AI: There is value_contents so rather LEN and PADDED_LEN can be ssize_t instead.
 
  LOC h8300-tdep.c:679
 - Function memcpy expects arg 3 to be size_t gets int: len
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - See above.
 
  LOC h8300-tdep.c:686
 - Right operand of && is non-boolean (int): len > wordsize && len % wordsize
 + Right operand of && is non-boolean (LONGEST): len > wordsize && len % wordsize
  LOC h8300-tdep.c:689
@@ -1112,10 +1216,11 @@
 + Function write_memory expects arg 3 to be ssize_t gets ULONGEST: (type)->length
  LOC hppa-tdep.c:766
 - Function memcpy expects arg 3 to be size_t gets UINT: (type)->length
 + Function memcpy expects arg 3 to be size_t gets ULONGEST: (type)->length
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed, responsibility of value_contents.
 
  LOC hppa-tdep.c:867
 - Variable len initialized to type UINT, expects int: (type)->length
 + Variable len initialized to type ULONGEST, expects int: (type)->length
  LOC hppa-tdep.c:889
@@ -1182,20 +1287,23 @@
 + Function write_memory expects arg 3 to be ssize_t gets LONGEST: len
  LOC i386-tdep.c:2470
 - Function memset expects arg 3 to be size_t gets int: len
 + Function memset expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Nothing needed, responsibility of the caller.
 
  LOC i386-tdep.c:2489
 - Function memcpy expects arg 3 to be size_t gets int: len
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Likewise.
 
  LOC i386-tdep.c:2496
 - Function memcpy expects arg 3 to be size_t gets int: len - low_size
 + Function memcpy expects arg 3 to be size_t gets LONGEST: len - low_size
 *** REVIEW: Might need iterative copying when sizeof(ULONGEST) > sizeof(size_t)
+ - Likewise.
 
  LOC i386-tdep.c:2556 (i386-tdep.c:2565)
 - New fresh storage (type char *) passed as implicitly temp (not released): gettext("Cannot store return value of %d bytes long.")
 + Function regcache_raw_write_part expects arg 4 to be int gets LONGEST: len
 +LOC i386-tdep.c:2561

Re: Splint report and scripts for gdb (was: Expand bitpos and type.length)

[Siddhesh Poyarekar]

On Sun, 1 Jul 2012 21:37:55 +0200, Jan wrote:
> While you now consider it as two different records.  But this should
> not miss any record, it just adds more records to process - although
> you discard these in splint-siddhesh-process-locdiff by rules like so
> it does not matter: if (/Previous definition of \w+: \w+/) {
> Not sure if it was all intentional or not.

It was intentional, because the extra spaces are also used in cases
where there are two warnings for one line.
 
>         # If the TYPE_LENGTH expression is a builtin_type then we're
> safe. $_ = $expr;
>         return 1 if (/builtin_type[^ ]+->length/);
>  - I would not call it safe, it can be a part of an arbitrary large
> expression. But the 6 cases where it applies are OK.

Yes, this was hard-coding on my part based on the input.

>         if (/^ (Right)|(Left) operand of \W+ /) {
>  - it did not discard any important line but the regex is dangerously
> wrong, it does:
>         if (/^ Right/ || /Left operand of \W+ /) {

This should have been:

if (/^ ((Right)|(Left)) operand of \W+ /) {

> 
> Otherwise I have made comments to your .report file.  I believe we
> should follow now the 'compact' form instead.  It is a lot of work as
> I see.

Thanks, that was the report before I did further work on the script.
I'll work on the rest and post the report with the compact form next.

Regards,
Siddhesh