Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 8/9/21 4:37 PM, Eric Botcazou wrote:
>> But it is okay that we can set a breakpoint on defs__struct1IP,
>> in the test case of PR 101598.
>> And the debugger shall only show assembler here.
>> Right?
> 
> The defs__struct1IP procedure should be totally transparent for the user, so 
> setting a breakpoint in it is not supposed to come into play.
> 

The symbol defs__struct1IP is a global symbol, so once a user know its name,
that user can set a breakpoint there, has never been changed, and I don't know
how to avoid that.

>> Do you have an example where this location information is used in the
>> compiler itself for debugging?
> 
> That's useful when you compile the code with -gnatD, i.e when you debug the 
> intermediate code generated by the front-end.
> 

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.

>> I assume You would agree that having the location for Test2 is better
>> than no debug info at all?
> 
> But we want no debug info at all for these IP functions.
> 
>> What do you think?
> 
> I guess I still don't understand why DECL_IGNORED_P was changed.
> 

We have several issues here.

First we have DECL_IGNORED_P = true functions, where DECL_SOURCE_LOCATION
is UNKNOWN_LOCATION, but they have *wrong* line numbers, either because a
preceding function's last line number extends to the ignored function, or
because of a bug in the assembler which mostly affects -gdwarf-4 with certain
versions if the binutils toolchain.

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:

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.


Thanks
Bernd.