On 12/17/19 3:44 AM, Simon Marchi wrote:
> On 2019-12-15 1:12 p.m., Bernd Edlinger wrote:
>> On 12/15/19 2:05 PM, Simon Marchi wrote:
>>> On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> this is the split out patch on skip.exp which fixes a pre-existing
>>>> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
>>>> trunk of a few weeks ago at least, likely other versions too).
>>>>
>>>>
>>>> Is it OK for trunk?
>>>>
>>>>
>>>> Thanks
>>>> Bernd.
>>>>
>>>>
>>>
>>> Hi Bernd,
>>>
>>> Just wondering, were you able to figure out which change in debug info lead
>>> to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
>>> I think your patch is ok.
>>>
>>
>> Yes indeed.  The change started with gcc-8.1.0 when -gcolumn-info was enabled
>> by default.  -gcolumn-info was first implemented in gcc-7.1.0 but default-disabled,
>> so you can get the altered behavior already with gcc-7 if you manually enable
>> -gcolumn-info.
>>
>> So previously there was just one point where line line 30 (of skip.c) started:
>>
>>   [0x00000032]  Advance Line by 27 to 28
>>   [0x00000034]  Copy
>>   [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
>>   [0x00000036]  Advance PC by constant 17 to 0x4004dc
>>   [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32
>>
>> with column-info we have line 30 three times with different column:
>>
>>   [0x00000034]  Advance Line by 27 to 28
>>   [0x00000036]  Copy
>>   [0x00000037]  Set column to 9
>>   [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
>>   [0x0000003a]  Set column to 17
>>   [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
>>   [0x0000003d]  Set column to 3
>>   [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
>>   [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32
>>
>>
>> That could probably be filtered in dwarf2read.c to keep the old behavior, but I agree
>> that the new behavior makes still sense, even if we cannot really use the column info
>> in the line number info.
> 
> That is actually some very good info for anyone wondering why this change was introduced!
> 
> Could you please put it in the commit message for posterity?
> 

Sure, good point.

Attached is the new, reworded version of the patch.
When I looked at it again, I stumbled over two wrong comments nearby,
telling "# Return from foo()" when we actually return from bar(),
and "# Return from bar()" when we actually return from foo()... :)

>>> I would just remove the unrelated whitespace fix before merging.
>>>
>>
>> Okay, I just replicated you advice regarding 8-space tab columns on
>> skip-inline.exp in the other patch.  Exactly those 2 lines were copied
>> where the tabs were not used correctly.
> 
> Great thanks.  Note that we can also fix skip.exp, just in another, obvious patch.
> 

Okay, will send a separate patch for the whitespace.


Thanks
Bernd.