On 8/10/21 10:56 PM, Eric Botcazou wrote:
>> Ah, thanks, I tried it but the defs__struct1IP function has
>> DECL_IGNORED_P = false when I build it with -gnatD.
>>
>> So that would not be affected by setting DECL_SOURCE_LOCATION to
>> UNKNOWN_LOCATION as done in the proposed patch, because that is only
>> done for functions with DECL_IGNORED_P = true.
>
> Ouch, I should have checked it myself... Thanks for doing the investigation.
>
no problem.
>> Then we have ordinary functions with DECL_IGNORED_P = false,
>> but they are morphed into a thunk calling a similar looking function
>> and the thunk has now DECL_IGNORED_P = true, and all debug info
>> removed, except the DECL_SOURCE_LOCATION. To make this even worse,
>> the similar looking function is inlined into the thunk again, and
>> all debug info is again stripped away.
>>
>> And when this happens it is desirable to at least see the source line where
>> the original function was declared.
>>
>>
>> As an example, please consider this test case:
>>
>> $ cat test.ads
>> package test is
>>
>> type Func_Ptr is access function (X : Integer) return Integer;
>>
>> function Test1 (X : Integer) return Integer;
>> function Test2 (X : Integer) return Integer;
>> function DoIt (X : Integer; Func : Func_Ptr) return Integer;
>>
>> end test;
>> $ cat test.ads
>> package test is
>>
>> type Func_Ptr is access function (X : Integer) return Integer;
>>
>> function Test1 (X : Integer) return Integer;
>> function Test2 (X : Integer) return Integer;
>> function DoIt (X : Integer; Func : Func_Ptr) return Integer;
>>
>> end test;
>>
>> $ cat test.adb
>> package body test is
>>
>> function Test1 (X : Integer) return Integer is
>> begin
>> return X;
>> end Test1;
>>
>> function Test2 (X : Integer) return Integer is
>> begin
>> return X;
>> end Test2;
>>
>> function DoIt (X : Integer; Func : Func_Ptr) return Integer is
>> begin
>> return Func (X);
>> end DoIt;
>>
>> end test;
>>
>> $ cat main.adb
>> with Ada.Text_IO; use Ada.Text_IO;
>> with test; use test;
>>
>> procedure Main is
>>
>> X : Integer := 7;
>> Y : Integer := DoIt (X, Test1'Access);
>> Z : Integer := DoIt (X, Test2'Access);
>>
>> begin
>> Put_Line (X'Img & " " & Y'Img & " " & Z'Img);
>> end Main;
>>
>> $ gnatmake -f -g -O2 main.adb -save-temp -fdump-tree-all-lineno
>>
>> Now Test1 and Test2 are ordinary functions, but Test2 ends up as
>> DECL_IGNORED_P = true, that happens between test.adb.052t.local-fnsummary2
>> and test.adb.092t.fixup_cfg3:
>
> Ouch (bis). Quite unexpected that DECL_IGNORED_P is set out of nowhere.
> It's
> Identical Code Folding which turns Test2 into a thunk?
>
Yes, the identical functions trigger this folding.
Originally I discovered this when I wanted to debug the
arm back-end, where we have several identical functions:
static bool
arm_align_anon_bitfield (void)
{
return TARGET_AAPCS_BASED;
}
[...]
static bool
arm_cxx_guard_mask_bit (void)
{
return TARGET_AAPCS_BASED;
}
the second one had no line numbers, but since it was not
at the beginning of the assembly gdb did show a totally
unrelated line.
In general I can say that debugging optimized code is not
a very pleasant experience. But this issue happens only
rarely, while another issue is much more dominant.
You will probably see it quite often while debugging
the gcc middle end that stepping over inline functions
does not work right, one often steps over some code,
and ends up in the tree_check inline function.
That's because of an ambiguity in the dwarf debug
info of inline ranges. It does not have the view number
where the inline range ends exactly, but only the PC.
I have been working on some gdb patches to work around the
problem, but they are stuck unfortunately :-(
"Improve debugging of optimized code"
https://sourceware.org/pipermail/gdb-patches/2021-January/175617.html
>> in test.adb.052t.local-fnsummary2 we have:
>>
>> integer test.test1 (integer x)
>> {
>> <bb 2> [local count: 1073741824]:
>> [test.adb:5:7] return x_1(D);
>>
>> }
>>
>> and
>>
>> integer test.test2 (integer x)
>> {
>> <bb 2> [local count: 1073741824]:
>> [test.adb:10:7] return x_1(D);
>>
>> }
>>
>>
>> but in test.adb.092t.fixup_cfg3
>>
>> we have
>>
>> integer test.test1 (integer x)
>> {
>> <bb 2> [local count: 1073741824]:
>> [test.adb:5:7] return x_1(D);
>>
>> }
>>
>> and
>>
>> integer test.test2 (integer x)
>> {
>> integer D.4674;
>> integer retval.5;
>> integer _4;
>>
>> <bb 2> [local count: 1073741824]:
>> # DEBUG x => x_1(D)
>> _4 = x_1(D);
>> # DEBUG x => NULL
>> retval.5_2 = _4;
>> return retval.5_2;
>>
>> }
>>
>>
>> the line numbers are gone, and the function has DECL_IGNORED_P,
>> but still a useful DECL_SOURCE_LOCATION, I don't know
>> where the DECL_SOURCE_LOCATION can be seen in the dump files,
>> but from debugging this effect, I know that quite well.
>>
>> This second effect is why as a special case DECL_IGNORED_P functions
>> with valid looking DECL_SOURCE_LOCATION have now a .loc statement,
>> because that is less surprising to a user than having no line numbers
>> at all here.
>
> OK, thanks for the explanation, the patch is OK then.
>
Thank You very much,
I will push it now.
Bernd.
