Re: [PATCH] Fix wrong assertions

[Yao Qi <qiyaoltc at gmail dot com>]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Hi, Jan,
thanks for your explanations... they are very helpful.

>> Further, what is "partially ambiguous result" in the comments below?
>
> The terminology seems bogus there.
>
> "partially ambiguous" was meant the chain:
> 	main -> a -> <???> -> d
> An intersection of all possible chains.
>

Sounds like "partially ambiguous" is equivalent to "ambiguous".

>
>> /* Determined tail calls for constructing virtual tail call frames.  */
>> 
>> struct call_site_chain
>>   {
>>     /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
>>        CALLERS + CALLEES < LENGTH.  */
>>     int callers, callees, length;
>> 
>>     /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
>>        top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
>>        (GDB "next") sites.  One is interested primarily in the PC field.  */
>>     struct call_site *call_site[1];
>>   };
>> 
>> I am confused by the usage of the variable-sized array call_site,
>> elements from 0 to CALLERS-1 are top sites, and elements from
>> LENGTH-CALLEES to LENGTH-1 are bottom sites, so I conclude that
>> CALLERS-1 < LENGTH-CALLEES, then CALLERS + CALLEES < LENGTH + 1,
>> then CALLERS + CALLEES =< LENGTH.  Is it right?
>
> Yes, that is right.  Initially there is some chain (let's say the longest one

If that is right, the assert below is too strict, isn't?

  /* See call_site_find_chain_1 why there is no way to reach the bottom callee
     PC again.  In such case there must be two different code paths to reach
     it, therefore some of the former determined intermediate PCs must differ
     and the unambiguous chain gets shortened.  */
  gdb_assert (result->callers + result->callees < result->length);

> but that doe snot matter).  Consequently its elements from the middle are
> being removed and there remains only some few unambiguous top and
> bottom ones.

If there is no call sites removed from the chain during the intersection,
CALLERS + CALLEES == LENGTH, right?  in function chain_candidate,
result->length is set by the length of a chain.  If this chain is the
shortest one, CALLERS + CALLEES == LENGTH otherwise,
CALLERS + CALLEES < LENGTH.  Is it right?  If so, we need to relax the
condition in the assert and update the comments.

>
> The original idea why the comparison should be sharp ("<") was that if there
> are multiple chains like (0xaddr show jmp instruction address):
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x200) -> c(0x300) -> d(0x400)
> then - such situation cannot exist - if two jmp instructions in "a" have the
> same address they must also jump to the same address (*).
>
> (*) jump to a computed address would be never considered for the DWARF
>     tail-call records.
>
> So there could be:
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x270) -> c(0x300) -> d(0x400)
> But then "a" frame itself is ambiguous and it must not be displayed.
>
> I did not realize that there can be self-tail-call:
> 	main(0x100) -> a(0x200) -> d(0x400)
> 	main(0x100) -> a(0x280) -> a(0x200) -> d(0x400)
> which intersects to:
> 	main(0x100) -> <???>? -> a(0x200) -> d(0x400)
> And so if the first chain was chosen the
> 	main(0x100) -> a(0x200) -> d(0x400)
> then the final intersection has callers+callees==length.

What are the definitions of CALLERS, CALLEES, top and bottom? given this example?

-- 
Yao (éå)