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 2009/05/18 16:56:44 UTC

segfault during merge (1.6.2)

Hi,

Crash report sent for TSVN:

A segfault in subversion\libsvn_client\merge.c, function
remove_noop_merge_ranges(), line 5022.

It seems that APR_ARRAY_IDX() return NULL (the changed_revs->nelts is
0). But the 'ranges->nelts' in the for-loop above it has a value of two.

This is a merge with:

svn_client_merge_peg3(https://url/to/file,
	revrange 1-HEAD,
	HEAD,
	path/to/wc/file/in/other/branch,
	infinity,
	TRUE,
	FALSE,
	FALSE,
	FALSE,
	"",
	&ctx, pool);


I've asked the user for more information, but haven't heard back yet.

Stefan

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

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2301638

Re: segfault during merge (1.6.2)

Posted by Paul Burba <pt...@gmail.com>.
On Tue, May 19, 2009 at 1:09 AM, Stefan Küng <to...@gmail.com> wrote:
> Paul Burba wrote:
>>  On Mon, May 18, 2009 at 12:56 PM, Stefan Küng <to...@gmail.com> wrote:
>>> Hi,
>>>
>>> Crash report sent for TSVN:
>>>
>>> A segfault in subversion\libsvn_client\merge.c, function
>>> remove_noop_merge_ranges(), line 5022.
>>>
>>> It seems that APR_ARRAY_IDX() return NULL (the changed_revs->nelts is
>>> 0). But the 'ranges->nelts' in the for-loop above it has a value of two.
>>>
>>> This is a merge with:
>>>
>>> svn_client_merge_peg3(https://url/to/file,
>>> Â  Â  Â  Â revrange 1-HEAD,
>>> Â  Â  Â  Â HEAD,
>>> Â  Â  Â  Â path/to/wc/file/in/other/branch,
>>> Â  Â  Â  Â infinity,
>>> Â  Â  Â  Â TRUE,
>>> Â  Â  Â  Â FALSE,
>>> Â  Â  Â  Â FALSE,
>>> Â  Â  Â  Â FALSE,
>>> Â  Â  Â  Â "",
>>> Â  Â  Â  Â &ctx, pool);
>>
>> Hi Stefan,
>>
>> Something is odd about this report.  You indicate the merge is with
>> --ignore-ancestry (6th argument to svn_client_merge_peg3).  This means
>> that merge.c:do_file_merge, the only caller of
>> remove_noop_merge_ranges, will set its local variable REMAINING_RANGES
>> to a single element array:
>
> Ups, sorry. It's the 'force' parameter that's true and the others are false.
>
>>   range.start = revision1;
>>   range.end = revision2;
>>   range.inheritable = TRUE;
>>   if (honor_mergeinfo)
>>     {
>>       ### --ignore-ancestry means we aren't honoring mergeinfo so we
>> never enter this block
>>     }
>>
>>   /* The simple cases where our remaining range is REVISION1:REVISION2. */
>>   if (!honor_mergeinfo || merge_b->record_only)
>>     {
>>       remaining_ranges = apr_array_make(pool, 1, sizeof(&range));
>>       APR_ARRAY_PUSH(remaining_ranges, svn_merge_range_t *) = &range;
>>     }
>>
>> Which in turn means that remove_noop_merge_ranges can never be called:
>>
>>   if (!merge_b->record_only)
>>     {
>>       apr_array_header_t *ranges_to_merge = remaining_ranges;
>>
>>       /* If we have ancestrally related sources and more than one
>>          range to merge, eliminate no-op ranges before going through
>>          the effort of downloading the many copies of the file
>>          required to do these merges (two copies per range). */
>>       if (merge_b->sources_ancestral && (remaining_ranges->nelts > 1))
>>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>         {
>>           const char *old_sess_url = NULL;
>>           SVN_ERR(svn_client__ensure_ra_session_url(&old_sess_url,
>>                                                     merge_b->ra_session1,
>>                                                     primary_url, subpool));
>>           SVN_ERR(remove_noop_merge_ranges(&ranges_to_merge,
>>                                            merge_b->ra_session1,
>>                                            remaining_ranges, subpool));
>>
>> Also, line 5022 in
>> http://svn.collab.net/repos/svn/tags/1.6.2/subversion/libsvn_client/merge.c
>> is:
>>
>> 5022     if ((! SVN_IS_VALID_REVNUM(oldest_rev)) || (min_rev < oldest_rev))
>
> Again, so sorry. I guess I had the cursor in the wrong line when I read
> the source line (VS shows the line where the cursor is on the bottom
> right - I don't have line numbers enabled elsewhere).
> It's line 5034 (the one with APR_ARRAY_IDX()).
>

Hi Stefan,

Thanks for the clarification.  I found the problem, fixed it and added
a test in r37779.  I will nominate for backport shortly.

Paul

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2315485


Re: segfault during merge (1.6.2)

Posted by Stefan Küng <to...@gmail.com>.
Paul Burba wrote:
>  On Mon, May 18, 2009 at 12:56 PM, Stefan Küng <to...@gmail.com> wrote:
>> Hi,
>>
>> Crash report sent for TSVN:
>>
>> A segfault in subversion\libsvn_client\merge.c, function
>> remove_noop_merge_ranges(), line 5022.
>>
>> It seems that APR_ARRAY_IDX() return NULL (the changed_revs->nelts is
>> 0). But the 'ranges->nelts' in the for-loop above it has a value of two.
>>
>> This is a merge with:
>>
>> svn_client_merge_peg3(https://url/to/file,
>> Â  Â  Â  Â revrange 1-HEAD,
>> Â  Â  Â  Â HEAD,
>> Â  Â  Â  Â path/to/wc/file/in/other/branch,
>> Â  Â  Â  Â infinity,
>> Â  Â  Â  Â TRUE,
>> Â  Â  Â  Â FALSE,
>> Â  Â  Â  Â FALSE,
>> Â  Â  Â  Â FALSE,
>> Â  Â  Â  Â "",
>> Â  Â  Â  Â &ctx, pool);
> 
> Hi Stefan,
> 
> Something is odd about this report.  You indicate the merge is with
> --ignore-ancestry (6th argument to svn_client_merge_peg3).  This means
> that merge.c:do_file_merge, the only caller of
> remove_noop_merge_ranges, will set its local variable REMAINING_RANGES
> to a single element array:

Ups, sorry. It's the 'force' parameter that's true and the others are false.

>   range.start = revision1;
>   range.end = revision2;
>   range.inheritable = TRUE;
>   if (honor_mergeinfo)
>     {
>       ### --ignore-ancestry means we aren't honoring mergeinfo so we
> never enter this block
>     }
> 
>   /* The simple cases where our remaining range is REVISION1:REVISION2. */
>   if (!honor_mergeinfo || merge_b->record_only)
>     {
>       remaining_ranges = apr_array_make(pool, 1, sizeof(&range));
>       APR_ARRAY_PUSH(remaining_ranges, svn_merge_range_t *) = &range;
>     }
> 
> Which in turn means that remove_noop_merge_ranges can never be called:
> 
>   if (!merge_b->record_only)
>     {
>       apr_array_header_t *ranges_to_merge = remaining_ranges;
> 
>       /* If we have ancestrally related sources and more than one
>          range to merge, eliminate no-op ranges before going through
>          the effort of downloading the many copies of the file
>          required to do these merges (two copies per range). */
>       if (merge_b->sources_ancestral && (remaining_ranges->nelts > 1))
>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         {
>           const char *old_sess_url = NULL;
>           SVN_ERR(svn_client__ensure_ra_session_url(&old_sess_url,
>                                                     merge_b->ra_session1,
>                                                     primary_url, subpool));
>           SVN_ERR(remove_noop_merge_ranges(&ranges_to_merge,
>                                            merge_b->ra_session1,
>                                            remaining_ranges, subpool));
> 
> Also, line 5022 in
> http://svn.collab.net/repos/svn/tags/1.6.2/subversion/libsvn_client/merge.c
> is:
> 
> 5022     if ((! SVN_IS_VALID_REVNUM(oldest_rev)) || (min_rev < oldest_rev))

Again, so sorry. I guess I had the cursor in the wrong line when I read
the source line (VS shows the line where the cursor is on the bottom
right - I don't have line numbers enabled elsewhere).
It's line 5034 (the one with APR_ARRAY_IDX()).

Stefan

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

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2307278

Re: segfault during merge (1.6.2)

Posted by Paul Burba <pt...@gmail.com>.
On Mon, May 18, 2009 at 12:56 PM, Stefan Küng <to...@gmail.com> wrote:
> Hi,
>
> Crash report sent for TSVN:
>
> A segfault in subversion\libsvn_client\merge.c, function
> remove_noop_merge_ranges(), line 5022.
>
> It seems that APR_ARRAY_IDX() return NULL (the changed_revs->nelts is
> 0). But the 'ranges->nelts' in the for-loop above it has a value of two.
>
> This is a merge with:
>
> svn_client_merge_peg3(https://url/to/file,
>        revrange 1-HEAD,
>        HEAD,
>        path/to/wc/file/in/other/branch,
>        infinity,
>        TRUE,
>        FALSE,
>        FALSE,
>        FALSE,
>        "",
>        &ctx, pool);

Hi Stefan,

Something is odd about this report.  You indicate the merge is with
--ignore-ancestry (6th argument to svn_client_merge_peg3).  This means
that merge.c:do_file_merge, the only caller of
remove_noop_merge_ranges, will set its local variable REMAINING_RANGES
to a single element array:

  range.start = revision1;
  range.end = revision2;
  range.inheritable = TRUE;
  if (honor_mergeinfo)
    {
      ### --ignore-ancestry means we aren't honoring mergeinfo so we
never enter this block
    }

  /* The simple cases where our remaining range is REVISION1:REVISION2. */
  if (!honor_mergeinfo || merge_b->record_only)
    {
      remaining_ranges = apr_array_make(pool, 1, sizeof(&range));
      APR_ARRAY_PUSH(remaining_ranges, svn_merge_range_t *) = &range;
    }

Which in turn means that remove_noop_merge_ranges can never be called:

  if (!merge_b->record_only)
    {
      apr_array_header_t *ranges_to_merge = remaining_ranges;

      /* If we have ancestrally related sources and more than one
         range to merge, eliminate no-op ranges before going through
         the effort of downloading the many copies of the file
         required to do these merges (two copies per range). */
      if (merge_b->sources_ancestral && (remaining_ranges->nelts > 1))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        {
          const char *old_sess_url = NULL;
          SVN_ERR(svn_client__ensure_ra_session_url(&old_sess_url,
                                                    merge_b->ra_session1,
                                                    primary_url, subpool));
          SVN_ERR(remove_noop_merge_ranges(&ranges_to_merge,
                                           merge_b->ra_session1,
                                           remaining_ranges, subpool));

Also, line 5022 in
http://svn.collab.net/repos/svn/tags/1.6.2/subversion/libsvn_client/merge.c
is:

5022     if ((! SVN_IS_VALID_REVNUM(oldest_rev)) || (min_rev < oldest_rev))

Paul

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2303460