Reachable abort() in 'svn merge'.

Paul Burba found a way to reach an abort() from 'svn merge'. He posted
about it at http://rifers.org/paste/show/7044. Here is a self-contained
reproduction recipe based on that paste:

   $ svn co -r30328 http://svn.collab.net/repos/svn/branches/1.5.x wc
   $ cd wc
   $ svn merge \
     http://svn.collab.net/repos/svn/branches/1.5.x-r30215 . --reintegrate
   [see abort happen]
   $

It's in libsvn_client/merge.c:calculate_left_hand_side(), here:

      /* We only got here because we had mergeinfo for the source; if
         there were no segments, then our logic was wrong. */
      abort();

In IRC just now, Paul made these comments

   <pburba> kfogel: It looks like this will occur anytime you copy A to
   B, then reintegrate B back to A without ever merging anything from A
   to B this will happen.

   <pburba> kfogel: B has no explicit mergeinfo from A, so once we
   'filter mergeinfo_catalog so that all of the ranges come fromthe
   target's history' then all we are left with is empty mergeinfo. This
   is still sufficient to make have_mergeinfo_for_source TRUE though, so
   we get into that 'else if (!have_mergeinfo_for_descendants)' block
   but end up with a NULL rangelist and that gets us to the abort().

It seems to me Paul is closer to fixing it than I am to finishing this
mail, so I'm just going to post now for the record. Paul can refer to
this thread in his commit, or just paste the repro recipe above into his
log message.

Below are some things I found poking around quickly in GDB, before he
made his comments.

   (gdb) p ((svn_ra_neon__session_t *) (ra_session->priv))->url->data
   $4 = 0x818ea88 "https://svn.collab.net/repos/svn"

So the session is rooted at the repository root, as it should be. Good.

   (gdb) p have_mergeinfo_for_source
   $5 = 1
   (gdb) p have_mergeinfo_for_descendants
   $6 = 0
   (gdb) p source_repos_rel_path
   $7 = 0x8288220 "branches/1.5.x-r30215"
   (gdb) p apr_hash_get(mergeinfo_catalog, source_repos_rel_path, -1)
   $8 = (void *) 0x8291c70
   (gdb) p apr_hash_count(mergeinfo_catalog)
   $9 = 1

Notice how source_repos_rel_path doesn't start with slash, even though
there's a TODO(reint) comment that says it should. Hmmm.

And here we see that source_mergeinfo has count == 0:

   (gdb) p *source_mergeinfo
   $14 = {pool = 0x81d9738, array = 0x8291c98,
          iterator = {ht = 0x0, this = 0x8291d40, next = 0x82908d0,
          index = 136911976}, count = 0, max = 15,
          hash_func = 0xb7daefa0 <apr_hashfunc_default>, free = 0x0}

Meanwhile, there are two segments:

   (gdb) p *segments
   $15 = {pool = 0x81d9738, elt_size = 4, nelts = 2, nalloc = 8,
          elts = 0x81d9788 <this part always looks like garbage>}

So yeah, the 'return' inside the for-loop just before the abort() will
never return, because

   rangelist = apr_hash_get(source_mergeinfo,
                            apr_pstrcat(iterpool, "/", segment->path, NULL),
                            APR_HASH_KEY_STRING);

will never retrieve a non-NULL rangelist.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-05 17:56:32 CEST