Bram Moolenaar wrote:
> Dominique Pelle wrote:
>
>> --001485f04092b96eb10469cb5cc9
>> Content-Type: text/plain; charset=ISO-8859-1
>> Content-Transfer-Encoding: quoted-printable
>>
>> Bram Moolenaar wrote:
>>
>> > Patch 7.2.167
>> > Problem: Splint doesn't work well for checking the code.
>> > Solution: Add splint arguments in the Makefile. Exclude some code from
>> > splint that it can't handle. Tune splint arguments to give
>> > reasonable errors. Add a filter for removing false warnings
>> > from
>> > splint output. Many small changes to avoid warnings. More to
>> > follow...
>> > Files: Filelist, src/Makefile, src/buffer.c, src/charset.c,
>> > src/cleanlint.vim, src/digraph.c, src/edit.c, src/ex_cmds.c,
>> > src/globals.h, src/ops.c, src/os_unix.c, src/os_unix.h,
>> > src/proto/buffer.pro, src/proto/edit.pro, src/screen.c,
>> > src/structs.h
>>
>> Patch 7.2.167 introduces a regression: test40 fails on one of
>> my machine (Linux x86_64) but succeeds on another (Linux x86).
>>
>> Running test40 with valgrind reveals this error (on both machines):
>>
>> ==10911== Conditional jump or move depends on uninitialised value(s)
>> ==10911== at 0x80521F4: buf_same_ino (buffer.c:2944)
>> ==10911== by 0x805212E: otherfile_buf (buffer.c:2905)
>> ==10911== by 0x8050F21: buflist_findname_stat (buffer.c:2013)
>> ==10911== by 0x80502F9: buflist_new (buffer.c:1519)
>> ==10911== by 0x8092A3F: do_ecmd (ex_cmds.c:3308)
>> ==10911== by 0x80A96DF: do_exedit (ex_docmd.c:7584)
>> ==10911== by 0x80A9356: ex_edit (ex_docmd.c:7479)
>> ==10911== by 0x80A1EE3: do_one_cmd (ex_docmd.c:2622)
>> ==10911== by 0x809F763: do_cmdline (ex_docmd.c:1096)
>> ==10911== by 0x8124EB0: nv_colon (normal.c:5227)
>> ==10911== by 0x811E6C8: normal_cmd (normal.c:1189)
>> ==10911== by 0x80E1745: main_loop (main.c:1180)
>> ==10911== by 0x80E1292: main (main.c:939)
>>
>> src/buffer.c:
>>
>> 2939 static int
>> 2940 buf_same_ino(buf, stp)
>> 2941 buf_T *buf;
>> 2942 struct stat *stp;
>> 2943 {
>> 2944 return (buf->b_dev >= 0
>> 2945 && stp->st_dev == buf->b_dev
>> 2946 && stp->st_ino == buf->b_ino);
>> 2947 }
>> 2948 #endif
>>
>> Test 'buf->b_dev >= 0' at buffer.c:2940 is incorrect because
>> type of b_dev field is now 'dev_t' which appears to be
>> unsigned so test is always true (prior to patch 7.2.167,
>> type was 'int'). So Vim accesses stp->st_ino which is
>> uninitialized.
>>
>> Attached patch fixes the problem. All tests are OK after
>> applying attached patch.
>
> Thanks for pinpointing this bug. So there must be a place where -1 is
> assigned to b_dev, assuming that this will never be a valid value. I'm
> actually not sure this is the case.
There are a couple of places where Vim uses -1 for b_dev or
st_dev fields:
$ egrep '_dev.*-1' *.c
buffer.c: st.st_dev = (dev_T)-1;
buffer.c: if (st.st_dev == (dev_T)-1)
buffer.c: buf->b_dev = -1;
buffer.c: st.st_dev = (dev_T)-1;
buffer.c: st.st_dev = (dev_T)-1;
buffer.c: st.st_dev = (dev_T)-1;
buffer.c: if (st.st_dev == (dev_T)-1)
buffer.c: buf->b_dev = -1;
buffer.c: if (buf->b_dev == (dev_t)-1 || mch_stat((char
*)ffname, &st) < 0)
buffer.c: st.st_dev = (dev_T)-1;
buffer.c: buf->b_dev = -1;
buffer.c: return (buf->b_dev != (dev_t)-1
ex_cmds2.c: ((stat_ok && si->sn_dev != -1)
ex_cmds2.c: si->sn_dev = -1;
fileio.c: else if (buf->b_dev == (dev_t)-1)
misc2.c: int ffv_dev; /* device number (-1
if not set) */
misc2.c: vp->ffv_dev = -1;
> The problem is that splint gives so many warnings for assigning an int
> to an unsigned that I ignore that warning.
>
> splint turns out to be not a good tool. With the GTK GUI enabled it
> runs out of memory. I had to change a few source files to avoid it from
> chocking on non-ASCII characters. It doesn't understand an array of
> pointers.
I agree. I'm not a fan of splint either. I once tried it but the
amount of noise or spurious errors was just too high to use it
systematically. Using stricter warning options of the C compiler
gives better results on my opinion than using splint.
> Does someone know a better lint than splint? Unfortunately the lint I
> was using on FreeBSD doesn't appear to be available for Linux. It
> wasn't great but I least I could use it for some basic sanity checking.
The only good static analyzer I've heard of is coverity. I once saw
a demo and it looked very good, but it's also very expensive. I see
in the link below that Vim has already been checked with coverity
(no idea which version of Vim was tested):
http://scan.coverity.com/rung1.html
I don't know when or if the coverity scan will be rerun. I see
that the last run was in March 2007:
http://scan.coverity.com/ladder.html#future
That was more than 2 years ago. Maybe the coverity scan
program is dead. I hope not.
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
