Re: crash fetching status

On Wed, Dec 28, 2011 at 1:01 AM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
> From crash dumps sent for TSVN, there's a crash happening when fetching the
> status. The stacktrace:
>
>        libsvn_tsvn32.dll!svn_relpath_join(const char * base=0x00000000,
> const char * component=0x01fe2154, apr_pool_t * pool=0x01fe2080)  Line 1158
> + 0xc bytes        C
>        libsvn_tsvn32.dll!make_file_baton(dir_baton *
> parent_dir_baton=0x01fe80e0, const char * path=0x01d81f70, apr_pool_t *
> pool=0x01fe2080)  Line 1583 + 0x2d bytes  C
>        libsvn_tsvn32.dll!add_file(const char * path=0x01d81f70, void *
> parent_baton=0x01fe80e0, const char * copyfrom_path=0x00000000, long
> copyfrom_revision=-1, apr_pool_t * pool=0x01fe2080, void * *
> file_baton=0x01fe20e4)  Line 2040     C
>        libsvn_tsvn32.dll!add_file(const char * path=0x01d81f70, void *
> parent_baton=0x01fe80d8, const char * copyfrom_path=0x00000000, long
> copyfrom_revision=-1, apr_pool_t * pool=0x01fe2080, void * *
> file_baton=0x01fe20c4)  Line 172 + 0x1f bytes C
>        libsvn_tsvn32.dll!ra_svn_handle_add_file(svn_ra_svn_conn_st *
> conn=0x01dc2390, apr_pool_t * pool=0x01d81d88, const apr_array_header_t *
> params=0x01d81e58, ra_svn_driver_state_t * ds=0x0254f084)  Line 653 + 0x23
> bytes        C
>        libsvn_tsvn32.dll!svn_ra_svn_drive_editor2(svn_ra_svn_conn_st *
> conn=0x01dc2390, apr_pool_t * pool=0x01d67d20, const svn_delta_editor_t *
> editor=0x01d85528, void * edit_baton=0x01d85568, int * aborted=0x00000000,
> int for_replay=0)  Line 916 + 0x1d bytes   C
>        libsvn_tsvn32.dll!ra_svn_finish_report(void * baton=0x01d6d0c0,
> apr_pool_t * pool=0x01d67d20)  Line 300 + 0x19 bytes    C
>        libsvn_tsvn32.dll!reporter_finish_report(void *
> report_baton=0x0254f1e4, apr_pool_t * pool=0x01d67d20)  Line 217 + 0xc bytes
>    C
>        libsvn_tsvn32.dll!svn_wc_crawl_revisions5(svn_wc_context_t *
> wc_ctx=0x01d67dc0, const char * local_abspath=0x01d685c0, const
> svn_ra_reporter3_t * reporter=0x6c984028, void * report_baton=0x0254f1e4,
> int restore_files=0, svn_depth_t depth=-2, int honor_depth_exclude=0, int
> depth_compatibility_trick=0, int use_commit_times=0, svn_error_t * (void *)*
> cancel_func=0x00360ee1, void * cancel_baton=0x00a8f81c, void (void *, const
> svn_wc_notify_t *, apr_pool_t *)* notify_func=0x00000000, void *
> notify_baton=0x00000000, apr_pool_t * scratch_pool=0x01d67d20)  Line 861 +
> 0xb bytes C
>        libsvn_tsvn32.dll!svn_client_status5(long * result_rev=0x0254f534,
> svn_client_ctx_t * ctx=0x01d67d60, const char * path=0x01d26dd8, const
> svn_opt_revision_t * revision=0x0254f2dc, svn_depth_t depth=-2, int
> get_all=1, int update=1, int no_ignore=0, int ignore_externals=0, int
> depth_as_sticky=1, const apr_array_header_t * changelists=0x00000000,
> svn_error_t * (void *, const char *, const svn_client_status_t *, apr_pool_t
> *)* status_func=0x00360ac4, void * status_baton=0x0254f2bc, apr_pool_t *
> pool=0x01d67d20)  Line 456 + 0x3c bytes C
>
>
> Problem is in libsvn_wc\status.c, make_file_baton():
> the line
>  f->repos_relpath = svn_relpath_join(find_dir_repos_relpath(pb, pool),
>                                      f->name, pool);
> calls svn_relpath_join() with the first parameter as NULL, which then leads
> to the crash because svn_relpath_join calls strlen() on the first parameter.
>
> The comments in find_dir_repos_relpath() clearly indicate that its return
> value can be NULL in certain situations, which is what happens here.

I looked at similar places elsewhere, and they seemed to follow a
pattern. The following patch is in a similar vein:

[[[
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 1225565)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -1795,6 +1795,7 @@
   struct dir_baton *pb = parent_dir_baton;
   struct edit_baton *eb = pb->edit_baton;
   struct file_baton *f = apr_pcalloc(pool, sizeof(*f));
+ const char *dir_repos_relpath;

   /* Finish populating the baton members. */
   f->local_abspath = svn_dirent_join(eb->anchor_abspath, path, pool);
@@ -1804,8 +1805,13 @@
   f->edit_baton = eb;
   f->ood_changed_rev = SVN_INVALID_REVNUM;
   f->ood_changed_date = 0;
- f->repos_relpath = svn_relpath_join(find_dir_repos_relpath(pb, pool),
- f->name, pool);
+
+ dir_repos_relpath = find_dir_repos_relpath(pb, pool);
+ if (dir_repos_relpath)
+ f->repos_relpath = svn_relpath_join(dir_repos_relpath, f->name, pool);
+ else
+ f->repos_relpath = apr_pstrdup(pool, f->name);
+
   f->ood_kind = svn_node_file;
   f->ood_changed_author = NULL;
   return f;
]]]

All the tests pass, though I'm not sure if there are other correctness
issues here or not, so I haven't committed this.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/