[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1559767 - /subversion/trunk/subversion/libsvn_delta/xdelta.c

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Thu, 20 Feb 2014 11:55:26 +0100

On Wed, Feb 19, 2014 at 2:34 AM, Philip Martin
<philip.martin_at_wandisco.com>wrote:

> Philip Martin <philip.martin_at_wandisco.com> writes:
>
> > $ valgrind -q .libs/lt-random-test 2
> > ==19698== Conditional jump or move depends on uninitialised value(s)
> > ==19698== at 0x402EAA3: bcmp (mc_replace_strmem.c:935)
> > ==19698== by 0x4052114: find_block (xdelta.c:201)
> > ==19698== by 0x4052314: find_match (xdelta.c:284)
> > ==19698== by 0x40527A3: compute_delta (xdelta.c:428)
> > ==19698== by 0x4052A4D: svn_txdelta__xdelta (xdelta.c:494)
> > ==19698== by 0x404FF49: compute_window (text_delta.c:160)
> > ==19698== by 0x4050718: txdelta_next_window (text_delta.c:392)
> > ==19698== by 0x40504D7: svn_txdelta_next_window (text_delta.c:346)
> > ==19698== by 0x402794: do_random_combine_test (random-test.c:446)
> > ==19698== by 0x402A4B: random_combine_test (random-test.c:499)
> > ==19698== by 0x403BDBB: do_test_num (svn_test_main.c:396)
> > ==19698== by 0x403CECD: main (svn_test_main.c:866)
>
> The following patch causes the test to abort showing that the
> pre-condition mentioned in the comment is not met:
>
> Index: sw/subversion/src2/subversion/libsvn_delta/xdelta.c
> ===================================================================
> --- sw/subversion/src2/subversion/libsvn_delta/xdelta.c (revision 1569558)
> +++ sw/subversion/src2/subversion/libsvn_delta/xdelta.c (working copy)
> @@ -412,6 +412,8 @@ compute_delta(svn_txdelta__ops_baton_t *build_bato
> {
> apr_size_t matchlen;
> apr_size_t apos;
> + apr_size_t lo_orig = lo;
> + apr_uint32_t rolling_orig = rolling;
>
> /* Quickly skip positions whose respective ROLLING checksums
> definitely do not match any SLOT in BLOCKS. */
> @@ -425,6 +427,7 @@ compute_delta(svn_txdelta__ops_baton_t *build_bato
> /* LO is still <= UPPER, i.e. the following lookup is legal:
> Closely check whether we've got a match for the current location.
> Due to the above pre-filter, chances are that we find one. */
> + SVN_ERR_ASSERT_NO_RETURN(lo <= upper);
> matchlen = find_match(&blocks, rolling, a, asize, b, bsize,
> &lo, &apos, pending_insert_start);
>
> With seed=4237012947 and catching the abort in gdb I see
>
> (gdb) p lo
> $1 = 7666
> (gdb) p upper
> $2 = 7610
> (gdb) p lo_orig
> $3 = 7666
>
> The previous iteration, when lo_orig=7591, calls find_match with lo=7592
> and that sets lo=7587 and matchlen=79. Then lo+=matchlen causes lo>upper.
>

Thanks to your analysis, Philip, this has been easy
to fix: r1570141.

-- Stefan^2.
Received on 2014-02-20 11:56:04 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.