Re: segfault during merge (1.6.2)

On Tue, May 19, 2009 at 1:09 AM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
> Paul Burba wrote:
>>  On Mon, May 18, 2009 at 12:56 PM, Stefan Küng <tortoisesvn_at_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
Received on 2009-05-19 18:56:52 CEST