tree conflicts: patch: segfault in merge_dir_opened()

Hi,

I've investigated the segmentation fault during merge (on the tree-conflicts
branch) and have, to my surprise, discovered that skipping tree-conflict
detection when adm_access==NULL is exactly the right thing to do.

I am thus posting this proper patch.

I am also appending three shell scripts that differ slightly (1: `svn rm;svn
ci', 2: `svn rm' without commit, 3: brute force `rm -rf'). Only 1 and 3
reproduce the segmentation fault.

Let me quote the explanatory comment contained in this patch:

  /* If adm_access == NULL, the tree conflict detection can be skipped,
   * because:
   *
   * adm_access refers to the parent(!) directory of the directory that
   * is to be opened. If adm_access == NULL, it means that the parent
   * of const char *path does not exist in the current working copy.
   *
   * We are at arbitrary depth in a directory subtree that does not exist
   * in the working copy, but nevertheless in a subtree off an existing
   * working copy directory (at least off the working copy "root").
   *
   * This function has already been called on the first non-existent
   * path element of this subtree, which has an existing parent (adm_access
   * != NULL), and a tree conflict has been triggered there.
   *
   * Even if we wanted to report another tree-conflict, there'd be no
   * working copy to mark the conflict in. Since the nearest existing parent
   * directory is already marked tree-conflicted, we can rest at that.
   */

[[[
Fixed a segmentation fault in tree-conflicts detection.

* subversion/libsvn_client/merge.c (merge_dir_opened): Disabled the
    superfluous tree-conflicts detection for the case adm_access == NULL,
    in effect avoiding a segmentation fault. Added a detailed comment.
]]]

Neels Janosch Hofmeyr wrote:
> Hi tree-conflicts folks,
>
> I discovered this case while writing a new set of tests for tree
> conflict detection, which tests for both file and directory victims in
> tree structures of various depths.
>
> I am faced with a segmentation fault in
>
> libsvn_client/merge.c: merge_dir_opened()
>
> when it is passed an adm_access == NULL.
>
> This occurs when a merge tries to add a file to a *directory* that
> existed in a common ancestry, but was deleted on the local branch.
>
> First, in subversion/libsvn_client/repos_diff.c, open_directory() looks
> up this previously deleted directory, using get_path_access(). It yields
> an adm_access == NULL, which is passed to merge_dir_opened().
>
> On trunk, merge_dir_opened() is empty. No segmentation fault occurs.
>
> On the tree-conflicts branch, merge_dir_opened() contains tree conflict
> detection code, which dereferences adm_access (== NULL).
>
> If I fix the first segmentation fault, the subsequent call to
> tree_conflict() causes another segmentation fault, since it too
> dereferences adm_access.
>
> For now, I am using a simple patch that simply skips the tree conflict
> detection in merge_dir_opened() when adm_access == NULL (see
> merge_patch.diff), and that's probably stupid.
>
> To reproduce the segmentation fault, apply the attached diff
> my_tc_tests.diff to today's tree-conflicts branch (r32499) and run my
> merge test `merge_tests.py 101'.
>
> For more details, see the attached backtrace.txt.
>
> Note: my_tc_tests.diff contains patches which I am about to post to the
> list separately. No need to discuss them here.
>
> Thanks a lot for any suggestions!
>
> Neels
>
>

-- 
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696  mobile: +49 177 2345869  fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194