On Sat, Nov 15, 2014 at 5:06 PM, Norihiro Tanaka <noritnk@kcn.ne.jp> wrote:
> On Sat, 15 Nov 2014 10:00:49 -0800
> Jim Meyering <jim@meyering.net> wrote:
>> Thank you for the patch.
>> That seems like a fine change, but so far, I cannot see how
>> it avoids accessing uninitialized memory.
>> I do see that it fixes an error whereby memcpy was being
>> called with its 2nd argument NULL, though in each case,
>> the third argument is always 0.  Passing a NULL pointer as
>> the 2nd argument to memcpy is officially "undefined
>> behavior", and I confirmed that building with gcc and its
>> "undefined behavior sanitizer", the problem was exposed,
>> and that your patch fixes it.
>>
>> Do you know of a way to make grep crash, as stated in your
>> proposed NEWS entry?  If so, please give details.
>>
>> It is UB after all.  Perhaps you found a system whose memcpy
>> dereferences the source pointer even when the size is 0?
>
> Thanks for the review.
>
> I ran accross this problem when I made next improvement.  If size is 0,
> when dfa_charclass_index has been called, the crash was caused.  And If
> I fixed it, the crash was not caused.  So I think that it is a bug.
>
> However, I deleted the branch as the improvement was bad.  And I cannot
> see cause of the bug in the source code.  I seem that the code has no bug.
> Further more, I could not reproduce it, though I re-wrote a similar code
> to the branch.
>
> Possibly other changes which I made are bad, and it might cause a
> buffer-overrun and override memory range for characlasses in the branch.

Thanks for confirming.
In that case, since I see no harm in calling xnmalloc with N = 0, I
will use a more conservative change: guard only the undefined use of
memcpy.
I've left your name on this amended patch.