Re: FYI: dfa: add an assertion to avoid coverity false positive

[Paul Eggert]

On 12/13/2016 10:49 PM, Jim Meyering wrote:
> It took me a few minutes to convince myself that a coverity warning
> was unwarranted, so I've added an assert that should suppress it.

I looked at it for a bit and found a way to justify some sort of warning 
in that area, although the failure is extremely unlikely. In 
realloc_trans_if_necessary, if new_state equals PTRDIFF_MAX - 1 then 
'new_state + 2' overflows, yielding undefined behavior. This can happen 
on weird platforms where PTRDIFF_MAX is much less than SIZE_MAX (e.g., 
32-bit ptrdiff_t and 64-bit size_t).

Admittedly I don't know of any such platforms. Still, over time I am 
becoming more inclined to like the Emacs model, where object counts are 
typically kept as nonnegative but signed integers. This approach makes C 
code a bit more reliable, as compiling with -fsanitize=undefined is more 
likely to catch integer overflow errors in index calculations (a real 
problem nowadays). The Emacs allocators check that counts fit into both 
ptrdiff_t and size_t (the latter for safety when dealing with low-level 
functions that use size_t). Although the Emacs approach cannot allocate 
char buffers with more than PTRDIFF_MAX bytes, in some sense this is a 
good thing since pointer subtraction does not work properly with those 
buffers anyway. Besides, dfa.c's state_num is already using ptrdiff_t 
for object counts, so it's not significantly restricting dfa.c if we 
change it to use ptrdiff_t in some more places.

To make a long story short, I installed the attached patch, and hope it 
pacifies Coverity without the need for that assertion.

0001-dfa-fix-some-unlikely-integer-overflows.patch
Description: Source code patch