You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2013/08/05 20:09:31 UTC

Re: crash in 1.8.1 on commit

On Sat, Jul 27, 2013 at 5:24 AM, Ben Reser <be...@reser.org> wrote:

> On Fri, Jul 26, 2013 at 8:28 AM, Stefan Küng <to...@gmail.com>
> wrote:
> > crash reports for 1.8.1 are coming in now. Top crash right now is this:
> > https://www.crash-server.com/Problem.aspx?ClientID=tsvn&ProblemID=31426
> >
> >> libsvn_tsvn.dll!svn_path_join_internal(const char *
> >> base=0xfffffffffcfcc6f7, const char * component=0x0000000000000000,
> >> apr_pool_t * pool=0x0000000003033880) Line 115  C
> >> libsvn_tsvn.dll!checkout_dir(dir_context_t * dir=0x0000000003017e78,
> >> apr_pool_t * scratch_pool=0x0000000002fce380) Line 403     C
> >> libsvn_tsvn.dll!delete_entry(const char * path=0x000007fedc1f7708, long
> >> revision=50429560, void * parent_baton=0x0000000003033880, apr_pool_t *
> >> pool=0x000007fedc1f7708) Line 1546      C
> >> libsvn_tsvn.dll!do_item_commit(void * * dir_baton=0x0000000003f5ebb8,
> void
> >> * parent_baton=0x0000000003033880, void *
> callback_baton=0x0000000003f5ec70,
> >> const char * path=0x0000000002fcc3a8, apr_pool_t *
> pool=0x0000000003039838)
> >> Line 1590   C
> >> libsvn_tsvn.dll!svn_delta_path_driver2(const svn_delta_editor_t *
> >> editor=0x000000000301f7e0, void * edit_baton=0x0000000002fce6a0, const
> >> apr_array_header_t * paths=0x0000000003031870, int sort_paths=50438336,
> >> svn_error_t * (void * *, void *, void *, const char *, apr_pool_t *) *
> >> callback_func=0x000007fede5ebbd0, void *
> callback_baton=0x0000000003f5ec70,
> >> apr_pool_t * pool=0x000000000301f768) Line 263      C
> >> libsvn_tsvn.dll!svn_client__do_commit(const char *
> >> base_url=0x0000000002fcc310, const apr_array_header_t *
> >> commit_items=0x000000000302f7e8, const svn_delta_editor_t *
> >> editor=0x0000000002fce6a0, void * edit_baton=0x0000000002fce380, const
> char
> >> * notify_path_prefix=0x0000000003017960, apr_hash_t * *
> >> sha1_checksums=0x0000000003f5eda8, svn_client_ctx_t *
> >> ctx=0x000000000301a0c0, apr_pool_t * result_pool=0x00000000030176f8,
> >> apr_pool_t * scratch_pool=0x000000000301f768) Line 1808   C
> >> libsvn_tsvn.dll!svn_client_commit6(const apr_array_header_t *
> >> targets=0x0000000003017770, svn_depth_t depth=34, int keep_locks=0, int
> >> keep_changelists=0, int commit_as_operations=1, int
> >> include_file_externals=0, int include_dir_externals=0, const
> >> apr_array_header_t * changelists=0x0000000003017770, const apr_hash_t *
> >> revprop_table=0x0000000000000000, svn_error_t * (const
> svn_commit_info_t *,
> >> void *, apr_pool_t *) * commit_callback=0x000000013f3cc770, void *
> >> commit_baton=0x0000000000158de0, svn_client_ctx_t *
> ctx=0x000000000301a0c0,
> >> apr_pool_t * pool=0x00000000030176f8) Line 965        C
> >
> >
> > the dir->parent_dir->working_url in libsvn_ra_serf/commit.c, line 402 is
> > NULL which makes svn_path_join_internal access the null pointer.
>
> There clearly is a bug where dir->parent_dir->working_url is NULL,
> however the backtrace above seems to be showing that the component
> variable passed into svn_path_join_internal is NULL.  checkout_dir()
> calls svn_path_url_add_component2() which uses uri_escape() on the
> component portion before calling svn_path_join_internal() with the
> result of uri_escape().  So I'm really not sure how the component is
> ending up NULL, but it appears to be a different issue from what you
> describe.
>


There's a small confusion about what the 'component' is I guess:


/* Implicitly checkout this dir now. */
dir->working_url = svn_path_url_add_component2(
                         dir->parent_dir->working_url,
                         dir->name, dir->pool);

It's actually dir->name that's NULL here, because:

const char *
svn_path_url_add_component2(const char *url,
                            const char *component,
                            apr_pool_t *pool)
{
  /* = svn_path_uri_encode() but without always copying */

  component = uri_escape(component, svn_uri__char_validity, pool);

  return svn_path_join_internal(url, component, pool);
}


which means 'component' (the second param to svn_path_join_internal)
is also the second param in svn_path_url_add_component2.


But looking at the code of svn_path_url_add_component2():
component = uri_escape(component, svn_uri__char_validity, pool);


how is that possible without the compiler complaining? 'component' is
const, so how is it possible to assign the return value of
uri_escape() to it?


Could it be that this strange use of the component param made the
compiler do something wrong and that's why I get this crash?


Stefan


-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: crash in 1.8.1 on commit

Posted by Branko Čibej <br...@wandisco.com>.
On 05.08.2013 20:09, Stefan Küng wrote:
> On Sat, Jul 27, 2013 at 5:24 AM, Ben Reser <ben@reser.org
> <ma...@reser.org>> wrote:
>
>     On Fri, Jul 26, 2013 at 8:28 AM, Stefan Küng
>     <tortoisesvn@gmail.com <ma...@gmail.com>> wrote:
>     > crash reports for 1.8.1 are coming in now. Top crash right now
>     is this:
>     >
>     https://www.crash-server.com/Problem.aspx?ClientID=tsvn&ProblemID=31426
>     >
>     >> libsvn_tsvn.dll!svn_path_join_internal(const char *
>     >> base=0xfffffffffcfcc6f7, const char * component=0x0000000000000000,
>     >> apr_pool_t * pool=0x0000000003033880) Line 115  C
>     >> libsvn_tsvn.dll!checkout_dir(dir_context_t *
>     dir=0x0000000003017e78,
>     >> apr_pool_t * scratch_pool=0x0000000002fce380) Line 403     C
>     >> libsvn_tsvn.dll!delete_entry(const char *
>     path=0x000007fedc1f7708, long
>     >> revision=50429560, void * parent_baton=0x0000000003033880,
>     apr_pool_t *
>     >> pool=0x000007fedc1f7708) Line 1546      C
>     >> libsvn_tsvn.dll!do_item_commit(void * *
>     dir_baton=0x0000000003f5ebb8, void
>     >> * parent_baton=0x0000000003033880, void *
>     callback_baton=0x0000000003f5ec70,
>     >> const char * path=0x0000000002fcc3a8, apr_pool_t *
>     pool=0x0000000003039838)
>     >> Line 1590   C
>     >> libsvn_tsvn.dll!svn_delta_path_driver2(const svn_delta_editor_t *
>     >> editor=0x000000000301f7e0, void *
>     edit_baton=0x0000000002fce6a0, const
>     >> apr_array_header_t * paths=0x0000000003031870, int
>     sort_paths=50438336,
>     >> svn_error_t * (void * *, void *, void *, const char *,
>     apr_pool_t *) *
>     >> callback_func=0x000007fede5ebbd0, void *
>     callback_baton=0x0000000003f5ec70,
>     >> apr_pool_t * pool=0x000000000301f768) Line 263      C
>     >> libsvn_tsvn.dll!svn_client__do_commit(const char *
>     >> base_url=0x0000000002fcc310, const apr_array_header_t *
>     >> commit_items=0x000000000302f7e8, const svn_delta_editor_t *
>     >> editor=0x0000000002fce6a0, void *
>     edit_baton=0x0000000002fce380, const char
>     >> * notify_path_prefix=0x0000000003017960, apr_hash_t * *
>     >> sha1_checksums=0x0000000003f5eda8, svn_client_ctx_t *
>     >> ctx=0x000000000301a0c0, apr_pool_t *
>     result_pool=0x00000000030176f8,
>     >> apr_pool_t * scratch_pool=0x000000000301f768) Line 1808   C
>     >> libsvn_tsvn.dll!svn_client_commit6(const apr_array_header_t *
>     >> targets=0x0000000003017770, svn_depth_t depth=34, int
>     keep_locks=0, int
>     >> keep_changelists=0, int commit_as_operations=1, int
>     >> include_file_externals=0, int include_dir_externals=0, const
>     >> apr_array_header_t * changelists=0x0000000003017770, const
>     apr_hash_t *
>     >> revprop_table=0x0000000000000000, svn_error_t * (const
>     svn_commit_info_t *,
>     >> void *, apr_pool_t *) * commit_callback=0x000000013f3cc770, void *
>     >> commit_baton=0x0000000000158de0, svn_client_ctx_t *
>     ctx=0x000000000301a0c0,
>     >> apr_pool_t * pool=0x00000000030176f8) Line 965        C
>     >
>     >
>     > the dir->parent_dir->working_url in libsvn_ra_serf/commit.c,
>     line 402 is
>     > NULL which makes svn_path_join_internal access the null pointer.
>
>     There clearly is a bug where dir->parent_dir->working_url is NULL,
>     however the backtrace above seems to be showing that the component
>     variable passed into svn_path_join_internal is NULL.  checkout_dir()
>     calls svn_path_url_add_component2() which uses uri_escape() on the
>     component portion before calling svn_path_join_internal() with the
>     result of uri_escape().  So I'm really not sure how the component is
>     ending up NULL, but it appears to be a different issue from what you
>     describe.
>
>
>
> There's a small confusion about what the 'component' is I guess:
>
> /* Implicitly checkout this dir now. */
> dir->working_url = svn_path_url_add_component2(
>                          dir->parent_dir->working_url,
>                          dir->name, dir->pool);
>
> It's actually dir->name that's NULL here, because:
>
>
>
> const char *
> svn_path_url_add_component2(const char *url,
>                             const char *component,
>                             apr_pool_t *pool)
> {
>   /* = svn_path_uri_encode() but without always copying */
>
>
>   component = uri_escape(component, svn_uri__char_validity, pool);
>
>   return svn_path_join_internal(url, component, pool);
> }
>
> which means 'component' (the second param to svn_path_join_internal) is also the second param in svn_path_url_add_component2.
>
> But looking at the code of svn_path_url_add_component2():
> component = uri_escape(component, svn_uri__char_validity, pool);
> how is that possible without the compiler complaining? 'component' is const, so how is it possible to assign the return value of uri_escape() to it?

Because it's not const. It's a pointer to a constant character, not a
constant pointer.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: crash in 1.8.1 on commit

Posted by Stefan Küng <to...@gmail.com>.
On Mon, Aug 5, 2013 at 8:09 PM, Stefan Küng <to...@gmail.com> wrote:

> On Sat, Jul 27, 2013 at 5:24 AM, Ben Reser <be...@reser.org> wrote:
>
>> On Fri, Jul 26, 2013 at 8:28 AM, Stefan Küng <to...@gmail.com>
>> wrote:
>> > crash reports for 1.8.1 are coming in now. Top crash right now is this:
>> > https://www.crash-server.com/Problem.aspx?ClientID=tsvn&ProblemID=31426
>> >
>> >> libsvn_tsvn.dll!svn_path_join_internal(const char *
>> >> base=0xfffffffffcfcc6f7, const char * component=0x0000000000000000,
>> >> apr_pool_t * pool=0x0000000003033880) Line 115  C
>> >> libsvn_tsvn.dll!checkout_dir(dir_context_t * dir=0x0000000003017e78,
>> >> apr_pool_t * scratch_pool=0x0000000002fce380) Line 403     C
>> >> libsvn_tsvn.dll!delete_entry(const char * path=0x000007fedc1f7708, long
>> >> revision=50429560, void * parent_baton=0x0000000003033880, apr_pool_t *
>> >> pool=0x000007fedc1f7708) Line 1546      C
>> >> libsvn_tsvn.dll!do_item_commit(void * * dir_baton=0x0000000003f5ebb8,
>> void
>> >> * parent_baton=0x0000000003033880, void *
>> callback_baton=0x0000000003f5ec70,
>> >> const char * path=0x0000000002fcc3a8, apr_pool_t *
>> pool=0x0000000003039838)
>> >> Line 1590   C
>> >> libsvn_tsvn.dll!svn_delta_path_driver2(const svn_delta_editor_t *
>> >> editor=0x000000000301f7e0, void * edit_baton=0x0000000002fce6a0, const
>> >> apr_array_header_t * paths=0x0000000003031870, int sort_paths=50438336,
>> >> svn_error_t * (void * *, void *, void *, const char *, apr_pool_t *) *
>> >> callback_func=0x000007fede5ebbd0, void *
>> callback_baton=0x0000000003f5ec70,
>> >> apr_pool_t * pool=0x000000000301f768) Line 263      C
>> >> libsvn_tsvn.dll!svn_client__do_commit(const char *
>> >> base_url=0x0000000002fcc310, const apr_array_header_t *
>> >> commit_items=0x000000000302f7e8, const svn_delta_editor_t *
>> >> editor=0x0000000002fce6a0, void * edit_baton=0x0000000002fce380, const
>> char
>> >> * notify_path_prefix=0x0000000003017960, apr_hash_t * *
>> >> sha1_checksums=0x0000000003f5eda8, svn_client_ctx_t *
>> >> ctx=0x000000000301a0c0, apr_pool_t * result_pool=0x00000000030176f8,
>> >> apr_pool_t * scratch_pool=0x000000000301f768) Line 1808   C
>> >> libsvn_tsvn.dll!svn_client_commit6(const apr_array_header_t *
>> >> targets=0x0000000003017770, svn_depth_t depth=34, int keep_locks=0, int
>> >> keep_changelists=0, int commit_as_operations=1, int
>> >> include_file_externals=0, int include_dir_externals=0, const
>> >> apr_array_header_t * changelists=0x0000000003017770, const apr_hash_t *
>> >> revprop_table=0x0000000000000000, svn_error_t * (const
>> svn_commit_info_t *,
>> >> void *, apr_pool_t *) * commit_callback=0x000000013f3cc770, void *
>> >> commit_baton=0x0000000000158de0, svn_client_ctx_t *
>> ctx=0x000000000301a0c0,
>> >> apr_pool_t * pool=0x00000000030176f8) Line 965        C
>> >
>> >
>> > the dir->parent_dir->working_url in libsvn_ra_serf/commit.c, line 402 is
>> > NULL which makes svn_path_join_internal access the null pointer.
>>
>> There clearly is a bug where dir->parent_dir->working_url is NULL,
>> however the backtrace above seems to be showing that the component
>> variable passed into svn_path_join_internal is NULL.  checkout_dir()
>> calls svn_path_url_add_component2() which uses uri_escape() on the
>> component portion before calling svn_path_join_internal() with the
>> result of uri_escape().  So I'm really not sure how the component is
>> ending up NULL, but it appears to be a different issue from what you
>> describe.
>>
>
>
> There's a small confusion about what the 'component' is I guess:
>
>
> /* Implicitly checkout this dir now. */
> dir->working_url = svn_path_url_add_component2(
>                          dir->parent_dir->working_url,
>                          dir->name, dir->pool);
>
> It's actually dir->name that's NULL here, because:
>
>
> const char *
> svn_path_url_add_component2(const char *url,
>                             const char *component,
>                             apr_pool_t *pool)
> {
>   /* = svn_path_uri_encode() but without always copying */
>
>
>   component = uri_escape(component, svn_uri__char_validity, pool);
>
>   return svn_path_join_internal(url, component, pool);
> }
>
>
> which means 'component' (the second param to svn_path_join_internal) is also the second param in svn_path_url_add_component2.
>
>
> But looking at the code of svn_path_url_add_component2():
> component = uri_escape(component, svn_uri__char_validity, pool);
>
>
> how is that possible without the compiler complaining? 'component' is const, so how is it possible to assign the return value of uri_escape() to it?
>
>
> Could it be that this strange use of the component param made the compiler do something wrong and that's why I get this crash?
>
>
Since the debug view didn't show enough information, I used the disassembly
view to gather more information:

A few things to notice here:
* the generated code is correct, even though uri_escape assigns a value to
a const param
* it's not 'component' that's NULL but 'base' (in svn_path_join_internal)
which means the url and one level up dir->parent_dir->working_url, is NULL.
That's why uri_escape works without a crash and the crash happens in
svn_path_join_internal.

Stefan


-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net