|
From: | Paul Eggert |
Subject: | Re: FYI: dfa: add an assertion to avoid coverity false positive |
Date: | Wed, 14 Dec 2016 13:46:24 -0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |