Fix regressions for improved manual dump and add warning

Viviane Garèse · Viviane Garese · commit d3c532982b62 · 2023-09-14T16:08:01.000Z
* Fix range-check failed regression in ada_manual_dump cause by
  accessing Main.Filename while creating the manual dump helper unit
* Modify ada_manual_no_dump_in_root to account for more cases and
  different expectations while running gnatcov natively or
  cross-platform
* Fix typo in test.opt of manual dump tests
* Don't prevent creation of manual dump helper unit when no dump
  procedure call was found
* Update documentation to highlight difference in source trace files
  creation between bin-file and base64-stdout dump config
* Add warning for dump buffers indications in subproject stating they
  will only produce an accurate coverage analysis for the subprojects
  and the projects it depends on
diff --git a/doc/gnatcov/src_traces.rst b/doc/gnatcov/src_traces.rst
@@ -288,6 +288,18 @@ at which point the source traces will be created during the execution of the
 program. Therefore, the pragma or comment should be placed at a location at
 which such a function call would be appropriate.
 
+A dump procedure is only able to dump the buffers of the project tree which
+root is the project it is called from. A dump procedure call done in a
+subproject will result in a trace containing all code of projects higher in the
+project tree marked as not covered.
+
+One source trace is dumped per call to the dump buffers procedure. For the
+``bin-file`` dump configuration, each trace is written in a file which name
+depends on the selected trace file naming scheme. For the ``base64-stdout``
+dump configuration the traces are sequentially dumped in the same output file,
+from which the |gcv| command ``extrace-base64-trace`` will be able to produce a
+source trace file.
+
 .. _instr-tracename:
 
 Controlling trace file names
diff --git a/testsuite/SCOV/minicheck.py b/testsuite/SCOV/minicheck.py
@@ -108,7 +108,7 @@ def build_and_run(gprsw, covlevel, mains, extra_coverage_args, scos=None,
     :param None|list[str] exec_args: List of arguments to pass to the
         executable. This will only work for native configurations.
     :param bool auto_languages: See SUITE.tutils.xcov.
-    :param None|str manual_proj_name: when the dump trigger is manual, several
+    :param None|str manual_prj_name: when the dump trigger is manual, several
         traces files (one per project) can be emitted if there are dump buffers
         procedure calls in at least two distinct projects. This is the name of
         the project which trace we want to consider.
diff --git a/testsuite/tests/instr-cov/ada_manual_dump/test.opt b/testsuite/tests/instr-cov/ada_manual_dump/test.opt
@@ -1 +1 @@
-!src-traces DEAD Checks a feature specific the instrumenter
+!src-traces DEAD Checks a feature specific to the instrumenter
diff --git a/testsuite/tests/instr-cov/ada_manual_dump/test.py b/testsuite/tests/instr-cov/ada_manual_dump/test.py
@@ -34,14 +34,17 @@
            objdir="obj",
            deps=["lib1", "lib2"])
 
+instr_warning = (r"warning: Manual dump trigger indications were found in.*")
+
 build_run_and_coverage(
     gprsw=GPRswitches(root_project=p, units=["lib1", "main"]),
     covlevel='stmt',
     mains=['main'],
     extra_coverage_args=['-axcov', '--output-dir=xcov'],
     trace_mode='src',
     dump_trigger="manual",
-    manual_prj_name="gen")
+    manual_prj_name="gen",
+    tolerate_instrument_messages=instr_warning)
 
 # Check that gnatcov inserted the call to the dump buffers procedure in the
 # lib2.adb which is not a unit of interest
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/lib1.adb b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/lib1.adb
@@ -1,11 +1,11 @@
-with Lib3;
+with Lib2;
 
 package body Lib1 is
    function Foo return Integer
    is
       Res : Integer := 1;
    begin
-      Res := Res + Lib3.Baz;
+      Res := Res + Lib2.Bar;
       pragma Annotate (Xcov, Dump_Buffers);
       return Res;
    end Foo;
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib2/lib2.adb b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib2/lib2.adb
@@ -3,7 +3,6 @@ package body Lib2 is
    is
       Res : constant Integer := 1;
    begin
-      pragma Annotate (Xcov, Dump_Buffers);
       return Res;
    end Bar;
 end Lib2;
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib2/lib2.ads b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib2/lib2.ads
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib3/lib3.adb b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib3/lib3.adb
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib3/lib3.ads b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src-lib1/src-lib3/lib3.ads
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src/main.adb b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/src/main.adb
@@ -1,7 +1,6 @@
 pragma Ada_2012;
 
 with Lib1;
-with Lib2;
 
 procedure Main is
 
@@ -13,15 +12,7 @@ procedure Main is
 
    I : Integer := 1;
 begin
-    -- The only call that should not count as a violation when never executed
-    -- is that of the dump buffers procedure.
-   if 1 = I + 1 then
-      pragma Annotate (Xcov, Dump_Buffers);
-      I := I+ 1;
-   end if;
-
    Increment (I);
    I := I + Lib1.Foo;
-   I := I + Lib2.Bar;
    Increment (I);
 end Main;
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/test.opt b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/test.opt
@@ -1 +1 @@
-!src-traces DEAD Checks a feature specific the instrumenter
+!src-traces DEAD Checks a feature specific to the instrumenter
diff --git a/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/test.py b/testsuite/tests/instr-cov/ada_manual_no_dump_in_root/test.py
@@ -15,59 +15,42 @@
 
 tmp = Wdir('tmp_')
 
-lib1_p = gprfor(mains=[],
-                prjid="lib1",
-                srcdirs="../src-lib1",
-                objdir="obj-lib1",
-                langs=["Ada"],
-                deps=["lib3"]
-                )
 
-lib2_p = gprfor(mains=[],
-                prjid="lib2",
-                srcdirs="../src-lib2",
-                objdir="obj-lib2",
-                langs=["Ada"],
-                )
+def make_lib_gpr(name, srcdirs, deps):
+    return gprfor(mains=[],
+                  prjid=name,
+                  srcdirs=srcdirs,
+                  objdir="obj-" + name,
+                  langs=["Ada"],
+                  deps=deps)
 
-lib2_p = gprfor(mains=[],
-                prjid="lib3",
-                srcdirs="../src-lib1/src-lib3",
-                objdir="obj-lib3",
-                langs=["Ada"],
-                )
+
+lib1_p = make_lib_gpr("lib1", "../src-lib1", ["lib2"])
+lib2_p = make_lib_gpr("lib2", "../src-lib1/src-lib2", None)
 
 p = gprfor(mains=["main.adb"],
            srcdirs=["../src"],
            objdir="obj",
-           deps=["lib1", "lib2", "lib3"])
+           deps=["lib1", "lib2"])
+
+# Check that we get the expected coverage reports
+
+# Running gnatcov natively allows to have one source trace file per
+# project.
+instr_warning = (r"warning: Manual dump trigger indications were found in.*")
 
 build_run_and_coverage(
-    gprsw=GPRswitches(root_project=p, units=["lib1", "lib3"]),
+    gprsw=GPRswitches(root_project=p, units=["lib1", "lib2"]),
     covlevel='stmt',
     mains=['main'],
     extra_coverage_args=['-axcov', '--output-dir=xcov'],
     trace_mode='src',
     dump_trigger="manual",
-    manual_prj_name="lib1")
-
-# Check that gnatcov inserted the call to the dump buffers procedure in the
-# lib2.adb which is not a unit of interest
-
-
-def check_call(file):
-    thistest.fail_if_no_match("missing dump buffers procedure call",
-                              "(\n|.)*GCVRT.DB_manual_lib2.Dump_Buffers;"
-                              "(\n|.)*",
-                              contents_of(file))
-
-
-check_call('obj-lib2/lib2-gnatcov-instr/lib2.adb')
-
-# Check that we get the expected coverage reports
+    manual_prj_name="lib1",
+    tolerate_instrument_messages=instr_warning)
 
 check_xcov_reports('xcov/*.xcov', {
     'xcov/lib1.adb.xcov': {'+': {6, 8}, '-': {10}},
-    'xcov/lib3.adb.xcov': {'+': {4, 6}}})
+    'xcov/lib2.adb.xcov': {'+': {4, 6}}})
 
 thistest.result()
diff --git a/testsuite/tests/instr-cov/c_manual_dump/test.opt b/testsuite/tests/instr-cov/c_manual_dump/test.opt
@@ -1 +1 @@
-!src-traces DEAD Checks a feature specific the instrumenter
+!src-traces DEAD Checks a feature specific to the instrumenter
diff --git a/testsuite/tests/instr-cov/c_manual_dump/test.py b/testsuite/tests/instr-cov/c_manual_dump/test.py
@@ -34,9 +34,12 @@
 
 gprsw = GPRswitches(root_project=src_gpr)
 
+instr_warning = (r"warning: Manual dump trigger indications were found in.*")
+
 build_run_and_coverage(gprsw=gprsw, covlevel="stmt", mains=["main"],
                        extra_coverage_args=["-axcov"], dump_trigger="manual",
-                       manual_prj_name="main")
+                       manual_prj_name="main",
+                       tolerate_instrument_messages=instr_warning)
 
 # Check that that the dump call indication was correctly replaced in the sub
 # project
diff --git a/tools/gnatcov/instrument-c.adb b/tools/gnatcov/instrument-c.adb
@@ -3786,7 +3786,10 @@ package body Instrument.C is
             --  get the current execution time.
 
             File.Put_Line (Indent2 & "STR ("""
-                           & Escape_Backslashes (+Main.Filename) & """),");
+                           & (if Dump_Config.Trigger = Manual
+                             then +Prj.Prj_Name
+                             else Escape_Backslashes (+Main.Filename))
+                           & """),");
             File.Put_Line (Indent2 & "0,");
             File.Put_Line (Indent2 & "STR ("""")");
 
diff --git a/tools/gnatcov/instrument-projects.adb b/tools/gnatcov/instrument-projects.adb
@@ -17,6 +17,7 @@
 ------------------------------------------------------------------------------
 
 with Ada.Characters.Handling; use Ada.Characters.Handling;
+with Ada.Characters.Latin_1;  use Ada.Characters.Latin_1;
 with Ada.Containers.Hashed_Maps;
 with Ada.Containers.Indefinite_Ordered_Maps;
 with Ada.Containers.Indefinite_Ordered_Sets;
@@ -823,6 +824,22 @@ is
       Instrumenter         : in out Language_Instrumenter'Class;
       Manual_Dump_Inserted : in out Boolean)
    is
+      function Dump_Helper_Output_Dir_Exists
+        (Source : File_Info; Prj : Prj_Desc)
+         return Boolean
+      is (Ada.Directories.Exists
+          (GNATCOLL.VFS."+" (Source.Project.Object_Dir.Base_Dir_Name))
+            and then Ada.Directories.Exists (+Prj.Output_Dir));
+      --  True if the project's object directory and the instrumented sources
+      --  directory exist, False otherwise.
+
+      package Non_Root_Src_Calls_Sets is new
+        Ada.Containers.Indefinite_Ordered_Sets (Element_Type => String);
+
+      Non_Root_Src_Calls : Non_Root_Src_Calls_Sets.Set;
+      --  Set of names of source files containing a dump buffers indication
+      --  that belong to a non-root project.
+
    begin
       for Source_C in Project_Sources.Iterate loop
          declare
@@ -834,21 +851,41 @@ is
 
                   Prj                  : constant Prj_Desc :=
                     Get_Or_Create_Project_Info (IC, Source.Project).Desc;
+                  Is_Root_Prj          : constant Boolean :=
+                    Prj.Prj_Name = Root_Project_Info.Project.Name;
+                  Source_Name          : constant String :=
+                    GNATCOLL.VFS."+" (Source.File.Full_Name);
                   Helper_Unit          : Unbounded_String;
                   Contained_Indication : Boolean := False;
+
                begin
 
                   Instrumenter.Replace_Manual_Dump_Indication
                     (Contained_Indication,
                      Prj,
                      Source);
 
-                  if Contained_Indication
-                    and then
-                      Prj_Has_Manual_Helper.Find (Prj.Prj_Name) = No_Element
+                  if Contained_Indication and then not Is_Root_Prj
                   then
-                     --  Only generate one manual dump helper unit per project
+                     --  A call to the dump buffers procedure is only able to
+                     --  dump the buffers of the project it is in and its
+                     --  subprojects, meaning coverage data for all projects
+                     --  higher in the project tree will be missing. Record
+                     --  what file this call was in to warn the user later.
+
+                     Non_Root_Src_Calls.Include (Source_Name);
+                  end if;
 
+                  --  Only generate one manual dump helper unit per project.
+                  --  At this point, if the project's object directory and the
+                  --  instrumented sources directory do not exist there is no
+                  --  need to emit the dump helper unit. There are no units of
+                  --  interest or call to a manual dump procedure for this
+                  --  project.
+
+                  if Prj_Has_Manual_Helper.Find (Prj.Prj_Name) = No_Element
+                    and then Dump_Helper_Output_Dir_Exists (Source, Prj)
+                  then
                      Instrumenter.Emit_Dump_Helper_Unit_Manual
                        (Helper_Unit, Dump_Config, Prj);
 
@@ -888,7 +925,7 @@ is
                         --  unit is already taken care of by the regular
                         --  instrumentation process, so skip it.
 
-                        if Prj.Prj_Name /= Root_Project_Info.Project.Name then
+                        if not Is_Root_Prj then
                            Instrumenter.Emit_Buffers_List_Unit
                              (Instr_Units, Prj);
                         end if;
@@ -905,6 +942,32 @@ is
             end if;
          end;
       end loop;
+
+      if not Non_Root_Src_Calls.Is_Empty then
+         --  For each manual dump call inserted in a file belonging to a
+         --  non-root project, warn the user the coverage data it will produce
+         --  will not cover the whole project tree.
+
+         declare
+            All_File_Names : Unbounded_String := +"";
+         begin
+            for File_Name of Non_Root_Src_Calls loop
+               All_File_Names :=
+                 All_File_Names & (+File_Name) & Ada.Characters.Latin_1.LF;
+            end loop;
+
+            Outputs.Warn ("Manual dump trigger indications were found in"
+                          & " subprojects in the following files:"
+                          & Ada.Characters.Latin_1.LF
+                          & (+All_File_Names)
+                          & "The coverage report built from the source traces"
+                          & " they will produce will show all code from"
+                          & " projects higher in the project tree as not"
+                          & " covered. To get a full coverage analysis,"
+                          & " consider placing the manual dump buffers"
+                          & " indication in the root project.");
+         end;
+      end if;
    end Insert_Manual_Dump_Trigger;
 
    ---------------------

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-!src-traces DEAD Checks a feature specific the instrumenter`
	`1`	`+!src-traces DEAD Checks a feature specific to the instrumenter`