[illumos-Developer] Off-by-one in _fgetws_impl()?

[Garrett D'Amore]

This is probably a bug I introduced, as I rewrote most of that function
from the original FreeBSD code that I imported.  :-/

Your fix looks good, but I think I would prefer to see it as:

	while (--n) {

This should also be slightly more efficient, since it can then generate
to simple branch-if-zero following the decrement, avoiding the extra
compare (subtract) against 1.

Its safe too, because the previous stanza guards against the case where
n == 0.

If you like my version of the fix, let me know, and I'll go ahead and
start the process to get it integrated.

	- Garrett


On Tue, 2011-03-08 at 00:00 -0800, Bryan Cantrill wrote:
> All,
> 
> In debugging another problem, sort(1) dumped core on me.  (Both the
> file that induced the core dump and the core dump itself are available
> -- for a limited time -- at ftp://dapsays.no.de.)
> 
> Here's the stack trace in the dump:
> 
>   > $c
>   libc_hwcap1.so.1`_fgetws_impl+0xa3(fe936ec4, 4f, 80757b0, 1, 8047c38)
>   libc_hwcap1.so.1`__fgetws_xpg5+0x23(fe936ec4, 4f, 80757b0, 808de80)
>   stream_wide_fetch+0x63(808de80, fe0384d0, 2, 0)
>   stream_insert+0xc0(806d380, 808de80, 808ddd0, 0)
>   internal_sort+0x18f(806d380, 3, 8047d14, feffbafc)
>   main+0x70(3, 8047d14, 8047d24, 805384f)
>   _start+0x7d(3, 8047de4, 8047df6, 8047df9, 0, 8047e00)
> 
> Died here:
> 
>   > libc_hwcap1.so.1`_fgetws_impl+0xa3/i
>   libc_hwcap1.so.1`_fgetws_impl+0xa3:             movl   %eax,(%edi)
>   > <edi=X
>                 fe937000
>   > <edi/X
>   mdb: failed to read data from target: no mapping for address
>   0xfe937000:
> 
> Suspiciously, %edi is exactly the input pointer (0xfe936ec4) plus the
> number of bytes (79) times the sizeof of a wchar_t (4).  It seems that
> _fgetws_impl() might have an off-by-one bug, and indeed, from the code
> it seems that -- in the case that we read to the end of the buffer --
> we overwrite the buffer by one byte.  The attached (excruciatingly
> simple) diff fixes the problem for me (dead reproducible on the
> input), but others may want to confirm that it's the proper fix...
> 
>         - Bryan
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer