All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] Improve code_cov_parse_info tool
@ 2024-02-15 10:27 Mauro Carvalho Chehab
  2024-02-15 10:27 ` [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default Mauro Carvalho Chehab
                   ` (18 more replies)
  0 siblings, 19 replies; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Add support for:
- handling JSON output format from gcov;
- use an enhanced JSON format;
- add a tool to help analyzing branch coverage;
- allow generating and reading compressed .info files;
- improve filtering logic;
- add support for Intel Xe driver default filter list;
- several fixes.

Mauro Carvalho Chehab (17):
  scripts/code_cov_parse_info: Silent some messages by default
  scripts/code_cov_parse_info: do some renames to make it more coherent
  scripts/code_cov_parse_info: better handle include regexes
  scripts/code_cov_parse_info: add support for filtering Xe driver data
  scripts/code_cov_parse_info: add a tool to analyze branch coverage
  scripts/code_cov_parse_info: prepare $all_branch to get function data
  scripts/code_cov_parse_info: use numberic sort for line numbers
  scripts/code_cov_parse_info: make parse_info_data more generic
  scripts/code_cov_parse_info: stop recording/writing too old fields
  scripts/code_cov_parse_info: add support for parsing JSON files
  scripts/code_cov_parse_info: add an internal JSON format
  scripts/code_cov_parse_info: add support for compressed files
  scripts/code_cov_parse_info: warn if branch block is not zero
  scripts/code_cov_parse_info: better handle branch filtering
  scripts/code_cov_parse_info: add support for filtering branches
  scripts/code_cov_parse_info: fix files statistics
  scripts/code_cov_parse_info: better check if source file exists

 scripts/code_cov_parse_info | 881 +++++++++++++++++++++++++++++++-----
 1 file changed, 768 insertions(+), 113 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:24   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent Mauro Carvalho Chehab
                   ` (17 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Those aren't really needed on normal output.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 72a1e8c2b0ad..1498bd09cdba 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -1030,7 +1030,7 @@ if ($has_filter) {
 }
 
 my $ntests=scalar(%test_names);
-printf "Number of tests: %d\n", $ntests if ($ntests > 1);
+printf "Number of tests: %d\n", $ntests if ($stat && $ntests > 1);
 
 if ($show_files) {
 	for my $f(sort keys %used_source) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
  2024-02-15 10:27 ` [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:27   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes Mauro Carvalho Chehab
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Let the regex array to be clearer about include regexes, and
coherent with exclude ones.

While here, also rename the file exclude check function, to
have a name closer to its functions counterpart.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 38 ++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 1498bd09cdba..d3739211b68a 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -18,10 +18,10 @@ my %all_line;
 my %used_source;
 my %record;
 my %files;
-my @func_regexes;
+my @func_include_regexes;
 my @func_exclude_regexes;
 my %test_names;
-my @src_regexes;
+my @src_include_regexes;
 my @src_exclude_regexes;
 
 my $verbose = 0;
@@ -30,7 +30,7 @@ my $skip_func = 0;
 
 sub is_function_excluded($)
 {
-	return 0 if (!@func_regexes && !@func_exclude_regexes);
+	return 0 if (!@func_include_regexes && !@func_exclude_regexes);
 
 	my $func = shift;
 
@@ -38,28 +38,28 @@ sub is_function_excluded($)
 		return 1 if ($func =~ m/$r/);
 	}
 
-	return 0 if (!@func_regexes);
+	return 0 if (!@func_include_regexes);
 
-	foreach my $r (@func_regexes) {
+	foreach my $r (@func_include_regexes) {
 		return 0 if ($func =~ m/$r/);
 	}
 
 	return 1;
 }
 
-sub filter_file($)
+sub is_file_excluded($)
 {
 	my $s = shift;
 
-	return 0 if (!@src_regexes && !@src_exclude_regexes);
+	return 0 if (!@src_include_regexes && !@src_exclude_regexes);
 
 	foreach my $r (@src_exclude_regexes) {
 		return 1 if ($s =~ m/$r/);
 	}
 
-	return 0 if (!@src_regexes);
+	return 0 if (!@src_include_regexes);
 
-	foreach my $r (@src_regexes) {
+	foreach my $r (@src_include_regexes) {
 		return 0 if ($s =~ m/$r/);
 	}
 
@@ -107,7 +107,7 @@ sub parse_info_data($)
 			$files{$source} = 1;
 
 			# Just ignore files explictly set as such
-			$ignore = filter_file($source);
+			$ignore = is_file_excluded($source);
 			next;
 		}
 
@@ -189,7 +189,7 @@ sub parse_info_data($)
 
 		# Ignore DA/BRDA that aren't associated with functions
 		# Those are present on header files (maybe defines?)
-		next if (@func_regexes && !$has_func);
+		next if (@func_include_regexes && !$has_func);
 
 		# FNF:<number of functions found>
 		if (m/^FNF:(-?\d+)/) {
@@ -899,10 +899,10 @@ GetOptions(
 	"only-i915|only_i915" => \$only_i915,
 	"only-drm|only_drm" => \$only_drm,
 	"func-filters|f=s" => \$func_filters,
-	"include-func=s" => \@func_regexes,
+	"include-func=s" => \@func_include_regexes,
 	"exclude-func=s" => \@func_exclude_regexes,
 	"source-filters|S=s" => \$src_filters,
-	"include-source=s" => \@src_regexes,
+	"include-source=s" => \@src_include_regexes,
 	"exclude-source=s" => \@src_exclude_regexes,
 	"show-files|show_files" => \$show_files,
 	"show-lines|show_lines" => \$show_lines,
@@ -935,9 +935,9 @@ my $str;
 if ($only_i915) {
 	# Please keep in sync with the documentation
 	push @src_exclude_regexes, "selftest";
-	push @src_regexes, "drm/i915";
-	push @src_regexes, "drm/ttm";
-	push @src_regexes, "drm/vgem";
+	push @src_include_regexes, "drm/i915";
+	push @src_include_regexes, "drm/ttm";
+	push @src_include_regexes, "drm/vgem";
 }
 
 if ($only_drm) {
@@ -946,21 +946,21 @@ if ($only_drm) {
 	push @src_exclude_regexes, "^/drm/";
 }
 
-$str = open_filter_file($func_filters, \@func_regexes, \@func_exclude_regexes);
+$str = open_filter_file($func_filters, \@func_include_regexes, \@func_exclude_regexes);
 if ($str) {
 	$filter_str .= "," if ($filter_str ne "");
 	$filter_str .= " function regex ($str)";
 	$has_filter = 1;
 }
 
-$str = open_filter_file($src_filters, \@src_regexes, \@src_exclude_regexes);
+$str = open_filter_file($src_filters, \@src_include_regexes, \@src_exclude_regexes);
 if ($str) {
 	$filter_str .= "," if ($filter_str ne "");
 	$filter_str .= " source regex ($str)";
 	$has_filter = 1;
 }
 
-$ignore_unused = 1 if (@func_regexes || @func_exclude_regexes);
+$ignore_unused = 1 if (@func_include_regexes || @func_exclude_regexes);
 
 if ($ignore_unused) {
 	$filter_str .= "," if ($filter_str ne "");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
  2024-02-15 10:27 ` [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default Mauro Carvalho Chehab
  2024-02-15 10:27 ` [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:35   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data Mauro Carvalho Chehab
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

When there are both exclude and include regexes, the include ones
may be overriding the more generic excluded ones. So, handle them
first.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 38 ++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index d3739211b68a..ceea67a13d5a 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -34,17 +34,24 @@ sub is_function_excluded($)
 
 	my $func = shift;
 
+	# Handle includes first, as, when there are both include and exclude
+	# includes should take preference, as they can be overriding exclude
+	# rules
+	foreach my $r (@func_include_regexes) {
+	    return 0 if ($func =~ m/$r/);
+	}
+
 	foreach my $r (@func_exclude_regexes) {
 		return 1 if ($func =~ m/$r/);
 	}
 
-	return 0 if (!@func_include_regexes);
-
-	foreach my $r (@func_include_regexes) {
-		return 0 if ($func =~ m/$r/);
+	# If there are no exclude regexes, only include functions that are
+	# explicitly included.
+	if ($#func_exclude_regexes == 0) {
+		return 1;
 	}
 
-	return 1;
+	return 0;
 }
 
 sub is_file_excluded($)
@@ -1178,9 +1185,10 @@ Each line at B<[filter's file]> may contain a new regex:
 
 =back
 
-When both include and exclude regexes are found, exclude regexes are
-applied first and any functions that don't match the include regular
-expressions from the B<[filter's file]> will be ignored.
+Include regexes are handled first, as they can override an exclude regex.
+
+When just include regexes are used, any functions that don't match the
+include regular expressions from the B<[filter's file]> will be ignored.
 
 Please notice that, when this filter is used, B<--ignore-unused> will be
 automaticaly enabled, as the final goal is to report per-function usage.
@@ -1189,7 +1197,8 @@ automaticaly enabled, as the final goal is to report per-function usage.
 
 Include B<regex> to the function filter. Can be used multiple times.
 
-When used together with B<--func-filters>, regexes here are handled first.
+When used together with B<--func-filters> or B<--exclude-func>, regexes
+here are handled first.
 
 Please notice that, when this filter is used, B<--ignore-unused> will be
 automaticaly enabled, as the final goal is to report per-function usage.
@@ -1198,8 +1207,6 @@ automaticaly enabled, as the final goal is to report per-function usage.
 
 Include B<regex> to the function filter. Can be used multiple times.
 
-When used together with B<--func-filters>, regexes here are handled first.
-
 Please notice that, when this filter is used, B<--ignore-unused> will be
 automaticaly enabled, as the final goal is to report per-function usage.
 
@@ -1229,22 +1236,19 @@ Each line at B<[filter's file]> may contain a new regex:
 
 =back
 
-When both include and exclude regexes are found, exclude regexes are
-applied first and any functions that don't match the include regular
-expressions from the B<[filter's file]> will be ignored.
+Include regexes are handled first, as they can override an exclude regex.
 
 =item B<--include-src> B<regex>
 
 Include B<regex> to the sources filter. Can be used multiple times.
 
-When used together with B<--src-filters>, regexes here are handled first.
+When used together with B<--src-filters> and B<--exclude-src>, regexes
+here are handled first.
 
 =item B<--exclude-src> B<regex>
 
 Include B<regex> to the sources filter. Can be used multiple times.
 
-When used together with B<--src-filters>, regexes here are handled first.
-
 =item B<--ignore-unused> or B<--ignore_unused>
 
 Filters out unused C files and headers from the code coverage results.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:36   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage Mauro Carvalho Chehab
                   ` (14 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Just like it does for i915, add an option to get code coverage
data from Xe driver.

For now, it won't be taking DRM core stuff into account; just
the Xe driver code itself.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index ceea67a13d5a..f0fb5716a6ca 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -894,6 +894,7 @@ my $src_filters;
 my $show_files;
 my $show_lines;
 my $only_i915;
+my $only_xe;
 my $only_drm;
 
 GetOptions(
@@ -904,6 +905,7 @@ GetOptions(
 	"verbose|v" => \$verbose,
 	"ignore-unused|ignore_unused" => \$ignore_unused,
 	"only-i915|only_i915" => \$only_i915,
+	"only-xe|only_xe" => \$only_xe,
 	"only-drm|only_drm" => \$only_drm,
 	"func-filters|f=s" => \$func_filters,
 	"include-func=s" => \@func_include_regexes,
@@ -947,6 +949,14 @@ if ($only_i915) {
 	push @src_include_regexes, "drm/vgem";
 }
 
+if ($only_xe) {
+	# Please keep in sync with the documentation
+	push @src_exclude_regexes, "selftest";
+	push @src_include_regexes, "drm/xe";
+#	push @src_include_regexes, "drm/ttm";
+#	push @src_include_regexes, "drm/vgem";
+}
+
 if ($only_drm) {
 	# Please keep in sync with the documentation
 	push @src_exclude_regexes, "trace.*\.h\$";
@@ -980,7 +990,9 @@ foreach my $f (@ARGV) {
 
 	if ($gen_report) {
 		$f =~ s,.*/,,;
+		$f =~ s/\.gz$//;
 		$f =~ s/\.info$//;
+		$f =~ s/\.json$//;
 
 		gen_stats();
 
@@ -1162,6 +1174,19 @@ Excluding files that match:
 
 	- selftest
 
+=item B<--only-xe> or B<--only_xe>
+
+Filters out C files and headers outside drm core and drm/i915.
+
+E. g. code coverage results will include only the files that that match
+the following regular expressions:
+
+	- drm/xe/
+
+Excluding files that match:
+
+	- selftest
+
 =item B<--func-filters>  B<[filter's file]> or B<-f>  B<[filter's file]>
 
 Use a file containing regular expressions (regex) to filter functions.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:40   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data Mauro Carvalho Chehab
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Branch coverage is trickier than functions. Add a tool that displays
the starting line that it is not being covered, together with up
to 10 context lines before.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 86 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 85 insertions(+), 1 deletion(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index f0fb5716a6ca..1be7fddf3f3e 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -879,6 +879,77 @@ sub generate_report($)
 	close OUT;
 }
 
+sub check_source_branches()
+{
+	foreach my $source (sort keys(%all_branch)) {
+		next if (!$used_source{$source});
+		next if (is_file_excluded($source));
+
+		my @lines;
+		foreach my $where (sort keys %{$all_branch{$source}}) {
+			my $taken = $all_branch{$source}{$where};
+			next if ($taken > 0);
+
+			next if !($where =~ m/^(-?\d+),(-?\d+),(-?\d+)/);
+
+			my ($ln, $block, $branch, $ctx_lines);
+			$ln = $1;
+			$block = $2;
+			$branch = $3;
+
+			if (!@lines) {
+				open IN, "$source" || die "File $source not found. Can't check branches\n";
+				@lines = <IN>;
+				close IN;
+			}
+
+			if ($ln >= $#lines) {
+				die "$source:$ln line is bigger than the number of lines at the file ($#lines lines)\n";
+				return;
+			}
+
+			my $func = $all_branch{$source}{$where}{func};
+			my $func_ln = $all_branch{$source}{$where}{func_ln};
+
+			print "Branch $source:$ln, ";
+			if ($func) {
+				if ($func_ln) {
+					print "func: $func, ";
+				} else {
+					print "func: $func:$func_ln, ";
+				}
+			}
+			print "block $block, branch $branch not taken:\n";
+
+			if ($func_ln) {
+				$ctx_lines = $ln - $func_ln;
+			} else {
+				$ctx_lines = 10;
+			}
+
+
+			# Search for up to $ctx_lines before the occurrence
+			my $context = "";
+			my $has_if;
+			$ln-- if ($ln > 0);
+			my $delim = "->";
+			for (my $if_ln = $ln; $if_ln >= 0 && (($ln - $if_ln) < $ctx_lines); $if_ln--) {
+				$context = "$if_ln$delim\t$lines[$if_ln]" . $context;
+				$delim = "";
+				if ($lines[$if_ln] =~ m/(\bif\b|#if)/) {
+					$has_if = 1;
+					last;
+				}
+			}
+			if ($has_if) {
+				print $context;
+			} else {
+				print "$ln->\t$lines[$ln]";
+			}
+		}
+	}
+}
+
 #
 # Argument handling
 #
@@ -896,6 +967,7 @@ my $show_lines;
 my $only_i915;
 my $only_xe;
 my $only_drm;
+my $check_branches;
 
 GetOptions(
 	"print-coverage|print_coverage|print|p" => \$print_used,
@@ -916,6 +988,7 @@ GetOptions(
 	"show-files|show_files" => \$show_files,
 	"show-lines|show_lines" => \$show_lines,
 	"report|r=s" => \$gen_report,
+	"check-branches" => \$check_branches,
 	"css-file|css|c=s" => \$css_file,
 	"title|t=s" => \$title,
 	"html-prolog|prolog=s" => \$html_prolog,
@@ -933,7 +1006,7 @@ if ($#ARGV < 0) {
 }
 
 # At least one action should be specified
-pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report);
+pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report && !$check_branches);
 
 pod2usage(1) if ($gen_report && ($print_used || $filter || $stat || $print_unused));
 
@@ -1024,6 +1097,11 @@ if ($gen_report) {
 
 gen_stats();
 
+
+if ($check_branches) {
+	check_source_branches();
+}
+
 die "Nothing counted. Wrong input files?" if (!$stats{"all_files"});
 
 print_code_coverage($print_used, $print_unused, $show_lines);
@@ -1294,6 +1372,12 @@ This option is automaticaly enabled when B<--func-filters> is used.
 Shows the list of files that were used to produce the code coverage
 results.
 
+=item B<--check-branches>
+
+Checks at the Linux Kernel source files what's the contents of the
+branches that weren't taken. The directory should match what's
+stored inside the info files.
+
 =item B<--verbose> or B<-v>
 
 Prints the name of each parsed file.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:41   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers Mauro Carvalho Chehab
                   ` (12 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Right now, we store only the branch counts. We also need to store
the functions that each branch belongs. So, add a new hash in
order to allow adding function data later on.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 1be7fddf3f3e..d28217cc196d 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -233,7 +233,7 @@ sub parse_info_data($)
 			$was_used = 1 if ($taken > 0);
 
 			$record{$source}{$func}{brda}{$where} += $taken;
-			$all_branch{$source}{"$where"} += $taken;
+			$all_branch{$source}{"$where"}{count} += $taken;
 			next;
 		}
 
@@ -448,7 +448,7 @@ sub gen_stats()
 
 		foreach my $where (keys(%{$all_branch{$source}})) {
 			$stats{"branch_count"}++;
-			$stats{"branch_reached"}++ if ($all_branch{$source}{$where} != 0);
+			$stats{"branch_reached"}++ if ($all_branch{$source}{$where}{count} != 0);
 		}
 	}
 
@@ -607,7 +607,7 @@ sub generate_report($)
 		}
 		foreach my $source (keys(%{$report{$f}{"all_branch"}})) {
 			foreach my $where (keys(%{$report{$f}{"all_branch"}{$source}})) {
-				$all_branch{$source}{"$where"} += $report{$f}{"all_branch"}{$source}{$where};
+				$all_branch{$source}{"$where"}{count} += $report{$f}{"all_branch"}{$source}{$where}{count};
 			}
 		}
 		for my $source(keys(%{$report{$f}{"files"}})) {
@@ -887,7 +887,7 @@ sub check_source_branches()
 
 		my @lines;
 		foreach my $where (sort keys %{$all_branch{$source}}) {
-			my $taken = $all_branch{$source}{$where};
+			my $taken = $all_branch{$source}{$where}{count};
 			next if ($taken > 0);
 
 			next if !($where =~ m/^(-?\d+),(-?\d+),(-?\d+)/);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 16:55   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic Mauro Carvalho Chehab
                   ` (11 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The DA and BRDA information is originally numerically sorted.

Sort it the same way at the output data.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index d28217cc196d..56b27faee812 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -291,6 +291,21 @@ sub parse_info_data($)
 	close IN or die;
 }
 
+sub sort_where($$)
+{
+	my @a = split ",", shift;
+	my @b = split ",", shift;
+	my $ret;
+
+	$ret = $a[0] <=> $b[0];
+	return $ret if ($ret);
+
+	$ret = $a[1] <=> $b[1];
+	return $ret if ($ret);
+
+	return $a[2] <=> $b[2];
+}
+
 sub write_filtered_file($)
 {
 	my $filter = shift;
@@ -332,10 +347,10 @@ sub write_filtered_file($)
 				}
 			}
 
-			foreach my $ln(sort keys %{ $record{$source}{$func}{da} }) {
+			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{$func}{da} }) {
 				$filtered .= "DA:$ln," . $record{$source}{$func}{da}{$ln} . "\n";
 			}
-			foreach my $where(sort keys %{ $record{$source}{$func}{brda} }) {
+			foreach my $where(sort sort_where keys %{ $record{$source}{$func}{brda} }) {
 				my $taken = $record{$source}{$func}{brda}{$where};
 				$taken = "-" if (!$taken);
 				$filtered .= "BRDA:$where,$taken\n";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:01   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields Mauro Carvalho Chehab
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

In preparation for adding support for JSON files, make the
function more generic.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 67 +++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 33 deletions(-)
 mode change 100755 => 100644 scripts/code_cov_parse_info

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
old mode 100755
new mode 100644
index 56b27faee812..92091215d224
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -98,6 +98,7 @@ sub parse_info_data($)
 			if ($1 ne $cur_test) {
 				$cur_test = $1;
 				$test_names{$cur_test} = 1;
+				$record{tests}{$cur_test} = 1;
 			}
 			$source = $before_sf;
 			$func = $before_sf;
@@ -158,7 +159,7 @@ sub parse_info_data($)
 
 			$skip_func = 0;
 
-			$record{$source}{$func}{fn} = $ln;
+			$record{$source}{func}{$func}{fn} = $ln;
 			$all_func{$func}{$source}->{ln} = $ln;
 			next;
 		}
@@ -186,7 +187,7 @@ sub parse_info_data($)
 			$skip_func = 0;
 			$was_used = 1;
 
-			$record{$source}{$func}{fnda} += $count;
+			$record{$source}{func}{$func}{fnda} += $count;
 			$used_func{$func}{$source}->{count} += $count;
 			next;
 		}
@@ -200,14 +201,14 @@ sub parse_info_data($)
 
 		# FNF:<number of functions found>
 		if (m/^FNF:(-?\d+)/) {
-			$record{$source}{$func}{fnf} = $1;
+			$record{$source}{func}{$func}{fnf} = $1;
 			next;
 		}
 		# FNH:<number of function hit>
 		if (m/^FNH:(-?\d+)/) {
 			my $hits = $1;
-			if (!defined($record{$source}{$func}{fnh}) || $record{$source}{$func}{fnh} < $hits) {
-				$record{$source}{$func}{fnh} = $hits;
+			if (!defined($record{$source}{func}{$func}{fnh}) || $record{$source}{func}{$func}{fnh} < $hits) {
+				$record{$source}{func}{$func}{fnh} = $hits;
 			}
 			next;
 		}
@@ -232,21 +233,21 @@ sub parse_info_data($)
 
 			$was_used = 1 if ($taken > 0);
 
-			$record{$source}{$func}{brda}{$where} += $taken;
-			$all_branch{$source}{"$where"}{count} += $taken;
+			$record{files}{$source}{line}{$ln}{branches}[$branch]{count} += $taken;
+			$all_branch{$source}{$where}{count} += $taken;
 			next;
 		}
 
 		# BRF:<number of branches found>
 		if (m/^BRF:(-?\d+)/) {
-			$record{$source}{$func}{brf} = $1;
+			$record{$source}{func}{$func}{brf} = $1;
 			next;
 		}
 		# BRH:<number of branches hit>
 		if (m/^BRH:(-?\d+)/) {
 			my $hits = $1;
-			if (!defined($record{$source}{$func}{brh}) || $record{$source}{$func}{brh} < $hits) {
-				$record{$source}{$func}{brh} = $hits;
+			if (!defined($record{$source}{func}{$func}{brh}) || $record{$source}{func}{$func}{brh} < $hits) {
+				$record{$source}{func}{$func}{brh} = $hits;
 			}
 			next;
 		}
@@ -265,22 +266,22 @@ sub parse_info_data($)
 
 			$was_used = 1 if ($count > 0);
 
-			$record{$source}{$func}{da}{$ln} += $count;
-			$all_line{$source}{"$ln"} += $count;
+			$record{$source}{func}{$func}{da}{$ln} += $count;
+			$all_line{$source}{$ln} += $count;
 			next;
 		}
 
 		# LF:<number of instrumented lines>
 		if (m/^LF:(-?\d+)/) {
-			$record{$source}{$func}{lf} = $1;
+			$record{$source}{func}{$func}{lf} = $1;
 			next;
 		}
 
 		# LH:<number of lines with a non-zero execution count>
 		if (m/^LH:(-?\d+)/) {
 			my $hits = $1;
-			if (!defined($record{$source}{$func}{lh}) || $record{$source}{$func}{lh} < $hits) {
-				$record{$source}{$func}{lh} = $hits;
+			if (!defined($record{$source}{func}{$func}{lh}) || $record{$source}{func}{$func}{lh} < $hits) {
+				$record{$source}{func}{$func}{lh} = $hits;
 			}
 			next;
 		}
@@ -333,39 +334,39 @@ sub write_filtered_file($)
 				my $fn;
 				my $fnda;
 
-				if (defined($record{$source}{$func}{fn})) {
-					$filtered .= "FN:" . $record{$source}{$func}{fn} . ",$func\n";
+				if (defined($record{$source}{func}{$func}{fn})) {
+					$filtered .= "FN:" . $record{$source}{func}{$func}{fn} . ",$func\n";
 				}
-				if (defined($record{$source}{$func}{fnda})) {
-					$filtered .= "FNDA:" . $record{$source}{$func}{fnda} . ",$func\n";
+				if (defined($record{$source}{func}{$func}{fnda})) {
+					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
 				}
 				if ($record{$source}{fnf}) {
-					$filtered .= "FNF:". $record{$source}{$func}{fnf} ."\n";
+					$filtered .= "FNF:". $record{$source}{func}{$func}{fnf} ."\n";
 				}
 				if ($record{$source}{fnh}) {
-					$filtered .= "FNH:". $record{$source}{$func}{fnh} ."\n";
+					$filtered .= "FNH:". $record{$source}{func}{$func}{fnh} ."\n";
 				}
 			}
 
-			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{$func}{da} }) {
-				$filtered .= "DA:$ln," . $record{$source}{$func}{da}{$ln} . "\n";
+			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{func}{$func}{da} }) {
+				$filtered .= "DA:$ln," . $record{$source}{func}{$func}{da}{$ln} . "\n";
 			}
-			foreach my $where(sort sort_where keys %{ $record{$source}{$func}{brda} }) {
-				my $taken = $record{$source}{$func}{brda}{$where};
+			foreach my $where(sort sort_where keys %{ $record{$source}{func}{$func}{brda} }) {
+				my $taken = $record{$source}{func}{$func}{brda}{$where};
 				$taken = "-" if (!$taken);
 				$filtered .= "BRDA:$where,$taken\n";
 			}
-			if ($record{$source}{$func}{brf}) {
-				$filtered .= "BRF:". $record{$source}{$func}{brf} ."\n";
+			if ($record{$source}{func}{$func}{brf}) {
+				$filtered .= "BRF:". $record{$source}{func}{$func}{brf} ."\n";
 			}
-			if ($record{$source}{$func}{brh}) {
-				$filtered .= "BRH:". $record{$source}{$func}{brh} ."\n";
+			if ($record{$source}{func}{$func}{brh}) {
+				$filtered .= "BRH:". $record{$source}{func}{$func}{brh} ."\n";
 			}
-			if ($record{$source}{$func}{lf}) {
-				$filtered .= "LF:". $record{$source}{$func}{lf} ."\n";
+			if ($record{$source}{func}{$func}{lf}) {
+				$filtered .= "LF:". $record{$source}{func}{$func}{lf} ."\n";
 			}
-			if ($record{$source}{$func}{lh}) {
-				$filtered .= "LH:". $record{$source}{$func}{lh} ."\n";
+			if ($record{$source}{func}{$func}{lh}) {
+				$filtered .= "LH:". $record{$source}{func}{$func}{lh} ."\n";
 			}
 		}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (7 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:09   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files Mauro Carvalho Chehab
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

As we'll be using json as basis, drop support for the fields that:

- aren't used at the reports;
- aren't even present when lcov parses gcc json format.

Such fields are only handled on lcov for very old gcc versions.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 92091215d224..038440a9c3e3 100644
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -201,15 +201,10 @@ sub parse_info_data($)
 
 		# FNF:<number of functions found>
 		if (m/^FNF:(-?\d+)/) {
-			$record{$source}{func}{$func}{fnf} = $1;
 			next;
 		}
 		# FNH:<number of function hit>
 		if (m/^FNH:(-?\d+)/) {
-			my $hits = $1;
-			if (!defined($record{$source}{func}{$func}{fnh}) || $record{$source}{func}{$func}{fnh} < $hits) {
-				$record{$source}{func}{$func}{fnh} = $hits;
-			}
 			next;
 		}
 
@@ -240,15 +235,10 @@ sub parse_info_data($)
 
 		# BRF:<number of branches found>
 		if (m/^BRF:(-?\d+)/) {
-			$record{$source}{func}{$func}{brf} = $1;
 			next;
 		}
 		# BRH:<number of branches hit>
 		if (m/^BRH:(-?\d+)/) {
-			my $hits = $1;
-			if (!defined($record{$source}{func}{$func}{brh}) || $record{$source}{func}{$func}{brh} < $hits) {
-				$record{$source}{func}{$func}{brh} = $hits;
-			}
 			next;
 		}
 
@@ -340,34 +330,11 @@ sub write_filtered_file($)
 				if (defined($record{$source}{func}{$func}{fnda})) {
 					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
 				}
-				if ($record{$source}{fnf}) {
-					$filtered .= "FNF:". $record{$source}{func}{$func}{fnf} ."\n";
-				}
-				if ($record{$source}{fnh}) {
-					$filtered .= "FNH:". $record{$source}{func}{$func}{fnh} ."\n";
-				}
-			}
 
-			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{func}{$func}{da} }) {
-				$filtered .= "DA:$ln," . $record{$source}{func}{$func}{da}{$ln} . "\n";
 			}
-			foreach my $where(sort sort_where keys %{ $record{$source}{func}{$func}{brda} }) {
-				my $taken = $record{$source}{func}{$func}{brda}{$where};
 				$taken = "-" if (!$taken);
 				$filtered .= "BRDA:$where,$taken\n";
 			}
-			if ($record{$source}{func}{$func}{brf}) {
-				$filtered .= "BRF:". $record{$source}{func}{$func}{brf} ."\n";
-			}
-			if ($record{$source}{func}{$func}{brh}) {
-				$filtered .= "BRH:". $record{$source}{func}{$func}{brh} ."\n";
-			}
-			if ($record{$source}{func}{$func}{lf}) {
-				$filtered .= "LF:". $record{$source}{func}{$func}{lf} ."\n";
-			}
-			if ($record{$source}{func}{$func}{lh}) {
-				$filtered .= "LH:". $record{$source}{func}{$func}{lh} ."\n";
-			}
 		}
 
 		$filtered .= "end_of_record\n";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (8 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:12   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format Mauro Carvalho Chehab
                   ` (8 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Currently, the tool supports only info files, provided by
lcov tool. While this works, the info output lacks support to
properly identify what function is related to branches and
lines. Such limitation doesn't exist with json. So, add
support for parsing it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 210 +++++++++++++++++++++++++++++++++---
 1 file changed, 197 insertions(+), 13 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 038440a9c3e3..97c94a81004f 100644
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -28,6 +28,16 @@ my $verbose = 0;
 my $ignore_unused = 0;
 my $skip_func = 0;
 
+my $has_json_support = eval {
+	require Cpanel::JSON::XS;
+	Cpanel::JSON::XS->import(qw(decode_json));
+	1;
+};
+
+if (!$has_json_support) {
+	print "Warning: System doesn't have Cpanel::JSON::XS. Can't use gcov directly.\n";
+}
+
 sub is_function_excluded($)
 {
 	return 0 if (!@func_include_regexes && !@func_exclude_regexes);
@@ -76,7 +86,169 @@ sub is_file_excluded($)
 # Use something that comes before any real function
 my $before_sf = "!!!!";
 
-sub parse_info_data($)
+sub parse_json_gcov_v1($$)
+{
+	my $file = shift;
+	my $json = shift;
+
+	my $was_used = 0;
+	my $has_func = 0;
+	my $ignore = 0;
+
+	my $cur_test = $file;
+	$cur_test =~ s#^.*/##;
+	$cur_test =~ s#\.json$##;
+	$test_names{$cur_test} = 1;
+
+	# Store the common JSON data into the record
+	for my $key (keys %$json) {
+		next if ($key eq "files");
+		next if ($key eq "functions");
+		next if ($key eq "lines");
+		# Store any extra data
+		$record{$key} = $json->{$key};
+	}
+	# Store test name at the record
+	$record{tests}{$cur_test} = 1;
+
+	for my $file_ref (@{$json->{'files'}}) {
+		my $source = $file_ref->{'file'};
+
+		$files{$source} = 1;
+		next if is_file_excluded($source);
+
+		# Parse functions
+		for my $func_ref (@{$file_ref->{'functions'}}) {
+			my $func = $func_ref->{'name'};
+
+			next if is_function_excluded($func);
+
+			# Negative gcov results are possible, as reported at:
+			# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
+			# Lcov ignores those. So, let's do the same here.
+			$func_ref->{'execution_count'} = 0 if ($func_ref->{'execution_count'} < 0);
+
+			# Store the record
+			for my $key (keys %$func_ref) {
+				if ($key eq "execution_count") {
+					$record{$source}{func}{$func}{$key} += $func_ref->{$key};
+				} else {
+					$record{$source}{func}{$func}{$key} = $func_ref->{$key};
+				}
+			}
+
+			$all_func{$func}{$source}->{ln} = $func_ref->{'start_line'};
+			$all_func{$func}{$source}->{end_ln} = $func_ref->{'end_line'};
+
+			if ($func_ref->{'execution_count'} > 0) {
+				$used_func{$func}{$source}->{count} += $func_ref->{'execution_count'};
+				$was_used = 1;
+			}
+		}
+		next if ($ignore_unused && !$was_used);
+		$used_source{$source} = 1;
+
+		# Parse lines and branches
+		for my $line_ref (@{$file_ref->{'lines'}}) {
+			my $ln = $line_ref->{'line_number'};
+
+			# Negative gcov results are possible, as reported at:
+			# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
+			# Lcov ignores those. So, let's do the same here.
+			$line_ref->{'count'} = 0 if ($line_ref->{'count'} < 0);
+
+			my $func = $line_ref->{'function_name'};
+			if (!$func) {
+				# Ignore DA/BRDA that aren't associated with
+				# functions. Those are present on header files
+				# (maybe defines?)
+				next if (@func_include_regexes);
+
+				# Otherwise place them in separate
+				$func = $before_sf;
+			} else {
+				next if is_function_excluded($func);
+			}
+
+			# Store the record
+			for my $key (keys %$line_ref) {
+				next if ($key eq "line_number");
+
+				# Branches will be handled in separate
+				next if ($key eq "branches");
+				if ($key eq "count") {
+					$record{$source}{line}{$ln}{$key} += $line_ref->{$key};
+				} else {
+					$record{$source}{line}{$ln}{$key} = $line_ref->{$key};
+				}
+			}
+			$all_line{$source}{$ln} += $line_ref->{'count'};
+
+			my $i = 0;
+			for my $branch_ref (@{$line_ref->{'branches'}}) {
+				my $taken = $branch_ref->{'count'};
+				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
+
+				# Negative gcov results are possible, as
+				# reported at:
+				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
+				# Lcov ignores those. So, let's do the same
+				# here.
+				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
+
+				for my $key (keys %$branch_ref) {
+					if ($key eq "count") {
+						$record{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
+					} else {
+						$record{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
+					}
+				}
+
+				$all_branch{$source}{$where}{count} += $taken;
+				$i++;
+			}
+			if (!defined($record{$source}{line}{$ln}{branches})) {
+				@{$record{$source}{line}{$ln}{branches}} = ();
+			}
+		}
+	}
+
+	# As the record was changed, we need to use a different format name
+	$record{format_version} = "parse_info v1.0";
+}
+
+sub read_json($)
+{
+	my $file = shift;
+
+	if (!$has_json_support) {
+		die "Can't parse json as system doesn't have Cpanel::JSON::XS.\n";
+	}
+
+	# Read JSON data
+	open IN, $file or die "can't open $file";
+	while (<IN>) {
+		my $json = eval {
+			Cpanel::JSON::XS::decode_json($_)
+		};
+
+		if (!$json) {
+			printf "Failed to parse file $file, line $.\n";
+			next;
+		}
+		if ($json->{'format_version'} eq '1') {
+			parse_json_gcov_v1($file, $json);
+		} else {
+			if ($json->{'format_version'}) {
+				die "Can't parse JSON version %d on file $file\n", $json->{'format_version'};
+			}
+			die "Unknown JSON format on file $file\n";
+		}
+	}
+	close IN;
+}
+
+sub read_info($)
 {
 	my $file = shift;
 	my $was_used = 0;
@@ -159,7 +331,7 @@ sub parse_info_data($)
 
 			$skip_func = 0;
 
-			$record{$source}{func}{$func}{fn} = $ln;
+			$record{$source}{func}{$func}{start_line} = $ln;
 			$all_func{$func}{$source}->{ln} = $ln;
 			next;
 		}
@@ -256,23 +428,23 @@ sub parse_info_data($)
 
 			$was_used = 1 if ($count > 0);
 
-			$record{$source}{func}{$func}{da}{$ln} += $count;
+			$record{$source}{line}{$ln}{count} += $count;
+			if (!defined($record{$source}{line}{$ln}{branches})) {
+				@{$record{$source}{line}{$ln}{branches}} = ();
+			}
+
 			$all_line{$source}{$ln} += $count;
+
 			next;
 		}
 
 		# LF:<number of instrumented lines>
 		if (m/^LF:(-?\d+)/) {
-			$record{$source}{func}{$func}{lf} = $1;
 			next;
 		}
 
 		# LH:<number of lines with a non-zero execution count>
 		if (m/^LH:(-?\d+)/) {
-			my $hits = $1;
-			if (!defined($record{$source}{func}{$func}{lh}) || $record{$source}{func}{$func}{lh} < $hits) {
-				$record{$source}{func}{$func}{lh} = $hits;
-			}
 			next;
 		}
 
@@ -319,21 +491,29 @@ sub write_filtered_file($)
 			$filtered .= "SF:$source\n";
 		}
 
-		foreach my $func(sort keys %{ $record{$source} }) {
+		foreach my $func(sort keys %{ $record{$source}{func} }) {
 			if ($func ne $before_sf) {
 				my $fn;
 				my $fnda;
 
-				if (defined($record{$source}{func}{$func}{fn})) {
-					$filtered .= "FN:" . $record{$source}{func}{$func}{fn} . ",$func\n";
+				if (defined($record{$source}{func}{$func}{start_line})) {
+					$filtered .= "FN:" . $record{$source}{func}{$func}{start_line} . ",$func\n";
 				}
 				if (defined($record{$source}{func}{$func}{fnda})) {
 					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
 				}
 
 			}
+		}
+
+		foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{line} }) {
+			$filtered .= "DA:$ln," . $record{$source}{line}{$ln}{count} . "\n";
+
+			my $i = 0;
+			for ($i = 0, $i < scalar(@{$record{$source}{line}{$ln}{branches}}), $i++) {
+				my $taken = $record{$source}{line}{$ln}{branches}[$i]{count};
 				$taken = "-" if (!$taken);
-				$filtered .= "BRDA:$where,$taken\n";
+				$filtered .= "BRDA:$ln,0,$i,$taken\n";
 			}
 		}
 
@@ -1042,7 +1222,11 @@ if ($ignore_unused) {
 }
 
 foreach my $f (@ARGV) {
-	parse_info_data($f);
+	if ($f =~ /.json$/) {
+		read_json($f);
+	} else {
+		read_info($f);
+	}
 
 	if ($gen_report) {
 		$f =~ s,.*/,,;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (9 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:15   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files Mauro Carvalho Chehab
                   ` (7 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

In order to be able to join data from different JSON files, the
JSON format needs to be different than the standard gcov one,
as:

- it should contain test names;
- it should use hashes instead of arrays for functions and lines.

So, at the end of the day, it needs a different format version.
Add support for read/write on a new format.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 222 +++++++++++++++++++++++++++++-------
 1 file changed, 179 insertions(+), 43 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 97c94a81004f..8180b9c1f82e 100644
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -131,9 +131,9 @@ sub parse_json_gcov_v1($$)
 			# Store the record
 			for my $key (keys %$func_ref) {
 				if ($key eq "execution_count") {
-					$record{$source}{func}{$func}{$key} += $func_ref->{$key};
+					$record{files}{$source}{func}{$func}{$key} += $func_ref->{$key};
 				} else {
-					$record{$source}{func}{$func}{$key} = $func_ref->{$key};
+					$record{files}{$source}{func}{$func}{$key} = $func_ref->{$key};
 				}
 			}
 
@@ -177,9 +177,9 @@ sub parse_json_gcov_v1($$)
 				# Branches will be handled in separate
 				next if ($key eq "branches");
 				if ($key eq "count") {
-					$record{$source}{line}{$ln}{$key} += $line_ref->{$key};
+					$record{files}{$source}{line}{$ln}{$key} += $line_ref->{$key};
 				} else {
-					$record{$source}{line}{$ln}{$key} = $line_ref->{$key};
+					$record{files}{$source}{line}{$ln}{$key} = $line_ref->{$key};
 				}
 			}
 			$all_line{$source}{$ln} += $line_ref->{'count'};
@@ -198,17 +198,17 @@ sub parse_json_gcov_v1($$)
 
 				for my $key (keys %$branch_ref) {
 					if ($key eq "count") {
-						$record{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
+						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
 					} else {
-						$record{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
+						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
 					}
 				}
 
 				$all_branch{$source}{$where}{count} += $taken;
 				$i++;
 			}
-			if (!defined($record{$source}{line}{$ln}{branches})) {
-				@{$record{$source}{line}{$ln}{branches}} = ();
+			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
+				@{$record{files}{$source}{line}{$ln}{branches}} = ();
 			}
 		}
 	}
@@ -217,6 +217,121 @@ sub parse_json_gcov_v1($$)
 	$record{format_version} = "parse_info v1.0";
 }
 
+sub parse_json_internal_format_v1($$)
+{
+	my $file = shift;
+	my $json = shift;
+
+	my $was_used = 0;
+	my $has_func = 0;
+	my $ignore = 0;
+
+	# Store the common JSON data into the record
+	for my $key (keys %$json) {
+		next if ($key eq "files");
+		next if ($key eq "functions");
+		next if ($key eq "lines");
+		# Store any extra data
+		$record{$key} = $json->{$key};
+	}
+
+	for my $test (keys %{$json->{'tests'}}) {
+		$test_names{$test} = 1;
+	}
+
+	for my $source (keys %{$json->{'files'}}) {
+		$files{$source} = 1;
+		next if is_file_excluded($source);
+
+		my $file_ref = \%{$json->{'files'}{$source}};
+
+		# Parse functions
+		for my $func (keys %{$file_ref->{func}}) {
+			next if is_function_excluded($func);
+
+			my $func_ref = \%{$file_ref->{func}{$func}};
+
+			# Store the record
+			for my $key (keys %$func_ref) {
+				if ($key eq "execution_count") {
+					$record{files}{$source}{func}{$func}{$key} += $func_ref->{$key};
+				} else {
+					$record{files}{$source}{func}{$func}{$key} = $func_ref->{$key};
+				}
+			}
+
+			$all_func{$func}{$source}->{ln} = $func_ref->{'start_line'};
+			$all_func{$func}{$source}->{end_ln} = $func_ref->{'end_line'};
+
+			if ($func_ref->{'execution_count'} > 0) {
+				$used_func{$func}{$source}->{count} += $func_ref->{'execution_count'};
+				$was_used = 1;
+			}
+		}
+		next if ($ignore_unused && !$was_used);
+		$used_source{$source} = 1;
+
+		# Parse lines and branches
+		for my $ln (keys %{$file_ref->{line}}) {
+			my $line_ref = \%{$file_ref->{line}{$ln}};
+			my $func = $line_ref->{'function_name'};
+			if (!$func) {
+				# Ignore DA/BRDA that aren't associated with
+				# functions. Those are present on header files
+				# (maybe defines?)
+				next if (@func_include_regexes);
+
+				# Otherwise place them in separate
+				$func = $before_sf;
+			} else {
+				next if is_function_excluded($func);
+			}
+
+			# Store the record
+			for my $key (keys %$line_ref) {
+				next if ($key eq "line_number");
+
+				# Branches will be handled in separate
+				next if ($key eq "branches");
+				if ($key eq "count") {
+					$record{files}{$source}{line}{$ln}{$key} += $line_ref->{$key};
+				} else {
+					$record{files}{$source}{line}{$ln}{$key} = $line_ref->{$key};
+				}
+			}
+			$all_line{$source}{$ln} += $line_ref->{'count'};
+
+			my $i = 0;
+			for my $branch_ref (@{$line_ref->{'branches'}}) {
+				my $taken = $branch_ref->{'count'};
+				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
+
+				# Negative gcov results are possible, as
+				# reported at:
+				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
+				# Lcov ignores those. So, let's do the same
+				# here.
+				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
+
+				for my $key (keys %$branch_ref) {
+					next if (!$record{files}{$source}{line}{$ln}{branches});
+					if ($key eq "count") {
+						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
+					} else {
+						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
+					}
+				}
+
+				$all_branch{$source}{$where}{count} += $taken;
+				$i++;
+			}
+			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
+				@{$record{files}{$source}{line}{$ln}{branches}} = ();
+			}
+		}
+	}
+}
+
 sub read_json($)
 {
 	my $file = shift;
@@ -238,6 +353,8 @@ sub read_json($)
 		}
 		if ($json->{'format_version'} eq '1') {
 			parse_json_gcov_v1($file, $json);
+		} elsif ($json->{'format_version'} eq 'parse_info v1.0') {
+			parse_json_internal_format_v1($file, $json);
 		} else {
 			if ($json->{'format_version'}) {
 				die "Can't parse JSON version %d on file $file\n", $json->{'format_version'};
@@ -331,7 +448,7 @@ sub read_info($)
 
 			$skip_func = 0;
 
-			$record{$source}{func}{$func}{start_line} = $ln;
+			$record{files}{$source}{func}{$func}{start_line} = $ln;
 			$all_func{$func}{$source}->{ln} = $ln;
 			next;
 		}
@@ -359,7 +476,7 @@ sub read_info($)
 			$skip_func = 0;
 			$was_used = 1;
 
-			$record{$source}{func}{$func}{fnda} += $count;
+			$record{files}{$source}{func}{$func}{execution_count} += $count;
 			$used_func{$func}{$source}->{count} += $count;
 			next;
 		}
@@ -428,9 +545,9 @@ sub read_info($)
 
 			$was_used = 1 if ($count > 0);
 
-			$record{$source}{line}{$ln}{count} += $count;
-			if (!defined($record{$source}{line}{$ln}{branches})) {
-				@{$record{$source}{line}{$ln}{branches}} = ();
+			$record{files}{$source}{line}{$ln}{count} += $count;
+			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
+				@{$record{files}{$source}{line}{$ln}{branches}} = ();
 			}
 
 			$all_line{$source}{$ln} += $count;
@@ -469,59 +586,74 @@ sub sort_where($$)
 	return $a[2] <=> $b[2];
 }
 
-sub write_filtered_file($)
+sub write_json_file($)
 {
-	my $filter = shift;
+	my $fname = shift;
 
-	my $filtered = "";
+	if (!$has_json_support) {
+		die "Can't parse json as system doesn't have Cpanel::JSON::XS.\n";
+	}
+
+	my $data = eval {
+		Cpanel::JSON::XS::encode_json(\%record)
+	};
+
+	open OUT, ">$fname" or die "Can't open $fname";
+	print OUT $data or die "Failed to write to $fname";
+	close OUT or die "Failed to close to $fname";
+}
+
+sub write_info_file($)
+{
+	my $fname = shift;
+	my $data = "";
 
 	if ($title eq "") {
 		foreach my $testname(sort keys %test_names) {
-			$filtered .= "TN:$testname\n";
+			$data .= "TN:$testname\n";
 		}
 	} else {
-		$filtered .= "TN:$title\n";
+		$data .= "TN:$title\n";
 	}
 
-	# Generates filtered data
-	foreach my $source(sort keys %record) {
+	# Fills $data with the contents to be stored at the file
+	foreach my $source(sort keys %{$record{files}}) {
 		next if (!$used_source{$source});
 
 		if ($source ne $before_sf) {
-			$filtered .= "SF:$source\n";
+			$data .= "SF:$source\n";
 		}
 
-		foreach my $func(sort keys %{ $record{$source}{func} }) {
+		foreach my $func(sort keys %{ $record{files}{$source}{func} }) {
 			if ($func ne $before_sf) {
 				my $fn;
 				my $fnda;
 
-				if (defined($record{$source}{func}{$func}{start_line})) {
-					$filtered .= "FN:" . $record{$source}{func}{$func}{start_line} . ",$func\n";
+				if (defined($record{files}{$source}{func}{$func}{start_line})) {
+					$data .= "FN:" . $record{files}{$source}{func}{$func}{start_line} . ",$func\n";
 				}
-				if (defined($record{$source}{func}{$func}{fnda})) {
-					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
+				if (defined($record{files}{$source}{func}{$func}{execution_count})) {
+					$data .= "FNDA:" . $record{files}{$source}{func}{$func}{execution_count} . ",$func\n";
 				}
 
 			}
 		}
 
-		foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{line} }) {
-			$filtered .= "DA:$ln," . $record{$source}{line}{$ln}{count} . "\n";
-
-			my $i = 0;
-			for ($i = 0, $i < scalar(@{$record{$source}{line}{$ln}{branches}}), $i++) {
-				my $taken = $record{$source}{line}{$ln}{branches}[$i]{count};
+		foreach my $ln(sort { $a <=> $b } keys %{ $record{files}{$source}{line} }) {
+			$data .= "DA:$ln," . $record{files}{$source}{line}{$ln}{count} . "\n";
+			next if (!$record{files}{$source}{line}{$ln}{branches});
+			for (my $i = 0; $i < scalar @{$record{files}{$source}{line}{$ln}{branches}}; $i++) {
+				my $taken = $record{files}{$source}{line}{$ln}{branches}[$i]{count};
 				$taken = "-" if (!$taken);
-				$filtered .= "BRDA:$ln,0,$i,$taken\n";
+				$data .= "BRDA:$ln,0,$i,$taken\n";
 			}
 		}
 
-		$filtered .= "end_of_record\n";
+		$data .= "end_of_record\n";
 	}
-	open OUT, ">$filter" or die "Can't open $filter";
-	print OUT $filtered or die "Failed to write to $filter";
-	close OUT or die "Failed to close to $filter";
+	open OUT, ">$fname" or die "Can't open $fname";
+	print OUT $data or die "Failed to write to $fname";
+	close OUT or die "Failed to close to $fname";
 }
 
 sub print_code_coverage($$$)
@@ -1120,7 +1252,7 @@ sub check_source_branches()
 my $print_used;
 my $print_unused;
 my $stat;
-my $filter;
+my $output_file;
 my $help;
 my $man;
 my $func_filters;
@@ -1136,7 +1268,7 @@ GetOptions(
 	"print-coverage|print_coverage|print|p" => \$print_used,
 	"print-unused|u" => \$print_unused,
 	"stat|statistics" => \$stat,
-	"output|o=s" => \$filter,
+	"output|o=s" => \$output_file,
 	"verbose|v" => \$verbose,
 	"ignore-unused|ignore_unused" => \$ignore_unused,
 	"only-i915|only_i915" => \$only_i915,
@@ -1169,9 +1301,9 @@ if ($#ARGV < 0) {
 }
 
 # At least one action should be specified
-pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report && !$check_branches);
+pod2usage(1) if (!$print_used && !$output_file && !$stat && !$print_unused && !$gen_report && !$check_branches);
 
-pod2usage(1) if ($gen_report && ($print_used || $filter || $stat || $print_unused));
+pod2usage(1) if ($gen_report && ($print_used || $output_file || $stat || $print_unused));
 
 my $filter_str = "";
 my $has_filter;
@@ -1302,8 +1434,12 @@ if ($show_files) {
 	}
 }
 
-if ($filter) {
-	write_filtered_file($filter);
+if ($output_file) {
+	if ($output_file =~ /.json$/) {
+		write_json_file($output_file);
+	} else {
+		write_info_file($output_file);
+	}
 }
 
 __END__
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (10 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:18   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Code coverage files are big. Add support for read/write gzipped
files, in order to save I/O, and, depending on the disk, speeding
up the tool.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 56 ++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 16 deletions(-)
 mode change 100644 => 100755 scripts/code_cov_parse_info

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
old mode 100644
new mode 100755
index 8180b9c1f82e..50d10a30d7f9
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -6,6 +6,8 @@ use Getopt::Long;
 BEGIN { $Pod::Usage::Formatter = 'Pod::Text::Termcap'; }
 use Pod::Usage;
 use Pod::Man;
+use IO::File;
+use IO::Zlib;
 
 my $prefix = qr ".*?(linux)\w*/";
 
@@ -341,8 +343,14 @@ sub read_json($)
 	}
 
 	# Read JSON data
-	open IN, $file or die "can't open $file";
-	while (<IN>) {
+	my $fh;
+	if ($file =~ m/\.gz$/) {
+		$fh = IO::Zlib->new($file, "rb") or die "can't open $file";
+	} else {
+		$fh = IO::File->new($file) or die "can't open $file";
+	}
+	print "reading $file...\n" if ($verbose);
+	while (<$fh>) {
 		my $json = eval {
 			Cpanel::JSON::XS::decode_json($_)
 		};
@@ -362,7 +370,7 @@ sub read_json($)
 			die "Unknown JSON format on file $file\n";
 		}
 	}
-	close IN;
+	$fh->close() or die "Failed to close file $file";
 }
 
 sub read_info($)
@@ -377,11 +385,16 @@ sub read_info($)
 
 	# First step: parse data
 
-	print "reading $file...\n" if ($verbose);
-	open IN, $file or die "can't open $file";
 	# For details on .info file format, see "man geninfo"
 	# http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php
-	while (<IN>) {
+	my $fh;
+	if ($file =~ m/\.gz$/) {
+		$fh = IO::Zlib->new($file, "rb") or die "can't open $file";
+	} else {
+		$fh = IO::File->new($file) or die "can't open $file";
+	}
+	print "reading $file...\n" if ($verbose);
+	while (<$fh>) {
 		# TN:<test name>
 		if (m/^TN:(.*)/) {
 			if ($1 ne $cur_test) {
@@ -568,7 +581,7 @@ sub read_info($)
 		printf("Warning: invalid line: $_");
 	}
 
-	close IN or die;
+	$fh->close() or die "Failed to close file $file";
 }
 
 sub sort_where($$)
@@ -598,9 +611,15 @@ sub write_json_file($)
 		Cpanel::JSON::XS::encode_json(\%record)
 	};
 
-	open OUT, ">$fname" or die "Can't open $fname";
-	print OUT $data or die "Failed to write to $fname";
-	close OUT or die "Failed to close to $fname";
+	if ($fname =~ m/\.gz$/) {
+		my $fh = IO::Zlib->new($fname, "wb") or die "can't open $fname";
+		print $fh $data or die "Failed to write to $fname";
+		$fh->close or die "Failed to write to $fname";
+	} else {
+		open OUT, ">$fname" or die "Can't open $fname";
+		print OUT $data or die "Failed to write to $fname";
+		close OUT or die "Failed to close to $fname";
+	}
 }
 
 sub write_info_file($)
@@ -648,12 +667,17 @@ sub write_info_file($)
 				$data .= "BRDA:$ln,0,$i,$taken\n";
 			}
 		}
-
 		$data .= "end_of_record\n";
 	}
-	open OUT, ">$fname" or die "Can't open $fname";
-	print OUT $data or die "Failed to write to $fname";
-	close OUT or die "Failed to close to $fname";
+	if ($fname =~ m/\.gz$/) {
+		my $fh = IO::Zlib->new($fname, "wb") or die "can't open $fname";
+		print $fh $data or die "Failed to write to $fname";
+		$fh->close or die "Failed to write to $fname";
+	} else {
+		open OUT, ">$fname" or die "Can't open $fname";
+		print OUT $data or die "Failed to write to $fname";
+		close OUT or die "Failed to close to $fname";
+	}
 }
 
 sub print_code_coverage($$$)
@@ -1354,7 +1378,7 @@ if ($ignore_unused) {
 }
 
 foreach my $f (@ARGV) {
-	if ($f =~ /.json$/) {
+	if ($f =~ /\.json(\.gz)?$/) {
 		read_json($f);
 	} else {
 		read_info($f);
@@ -1435,7 +1459,7 @@ if ($show_files) {
 }
 
 if ($output_file) {
-	if ($output_file =~ /.json$/) {
+	if ($output_file =~ /.json(.gz)?$/) {
 		write_json_file($output_file);
 	} else {
 		write_info_file($output_file);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (11 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:19   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The lcov's info format has a field to be used for report branch
block. This is unused on json format (newer gcc), being always
zero. JSON doesn't contain it, so the parser doesn't support
it to be non-zero anymore. Print a warning if this happens.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 50d10a30d7f9..ea4f64b34d2f 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -519,6 +519,10 @@ sub read_info($)
 			my $branch = $3;
 			my $taken = $4;
 
+			if ($block != 0) {
+				print "Warning: unexpected block $block at line $.\n";
+			}
+
 			my $where = "$ln,$block,$branch";
 
 			$taken = 0 if ($taken eq '-');
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (12 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:21   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Add an option to filter out branches that aren't associated with
any functions inside the file (e. g. they came from #inline
directives).

While here, also address some issues at the branch report.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 71 ++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 20 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index ea4f64b34d2f..57bc4c2f0b4b 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -25,6 +25,8 @@ my @func_exclude_regexes;
 my %test_names;
 my @src_include_regexes;
 my @src_exclude_regexes;
+my $can_filter_lines = 1;
+my $ignore_lines_without_functions = 1;
 
 my $verbose = 0;
 my $ignore_unused = 0;
@@ -161,12 +163,6 @@ sub parse_json_gcov_v1($$)
 
 			my $func = $line_ref->{'function_name'};
 			if (!$func) {
-				# Ignore DA/BRDA that aren't associated with
-				# functions. Those are present on header files
-				# (maybe defines?)
-				next if (@func_include_regexes);
-
-				# Otherwise place them in separate
 				$func = $before_sf;
 			} else {
 				next if is_function_excluded($func);
@@ -188,9 +184,17 @@ sub parse_json_gcov_v1($$)
 
 			my $i = 0;
 			for my $branch_ref (@{$line_ref->{'branches'}}) {
-				my $taken = $branch_ref->{'count'};
 				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
 
+				if ($func eq $before_sf) {
+					# Ignore DA/BRDA that aren't associated with
+					# functions. Those are present on header files
+					# (maybe defines?)
+					next if ($ignore_lines_without_functions);
+				} else {
+					$all_branch{$source}{$where}{func} = $func;
+				}
+
 				# Negative gcov results are possible, as
 				# reported at:
 				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
@@ -206,7 +210,8 @@ sub parse_json_gcov_v1($$)
 					}
 				}
 
-				$all_branch{$source}{$where}{count} += $taken;
+				$all_branch{$source}{$where}{count} += $branch_ref->{'count'};
+
 				$i++;
 			}
 			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
@@ -383,6 +388,10 @@ sub read_info($)
 	my $func = $before_sf;
 	my $cur_test = "";
 
+	# Info files don't contain functions for lines. So, they can't
+	# be used to filter lines and branches used inside functions.
+	$can_filter_lines = 0;
+
 	# First step: parse data
 
 	# For details on .info file format, see "man geninfo"
@@ -1231,18 +1240,15 @@ sub check_source_branches()
 				return;
 			}
 
+			my $func_ln;
 			my $func = $all_branch{$source}{$where}{func};
-			my $func_ln = $all_branch{$source}{$where}{func_ln};
-
-			print "Branch $source:$ln, ";
 			if ($func) {
-				if ($func_ln) {
-					print "func: $func, ";
-				} else {
-					print "func: $func:$func_ln, ";
-				}
+				$func_ln = $all_func{$func}{$source}->{ln};
 			}
-			print "block $block, branch $branch not taken:\n";
+
+			print "Branch $branch on $source:$ln";
+			print ", function: $func" if ($func);
+			print " not taken:\n";
 
 			if ($func_ln) {
 				$ctx_lines = $ln - $func_ln;
@@ -1250,14 +1256,14 @@ sub check_source_branches()
 				$ctx_lines = 10;
 			}
 
-
 			# Search for up to $ctx_lines before the occurrence
 			my $context = "";
 			my $has_if;
 			$ln-- if ($ln > 0);
 			my $delim = "->";
 			for (my $if_ln = $ln; $if_ln >= 0 && (($ln - $if_ln) < $ctx_lines); $if_ln--) {
-				$context = "$if_ln$delim\t$lines[$if_ln]" . $context;
+				my $cur_ln = $if_ln + 1;
+				$context = "$cur_ln$delim\t$lines[$if_ln]" . $context;
 				$delim = "";
 				if ($lines[$if_ln] =~ m/(\bif\b|#if)/) {
 					$has_if = 1;
@@ -1267,7 +1273,8 @@ sub check_source_branches()
 			if ($has_if) {
 				print $context;
 			} else {
-				print "$ln->\t$lines[$ln]";
+				my $cur_ln = $ln + 1;
+				print "$cur_ln->\t$lines[$ln]";
 			}
 		}
 	}
@@ -1308,6 +1315,7 @@ GetOptions(
 	"source-filters|S=s" => \$src_filters,
 	"include-source=s" => \@src_include_regexes,
 	"exclude-source=s" => \@src_exclude_regexes,
+	"ignore-lines-without-functions!" => \$ignore_lines_without_functions,
 	"show-files|show_files" => \$show_files,
 	"show-lines|show_lines" => \$show_lines,
 	"report|r=s" => \$gen_report,
@@ -1438,6 +1446,10 @@ print_summary() if ($stat);
 if ($has_filter) {
 	my $percent = 100. * $stats{"used_files"} / $stats{"all_files"};
 
+	if (!$can_filter_lines) {
+		printf "Warning......: Function filters won't work with lines/branches\n";
+	}
+
 	printf "Filters......:%s.\n", $filter_str;
 	printf "Source files.: %.2f%% (%d of %d total)",
 		$percent, $stats{"used_files"}, $stats{"all_files"};
@@ -1696,6 +1708,25 @@ report and decrease the code coverage statistics.
 
 This option is automaticaly enabled when B<--func-filters> is used.
 
+=item B<--ignore-lines-without-functions>
+
+This option works only when the input file is in JSON format.
+
+Basically, include files may contain several lines of codes that aren't
+assigned to any functions inside a source file. This is a common behavior
+for macros and inlined functions inside headers.
+
+When this option is selected, the branches stat won't contain any such code.
+
+Please notice that this is enabled by default.
+
+Use B<--no-ignore-lines-without-functions> to disable it.
+
+=item B<--no-ignore-lines-without-functions>
+
+Disables filtering out branches that are not associated with any functions
+inside the source file, but were imported via includes.
+
 =back
 
 =item B<--show-files> or B<--show_files>
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (13 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 18:41   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Add support for passing regexes to be used to filter branches.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 208 +++++++++++++++++++++++++++++++++---
 1 file changed, 192 insertions(+), 16 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 57bc4c2f0b4b..3ce26e545aee 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -22,11 +22,16 @@ my %record;
 my %files;
 my @func_include_regexes;
 my @func_exclude_regexes;
+my @branch_include_regexes;
+my @branch_exclude_regexes;
 my %test_names;
 my @src_include_regexes;
 my @src_exclude_regexes;
+my $source_dir = ".";
 my $can_filter_lines = 1;
 my $ignore_lines_without_functions = 1;
+my $ignore_branches_on_headers = 1;
+my $has_branch_filter;
 
 my $verbose = 0;
 my $ignore_unused = 0;
@@ -87,6 +92,32 @@ sub is_file_excluded($)
 	return 1;
 }
 
+sub is_branch_excluded($)
+{
+	return 0 if (!@branch_include_regexes && !@branch_exclude_regexes);
+
+	my $branch = shift;
+
+	# Handle includes first, as, when there are both include and exclude
+	# includes should take preference, as they can be overriding exclude
+	# rules
+	foreach my $r (@branch_include_regexes) {
+	    return 0 if ($branch =~ m/$r/);
+	}
+
+	foreach my $r (@branch_exclude_regexes) {
+		return 1 if ($branch =~ m/$r/);
+	}
+
+	# If there are no exclude regexes, only include branches that are
+	# explicitly included.
+	if ($#branch_exclude_regexes == 0) {
+		return 1;
+	}
+
+	return 0;
+}
+
 # Use something that comes before any real function
 my $before_sf = "!!!!";
 
@@ -115,6 +146,7 @@ sub parse_json_gcov_v1($$)
 	# Store test name at the record
 	$record{tests}{$cur_test} = 1;
 
+	my %cached;
 	for my $file_ref (@{$json->{'files'}}) {
 		my $source = $file_ref->{'file'};
 
@@ -182,16 +214,40 @@ sub parse_json_gcov_v1($$)
 			}
 			$all_line{$source}{$ln} += $line_ref->{'count'};
 
+			if ($ignore_branches_on_headers) {
+				next if ($source =~ m/.h$/);
+			}
+
 			my $i = 0;
 			for my $branch_ref (@{$line_ref->{'branches'}}) {
 				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
 
+				# Filter out branches
+				if ($has_branch_filter) {
+					if (!$cached{$source}) {
+						open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
+						push @{$cached{$source}}, <IN>;
+						close IN;
+					}
+					my $nlines = scalar(@{$cached{$source}});
+					if ($ln > $nlines) {
+						die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
+						return;
+					}
+					next if (is_branch_excluded($cached{$source}[$ln - 1]));
+				}
+
 				if ($func eq $before_sf) {
 					# Ignore DA/BRDA that aren't associated with
 					# functions. Those are present on header files
 					# (maybe defines?)
 					next if ($ignore_lines_without_functions);
+
+					# Otherwise place them in separate
+					$func = $before_sf;
 				} else {
+					next if is_function_excluded($func);
+
 					$all_branch{$source}{$where}{func} = $func;
 				}
 
@@ -232,6 +288,7 @@ sub parse_json_internal_format_v1($$)
 	my $was_used = 0;
 	my $has_func = 0;
 	my $ignore = 0;
+	my %cached;
 
 	# Store the common JSON data into the record
 	for my $key (keys %$json) {
@@ -308,17 +365,30 @@ sub parse_json_internal_format_v1($$)
 			}
 			$all_line{$source}{$ln} += $line_ref->{'count'};
 
+			if ($ignore_branches_on_headers) {
+				next if ($source =~ m/.h$/);
+			}
+
 			my $i = 0;
 			for my $branch_ref (@{$line_ref->{'branches'}}) {
 				my $taken = $branch_ref->{'count'};
 				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
 
-				# Negative gcov results are possible, as
-				# reported at:
-				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
-				# Lcov ignores those. So, let's do the same
-				# here.
-				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
+
+				# Filter out branches
+				if ($has_branch_filter) {
+					if (!$cached{$source}) {
+						open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
+						push @{$cached{$source}}, <IN>;
+						close IN;
+					}
+					my $nlines = scalar(@{$cached{$source}});
+					if ($ln > $nlines) {
+						die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
+						return;
+					}
+					next if (is_branch_excluded($cached{$source}[$ln - 1]));
+				}
 
 				for my $key (keys %$branch_ref) {
 					next if (!$record{files}{$source}{line}{$ln}{branches});
@@ -387,6 +457,7 @@ sub read_info($)
 	my $source = $before_sf;
 	my $func = $before_sf;
 	my $cur_test = "";
+	my %cached;
 
 	# Info files don't contain functions for lines. So, they can't
 	# be used to filter lines and branches used inside functions.
@@ -528,6 +599,25 @@ sub read_info($)
 			my $branch = $3;
 			my $taken = $4;
 
+			if ($ignore_branches_on_headers) {
+				next if ($source =~ m/.h$/);
+			}
+
+			# Filter out branches
+			if ($has_branch_filter) {
+				if (!$cached{$source}) {
+					open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
+					push @{$cached{$source}}, <IN>;
+					close IN;
+				}
+				my $nlines = scalar(@{$cached{$source}});
+				if ($ln > $nlines) {
+					die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
+					return;
+				}
+				next if (is_branch_excluded($cached{$source}[$ln - 1]));
+			}
+
 			if ($block != 0) {
 				print "Warning: unexpected block $block at line $.\n";
 			}
@@ -1217,7 +1307,10 @@ sub check_source_branches()
 		next if (!$used_source{$source});
 		next if (is_file_excluded($source));
 
-		my @lines;
+		open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't check branches\n";
+		my @lines = <IN>;
+		close IN;
+
 		foreach my $where (sort keys %{$all_branch{$source}}) {
 			my $taken = $all_branch{$source}{$where}{count};
 			next if ($taken > 0);
@@ -1229,13 +1322,7 @@ sub check_source_branches()
 			$block = $2;
 			$branch = $3;
 
-			if (!@lines) {
-				open IN, "$source" || die "File $source not found. Can't check branches\n";
-				@lines = <IN>;
-				close IN;
-			}
-
-			if ($ln >= $#lines) {
+			if ($ln > $#lines) {
 				die "$source:$ln line is bigger than the number of lines at the file ($#lines lines)\n";
 				return;
 			}
@@ -1292,6 +1379,7 @@ my $help;
 my $man;
 my $func_filters;
 my $src_filters;
+my $branch_filters;
 my $show_files;
 my $show_lines;
 my $only_i915;
@@ -1304,6 +1392,7 @@ GetOptions(
 	"print-unused|u" => \$print_unused,
 	"stat|statistics" => \$stat,
 	"output|o=s" => \$output_file,
+	"source-dir|source_dir=s" => \$source_dir,
 	"verbose|v" => \$verbose,
 	"ignore-unused|ignore_unused" => \$ignore_unused,
 	"only-i915|only_i915" => \$only_i915,
@@ -1312,14 +1401,18 @@ GetOptions(
 	"func-filters|f=s" => \$func_filters,
 	"include-func=s" => \@func_include_regexes,
 	"exclude-func=s" => \@func_exclude_regexes,
+	"branch-filters|f=s" => \$branch_filters,
+	"include-branch=s" => \@branch_include_regexes,
+	"exclude-branch=s" => \@branch_exclude_regexes,
 	"source-filters|S=s" => \$src_filters,
 	"include-source=s" => \@src_include_regexes,
 	"exclude-source=s" => \@src_exclude_regexes,
 	"ignore-lines-without-functions!" => \$ignore_lines_without_functions,
+	"ignore-branches-on-headers!" => \$ignore_branches_on_headers,
 	"show-files|show_files" => \$show_files,
 	"show-lines|show_lines" => \$show_lines,
 	"report|r=s" => \$gen_report,
-	"check-branches" => \$check_branches,
+	"check-branches|check_branches" => \$check_branches,
 	"css-file|css|c=s" => \$css_file,
 	"title|t=s" => \$title,
 	"html-prolog|prolog=s" => \$html_prolog,
@@ -1381,6 +1474,14 @@ if ($str) {
 	$has_filter = 1;
 }
 
+$str = open_filter_file($branch_filters, \@branch_include_regexes, \@branch_exclude_regexes);
+if ($str) {
+	$filter_str .= "," if ($filter_str ne "");
+	$filter_str .= " branch regex ($str)";
+	$has_filter = 1;
+	$has_branch_filter = 1;
+}
+
 $ignore_unused = 1 if (@func_include_regexes || @func_exclude_regexes);
 
 if ($ignore_unused) {
@@ -1567,6 +1668,12 @@ Produce an output file merging all input files.
 
 The generated output file is affected by the applied filters.
 
+=item B<--source-dir> or B<--source_dir>
+
+Sets the source directory baseline. This is used together with other
+options that require to parse the source files (currently, only
+B<--check-branches).
+
 =item B<--only-drm> or B<--only_drm>
 
 Filters out includes outside the DRM subsystem, plus trace files.
@@ -1656,6 +1763,51 @@ Include B<regex> to the function filter. Can be used multiple times.
 Please notice that, when this filter is used, B<--ignore-unused> will be
 automaticaly enabled, as the final goal is to report per-function usage.
 
+=item B<--branch-filters>  B<[filter's file]> or B<-f>  B<[filter's file]>
+
+Use a file containing regular expressions (regex) to filter branches.
+
+Each line at B<[filter's file]> may contain a new regex:
+
+=over 4
+
+- Blank lines and lines starting with B<#> will be ignored;
+
+- Each line of the file will be handled as a new regex;
+
+- If B<+regex> is used, the filter will include B<regex> to the matches;
+
+- If B<-regex> is used, the filter will exclude B<regex> from the matches;
+
+- If the line doesn't start with neither B<+> nor B<->, containing just
+  B<regex>, the filter will include B<regex> to the matches.
+
+- Any whitespace/tab before or after B<regex> will be ignored.
+
+=back
+
+Include regexes are handled first, as they can override an exclude regex.
+
+When just include regexes are used, any branches that don't match the
+include regular expressions from the B<[filter's file]> will be ignored.
+
+=item B<--include-branch> B<regex>
+
+Include B<regex> to the branch filter. Can be used multiple times.
+
+When used together with B<--branch-filters> or B<--exclude-branch>, regexes
+here are handled first.
+
+Please notice that, when this filter is used, B<--ignore-unused> will be
+automaticaly enabled, as the final goal is to report per-branch usage.
+
+=item B<--exclude-branch> B<regex>
+
+Include B<regex> to the branchtion filter. Can be used multiple times.
+
+Please notice that, when this filter is used, B<--ignore-unused> will be
+automaticaly enabled, as the final goal is to report per-branch usage.
+
 =item B<--source-filters>  B<[filter's file]> or B<-S>  B<[filter's file]>
 
 Use a file containing regular expressions to filter source files.
@@ -1727,6 +1879,30 @@ Use B<--no-ignore-lines-without-functions> to disable it.
 Disables filtering out branches that are not associated with any functions
 inside the source file, but were imported via includes.
 
+See B<--ignore-lines-without-functions> for more details.
+
+=item B<--ignore-branches-on-headers>
+
+Branches on header files are really tricky to parse, as they depend
+on how gcc optimizes the output code. That's specially hard to use on
+Linux Kernel, as there are lots of complex macros that can be optimized
+on different ways. There are even some cases where the same macro sometimes
+have zero branches, while on other cases it can contain dozen ones.
+
+When this option is selected, all branches inside header files will be
+ignored.
+
+Please notice that this is enabled by default.
+
+Use B<--no-ignore-branches-on-headers> to disable this filter, preserving
+data from all branches.
+
+=item B<--no-ignore-branches-on-headers>
+
+Disables filtering out branches that are inside header files.
+
+See B<--ignore-branches-on-headers> for more details.
+
 =back
 
 =item B<--show-files> or B<--show_files>
@@ -1734,7 +1910,7 @@ inside the source file, but were imported via includes.
 Shows the list of files that were used to produce the code coverage
 results.
 
-=item B<--check-branches>
+=item B<--check-branches> or B<--check_branches>
 
 Checks at the Linux Kernel source files what's the contents of the
 branches that weren't taken. The directory should match what's
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (14 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:26   ` Kamil Konieczny
  2024-02-15 10:27 ` [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

With fields rename at %record, the number of filtered files
is not reflecting what it was actually there. Also, the was_used
logic for json format was not parsing the same way as it used to
be with info format.

Fix those.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 3ce26e545aee..9d043a987f44 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -126,7 +126,6 @@ sub parse_json_gcov_v1($$)
 	my $file = shift;
 	my $json = shift;
 
-	my $was_used = 0;
 	my $has_func = 0;
 	my $ignore = 0;
 
@@ -149,6 +148,7 @@ sub parse_json_gcov_v1($$)
 	my %cached;
 	for my $file_ref (@{$json->{'files'}}) {
 		my $source = $file_ref->{'file'};
+		my $was_used = 0;
 
 		$files{$source} = 1;
 		next if is_file_excluded($source);
@@ -181,8 +181,6 @@ sub parse_json_gcov_v1($$)
 				$was_used = 1;
 			}
 		}
-		next if ($ignore_unused && !$was_used);
-		$used_source{$source} = 1;
 
 		# Parse lines and branches
 		for my $line_ref (@{$file_ref->{'lines'}}) {
@@ -267,6 +265,7 @@ sub parse_json_gcov_v1($$)
 				}
 
 				$all_branch{$source}{$where}{count} += $branch_ref->{'count'};
+				$was_used = 1 if ($branch_ref->{'count'} > 0);
 
 				$i++;
 			}
@@ -274,6 +273,8 @@ sub parse_json_gcov_v1($$)
 				@{$record{files}{$source}{line}{$ln}{branches}} = ();
 			}
 		}
+		next if ($ignore_unused && !$was_used);
+		$used_source{$source} = 1;
 	}
 
 	# As the record was changed, we need to use a different format name
@@ -285,7 +286,6 @@ sub parse_json_internal_format_v1($$)
 	my $file = shift;
 	my $json = shift;
 
-	my $was_used = 0;
 	my $has_func = 0;
 	my $ignore = 0;
 	my %cached;
@@ -305,6 +305,8 @@ sub parse_json_internal_format_v1($$)
 
 	for my $source (keys %{$json->{'files'}}) {
 		$files{$source} = 1;
+		my $was_used = 0;
+
 		next if is_file_excluded($source);
 
 		my $file_ref = \%{$json->{'files'}{$source}};
@@ -332,8 +334,6 @@ sub parse_json_internal_format_v1($$)
 				$was_used = 1;
 			}
 		}
-		next if ($ignore_unused && !$was_used);
-		$used_source{$source} = 1;
 
 		# Parse lines and branches
 		for my $ln (keys %{$file_ref->{line}}) {
@@ -400,12 +400,15 @@ sub parse_json_internal_format_v1($$)
 				}
 
 				$all_branch{$source}{$where}{count} += $taken;
+				$was_used = 1 if ($taken > 0);
 				$i++;
 			}
 			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
 				@{$record{files}{$source}{line}{$ln}{branches}} = ();
 			}
 		}
+		next if ($ignore_unused && !$was_used);
+		$used_source{$source} = 1;
 	}
 }
 
@@ -876,7 +879,7 @@ sub gen_stats()
 
 	# per-file coverage stats
 	$stats{"all_files"} = scalar keys(%files);
-	$stats{"filtered_files"} = scalar keys(%record);
+	$stats{"filtered_files"} = scalar keys(%{$record{files}});
 	$stats{"used_files"} = scalar keys(%used_source);
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (15 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics Mauro Carvalho Chehab
@ 2024-02-15 10:27 ` Mauro Carvalho Chehab
  2024-02-15 17:24   ` Kamil Konieczny
  2024-02-15 10:59 ` ✓ CI.xeBAT: success for Improve code_cov_parse_info tool Patchwork
  2024-02-15 11:23 ` ✓ Fi.CI.BAT: " Patchwork
  18 siblings, 1 reply; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 10:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Not sure why, but opening a non-exisiting file is not returning
an error. Well, let's add another check to cover troubles, by
identifying if no lines were filled at the cache array.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/code_cov_parse_info | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
index 9d043a987f44..d133ef1a18d4 100755
--- a/scripts/code_cov_parse_info
+++ b/scripts/code_cov_parse_info
@@ -614,6 +614,7 @@ sub read_info($)
 					close IN;
 				}
 				my $nlines = scalar(@{$cached{$source}});
+				die "File $source_dir/$source not found or it is empty. Can't filter branches\n" if (!$nlines);
 				if ($ln > $nlines) {
 					die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
 					return;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* ✓ CI.xeBAT: success for Improve code_cov_parse_info tool
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (16 preceding siblings ...)
  2024-02-15 10:27 ` [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists Mauro Carvalho Chehab
@ 2024-02-15 10:59 ` Patchwork
  2024-02-15 11:23 ` ✓ Fi.CI.BAT: " Patchwork
  18 siblings, 0 replies; 38+ messages in thread
From: Patchwork @ 2024-02-15 10:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 7649 bytes --]

== Series Details ==

Series: Improve code_cov_parse_info tool
URL   : https://patchwork.freedesktop.org/series/129937/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7714_BAT -> XEIGTPW_10677_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in XEIGTPW_10677_BAT that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - bat-dg2-oem2:       [SKIP][1] ([Intel XE#1136]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@core_hotunplug@unbind-rebind.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_addfb_basic@bad-pitch-128:
    - bat-dg2-oem2:       [SKIP][3] ([i915#2575]) -> [PASS][4] +49 other tests pass
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_addfb_basic@bad-pitch-128.html
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_addfb_basic@bad-pitch-128.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-dg2-oem2:       [SKIP][5] ([Intel XE#1134]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_frontbuffer_tracking@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_frontbuffer_tracking@basic.html

  * igt@xe_intel_bb@create-in-region:
    - bat-dg2-oem2:       [SKIP][7] ([Intel XE#1130]) -> [PASS][8] +154 other tests pass
   [7]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_intel_bb@create-in-region.html
   [8]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_intel_bb@create-in-region.html

  * {igt@xe_live_ktest@xe_bo@xe_ccs_migrate_kunit}:
    - bat-dg2-oem2:       [SKIP][9] ([Intel XE#1245]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_live_ktest@xe_bo@xe_ccs_migrate_kunit.html
   [10]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_live_ktest@xe_bo@xe_ccs_migrate_kunit.html

  * {igt@xe_live_ktest@xe_migrate}:
    - bat-dg2-oem2:       [SKIP][11] ([Intel XE#455]) -> [PASS][12] +5 other tests pass
   [11]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_live_ktest@xe_migrate.html
   [12]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_live_ktest@xe_migrate.html

  * igt@xe_module_load@load:
    - bat-dg2-oem2:       [FAIL][13] ([Intel XE#1244]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_module_load@load.html

  
#### Warnings ####

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-dg2-oem2:       [SKIP][15] ([i915#2575]) -> [SKIP][16] ([Intel XE#623])
   [15]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html
   [16]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-dg2-oem2:       [SKIP][17] ([Intel XE#1134]) -> [SKIP][18] ([Intel XE#455])
   [17]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_dsc@dsc-basic.html
   [18]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-dg2-oem2:       [SKIP][19] ([i915#2575]) -> [SKIP][20] ([i915#5274])
   [19]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_force_connector_basic@prune-stale-modes.html
   [20]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_hdmi_inject@inject-audio:
    - bat-dg2-oem2:       [SKIP][21] ([i915#2575]) -> [SKIP][22] ([Intel XE#417])
   [21]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@kms_hdmi_inject@inject-audio.html
   [22]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@kms_hdmi_inject@inject-audio.html

  * igt@xe_exec_fault_mode@twice-bindexecqueue-userptr-invalidate:
    - bat-dg2-oem2:       [SKIP][23] ([Intel XE#1130]) -> [SKIP][24] ([Intel XE#288]) +32 other tests skip
   [23]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_exec_fault_mode@twice-bindexecqueue-userptr-invalidate.html
   [24]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_exec_fault_mode@twice-bindexecqueue-userptr-invalidate.html

  * igt@xe_huc_copy@huc_copy:
    - bat-dg2-oem2:       [SKIP][25] ([Intel XE#1130]) -> [SKIP][26] ([Intel XE#255])
   [25]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_huc_copy@huc_copy.html
   [26]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_huc_copy@huc_copy.html

  * igt@xe_pat@pat-index-xe2:
    - bat-dg2-oem2:       [SKIP][27] ([Intel XE#1130]) -> [SKIP][28] ([Intel XE#977])
   [27]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_pat@pat-index-xe2.html
   [28]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_pat@pat-index-xe2.html

  * igt@xe_pat@pat-index-xehpc:
    - bat-dg2-oem2:       [SKIP][29] ([Intel XE#1130]) -> [SKIP][30] ([Intel XE#979]) +1 other test skip
   [29]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7714/bat-dg2-oem2/igt@xe_pat@pat-index-xehpc.html
   [30]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/bat-dg2-oem2/igt@xe_pat@pat-index-xehpc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [Intel XE#1130]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1130
  [Intel XE#1134]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1134
  [Intel XE#1136]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1136
  [Intel XE#1244]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1244
  [Intel XE#1245]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1245
  [Intel XE#255]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/255
  [Intel XE#288]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/288
  [Intel XE#417]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/417
  [Intel XE#455]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/455
  [Intel XE#623]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/623
  [Intel XE#929]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/929
  [Intel XE#977]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/977
  [Intel XE#979]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/979
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274


Build changes
-------------

  * IGT: IGT_7714 -> IGTPW_10677
  * Linux: xe-783-16b37f41eb6fd6e98de452231506d68635fdf0c5 -> xe-784-0b36e01cb22c6d9d541f379253b4b2ab0c805646

  IGTPW_10677: 10677
  IGT_7714: 7714
  xe-783-16b37f41eb6fd6e98de452231506d68635fdf0c5: 16b37f41eb6fd6e98de452231506d68635fdf0c5
  xe-784-0b36e01cb22c6d9d541f379253b4b2ab0c805646: 0b36e01cb22c6d9d541f379253b4b2ab0c805646

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10677/index.html

[-- Attachment #2: Type: text/html, Size: 9114 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* ✓ Fi.CI.BAT: success for Improve code_cov_parse_info tool
  2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
                   ` (17 preceding siblings ...)
  2024-02-15 10:59 ` ✓ CI.xeBAT: success for Improve code_cov_parse_info tool Patchwork
@ 2024-02-15 11:23 ` Patchwork
  18 siblings, 0 replies; 38+ messages in thread
From: Patchwork @ 2024-02-15 11:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 3106 bytes --]

== Series Details ==

Series: Improve code_cov_parse_info tool
URL   : https://patchwork.freedesktop.org/series/129937/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_14277 -> IGTPW_10677
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/index.html

Participating hosts (35 -> 32)
------------------------------

  Missing    (3): fi-glk-j4005 fi-apl-guc fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_10677:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_fence@basic-busy@vcs1:
    - {bat-arls-1}:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14277/bat-arls-1/igt@gem_exec_fence@basic-busy@vcs1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/bat-arls-1/igt@gem_exec_fence@basic-busy@vcs1.html

  
Known issues
------------

  Here are the changes found in IGTPW_10677 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/fi-cfl-8109u/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/fi-cfl-8109u/igt@gem_lmem_swapping@verify-random.html

  * igt@kms_pm_backlight@basic-brightness:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][5] ([fdo#109271]) +7 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/fi-cfl-8109u/igt@kms_pm_backlight@basic-brightness.html

  
#### Possible fixes ####

  * igt@gem_exec_create@basic@smem:
    - {bat-arls-1}:       [DMESG-WARN][6] ([i915#10194]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14277/bat-arls-1/igt@gem_exec_create@basic@smem.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/bat-arls-1/igt@gem_exec_create@basic@smem.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#10194]: https://gitlab.freedesktop.org/drm/intel/issues/10194
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7714 -> IGTPW_10677

  CI-20190529: 20190529
  CI_DRM_14277: 0b36e01cb22c6d9d541f379253b4b2ab0c805646 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10677: 10677
  IGT_7714: 7714

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10677/index.html

[-- Attachment #2: Type: text/html, Size: 3950 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default
  2024-02-15 10:27 ` [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default Mauro Carvalho Chehab
@ 2024-02-15 16:24   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:24 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:10 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Those aren't really needed on normal output.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 72a1e8c2b0ad..1498bd09cdba 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -1030,7 +1030,7 @@ if ($has_filter) {
>  }
>  
>  my $ntests=scalar(%test_names);
> -printf "Number of tests: %d\n", $ntests if ($ntests > 1);
> +printf "Number of tests: %d\n", $ntests if ($stat && $ntests > 1);
>  
>  if ($show_files) {
>  	for my $f(sort keys %used_source) {
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent
  2024-02-15 10:27 ` [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent Mauro Carvalho Chehab
@ 2024-02-15 16:27   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:11 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Let the regex array to be clearer about include regexes, and
> coherent with exclude ones.
> 
> While here, also rename the file exclude check function, to
> have a name closer to its functions counterpart.
> 
> No functional changes.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 38 ++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 1498bd09cdba..d3739211b68a 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -18,10 +18,10 @@ my %all_line;
>  my %used_source;
>  my %record;
>  my %files;
> -my @func_regexes;
> +my @func_include_regexes;
>  my @func_exclude_regexes;
>  my %test_names;
> -my @src_regexes;
> +my @src_include_regexes;
>  my @src_exclude_regexes;
>  
>  my $verbose = 0;
> @@ -30,7 +30,7 @@ my $skip_func = 0;
>  
>  sub is_function_excluded($)
>  {
> -	return 0 if (!@func_regexes && !@func_exclude_regexes);
> +	return 0 if (!@func_include_regexes && !@func_exclude_regexes);
>  
>  	my $func = shift;
>  
> @@ -38,28 +38,28 @@ sub is_function_excluded($)
>  		return 1 if ($func =~ m/$r/);
>  	}
>  
> -	return 0 if (!@func_regexes);
> +	return 0 if (!@func_include_regexes);
>  
> -	foreach my $r (@func_regexes) {
> +	foreach my $r (@func_include_regexes) {
>  		return 0 if ($func =~ m/$r/);
>  	}
>  
>  	return 1;
>  }
>  
> -sub filter_file($)
> +sub is_file_excluded($)
>  {
>  	my $s = shift;
>  
> -	return 0 if (!@src_regexes && !@src_exclude_regexes);
> +	return 0 if (!@src_include_regexes && !@src_exclude_regexes);
>  
>  	foreach my $r (@src_exclude_regexes) {
>  		return 1 if ($s =~ m/$r/);
>  	}
>  
> -	return 0 if (!@src_regexes);
> +	return 0 if (!@src_include_regexes);
>  
> -	foreach my $r (@src_regexes) {
> +	foreach my $r (@src_include_regexes) {
>  		return 0 if ($s =~ m/$r/);
>  	}
>  
> @@ -107,7 +107,7 @@ sub parse_info_data($)
>  			$files{$source} = 1;
>  
>  			# Just ignore files explictly set as such
> -			$ignore = filter_file($source);
> +			$ignore = is_file_excluded($source);
>  			next;
>  		}
>  
> @@ -189,7 +189,7 @@ sub parse_info_data($)
>  
>  		# Ignore DA/BRDA that aren't associated with functions
>  		# Those are present on header files (maybe defines?)
> -		next if (@func_regexes && !$has_func);
> +		next if (@func_include_regexes && !$has_func);
>  
>  		# FNF:<number of functions found>
>  		if (m/^FNF:(-?\d+)/) {
> @@ -899,10 +899,10 @@ GetOptions(
>  	"only-i915|only_i915" => \$only_i915,
>  	"only-drm|only_drm" => \$only_drm,
>  	"func-filters|f=s" => \$func_filters,
> -	"include-func=s" => \@func_regexes,
> +	"include-func=s" => \@func_include_regexes,
>  	"exclude-func=s" => \@func_exclude_regexes,
>  	"source-filters|S=s" => \$src_filters,
> -	"include-source=s" => \@src_regexes,
> +	"include-source=s" => \@src_include_regexes,
>  	"exclude-source=s" => \@src_exclude_regexes,
>  	"show-files|show_files" => \$show_files,
>  	"show-lines|show_lines" => \$show_lines,
> @@ -935,9 +935,9 @@ my $str;
>  if ($only_i915) {
>  	# Please keep in sync with the documentation
>  	push @src_exclude_regexes, "selftest";
> -	push @src_regexes, "drm/i915";
> -	push @src_regexes, "drm/ttm";
> -	push @src_regexes, "drm/vgem";
> +	push @src_include_regexes, "drm/i915";
> +	push @src_include_regexes, "drm/ttm";
> +	push @src_include_regexes, "drm/vgem";
>  }
>  
>  if ($only_drm) {
> @@ -946,21 +946,21 @@ if ($only_drm) {
>  	push @src_exclude_regexes, "^/drm/";
>  }
>  
> -$str = open_filter_file($func_filters, \@func_regexes, \@func_exclude_regexes);
> +$str = open_filter_file($func_filters, \@func_include_regexes, \@func_exclude_regexes);
>  if ($str) {
>  	$filter_str .= "," if ($filter_str ne "");
>  	$filter_str .= " function regex ($str)";
>  	$has_filter = 1;
>  }
>  
> -$str = open_filter_file($src_filters, \@src_regexes, \@src_exclude_regexes);
> +$str = open_filter_file($src_filters, \@src_include_regexes, \@src_exclude_regexes);
>  if ($str) {
>  	$filter_str .= "," if ($filter_str ne "");
>  	$filter_str .= " source regex ($str)";
>  	$has_filter = 1;
>  }
>  
> -$ignore_unused = 1 if (@func_regexes || @func_exclude_regexes);
> +$ignore_unused = 1 if (@func_include_regexes || @func_exclude_regexes);
>  
>  if ($ignore_unused) {
>  	$filter_str .= "," if ($filter_str ne "");
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes
  2024-02-15 10:27 ` [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes Mauro Carvalho Chehab
@ 2024-02-15 16:35   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:12 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> When there are both exclude and include regexes, the include ones
> may be overriding the more generic excluded ones. So, handle them
> first.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 38 ++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index d3739211b68a..ceea67a13d5a 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -34,17 +34,24 @@ sub is_function_excluded($)
>  
>  	my $func = shift;
>  
> +	# Handle includes first, as, when there are both include and exclude
> +	# includes should take preference, as they can be overriding exclude
> +	# rules
> +	foreach my $r (@func_include_regexes) {
> +	    return 0 if ($func =~ m/$r/);
> +	}
> +
>  	foreach my $r (@func_exclude_regexes) {
>  		return 1 if ($func =~ m/$r/);
>  	}
>  
> -	return 0 if (!@func_include_regexes);
> -
> -	foreach my $r (@func_include_regexes) {
> -		return 0 if ($func =~ m/$r/);
> +	# If there are no exclude regexes, only include functions that are
> +	# explicitly included.
> +	if ($#func_exclude_regexes == 0) {
> +		return 1;
>  	}
>  
> -	return 1;
> +	return 0;
>  }
>  
>  sub is_file_excluded($)
> @@ -1178,9 +1185,10 @@ Each line at B<[filter's file]> may contain a new regex:
>  
>  =back
>  
> -When both include and exclude regexes are found, exclude regexes are
> -applied first and any functions that don't match the include regular
> -expressions from the B<[filter's file]> will be ignored.
> +Include regexes are handled first, as they can override an exclude regex.
> +
> +When just include regexes are used, any functions that don't match the
> +include regular expressions from the B<[filter's file]> will be ignored.
>  
>  Please notice that, when this filter is used, B<--ignore-unused> will be
>  automaticaly enabled, as the final goal is to report per-function usage.
> @@ -1189,7 +1197,8 @@ automaticaly enabled, as the final goal is to report per-function usage.
>  
>  Include B<regex> to the function filter. Can be used multiple times.
>  
> -When used together with B<--func-filters>, regexes here are handled first.
> +When used together with B<--func-filters> or B<--exclude-func>, regexes
> +here are handled first.
>  
>  Please notice that, when this filter is used, B<--ignore-unused> will be
>  automaticaly enabled, as the final goal is to report per-function usage.
> @@ -1198,8 +1207,6 @@ automaticaly enabled, as the final goal is to report per-function usage.
>  
>  Include B<regex> to the function filter. Can be used multiple times.
>  
> -When used together with B<--func-filters>, regexes here are handled first.
> -
>  Please notice that, when this filter is used, B<--ignore-unused> will be
>  automaticaly enabled, as the final goal is to report per-function usage.
>  
> @@ -1229,22 +1236,19 @@ Each line at B<[filter's file]> may contain a new regex:
>  
>  =back
>  
> -When both include and exclude regexes are found, exclude regexes are
> -applied first and any functions that don't match the include regular
> -expressions from the B<[filter's file]> will be ignored.
> +Include regexes are handled first, as they can override an exclude regex.
>  
>  =item B<--include-src> B<regex>
>  
>  Include B<regex> to the sources filter. Can be used multiple times.
>  
> -When used together with B<--src-filters>, regexes here are handled first.
> +When used together with B<--src-filters> and B<--exclude-src>, regexes
> +here are handled first.
>  
>  =item B<--exclude-src> B<regex>
>  
>  Include B<regex> to the sources filter. Can be used multiple times.
>  
> -When used together with B<--src-filters>, regexes here are handled first.
> -
>  =item B<--ignore-unused> or B<--ignore_unused>
>  
>  Filters out unused C files and headers from the code coverage results.
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data
  2024-02-15 10:27 ` [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data Mauro Carvalho Chehab
@ 2024-02-15 16:36   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:36 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:13 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Just like it does for i915, add an option to get code coverage
> data from Xe driver.
> 
> For now, it won't be taking DRM core stuff into account; just
> the Xe driver code itself.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index ceea67a13d5a..f0fb5716a6ca 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -894,6 +894,7 @@ my $src_filters;
>  my $show_files;
>  my $show_lines;
>  my $only_i915;
> +my $only_xe;
>  my $only_drm;
>  
>  GetOptions(
> @@ -904,6 +905,7 @@ GetOptions(
>  	"verbose|v" => \$verbose,
>  	"ignore-unused|ignore_unused" => \$ignore_unused,
>  	"only-i915|only_i915" => \$only_i915,
> +	"only-xe|only_xe" => \$only_xe,
>  	"only-drm|only_drm" => \$only_drm,
>  	"func-filters|f=s" => \$func_filters,
>  	"include-func=s" => \@func_include_regexes,
> @@ -947,6 +949,14 @@ if ($only_i915) {
>  	push @src_include_regexes, "drm/vgem";
>  }
>  
> +if ($only_xe) {
> +	# Please keep in sync with the documentation
> +	push @src_exclude_regexes, "selftest";
> +	push @src_include_regexes, "drm/xe";
> +#	push @src_include_regexes, "drm/ttm";
> +#	push @src_include_regexes, "drm/vgem";
> +}
> +
>  if ($only_drm) {
>  	# Please keep in sync with the documentation
>  	push @src_exclude_regexes, "trace.*\.h\$";
> @@ -980,7 +990,9 @@ foreach my $f (@ARGV) {
>  
>  	if ($gen_report) {
>  		$f =~ s,.*/,,;
> +		$f =~ s/\.gz$//;
>  		$f =~ s/\.info$//;
> +		$f =~ s/\.json$//;
>  
>  		gen_stats();
>  
> @@ -1162,6 +1174,19 @@ Excluding files that match:
>  
>  	- selftest
>  
> +=item B<--only-xe> or B<--only_xe>
> +
> +Filters out C files and headers outside drm core and drm/i915.
> +
> +E. g. code coverage results will include only the files that that match
> +the following regular expressions:
> +
> +	- drm/xe/
> +
> +Excluding files that match:
> +
> +	- selftest
> +
>  =item B<--func-filters>  B<[filter's file]> or B<-f>  B<[filter's file]>
>  
>  Use a file containing regular expressions (regex) to filter functions.
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage
  2024-02-15 10:27 ` [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage Mauro Carvalho Chehab
@ 2024-02-15 16:40   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:40 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:14 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Branch coverage is trickier than functions. Add a tool that displays
> the starting line that it is not being covered, together with up
> to 10 context lines before.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 86 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index f0fb5716a6ca..1be7fddf3f3e 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -879,6 +879,77 @@ sub generate_report($)
>  	close OUT;
>  }
>  
> +sub check_source_branches()
> +{
> +	foreach my $source (sort keys(%all_branch)) {
> +		next if (!$used_source{$source});
> +		next if (is_file_excluded($source));
> +
> +		my @lines;
> +		foreach my $where (sort keys %{$all_branch{$source}}) {
> +			my $taken = $all_branch{$source}{$where};
> +			next if ($taken > 0);
> +
> +			next if !($where =~ m/^(-?\d+),(-?\d+),(-?\d+)/);
> +
> +			my ($ln, $block, $branch, $ctx_lines);
> +			$ln = $1;
> +			$block = $2;
> +			$branch = $3;
> +
> +			if (!@lines) {
> +				open IN, "$source" || die "File $source not found. Can't check branches\n";
> +				@lines = <IN>;
> +				close IN;
> +			}
> +
> +			if ($ln >= $#lines) {
> +				die "$source:$ln line is bigger than the number of lines at the file ($#lines lines)\n";
> +				return;
> +			}
> +
> +			my $func = $all_branch{$source}{$where}{func};
> +			my $func_ln = $all_branch{$source}{$where}{func_ln};
> +
> +			print "Branch $source:$ln, ";
> +			if ($func) {
> +				if ($func_ln) {
> +					print "func: $func, ";
> +				} else {
> +					print "func: $func:$func_ln, ";
> +				}
> +			}
> +			print "block $block, branch $branch not taken:\n";
> +
> +			if ($func_ln) {
> +				$ctx_lines = $ln - $func_ln;
> +			} else {
> +				$ctx_lines = 10;
> +			}
> +
> +
> +			# Search for up to $ctx_lines before the occurrence
> +			my $context = "";
> +			my $has_if;
> +			$ln-- if ($ln > 0);
> +			my $delim = "->";
> +			for (my $if_ln = $ln; $if_ln >= 0 && (($ln - $if_ln) < $ctx_lines); $if_ln--) {
> +				$context = "$if_ln$delim\t$lines[$if_ln]" . $context;
> +				$delim = "";
> +				if ($lines[$if_ln] =~ m/(\bif\b|#if)/) {
> +					$has_if = 1;
> +					last;
> +				}
> +			}
> +			if ($has_if) {
> +				print $context;
> +			} else {
> +				print "$ln->\t$lines[$ln]";
> +			}
> +		}
> +	}
> +}
> +
>  #
>  # Argument handling
>  #
> @@ -896,6 +967,7 @@ my $show_lines;
>  my $only_i915;
>  my $only_xe;
>  my $only_drm;
> +my $check_branches;
>  
>  GetOptions(
>  	"print-coverage|print_coverage|print|p" => \$print_used,
> @@ -916,6 +988,7 @@ GetOptions(
>  	"show-files|show_files" => \$show_files,
>  	"show-lines|show_lines" => \$show_lines,
>  	"report|r=s" => \$gen_report,
> +	"check-branches" => \$check_branches,
>  	"css-file|css|c=s" => \$css_file,
>  	"title|t=s" => \$title,
>  	"html-prolog|prolog=s" => \$html_prolog,
> @@ -933,7 +1006,7 @@ if ($#ARGV < 0) {
>  }
>  
>  # At least one action should be specified
> -pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report);
> +pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report && !$check_branches);
>  
>  pod2usage(1) if ($gen_report && ($print_used || $filter || $stat || $print_unused));
>  
> @@ -1024,6 +1097,11 @@ if ($gen_report) {
>  
>  gen_stats();
>  
> +
> +if ($check_branches) {
> +	check_source_branches();
> +}
> +
>  die "Nothing counted. Wrong input files?" if (!$stats{"all_files"});
>  
>  print_code_coverage($print_used, $print_unused, $show_lines);
> @@ -1294,6 +1372,12 @@ This option is automaticaly enabled when B<--func-filters> is used.
>  Shows the list of files that were used to produce the code coverage
>  results.
>  
> +=item B<--check-branches>
> +
> +Checks at the Linux Kernel source files what's the contents of the
> +branches that weren't taken. The directory should match what's
> +stored inside the info files.
> +
>  =item B<--verbose> or B<-v>
>  
>  Prints the name of each parsed file.
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data
  2024-02-15 10:27 ` [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data Mauro Carvalho Chehab
@ 2024-02-15 16:41   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:41 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:15 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Right now, we store only the branch counts. We also need to store
> the functions that each branch belongs. So, add a new hash in
> order to allow adding function data later on.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 1be7fddf3f3e..d28217cc196d 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -233,7 +233,7 @@ sub parse_info_data($)
>  			$was_used = 1 if ($taken > 0);
>  
>  			$record{$source}{$func}{brda}{$where} += $taken;
> -			$all_branch{$source}{"$where"} += $taken;
> +			$all_branch{$source}{"$where"}{count} += $taken;
>  			next;
>  		}
>  
> @@ -448,7 +448,7 @@ sub gen_stats()
>  
>  		foreach my $where (keys(%{$all_branch{$source}})) {
>  			$stats{"branch_count"}++;
> -			$stats{"branch_reached"}++ if ($all_branch{$source}{$where} != 0);
> +			$stats{"branch_reached"}++ if ($all_branch{$source}{$where}{count} != 0);
>  		}
>  	}
>  
> @@ -607,7 +607,7 @@ sub generate_report($)
>  		}
>  		foreach my $source (keys(%{$report{$f}{"all_branch"}})) {
>  			foreach my $where (keys(%{$report{$f}{"all_branch"}{$source}})) {
> -				$all_branch{$source}{"$where"} += $report{$f}{"all_branch"}{$source}{$where};
> +				$all_branch{$source}{"$where"}{count} += $report{$f}{"all_branch"}{$source}{$where}{count};
>  			}
>  		}
>  		for my $source(keys(%{$report{$f}{"files"}})) {
> @@ -887,7 +887,7 @@ sub check_source_branches()
>  
>  		my @lines;
>  		foreach my $where (sort keys %{$all_branch{$source}}) {
> -			my $taken = $all_branch{$source}{$where};
> +			my $taken = $all_branch{$source}{$where}{count};
>  			next if ($taken > 0);
>  
>  			next if !($where =~ m/^(-?\d+),(-?\d+),(-?\d+)/);
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers
  2024-02-15 10:27 ` [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers Mauro Carvalho Chehab
@ 2024-02-15 16:55   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 16:55 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:16 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>

Small nit:

[PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for

s/numberic/numeric/

with that:

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> 
> The DA and BRDA information is originally numerically sorted.
> 
> Sort it the same way at the output data.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> ---
>  scripts/code_cov_parse_info | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index d28217cc196d..56b27faee812 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -291,6 +291,21 @@ sub parse_info_data($)
>  	close IN or die;
>  }
>  
> +sub sort_where($$)
> +{
> +	my @a = split ",", shift;
> +	my @b = split ",", shift;
> +	my $ret;
> +
> +	$ret = $a[0] <=> $b[0];
> +	return $ret if ($ret);
> +
> +	$ret = $a[1] <=> $b[1];
> +	return $ret if ($ret);
> +
> +	return $a[2] <=> $b[2];
> +}
> +
>  sub write_filtered_file($)
>  {
>  	my $filter = shift;
> @@ -332,10 +347,10 @@ sub write_filtered_file($)
>  				}
>  			}
>  
> -			foreach my $ln(sort keys %{ $record{$source}{$func}{da} }) {
> +			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{$func}{da} }) {
>  				$filtered .= "DA:$ln," . $record{$source}{$func}{da}{$ln} . "\n";
>  			}
> -			foreach my $where(sort keys %{ $record{$source}{$func}{brda} }) {
> +			foreach my $where(sort sort_where keys %{ $record{$source}{$func}{brda} }) {
>  				my $taken = $record{$source}{$func}{brda}{$where};
>  				$taken = "-" if (!$taken);
>  				$filtered .= "BRDA:$where,$taken\n";
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic
  2024-02-15 10:27 ` [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic Mauro Carvalho Chehab
@ 2024-02-15 17:01   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:17 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> In preparation for adding support for JSON files, make the
> function more generic.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 67 +++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 33 deletions(-)
>  mode change 100755 => 100644 scripts/code_cov_parse_info
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> old mode 100755
> new mode 100644
> index 56b27faee812..92091215d224
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -98,6 +98,7 @@ sub parse_info_data($)
>  			if ($1 ne $cur_test) {
>  				$cur_test = $1;
>  				$test_names{$cur_test} = 1;
> +				$record{tests}{$cur_test} = 1;
>  			}
>  			$source = $before_sf;
>  			$func = $before_sf;
> @@ -158,7 +159,7 @@ sub parse_info_data($)
>  
>  			$skip_func = 0;
>  
> -			$record{$source}{$func}{fn} = $ln;
> +			$record{$source}{func}{$func}{fn} = $ln;
>  			$all_func{$func}{$source}->{ln} = $ln;
>  			next;
>  		}
> @@ -186,7 +187,7 @@ sub parse_info_data($)
>  			$skip_func = 0;
>  			$was_used = 1;
>  
> -			$record{$source}{$func}{fnda} += $count;
> +			$record{$source}{func}{$func}{fnda} += $count;
>  			$used_func{$func}{$source}->{count} += $count;
>  			next;
>  		}
> @@ -200,14 +201,14 @@ sub parse_info_data($)
>  
>  		# FNF:<number of functions found>
>  		if (m/^FNF:(-?\d+)/) {
> -			$record{$source}{$func}{fnf} = $1;
> +			$record{$source}{func}{$func}{fnf} = $1;
>  			next;
>  		}
>  		# FNH:<number of function hit>
>  		if (m/^FNH:(-?\d+)/) {
>  			my $hits = $1;
> -			if (!defined($record{$source}{$func}{fnh}) || $record{$source}{$func}{fnh} < $hits) {
> -				$record{$source}{$func}{fnh} = $hits;
> +			if (!defined($record{$source}{func}{$func}{fnh}) || $record{$source}{func}{$func}{fnh} < $hits) {
> +				$record{$source}{func}{$func}{fnh} = $hits;
>  			}
>  			next;
>  		}
> @@ -232,21 +233,21 @@ sub parse_info_data($)
>  
>  			$was_used = 1 if ($taken > 0);
>  
> -			$record{$source}{$func}{brda}{$where} += $taken;
> -			$all_branch{$source}{"$where"}{count} += $taken;
> +			$record{files}{$source}{line}{$ln}{branches}[$branch]{count} += $taken;
> +			$all_branch{$source}{$where}{count} += $taken;
>  			next;
>  		}
>  
>  		# BRF:<number of branches found>
>  		if (m/^BRF:(-?\d+)/) {
> -			$record{$source}{$func}{brf} = $1;
> +			$record{$source}{func}{$func}{brf} = $1;
>  			next;
>  		}
>  		# BRH:<number of branches hit>
>  		if (m/^BRH:(-?\d+)/) {
>  			my $hits = $1;
> -			if (!defined($record{$source}{$func}{brh}) || $record{$source}{$func}{brh} < $hits) {
> -				$record{$source}{$func}{brh} = $hits;
> +			if (!defined($record{$source}{func}{$func}{brh}) || $record{$source}{func}{$func}{brh} < $hits) {
> +				$record{$source}{func}{$func}{brh} = $hits;
>  			}
>  			next;
>  		}
> @@ -265,22 +266,22 @@ sub parse_info_data($)
>  
>  			$was_used = 1 if ($count > 0);
>  
> -			$record{$source}{$func}{da}{$ln} += $count;
> -			$all_line{$source}{"$ln"} += $count;
> +			$record{$source}{func}{$func}{da}{$ln} += $count;
> +			$all_line{$source}{$ln} += $count;
>  			next;
>  		}
>  
>  		# LF:<number of instrumented lines>
>  		if (m/^LF:(-?\d+)/) {
> -			$record{$source}{$func}{lf} = $1;
> +			$record{$source}{func}{$func}{lf} = $1;
>  			next;
>  		}
>  
>  		# LH:<number of lines with a non-zero execution count>
>  		if (m/^LH:(-?\d+)/) {
>  			my $hits = $1;
> -			if (!defined($record{$source}{$func}{lh}) || $record{$source}{$func}{lh} < $hits) {
> -				$record{$source}{$func}{lh} = $hits;
> +			if (!defined($record{$source}{func}{$func}{lh}) || $record{$source}{func}{$func}{lh} < $hits) {
> +				$record{$source}{func}{$func}{lh} = $hits;
>  			}
>  			next;
>  		}
> @@ -333,39 +334,39 @@ sub write_filtered_file($)
>  				my $fn;
>  				my $fnda;
>  
> -				if (defined($record{$source}{$func}{fn})) {
> -					$filtered .= "FN:" . $record{$source}{$func}{fn} . ",$func\n";
> +				if (defined($record{$source}{func}{$func}{fn})) {
> +					$filtered .= "FN:" . $record{$source}{func}{$func}{fn} . ",$func\n";
>  				}
> -				if (defined($record{$source}{$func}{fnda})) {
> -					$filtered .= "FNDA:" . $record{$source}{$func}{fnda} . ",$func\n";
> +				if (defined($record{$source}{func}{$func}{fnda})) {
> +					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
>  				}
>  				if ($record{$source}{fnf}) {
> -					$filtered .= "FNF:". $record{$source}{$func}{fnf} ."\n";
> +					$filtered .= "FNF:". $record{$source}{func}{$func}{fnf} ."\n";
>  				}
>  				if ($record{$source}{fnh}) {
> -					$filtered .= "FNH:". $record{$source}{$func}{fnh} ."\n";
> +					$filtered .= "FNH:". $record{$source}{func}{$func}{fnh} ."\n";
>  				}
>  			}
>  
> -			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{$func}{da} }) {
> -				$filtered .= "DA:$ln," . $record{$source}{$func}{da}{$ln} . "\n";
> +			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{func}{$func}{da} }) {
> +				$filtered .= "DA:$ln," . $record{$source}{func}{$func}{da}{$ln} . "\n";
>  			}
> -			foreach my $where(sort sort_where keys %{ $record{$source}{$func}{brda} }) {
> -				my $taken = $record{$source}{$func}{brda}{$where};
> +			foreach my $where(sort sort_where keys %{ $record{$source}{func}{$func}{brda} }) {
> +				my $taken = $record{$source}{func}{$func}{brda}{$where};
>  				$taken = "-" if (!$taken);
>  				$filtered .= "BRDA:$where,$taken\n";
>  			}
> -			if ($record{$source}{$func}{brf}) {
> -				$filtered .= "BRF:". $record{$source}{$func}{brf} ."\n";
> +			if ($record{$source}{func}{$func}{brf}) {
> +				$filtered .= "BRF:". $record{$source}{func}{$func}{brf} ."\n";
>  			}
> -			if ($record{$source}{$func}{brh}) {
> -				$filtered .= "BRH:". $record{$source}{$func}{brh} ."\n";
> +			if ($record{$source}{func}{$func}{brh}) {
> +				$filtered .= "BRH:". $record{$source}{func}{$func}{brh} ."\n";
>  			}
> -			if ($record{$source}{$func}{lf}) {
> -				$filtered .= "LF:". $record{$source}{$func}{lf} ."\n";
> +			if ($record{$source}{func}{$func}{lf}) {
> +				$filtered .= "LF:". $record{$source}{func}{$func}{lf} ."\n";
>  			}
> -			if ($record{$source}{$func}{lh}) {
> -				$filtered .= "LH:". $record{$source}{$func}{lh} ."\n";
> +			if ($record{$source}{func}{$func}{lh}) {
> +				$filtered .= "LH:". $record{$source}{func}{$func}{lh} ."\n";
>  			}
>  		}
>  
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields
  2024-02-15 10:27 ` [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields Mauro Carvalho Chehab
@ 2024-02-15 17:09   ` Kamil Konieczny
  2024-02-15 19:55     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:09 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:18 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> As we'll be using json as basis, drop support for the fields that:
> 
> - aren't used at the reports;
> - aren't even present when lcov parses gcc json format.
> 
> Such fields are only handled on lcov for very old gcc versions.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> ---
>  scripts/code_cov_parse_info | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 92091215d224..038440a9c3e3 100644
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -201,15 +201,10 @@ sub parse_info_data($)
>  
>  		# FNF:<number of functions found>
>  		if (m/^FNF:(-?\d+)/) {
> -			$record{$source}{func}{$func}{fnf} = $1;
>  			next;
>  		}
>  		# FNH:<number of function hit>
>  		if (m/^FNH:(-?\d+)/) {
> -			my $hits = $1;
> -			if (!defined($record{$source}{func}{$func}{fnh}) || $record{$source}{func}{$func}{fnh} < $hits) {
> -				$record{$source}{func}{$func}{fnh} = $hits;
> -			}
>  			next;
>  		}
>  
> @@ -240,15 +235,10 @@ sub parse_info_data($)
>  
>  		# BRF:<number of branches found>
>  		if (m/^BRF:(-?\d+)/) {
> -			$record{$source}{func}{$func}{brf} = $1;
>  			next;
>  		}
>  		# BRH:<number of branches hit>
>  		if (m/^BRH:(-?\d+)/) {
> -			my $hits = $1;
> -			if (!defined($record{$source}{func}{$func}{brh}) || $record{$source}{func}{$func}{brh} < $hits) {
> -				$record{$source}{func}{$func}{brh} = $hits;
> -			}

Above is self-explanatory.

>  			next;
>  		}
>  
> @@ -340,34 +330,11 @@ sub write_filtered_file($)
>  				if (defined($record{$source}{func}{$func}{fnda})) {
>  					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
>  				}
> -				if ($record{$source}{fnf}) {
> -					$filtered .= "FNF:". $record{$source}{func}{$func}{fnf} ."\n";
> -				}
> -				if ($record{$source}{fnh}) {
> -					$filtered .= "FNH:". $record{$source}{func}{$func}{fnh} ."\n";
> -				}
> -			}
>  
> -			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{func}{$func}{da} }) {
> -				$filtered .= "DA:$ln," . $record{$source}{func}{$func}{da}{$ln} . "\n";

Why are you removing DA here? It is used elswhere.

>  			}
> -			foreach my $where(sort sort_where keys %{ $record{$source}{func}{$func}{brda} }) {
> -				my $taken = $record{$source}{func}{$func}{brda}{$where};

Same here, BRDA.

>  				$taken = "-" if (!$taken);
>  				$filtered .= "BRDA:$where,$taken\n";
>  			}
> -			if ($record{$source}{func}{$func}{brf}) {
> -				$filtered .= "BRF:". $record{$source}{func}{$func}{brf} ."\n";
> -			}
> -			if ($record{$source}{func}{$func}{brh}) {
> -				$filtered .= "BRH:". $record{$source}{func}{$func}{brh} ."\n";
> -			}
> -			if ($record{$source}{func}{$func}{lf}) {
> -				$filtered .= "LF:". $record{$source}{func}{$func}{lf} ."\n";
> -			}
> -			if ($record{$source}{func}{$func}{lh}) {
> -				$filtered .= "LH:". $record{$source}{func}{$func}{lh} ."\n";

LF and LH?

If above is correct you can add my a-b

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> -			}
>  		}
>  
>  		$filtered .= "end_of_record\n";
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files
  2024-02-15 10:27 ` [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files Mauro Carvalho Chehab
@ 2024-02-15 17:12   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

Hi Mauro,
On 2024-02-15 at 11:27:19 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Currently, the tool supports only info files, provided by
> lcov tool. While this works, the info output lacks support to
> properly identify what function is related to branches and
> lines. Such limitation doesn't exist with json. So, add
> support for parsing it.

Small nit: Such limitation doesn't exist with json so use it.

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

with or without:
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 210 +++++++++++++++++++++++++++++++++---
>  1 file changed, 197 insertions(+), 13 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 038440a9c3e3..97c94a81004f 100644
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -28,6 +28,16 @@ my $verbose = 0;
>  my $ignore_unused = 0;
>  my $skip_func = 0;
>  
> +my $has_json_support = eval {
> +	require Cpanel::JSON::XS;
> +	Cpanel::JSON::XS->import(qw(decode_json));
> +	1;
> +};
> +
> +if (!$has_json_support) {
> +	print "Warning: System doesn't have Cpanel::JSON::XS. Can't use gcov directly.\n";
> +}
> +
>  sub is_function_excluded($)
>  {
>  	return 0 if (!@func_include_regexes && !@func_exclude_regexes);
> @@ -76,7 +86,169 @@ sub is_file_excluded($)
>  # Use something that comes before any real function
>  my $before_sf = "!!!!";
>  
> -sub parse_info_data($)
> +sub parse_json_gcov_v1($$)
> +{
> +	my $file = shift;
> +	my $json = shift;
> +
> +	my $was_used = 0;
> +	my $has_func = 0;
> +	my $ignore = 0;
> +
> +	my $cur_test = $file;
> +	$cur_test =~ s#^.*/##;
> +	$cur_test =~ s#\.json$##;
> +	$test_names{$cur_test} = 1;
> +
> +	# Store the common JSON data into the record
> +	for my $key (keys %$json) {
> +		next if ($key eq "files");
> +		next if ($key eq "functions");
> +		next if ($key eq "lines");
> +		# Store any extra data
> +		$record{$key} = $json->{$key};
> +	}
> +	# Store test name at the record
> +	$record{tests}{$cur_test} = 1;
> +
> +	for my $file_ref (@{$json->{'files'}}) {
> +		my $source = $file_ref->{'file'};
> +
> +		$files{$source} = 1;
> +		next if is_file_excluded($source);
> +
> +		# Parse functions
> +		for my $func_ref (@{$file_ref->{'functions'}}) {
> +			my $func = $func_ref->{'name'};
> +
> +			next if is_function_excluded($func);
> +
> +			# Negative gcov results are possible, as reported at:
> +			# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> +			# Lcov ignores those. So, let's do the same here.
> +			$func_ref->{'execution_count'} = 0 if ($func_ref->{'execution_count'} < 0);
> +
> +			# Store the record
> +			for my $key (keys %$func_ref) {
> +				if ($key eq "execution_count") {
> +					$record{$source}{func}{$func}{$key} += $func_ref->{$key};
> +				} else {
> +					$record{$source}{func}{$func}{$key} = $func_ref->{$key};
> +				}
> +			}
> +
> +			$all_func{$func}{$source}->{ln} = $func_ref->{'start_line'};
> +			$all_func{$func}{$source}->{end_ln} = $func_ref->{'end_line'};
> +
> +			if ($func_ref->{'execution_count'} > 0) {
> +				$used_func{$func}{$source}->{count} += $func_ref->{'execution_count'};
> +				$was_used = 1;
> +			}
> +		}
> +		next if ($ignore_unused && !$was_used);
> +		$used_source{$source} = 1;
> +
> +		# Parse lines and branches
> +		for my $line_ref (@{$file_ref->{'lines'}}) {
> +			my $ln = $line_ref->{'line_number'};
> +
> +			# Negative gcov results are possible, as reported at:
> +			# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> +			# Lcov ignores those. So, let's do the same here.
> +			$line_ref->{'count'} = 0 if ($line_ref->{'count'} < 0);
> +
> +			my $func = $line_ref->{'function_name'};
> +			if (!$func) {
> +				# Ignore DA/BRDA that aren't associated with
> +				# functions. Those are present on header files
> +				# (maybe defines?)
> +				next if (@func_include_regexes);
> +
> +				# Otherwise place them in separate
> +				$func = $before_sf;
> +			} else {
> +				next if is_function_excluded($func);
> +			}
> +
> +			# Store the record
> +			for my $key (keys %$line_ref) {
> +				next if ($key eq "line_number");
> +
> +				# Branches will be handled in separate
> +				next if ($key eq "branches");
> +				if ($key eq "count") {
> +					$record{$source}{line}{$ln}{$key} += $line_ref->{$key};
> +				} else {
> +					$record{$source}{line}{$ln}{$key} = $line_ref->{$key};
> +				}
> +			}
> +			$all_line{$source}{$ln} += $line_ref->{'count'};
> +
> +			my $i = 0;
> +			for my $branch_ref (@{$line_ref->{'branches'}}) {
> +				my $taken = $branch_ref->{'count'};
> +				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
> +
> +				# Negative gcov results are possible, as
> +				# reported at:
> +				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> +				# Lcov ignores those. So, let's do the same
> +				# here.
> +				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
> +
> +				for my $key (keys %$branch_ref) {
> +					if ($key eq "count") {
> +						$record{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
> +					} else {
> +						$record{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
> +					}
> +				}
> +
> +				$all_branch{$source}{$where}{count} += $taken;
> +				$i++;
> +			}
> +			if (!defined($record{$source}{line}{$ln}{branches})) {
> +				@{$record{$source}{line}{$ln}{branches}} = ();
> +			}
> +		}
> +	}
> +
> +	# As the record was changed, we need to use a different format name
> +	$record{format_version} = "parse_info v1.0";
> +}
> +
> +sub read_json($)
> +{
> +	my $file = shift;
> +
> +	if (!$has_json_support) {
> +		die "Can't parse json as system doesn't have Cpanel::JSON::XS.\n";
> +	}
> +
> +	# Read JSON data
> +	open IN, $file or die "can't open $file";
> +	while (<IN>) {
> +		my $json = eval {
> +			Cpanel::JSON::XS::decode_json($_)
> +		};
> +
> +		if (!$json) {
> +			printf "Failed to parse file $file, line $.\n";
> +			next;
> +		}
> +		if ($json->{'format_version'} eq '1') {
> +			parse_json_gcov_v1($file, $json);
> +		} else {
> +			if ($json->{'format_version'}) {
> +				die "Can't parse JSON version %d on file $file\n", $json->{'format_version'};
> +			}
> +			die "Unknown JSON format on file $file\n";
> +		}
> +	}
> +	close IN;
> +}
> +
> +sub read_info($)
>  {
>  	my $file = shift;
>  	my $was_used = 0;
> @@ -159,7 +331,7 @@ sub parse_info_data($)
>  
>  			$skip_func = 0;
>  
> -			$record{$source}{func}{$func}{fn} = $ln;
> +			$record{$source}{func}{$func}{start_line} = $ln;
>  			$all_func{$func}{$source}->{ln} = $ln;
>  			next;
>  		}
> @@ -256,23 +428,23 @@ sub parse_info_data($)
>  
>  			$was_used = 1 if ($count > 0);
>  
> -			$record{$source}{func}{$func}{da}{$ln} += $count;
> +			$record{$source}{line}{$ln}{count} += $count;
> +			if (!defined($record{$source}{line}{$ln}{branches})) {
> +				@{$record{$source}{line}{$ln}{branches}} = ();
> +			}
> +
>  			$all_line{$source}{$ln} += $count;
> +
>  			next;
>  		}
>  
>  		# LF:<number of instrumented lines>
>  		if (m/^LF:(-?\d+)/) {
> -			$record{$source}{func}{$func}{lf} = $1;
>  			next;
>  		}
>  
>  		# LH:<number of lines with a non-zero execution count>
>  		if (m/^LH:(-?\d+)/) {
> -			my $hits = $1;
> -			if (!defined($record{$source}{func}{$func}{lh}) || $record{$source}{func}{$func}{lh} < $hits) {
> -				$record{$source}{func}{$func}{lh} = $hits;
> -			}
>  			next;
>  		}
>  
> @@ -319,21 +491,29 @@ sub write_filtered_file($)
>  			$filtered .= "SF:$source\n";
>  		}
>  
> -		foreach my $func(sort keys %{ $record{$source} }) {
> +		foreach my $func(sort keys %{ $record{$source}{func} }) {
>  			if ($func ne $before_sf) {
>  				my $fn;
>  				my $fnda;
>  
> -				if (defined($record{$source}{func}{$func}{fn})) {
> -					$filtered .= "FN:" . $record{$source}{func}{$func}{fn} . ",$func\n";
> +				if (defined($record{$source}{func}{$func}{start_line})) {
> +					$filtered .= "FN:" . $record{$source}{func}{$func}{start_line} . ",$func\n";
>  				}
>  				if (defined($record{$source}{func}{$func}{fnda})) {
>  					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
>  				}
>  
>  			}
> +		}
> +
> +		foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{line} }) {
> +			$filtered .= "DA:$ln," . $record{$source}{line}{$ln}{count} . "\n";
> +
> +			my $i = 0;
> +			for ($i = 0, $i < scalar(@{$record{$source}{line}{$ln}{branches}}), $i++) {
> +				my $taken = $record{$source}{line}{$ln}{branches}[$i]{count};
>  				$taken = "-" if (!$taken);
> -				$filtered .= "BRDA:$where,$taken\n";
> +				$filtered .= "BRDA:$ln,0,$i,$taken\n";
>  			}
>  		}
>  
> @@ -1042,7 +1222,11 @@ if ($ignore_unused) {
>  }
>  
>  foreach my $f (@ARGV) {
> -	parse_info_data($f);
> +	if ($f =~ /.json$/) {
> +		read_json($f);
> +	} else {
> +		read_info($f);
> +	}
>  
>  	if ($gen_report) {
>  		$f =~ s,.*/,,;
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format
  2024-02-15 10:27 ` [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format Mauro Carvalho Chehab
@ 2024-02-15 17:15   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

Hi Mauro,
On 2024-02-15 at 11:27:20 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> In order to be able to join data from different JSON files, the
> JSON format needs to be different than the standard gcov one,
> as:
> 
> - it should contain test names;
> - it should use hashes instead of arrays for functions and lines.
> 
> So, at the end of the day, it needs a different format version.
- ^^^^^^^^^^^^^^^^^^^^^^^^^^
imho delete this one line.

> Add support for read/write on a new format.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 222 +++++++++++++++++++++++++++++-------
>  1 file changed, 179 insertions(+), 43 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 97c94a81004f..8180b9c1f82e 100644
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -131,9 +131,9 @@ sub parse_json_gcov_v1($$)
>  			# Store the record
>  			for my $key (keys %$func_ref) {
>  				if ($key eq "execution_count") {
> -					$record{$source}{func}{$func}{$key} += $func_ref->{$key};
> +					$record{files}{$source}{func}{$func}{$key} += $func_ref->{$key};
>  				} else {
> -					$record{$source}{func}{$func}{$key} = $func_ref->{$key};
> +					$record{files}{$source}{func}{$func}{$key} = $func_ref->{$key};
>  				}
>  			}
>  
> @@ -177,9 +177,9 @@ sub parse_json_gcov_v1($$)
>  				# Branches will be handled in separate
>  				next if ($key eq "branches");
>  				if ($key eq "count") {
> -					$record{$source}{line}{$ln}{$key} += $line_ref->{$key};
> +					$record{files}{$source}{line}{$ln}{$key} += $line_ref->{$key};
>  				} else {
> -					$record{$source}{line}{$ln}{$key} = $line_ref->{$key};
> +					$record{files}{$source}{line}{$ln}{$key} = $line_ref->{$key};
>  				}
>  			}
>  			$all_line{$source}{$ln} += $line_ref->{'count'};
> @@ -198,17 +198,17 @@ sub parse_json_gcov_v1($$)
>  
>  				for my $key (keys %$branch_ref) {
>  					if ($key eq "count") {
> -						$record{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
> +						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
>  					} else {
> -						$record{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
> +						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
>  					}
>  				}
>  
>  				$all_branch{$source}{$where}{count} += $taken;
>  				$i++;
>  			}
> -			if (!defined($record{$source}{line}{$ln}{branches})) {
> -				@{$record{$source}{line}{$ln}{branches}} = ();
> +			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
> +				@{$record{files}{$source}{line}{$ln}{branches}} = ();
>  			}
>  		}
>  	}
> @@ -217,6 +217,121 @@ sub parse_json_gcov_v1($$)
>  	$record{format_version} = "parse_info v1.0";
>  }
>  
> +sub parse_json_internal_format_v1($$)
> +{
> +	my $file = shift;
> +	my $json = shift;
> +
> +	my $was_used = 0;
> +	my $has_func = 0;
> +	my $ignore = 0;
> +
> +	# Store the common JSON data into the record
> +	for my $key (keys %$json) {
> +		next if ($key eq "files");
> +		next if ($key eq "functions");
> +		next if ($key eq "lines");
> +		# Store any extra data
> +		$record{$key} = $json->{$key};
> +	}
> +
> +	for my $test (keys %{$json->{'tests'}}) {
> +		$test_names{$test} = 1;
> +	}
> +
> +	for my $source (keys %{$json->{'files'}}) {
> +		$files{$source} = 1;
> +		next if is_file_excluded($source);
> +
> +		my $file_ref = \%{$json->{'files'}{$source}};
> +
> +		# Parse functions
> +		for my $func (keys %{$file_ref->{func}}) {
> +			next if is_function_excluded($func);
> +
> +			my $func_ref = \%{$file_ref->{func}{$func}};
> +
> +			# Store the record
> +			for my $key (keys %$func_ref) {
> +				if ($key eq "execution_count") {
> +					$record{files}{$source}{func}{$func}{$key} += $func_ref->{$key};
> +				} else {
> +					$record{files}{$source}{func}{$func}{$key} = $func_ref->{$key};
> +				}
> +			}
> +
> +			$all_func{$func}{$source}->{ln} = $func_ref->{'start_line'};
> +			$all_func{$func}{$source}->{end_ln} = $func_ref->{'end_line'};
> +
> +			if ($func_ref->{'execution_count'} > 0) {
> +				$used_func{$func}{$source}->{count} += $func_ref->{'execution_count'};
> +				$was_used = 1;
> +			}
> +		}
> +		next if ($ignore_unused && !$was_used);
> +		$used_source{$source} = 1;
> +
> +		# Parse lines and branches
> +		for my $ln (keys %{$file_ref->{line}}) {
> +			my $line_ref = \%{$file_ref->{line}{$ln}};
> +			my $func = $line_ref->{'function_name'};
> +			if (!$func) {
> +				# Ignore DA/BRDA that aren't associated with
> +				# functions. Those are present on header files
> +				# (maybe defines?)
> +				next if (@func_include_regexes);
> +
> +				# Otherwise place them in separate
> +				$func = $before_sf;
> +			} else {
> +				next if is_function_excluded($func);
> +			}
> +
> +			# Store the record
> +			for my $key (keys %$line_ref) {
> +				next if ($key eq "line_number");
> +
> +				# Branches will be handled in separate
> +				next if ($key eq "branches");
> +				if ($key eq "count") {
> +					$record{files}{$source}{line}{$ln}{$key} += $line_ref->{$key};
> +				} else {
> +					$record{files}{$source}{line}{$ln}{$key} = $line_ref->{$key};
> +				}
> +			}
> +			$all_line{$source}{$ln} += $line_ref->{'count'};
> +
> +			my $i = 0;
> +			for my $branch_ref (@{$line_ref->{'branches'}}) {
> +				my $taken = $branch_ref->{'count'};
> +				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
> +
> +				# Negative gcov results are possible, as
> +				# reported at:
> +				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> +				# Lcov ignores those. So, let's do the same
> +				# here.
> +				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
> +
> +				for my $key (keys %$branch_ref) {
> +					next if (!$record{files}{$source}{line}{$ln}{branches});
> +					if ($key eq "count") {
> +						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} += $branch_ref->{$key};
> +					} else {
> +						$record{files}{$source}{line}{$ln}{branches}[$i]{$key} = $branch_ref->{$key};
> +					}
> +				}
> +
> +				$all_branch{$source}{$where}{count} += $taken;
> +				$i++;
> +			}
> +			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
> +				@{$record{files}{$source}{line}{$ln}{branches}} = ();
> +			}
> +		}
> +	}
> +}
> +
>  sub read_json($)
>  {
>  	my $file = shift;
> @@ -238,6 +353,8 @@ sub read_json($)
>  		}
>  		if ($json->{'format_version'} eq '1') {
>  			parse_json_gcov_v1($file, $json);
> +		} elsif ($json->{'format_version'} eq 'parse_info v1.0') {
> +			parse_json_internal_format_v1($file, $json);
>  		} else {
>  			if ($json->{'format_version'}) {
>  				die "Can't parse JSON version %d on file $file\n", $json->{'format_version'};
> @@ -331,7 +448,7 @@ sub read_info($)
>  
>  			$skip_func = 0;
>  
> -			$record{$source}{func}{$func}{start_line} = $ln;
> +			$record{files}{$source}{func}{$func}{start_line} = $ln;
>  			$all_func{$func}{$source}->{ln} = $ln;
>  			next;
>  		}
> @@ -359,7 +476,7 @@ sub read_info($)
>  			$skip_func = 0;
>  			$was_used = 1;
>  
> -			$record{$source}{func}{$func}{fnda} += $count;
> +			$record{files}{$source}{func}{$func}{execution_count} += $count;
>  			$used_func{$func}{$source}->{count} += $count;
>  			next;
>  		}
> @@ -428,9 +545,9 @@ sub read_info($)
>  
>  			$was_used = 1 if ($count > 0);
>  
> -			$record{$source}{line}{$ln}{count} += $count;
> -			if (!defined($record{$source}{line}{$ln}{branches})) {
> -				@{$record{$source}{line}{$ln}{branches}} = ();
> +			$record{files}{$source}{line}{$ln}{count} += $count;
> +			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
> +				@{$record{files}{$source}{line}{$ln}{branches}} = ();
>  			}
>  
>  			$all_line{$source}{$ln} += $count;
> @@ -469,59 +586,74 @@ sub sort_where($$)
>  	return $a[2] <=> $b[2];
>  }
>  
> -sub write_filtered_file($)
> +sub write_json_file($)
>  {
> -	my $filter = shift;
> +	my $fname = shift;
>  
> -	my $filtered = "";
> +	if (!$has_json_support) {
> +		die "Can't parse json as system doesn't have Cpanel::JSON::XS.\n";
> +	}
> +
> +	my $data = eval {
> +		Cpanel::JSON::XS::encode_json(\%record)
> +	};
> +
> +	open OUT, ">$fname" or die "Can't open $fname";
> +	print OUT $data or die "Failed to write to $fname";
> +	close OUT or die "Failed to close to $fname";
> +}
> +
> +sub write_info_file($)
> +{
> +	my $fname = shift;
> +	my $data = "";
>  
>  	if ($title eq "") {
>  		foreach my $testname(sort keys %test_names) {
> -			$filtered .= "TN:$testname\n";
> +			$data .= "TN:$testname\n";
>  		}
>  	} else {
> -		$filtered .= "TN:$title\n";
> +		$data .= "TN:$title\n";
>  	}
>  
> -	# Generates filtered data
> -	foreach my $source(sort keys %record) {
> +	# Fills $data with the contents to be stored at the file
> +	foreach my $source(sort keys %{$record{files}}) {
>  		next if (!$used_source{$source});
>  
>  		if ($source ne $before_sf) {
> -			$filtered .= "SF:$source\n";
> +			$data .= "SF:$source\n";
>  		}
>  
> -		foreach my $func(sort keys %{ $record{$source}{func} }) {
> +		foreach my $func(sort keys %{ $record{files}{$source}{func} }) {
>  			if ($func ne $before_sf) {
>  				my $fn;
>  				my $fnda;
>  
> -				if (defined($record{$source}{func}{$func}{start_line})) {
> -					$filtered .= "FN:" . $record{$source}{func}{$func}{start_line} . ",$func\n";
> +				if (defined($record{files}{$source}{func}{$func}{start_line})) {
> +					$data .= "FN:" . $record{files}{$source}{func}{$func}{start_line} . ",$func\n";
>  				}
> -				if (defined($record{$source}{func}{$func}{fnda})) {
> -					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
> +				if (defined($record{files}{$source}{func}{$func}{execution_count})) {
> +					$data .= "FNDA:" . $record{files}{$source}{func}{$func}{execution_count} . ",$func\n";
>  				}
>  
>  			}
>  		}
>  
> -		foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{line} }) {
> -			$filtered .= "DA:$ln," . $record{$source}{line}{$ln}{count} . "\n";
> -
> -			my $i = 0;
> -			for ($i = 0, $i < scalar(@{$record{$source}{line}{$ln}{branches}}), $i++) {
> -				my $taken = $record{$source}{line}{$ln}{branches}[$i]{count};
> +		foreach my $ln(sort { $a <=> $b } keys %{ $record{files}{$source}{line} }) {
> +			$data .= "DA:$ln," . $record{files}{$source}{line}{$ln}{count} . "\n";
> +			next if (!$record{files}{$source}{line}{$ln}{branches});
> +			for (my $i = 0; $i < scalar @{$record{files}{$source}{line}{$ln}{branches}}; $i++) {
> +				my $taken = $record{files}{$source}{line}{$ln}{branches}[$i]{count};
>  				$taken = "-" if (!$taken);
> -				$filtered .= "BRDA:$ln,0,$i,$taken\n";
> +				$data .= "BRDA:$ln,0,$i,$taken\n";
>  			}
>  		}
>  
> -		$filtered .= "end_of_record\n";
> +		$data .= "end_of_record\n";
>  	}
> -	open OUT, ">$filter" or die "Can't open $filter";
> -	print OUT $filtered or die "Failed to write to $filter";
> -	close OUT or die "Failed to close to $filter";
> +	open OUT, ">$fname" or die "Can't open $fname";
> +	print OUT $data or die "Failed to write to $fname";
> +	close OUT or die "Failed to close to $fname";
>  }
>  
>  sub print_code_coverage($$$)
> @@ -1120,7 +1252,7 @@ sub check_source_branches()
>  my $print_used;
>  my $print_unused;
>  my $stat;
> -my $filter;
> +my $output_file;
>  my $help;
>  my $man;
>  my $func_filters;
> @@ -1136,7 +1268,7 @@ GetOptions(
>  	"print-coverage|print_coverage|print|p" => \$print_used,
>  	"print-unused|u" => \$print_unused,
>  	"stat|statistics" => \$stat,
> -	"output|o=s" => \$filter,
> +	"output|o=s" => \$output_file,
>  	"verbose|v" => \$verbose,
>  	"ignore-unused|ignore_unused" => \$ignore_unused,
>  	"only-i915|only_i915" => \$only_i915,
> @@ -1169,9 +1301,9 @@ if ($#ARGV < 0) {
>  }
>  
>  # At least one action should be specified
> -pod2usage(1) if (!$print_used && !$filter && !$stat && !$print_unused && !$gen_report && !$check_branches);
> +pod2usage(1) if (!$print_used && !$output_file && !$stat && !$print_unused && !$gen_report && !$check_branches);
>  
> -pod2usage(1) if ($gen_report && ($print_used || $filter || $stat || $print_unused));
> +pod2usage(1) if ($gen_report && ($print_used || $output_file || $stat || $print_unused));
>  
>  my $filter_str = "";
>  my $has_filter;
> @@ -1302,8 +1434,12 @@ if ($show_files) {
>  	}
>  }
>  
> -if ($filter) {
> -	write_filtered_file($filter);
> +if ($output_file) {
> +	if ($output_file =~ /.json$/) {
> +		write_json_file($output_file);
> +	} else {
> +		write_info_file($output_file);
> +	}
>  }
>  
>  __END__
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files
  2024-02-15 10:27 ` [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files Mauro Carvalho Chehab
@ 2024-02-15 17:18   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:21 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Code coverage files are big. Add support for read/write gzipped
> files, in order to save I/O, and, depending on the disk, speeding
> up the tool.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 56 ++++++++++++++++++++++++++-----------
>  1 file changed, 40 insertions(+), 16 deletions(-)
>  mode change 100644 => 100755 scripts/code_cov_parse_info
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> old mode 100644
> new mode 100755
> index 8180b9c1f82e..50d10a30d7f9
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -6,6 +6,8 @@ use Getopt::Long;
>  BEGIN { $Pod::Usage::Formatter = 'Pod::Text::Termcap'; }
>  use Pod::Usage;
>  use Pod::Man;
> +use IO::File;
> +use IO::Zlib;
>  
>  my $prefix = qr ".*?(linux)\w*/";
>  
> @@ -341,8 +343,14 @@ sub read_json($)
>  	}
>  
>  	# Read JSON data
> -	open IN, $file or die "can't open $file";
> -	while (<IN>) {
> +	my $fh;
> +	if ($file =~ m/\.gz$/) {
> +		$fh = IO::Zlib->new($file, "rb") or die "can't open $file";
> +	} else {
> +		$fh = IO::File->new($file) or die "can't open $file";
> +	}
> +	print "reading $file...\n" if ($verbose);
> +	while (<$fh>) {
>  		my $json = eval {
>  			Cpanel::JSON::XS::decode_json($_)
>  		};
> @@ -362,7 +370,7 @@ sub read_json($)
>  			die "Unknown JSON format on file $file\n";
>  		}
>  	}
> -	close IN;
> +	$fh->close() or die "Failed to close file $file";
>  }
>  
>  sub read_info($)
> @@ -377,11 +385,16 @@ sub read_info($)
>  
>  	# First step: parse data
>  
> -	print "reading $file...\n" if ($verbose);
> -	open IN, $file or die "can't open $file";
>  	# For details on .info file format, see "man geninfo"
>  	# http://ltp.sourceforge.net/coverage/lcov/geninfo.1.php
> -	while (<IN>) {
> +	my $fh;
> +	if ($file =~ m/\.gz$/) {
> +		$fh = IO::Zlib->new($file, "rb") or die "can't open $file";
> +	} else {
> +		$fh = IO::File->new($file) or die "can't open $file";
> +	}
> +	print "reading $file...\n" if ($verbose);
> +	while (<$fh>) {
>  		# TN:<test name>
>  		if (m/^TN:(.*)/) {
>  			if ($1 ne $cur_test) {
> @@ -568,7 +581,7 @@ sub read_info($)
>  		printf("Warning: invalid line: $_");
>  	}
>  
> -	close IN or die;
> +	$fh->close() or die "Failed to close file $file";
>  }
>  
>  sub sort_where($$)
> @@ -598,9 +611,15 @@ sub write_json_file($)
>  		Cpanel::JSON::XS::encode_json(\%record)
>  	};
>  
> -	open OUT, ">$fname" or die "Can't open $fname";
> -	print OUT $data or die "Failed to write to $fname";
> -	close OUT or die "Failed to close to $fname";
> +	if ($fname =~ m/\.gz$/) {
> +		my $fh = IO::Zlib->new($fname, "wb") or die "can't open $fname";
> +		print $fh $data or die "Failed to write to $fname";
> +		$fh->close or die "Failed to write to $fname";
> +	} else {
> +		open OUT, ">$fname" or die "Can't open $fname";
> +		print OUT $data or die "Failed to write to $fname";
> +		close OUT or die "Failed to close to $fname";
> +	}
>  }
>  
>  sub write_info_file($)
> @@ -648,12 +667,17 @@ sub write_info_file($)
>  				$data .= "BRDA:$ln,0,$i,$taken\n";
>  			}
>  		}
> -
>  		$data .= "end_of_record\n";
>  	}
> -	open OUT, ">$fname" or die "Can't open $fname";
> -	print OUT $data or die "Failed to write to $fname";
> -	close OUT or die "Failed to close to $fname";
> +	if ($fname =~ m/\.gz$/) {
> +		my $fh = IO::Zlib->new($fname, "wb") or die "can't open $fname";
> +		print $fh $data or die "Failed to write to $fname";
> +		$fh->close or die "Failed to write to $fname";
> +	} else {
> +		open OUT, ">$fname" or die "Can't open $fname";
> +		print OUT $data or die "Failed to write to $fname";
> +		close OUT or die "Failed to close to $fname";
> +	}
>  }
>  
>  sub print_code_coverage($$$)
> @@ -1354,7 +1378,7 @@ if ($ignore_unused) {
>  }
>  
>  foreach my $f (@ARGV) {
> -	if ($f =~ /.json$/) {
> +	if ($f =~ /\.json(\.gz)?$/) {
>  		read_json($f);
>  	} else {
>  		read_info($f);
> @@ -1435,7 +1459,7 @@ if ($show_files) {
>  }
>  
>  if ($output_file) {
> -	if ($output_file =~ /.json$/) {
> +	if ($output_file =~ /.json(.gz)?$/) {
>  		write_json_file($output_file);
>  	} else {
>  		write_info_file($output_file);
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero
  2024-02-15 10:27 ` [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero Mauro Carvalho Chehab
@ 2024-02-15 17:19   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:19 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:22 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> The lcov's info format has a field to be used for report branch
> block. This is unused on json format (newer gcc), being always
> zero. JSON doesn't contain it, so the parser doesn't support
> it to be non-zero anymore. Print a warning if this happens.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 50d10a30d7f9..ea4f64b34d2f 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -519,6 +519,10 @@ sub read_info($)
>  			my $branch = $3;
>  			my $taken = $4;
>  
> +			if ($block != 0) {
> +				print "Warning: unexpected block $block at line $.\n";
> +			}
> +
>  			my $where = "$ln,$block,$branch";
>  
>  			$taken = 0 if ($taken eq '-');
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering
  2024-02-15 10:27 ` [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering Mauro Carvalho Chehab
@ 2024-02-15 17:21   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:23 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Add an option to filter out branches that aren't associated with
> any functions inside the file (e. g. they came from #inline
> directives).
> 
> While here, also address some issues at the branch report.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 71 ++++++++++++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 20 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index ea4f64b34d2f..57bc4c2f0b4b 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -25,6 +25,8 @@ my @func_exclude_regexes;
>  my %test_names;
>  my @src_include_regexes;
>  my @src_exclude_regexes;
> +my $can_filter_lines = 1;
> +my $ignore_lines_without_functions = 1;
>  
>  my $verbose = 0;
>  my $ignore_unused = 0;
> @@ -161,12 +163,6 @@ sub parse_json_gcov_v1($$)
>  
>  			my $func = $line_ref->{'function_name'};
>  			if (!$func) {
> -				# Ignore DA/BRDA that aren't associated with
> -				# functions. Those are present on header files
> -				# (maybe defines?)
> -				next if (@func_include_regexes);
> -
> -				# Otherwise place them in separate
>  				$func = $before_sf;
>  			} else {
>  				next if is_function_excluded($func);
> @@ -188,9 +184,17 @@ sub parse_json_gcov_v1($$)
>  
>  			my $i = 0;
>  			for my $branch_ref (@{$line_ref->{'branches'}}) {
> -				my $taken = $branch_ref->{'count'};
>  				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
>  
> +				if ($func eq $before_sf) {
> +					# Ignore DA/BRDA that aren't associated with
> +					# functions. Those are present on header files
> +					# (maybe defines?)
> +					next if ($ignore_lines_without_functions);
> +				} else {
> +					$all_branch{$source}{$where}{func} = $func;
> +				}
> +
>  				# Negative gcov results are possible, as
>  				# reported at:
>  				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> @@ -206,7 +210,8 @@ sub parse_json_gcov_v1($$)
>  					}
>  				}
>  
> -				$all_branch{$source}{$where}{count} += $taken;
> +				$all_branch{$source}{$where}{count} += $branch_ref->{'count'};
> +
>  				$i++;
>  			}
>  			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
> @@ -383,6 +388,10 @@ sub read_info($)
>  	my $func = $before_sf;
>  	my $cur_test = "";
>  
> +	# Info files don't contain functions for lines. So, they can't
> +	# be used to filter lines and branches used inside functions.
> +	$can_filter_lines = 0;
> +
>  	# First step: parse data
>  
>  	# For details on .info file format, see "man geninfo"
> @@ -1231,18 +1240,15 @@ sub check_source_branches()
>  				return;
>  			}
>  
> +			my $func_ln;
>  			my $func = $all_branch{$source}{$where}{func};
> -			my $func_ln = $all_branch{$source}{$where}{func_ln};
> -
> -			print "Branch $source:$ln, ";
>  			if ($func) {
> -				if ($func_ln) {
> -					print "func: $func, ";
> -				} else {
> -					print "func: $func:$func_ln, ";
> -				}
> +				$func_ln = $all_func{$func}{$source}->{ln};
>  			}
> -			print "block $block, branch $branch not taken:\n";
> +
> +			print "Branch $branch on $source:$ln";
> +			print ", function: $func" if ($func);
> +			print " not taken:\n";
>  
>  			if ($func_ln) {
>  				$ctx_lines = $ln - $func_ln;
> @@ -1250,14 +1256,14 @@ sub check_source_branches()
>  				$ctx_lines = 10;
>  			}
>  
> -
>  			# Search for up to $ctx_lines before the occurrence
>  			my $context = "";
>  			my $has_if;
>  			$ln-- if ($ln > 0);
>  			my $delim = "->";
>  			for (my $if_ln = $ln; $if_ln >= 0 && (($ln - $if_ln) < $ctx_lines); $if_ln--) {
> -				$context = "$if_ln$delim\t$lines[$if_ln]" . $context;
> +				my $cur_ln = $if_ln + 1;
> +				$context = "$cur_ln$delim\t$lines[$if_ln]" . $context;
>  				$delim = "";
>  				if ($lines[$if_ln] =~ m/(\bif\b|#if)/) {
>  					$has_if = 1;
> @@ -1267,7 +1273,8 @@ sub check_source_branches()
>  			if ($has_if) {
>  				print $context;
>  			} else {
> -				print "$ln->\t$lines[$ln]";
> +				my $cur_ln = $ln + 1;
> +				print "$cur_ln->\t$lines[$ln]";
>  			}
>  		}
>  	}
> @@ -1308,6 +1315,7 @@ GetOptions(
>  	"source-filters|S=s" => \$src_filters,
>  	"include-source=s" => \@src_include_regexes,
>  	"exclude-source=s" => \@src_exclude_regexes,
> +	"ignore-lines-without-functions!" => \$ignore_lines_without_functions,
>  	"show-files|show_files" => \$show_files,
>  	"show-lines|show_lines" => \$show_lines,
>  	"report|r=s" => \$gen_report,
> @@ -1438,6 +1446,10 @@ print_summary() if ($stat);
>  if ($has_filter) {
>  	my $percent = 100. * $stats{"used_files"} / $stats{"all_files"};
>  
> +	if (!$can_filter_lines) {
> +		printf "Warning......: Function filters won't work with lines/branches\n";
> +	}
> +
>  	printf "Filters......:%s.\n", $filter_str;
>  	printf "Source files.: %.2f%% (%d of %d total)",
>  		$percent, $stats{"used_files"}, $stats{"all_files"};
> @@ -1696,6 +1708,25 @@ report and decrease the code coverage statistics.
>  
>  This option is automaticaly enabled when B<--func-filters> is used.
>  
> +=item B<--ignore-lines-without-functions>
> +
> +This option works only when the input file is in JSON format.
> +
> +Basically, include files may contain several lines of codes that aren't
> +assigned to any functions inside a source file. This is a common behavior
> +for macros and inlined functions inside headers.
> +
> +When this option is selected, the branches stat won't contain any such code.
> +
> +Please notice that this is enabled by default.
> +
> +Use B<--no-ignore-lines-without-functions> to disable it.
> +
> +=item B<--no-ignore-lines-without-functions>
> +
> +Disables filtering out branches that are not associated with any functions
> +inside the source file, but were imported via includes.
> +
>  =back
>  
>  =item B<--show-files> or B<--show_files>
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists
  2024-02-15 10:27 ` [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists Mauro Carvalho Chehab
@ 2024-02-15 17:24   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:24 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:26 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Not sure why, but opening a non-exisiting file is not returning
> an error. Well, let's add another check to cover troubles, by
> identifying if no lines were filled at the cache array.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 9d043a987f44..d133ef1a18d4 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -614,6 +614,7 @@ sub read_info($)
>  					close IN;
>  				}
>  				my $nlines = scalar(@{$cached{$source}});
> +				die "File $source_dir/$source not found or it is empty. Can't filter branches\n" if (!$nlines);
>  				if ($ln > $nlines) {
>  					die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
>  					return;
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics
  2024-02-15 10:27 ` [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics Mauro Carvalho Chehab
@ 2024-02-15 17:26   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 17:26 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev, Mauro Carvalho Chehab

Hi Mauro,
On 2024-02-15 at 11:27:25 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> With fields rename at %record, the number of filtered files
> is not reflecting what it was actually there. Also, the was_used
> logic for json format was not parsing the same way as it used to
> be with info format.
> 
> Fix those.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  scripts/code_cov_parse_info | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 3ce26e545aee..9d043a987f44 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -126,7 +126,6 @@ sub parse_json_gcov_v1($$)
>  	my $file = shift;
>  	my $json = shift;
>  
> -	my $was_used = 0;
>  	my $has_func = 0;
>  	my $ignore = 0;
>  
> @@ -149,6 +148,7 @@ sub parse_json_gcov_v1($$)
>  	my %cached;
>  	for my $file_ref (@{$json->{'files'}}) {
>  		my $source = $file_ref->{'file'};
> +		my $was_used = 0;
>  
>  		$files{$source} = 1;
>  		next if is_file_excluded($source);
> @@ -181,8 +181,6 @@ sub parse_json_gcov_v1($$)
>  				$was_used = 1;
>  			}
>  		}
> -		next if ($ignore_unused && !$was_used);
> -		$used_source{$source} = 1;
>  
>  		# Parse lines and branches
>  		for my $line_ref (@{$file_ref->{'lines'}}) {
> @@ -267,6 +265,7 @@ sub parse_json_gcov_v1($$)
>  				}
>  
>  				$all_branch{$source}{$where}{count} += $branch_ref->{'count'};
> +				$was_used = 1 if ($branch_ref->{'count'} > 0);
>  
>  				$i++;
>  			}
> @@ -274,6 +273,8 @@ sub parse_json_gcov_v1($$)
>  				@{$record{files}{$source}{line}{$ln}{branches}} = ();
>  			}
>  		}
> +		next if ($ignore_unused && !$was_used);
> +		$used_source{$source} = 1;
>  	}
>  
>  	# As the record was changed, we need to use a different format name
> @@ -285,7 +286,6 @@ sub parse_json_internal_format_v1($$)
>  	my $file = shift;
>  	my $json = shift;
>  
> -	my $was_used = 0;
>  	my $has_func = 0;
>  	my $ignore = 0;
>  	my %cached;
> @@ -305,6 +305,8 @@ sub parse_json_internal_format_v1($$)
>  
>  	for my $source (keys %{$json->{'files'}}) {
>  		$files{$source} = 1;
> +		my $was_used = 0;
> +
>  		next if is_file_excluded($source);
>  
>  		my $file_ref = \%{$json->{'files'}{$source}};
> @@ -332,8 +334,6 @@ sub parse_json_internal_format_v1($$)
>  				$was_used = 1;
>  			}
>  		}
> -		next if ($ignore_unused && !$was_used);
> -		$used_source{$source} = 1;
>  
>  		# Parse lines and branches
>  		for my $ln (keys %{$file_ref->{line}}) {
> @@ -400,12 +400,15 @@ sub parse_json_internal_format_v1($$)
>  				}
>  
>  				$all_branch{$source}{$where}{count} += $taken;
> +				$was_used = 1 if ($taken > 0);
>  				$i++;
>  			}
>  			if (!defined($record{files}{$source}{line}{$ln}{branches})) {
>  				@{$record{files}{$source}{line}{$ln}{branches}} = ();
>  			}
>  		}
> +		next if ($ignore_unused && !$was_used);
> +		$used_source{$source} = 1;
>  	}
>  }
>  
> @@ -876,7 +879,7 @@ sub gen_stats()
>  
>  	# per-file coverage stats
>  	$stats{"all_files"} = scalar keys(%files);
> -	$stats{"filtered_files"} = scalar keys(%record);
> +	$stats{"filtered_files"} = scalar keys(%{$record{files}});
>  	$stats{"used_files"} = scalar keys(%used_source);
>  }
>  
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches
  2024-02-15 10:27 ` [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches Mauro Carvalho Chehab
@ 2024-02-15 18:41   ` Kamil Konieczny
  0 siblings, 0 replies; 38+ messages in thread
From: Kamil Konieczny @ 2024-02-15 18:41 UTC (permalink / raw)
  To: igt-dev; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab

Hi igt-dev,
On 2024-02-15 at 11:27:24 +0100, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Add support for passing regexes to be used to filter branches.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> ---
>  scripts/code_cov_parse_info | 208 +++++++++++++++++++++++++++++++++---
>  1 file changed, 192 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> index 57bc4c2f0b4b..3ce26e545aee 100755
> --- a/scripts/code_cov_parse_info
> +++ b/scripts/code_cov_parse_info
> @@ -22,11 +22,16 @@ my %record;
>  my %files;
>  my @func_include_regexes;
>  my @func_exclude_regexes;
> +my @branch_include_regexes;
> +my @branch_exclude_regexes;
>  my %test_names;
>  my @src_include_regexes;
>  my @src_exclude_regexes;
> +my $source_dir = ".";
>  my $can_filter_lines = 1;
>  my $ignore_lines_without_functions = 1;
> +my $ignore_branches_on_headers = 1;
> +my $has_branch_filter;
>  
>  my $verbose = 0;
>  my $ignore_unused = 0;
> @@ -87,6 +92,32 @@ sub is_file_excluded($)
>  	return 1;
>  }
>  
> +sub is_branch_excluded($)
> +{
> +	return 0 if (!@branch_include_regexes && !@branch_exclude_regexes);
> +
> +	my $branch = shift;
> +
> +	# Handle includes first, as, when there are both include and exclude
> +	# includes should take preference, as they can be overriding exclude
> +	# rules
> +	foreach my $r (@branch_include_regexes) {
> +	    return 0 if ($branch =~ m/$r/);
> +	}
> +
> +	foreach my $r (@branch_exclude_regexes) {
> +		return 1 if ($branch =~ m/$r/);
> +	}
> +
> +	# If there are no exclude regexes, only include branches that are
> +	# explicitly included.
> +	if ($#branch_exclude_regexes == 0) {
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
>  # Use something that comes before any real function
>  my $before_sf = "!!!!";
>  
> @@ -115,6 +146,7 @@ sub parse_json_gcov_v1($$)
>  	# Store test name at the record
>  	$record{tests}{$cur_test} = 1;
>  
> +	my %cached;
>  	for my $file_ref (@{$json->{'files'}}) {
>  		my $source = $file_ref->{'file'};
>  
> @@ -182,16 +214,40 @@ sub parse_json_gcov_v1($$)
>  			}
>  			$all_line{$source}{$ln} += $line_ref->{'count'};
>  
> +			if ($ignore_branches_on_headers) {
> +				next if ($source =~ m/.h$/);
--------------------------------------^
imho you should escape dot here

> +			}
> +
>  			my $i = 0;
>  			for my $branch_ref (@{$line_ref->{'branches'}}) {
>  				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
>  
> +				# Filter out branches
> +				if ($has_branch_filter) {
> +					if (!$cached{$source}) {
> +						open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
> +						push @{$cached{$source}}, <IN>;
> +						close IN;
> +					}
> +					my $nlines = scalar(@{$cached{$source}});
> +					if ($ln > $nlines) {
> +						die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
> +						return;
> +					}
> +					next if (is_branch_excluded($cached{$source}[$ln - 1]));
> +				}
> +
>  				if ($func eq $before_sf) {
>  					# Ignore DA/BRDA that aren't associated with
>  					# functions. Those are present on header files
>  					# (maybe defines?)
>  					next if ($ignore_lines_without_functions);
> +
> +					# Otherwise place them in separate
> +					$func = $before_sf;
>  				} else {
> +					next if is_function_excluded($func);
> +
>  					$all_branch{$source}{$where}{func} = $func;
>  				}
>  
> @@ -232,6 +288,7 @@ sub parse_json_internal_format_v1($$)
>  	my $was_used = 0;
>  	my $has_func = 0;
>  	my $ignore = 0;
> +	my %cached;
>  
>  	# Store the common JSON data into the record
>  	for my $key (keys %$json) {
> @@ -308,17 +365,30 @@ sub parse_json_internal_format_v1($$)
>  			}
>  			$all_line{$source}{$ln} += $line_ref->{'count'};
>  
> +			if ($ignore_branches_on_headers) {
> +				next if ($source =~ m/.h$/);
--------------------------------------^
Same here, escape dot.

> +			}
> +
>  			my $i = 0;
>  			for my $branch_ref (@{$line_ref->{'branches'}}) {
>  				my $taken = $branch_ref->{'count'};
>  				my $where = sprintf "%d,%d,%d", $ln, 0, $i;
>  
> -				# Negative gcov results are possible, as
> -				# reported at:
> -				# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67937
> -				# Lcov ignores those. So, let's do the same
> -				# here.
> -				$branch_ref->{'count'} = 0 if ($branch_ref->{'count'} < 0);
> +
> +				# Filter out branches
> +				if ($has_branch_filter) {
> +					if (!$cached{$source}) {
> +						open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
> +						push @{$cached{$source}}, <IN>;
> +						close IN;
> +					}
> +					my $nlines = scalar(@{$cached{$source}});
> +					if ($ln > $nlines) {
> +						die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
> +						return;
> +					}
> +					next if (is_branch_excluded($cached{$source}[$ln - 1]));
> +				}
>  
>  				for my $key (keys %$branch_ref) {
>  					next if (!$record{files}{$source}{line}{$ln}{branches});
> @@ -387,6 +457,7 @@ sub read_info($)
>  	my $source = $before_sf;
>  	my $func = $before_sf;
>  	my $cur_test = "";
> +	my %cached;
>  
>  	# Info files don't contain functions for lines. So, they can't
>  	# be used to filter lines and branches used inside functions.
> @@ -528,6 +599,25 @@ sub read_info($)
>  			my $branch = $3;
>  			my $taken = $4;
>  
> +			if ($ignore_branches_on_headers) {
> +				next if ($source =~ m/.h$/);
--------------------------------------^
Same here, escape dot.

With this fixed,
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> +			}
> +
> +			# Filter out branches
> +			if ($has_branch_filter) {
> +				if (!$cached{$source}) {
> +					open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't filter branches\n";
> +					push @{$cached{$source}}, <IN>;
> +					close IN;
> +				}
> +				my $nlines = scalar(@{$cached{$source}});
> +				if ($ln > $nlines) {
> +					die "$source:$ln line is bigger than the number of lines at the file ($nlines lines)\n";
> +					return;
> +				}
> +				next if (is_branch_excluded($cached{$source}[$ln - 1]));
> +			}
> +
>  			if ($block != 0) {
>  				print "Warning: unexpected block $block at line $.\n";
>  			}
> @@ -1217,7 +1307,10 @@ sub check_source_branches()
>  		next if (!$used_source{$source});
>  		next if (is_file_excluded($source));
>  
> -		my @lines;
> +		open IN, "$source_dir/$source" || die "File $source_dir/$source not found. Can't check branches\n";
> +		my @lines = <IN>;
> +		close IN;
> +
>  		foreach my $where (sort keys %{$all_branch{$source}}) {
>  			my $taken = $all_branch{$source}{$where}{count};
>  			next if ($taken > 0);
> @@ -1229,13 +1322,7 @@ sub check_source_branches()
>  			$block = $2;
>  			$branch = $3;
>  
> -			if (!@lines) {
> -				open IN, "$source" || die "File $source not found. Can't check branches\n";
> -				@lines = <IN>;
> -				close IN;
> -			}
> -
> -			if ($ln >= $#lines) {
> +			if ($ln > $#lines) {
>  				die "$source:$ln line is bigger than the number of lines at the file ($#lines lines)\n";
>  				return;
>  			}
> @@ -1292,6 +1379,7 @@ my $help;
>  my $man;
>  my $func_filters;
>  my $src_filters;
> +my $branch_filters;
>  my $show_files;
>  my $show_lines;
>  my $only_i915;
> @@ -1304,6 +1392,7 @@ GetOptions(
>  	"print-unused|u" => \$print_unused,
>  	"stat|statistics" => \$stat,
>  	"output|o=s" => \$output_file,
> +	"source-dir|source_dir=s" => \$source_dir,
>  	"verbose|v" => \$verbose,
>  	"ignore-unused|ignore_unused" => \$ignore_unused,
>  	"only-i915|only_i915" => \$only_i915,
> @@ -1312,14 +1401,18 @@ GetOptions(
>  	"func-filters|f=s" => \$func_filters,
>  	"include-func=s" => \@func_include_regexes,
>  	"exclude-func=s" => \@func_exclude_regexes,
> +	"branch-filters|f=s" => \$branch_filters,
> +	"include-branch=s" => \@branch_include_regexes,
> +	"exclude-branch=s" => \@branch_exclude_regexes,
>  	"source-filters|S=s" => \$src_filters,
>  	"include-source=s" => \@src_include_regexes,
>  	"exclude-source=s" => \@src_exclude_regexes,
>  	"ignore-lines-without-functions!" => \$ignore_lines_without_functions,
> +	"ignore-branches-on-headers!" => \$ignore_branches_on_headers,
>  	"show-files|show_files" => \$show_files,
>  	"show-lines|show_lines" => \$show_lines,
>  	"report|r=s" => \$gen_report,
> -	"check-branches" => \$check_branches,
> +	"check-branches|check_branches" => \$check_branches,
>  	"css-file|css|c=s" => \$css_file,
>  	"title|t=s" => \$title,
>  	"html-prolog|prolog=s" => \$html_prolog,
> @@ -1381,6 +1474,14 @@ if ($str) {
>  	$has_filter = 1;
>  }
>  
> +$str = open_filter_file($branch_filters, \@branch_include_regexes, \@branch_exclude_regexes);
> +if ($str) {
> +	$filter_str .= "," if ($filter_str ne "");
> +	$filter_str .= " branch regex ($str)";
> +	$has_filter = 1;
> +	$has_branch_filter = 1;
> +}
> +
>  $ignore_unused = 1 if (@func_include_regexes || @func_exclude_regexes);
>  
>  if ($ignore_unused) {
> @@ -1567,6 +1668,12 @@ Produce an output file merging all input files.
>  
>  The generated output file is affected by the applied filters.
>  
> +=item B<--source-dir> or B<--source_dir>
> +
> +Sets the source directory baseline. This is used together with other
> +options that require to parse the source files (currently, only
> +B<--check-branches).
> +
>  =item B<--only-drm> or B<--only_drm>
>  
>  Filters out includes outside the DRM subsystem, plus trace files.
> @@ -1656,6 +1763,51 @@ Include B<regex> to the function filter. Can be used multiple times.
>  Please notice that, when this filter is used, B<--ignore-unused> will be
>  automaticaly enabled, as the final goal is to report per-function usage.
>  
> +=item B<--branch-filters>  B<[filter's file]> or B<-f>  B<[filter's file]>
> +
> +Use a file containing regular expressions (regex) to filter branches.
> +
> +Each line at B<[filter's file]> may contain a new regex:
> +
> +=over 4
> +
> +- Blank lines and lines starting with B<#> will be ignored;
> +
> +- Each line of the file will be handled as a new regex;
> +
> +- If B<+regex> is used, the filter will include B<regex> to the matches;
> +
> +- If B<-regex> is used, the filter will exclude B<regex> from the matches;
> +
> +- If the line doesn't start with neither B<+> nor B<->, containing just
> +  B<regex>, the filter will include B<regex> to the matches.
> +
> +- Any whitespace/tab before or after B<regex> will be ignored.
> +
> +=back
> +
> +Include regexes are handled first, as they can override an exclude regex.
> +
> +When just include regexes are used, any branches that don't match the
> +include regular expressions from the B<[filter's file]> will be ignored.
> +
> +=item B<--include-branch> B<regex>
> +
> +Include B<regex> to the branch filter. Can be used multiple times.
> +
> +When used together with B<--branch-filters> or B<--exclude-branch>, regexes
> +here are handled first.
> +
> +Please notice that, when this filter is used, B<--ignore-unused> will be
> +automaticaly enabled, as the final goal is to report per-branch usage.
> +
> +=item B<--exclude-branch> B<regex>
> +
> +Include B<regex> to the branchtion filter. Can be used multiple times.
> +
> +Please notice that, when this filter is used, B<--ignore-unused> will be
> +automaticaly enabled, as the final goal is to report per-branch usage.
> +
>  =item B<--source-filters>  B<[filter's file]> or B<-S>  B<[filter's file]>
>  
>  Use a file containing regular expressions to filter source files.
> @@ -1727,6 +1879,30 @@ Use B<--no-ignore-lines-without-functions> to disable it.
>  Disables filtering out branches that are not associated with any functions
>  inside the source file, but were imported via includes.
>  
> +See B<--ignore-lines-without-functions> for more details.
> +
> +=item B<--ignore-branches-on-headers>
> +
> +Branches on header files are really tricky to parse, as they depend
> +on how gcc optimizes the output code. That's specially hard to use on
> +Linux Kernel, as there are lots of complex macros that can be optimized
> +on different ways. There are even some cases where the same macro sometimes
> +have zero branches, while on other cases it can contain dozen ones.
> +
> +When this option is selected, all branches inside header files will be
> +ignored.
> +
> +Please notice that this is enabled by default.
> +
> +Use B<--no-ignore-branches-on-headers> to disable this filter, preserving
> +data from all branches.
> +
> +=item B<--no-ignore-branches-on-headers>
> +
> +Disables filtering out branches that are inside header files.
> +
> +See B<--ignore-branches-on-headers> for more details.
> +
>  =back
>  
>  =item B<--show-files> or B<--show_files>
> @@ -1734,7 +1910,7 @@ inside the source file, but were imported via includes.
>  Shows the list of files that were used to produce the code coverage
>  results.
>  
> -=item B<--check-branches>
> +=item B<--check-branches> or B<--check_branches>
>  
>  Checks at the Linux Kernel source files what's the contents of the
>  branches that weren't taken. The directory should match what's
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields
  2024-02-15 17:09   ` Kamil Konieczny
@ 2024-02-15 19:55     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 38+ messages in thread
From: Mauro Carvalho Chehab @ 2024-02-15 19:55 UTC (permalink / raw)
  To: Kamil Konieczny; +Cc: igt-dev, Mauro Carvalho Chehab

On Thu, 15 Feb 2024 18:09:46 +0100
Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:

> Hi Mauro,
> On 2024-02-15 at 11:27:18 +0100, Mauro Carvalho Chehab wrote:
> > From: Mauro Carvalho Chehab <mchehab@kernel.org>
> > 
> > As we'll be using json as basis, drop support for the fields that:
> > 
> > - aren't used at the reports;
> > - aren't even present when lcov parses gcc json format.
> > 
> > Such fields are only handled on lcov for very old gcc versions.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > ---
> >  scripts/code_cov_parse_info | 33 ---------------------------------
> >  1 file changed, 33 deletions(-)
> > 
> > diff --git a/scripts/code_cov_parse_info b/scripts/code_cov_parse_info
> > index 92091215d224..038440a9c3e3 100644
> > --- a/scripts/code_cov_parse_info
> > +++ b/scripts/code_cov_parse_info
> > @@ -201,15 +201,10 @@ sub parse_info_data($)
> >  
> >  		# FNF:<number of functions found>
> >  		if (m/^FNF:(-?\d+)/) {
> > -			$record{$source}{func}{$func}{fnf} = $1;
> >  			next;
> >  		}
> >  		# FNH:<number of function hit>
> >  		if (m/^FNH:(-?\d+)/) {
> > -			my $hits = $1;
> > -			if (!defined($record{$source}{func}{$func}{fnh}) || $record{$source}{func}{$func}{fnh} < $hits) {
> > -				$record{$source}{func}{$func}{fnh} = $hits;
> > -			}
> >  			next;
> >  		}
> >  
> > @@ -240,15 +235,10 @@ sub parse_info_data($)
> >  
> >  		# BRF:<number of branches found>
> >  		if (m/^BRF:(-?\d+)/) {
> > -			$record{$source}{func}{$func}{brf} = $1;
> >  			next;
> >  		}
> >  		# BRH:<number of branches hit>
> >  		if (m/^BRH:(-?\d+)/) {
> > -			my $hits = $1;
> > -			if (!defined($record{$source}{func}{$func}{brh}) || $record{$source}{func}{$func}{brh} < $hits) {
> > -				$record{$source}{func}{$func}{brh} = $hits;
> > -			}  
> 
> Above is self-explanatory.
> 
> >  			next;
> >  		}
> >  
> > @@ -340,34 +330,11 @@ sub write_filtered_file($)
> >  				if (defined($record{$source}{func}{$func}{fnda})) {
> >  					$filtered .= "FNDA:" . $record{$source}{func}{$func}{fnda} . ",$func\n";
> >  				}
> > -				if ($record{$source}{fnf}) {
> > -					$filtered .= "FNF:". $record{$source}{func}{$func}{fnf} ."\n";
> > -				}
> > -				if ($record{$source}{fnh}) {
> > -					$filtered .= "FNH:". $record{$source}{func}{$func}{fnh} ."\n";
> > -				}
> > -			}
> >  
> > -			foreach my $ln(sort { $a <=> $b } keys %{ $record{$source}{func}{$func}{da} }) {
> > -				$filtered .= "DA:$ln," . $record{$source}{func}{$func}{da}{$ln} . "\n";  
> 
> Why are you removing DA here? It is used elswhere.
> 
> >  			}
> > -			foreach my $where(sort sort_where keys %{ $record{$source}{func}{$func}{brda} }) {
> > -				my $taken = $record{$source}{func}{$func}{brda}{$where};  
> 
> Same here, BRDA.
> 
> >  				$taken = "-" if (!$taken);
> >  				$filtered .= "BRDA:$where,$taken\n";
> >  			}
> > -			if ($record{$source}{func}{$func}{brf}) {
> > -				$filtered .= "BRF:". $record{$source}{func}{$func}{brf} ."\n";
> > -			}
> > -			if ($record{$source}{func}{$func}{brh}) {
> > -				$filtered .= "BRH:". $record{$source}{func}{$func}{brh} ."\n";
> > -			}
> > -			if ($record{$source}{func}{$func}{lf}) {
> > -				$filtered .= "LF:". $record{$source}{func}{$func}{lf} ."\n";
> > -			}
> > -			if ($record{$source}{func}{$func}{lh}) {
> > -				$filtered .= "LH:". $record{$source}{func}{$func}{lh} ."\n";  
> 
> LF and LH?
> 
> If above is correct you can add my a-b
> 
> Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Basically, lcov has support for different versions of gcov and has
some sort of backward compatibility with older versions of lcov
tool. In practice, those fields aren't either be filled anymore for
a long time or just provide duplicated (incomplete) information.

The rationale behind this patch is to remove those duplicated info
from old versions that are either duplicated or not filled anymore
by gcov/lcov versions on Ubuntu 18.04 and above[1]. 

[1] btw, this script was never tested with versions older than
what's there on Ubuntu 18.04, so what has been removed is something
that it was not really used in practice.

> 
> > -			}
> >  		}
> >  
> >  		$filtered .= "end_of_record\n";
> > -- 
> > 2.43.0
> >   

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2024-02-15 19:55 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-15 10:27 [PATCH 00/17] Improve code_cov_parse_info tool Mauro Carvalho Chehab
2024-02-15 10:27 ` [PATCH 01/17] scripts/code_cov_parse_info: Silent some messages by default Mauro Carvalho Chehab
2024-02-15 16:24   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 02/17] scripts/code_cov_parse_info: do some renames to make it more coherent Mauro Carvalho Chehab
2024-02-15 16:27   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 03/17] scripts/code_cov_parse_info: better handle include regexes Mauro Carvalho Chehab
2024-02-15 16:35   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 04/17] scripts/code_cov_parse_info: add support for filtering Xe driver data Mauro Carvalho Chehab
2024-02-15 16:36   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 05/17] scripts/code_cov_parse_info: add a tool to analyze branch coverage Mauro Carvalho Chehab
2024-02-15 16:40   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 06/17] scripts/code_cov_parse_info: prepare $all_branch to get function data Mauro Carvalho Chehab
2024-02-15 16:41   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 07/17] scripts/code_cov_parse_info: use numberic sort for line numbers Mauro Carvalho Chehab
2024-02-15 16:55   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 08/17] scripts/code_cov_parse_info: make parse_info_data more generic Mauro Carvalho Chehab
2024-02-15 17:01   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 09/17] scripts/code_cov_parse_info: stop recording/writing too old fields Mauro Carvalho Chehab
2024-02-15 17:09   ` Kamil Konieczny
2024-02-15 19:55     ` Mauro Carvalho Chehab
2024-02-15 10:27 ` [PATCH 10/17] scripts/code_cov_parse_info: add support for parsing JSON files Mauro Carvalho Chehab
2024-02-15 17:12   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 11/17] scripts/code_cov_parse_info: add an internal JSON format Mauro Carvalho Chehab
2024-02-15 17:15   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 12/17] scripts/code_cov_parse_info: add support for compressed files Mauro Carvalho Chehab
2024-02-15 17:18   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 13/17] scripts/code_cov_parse_info: warn if branch block is not zero Mauro Carvalho Chehab
2024-02-15 17:19   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 14/17] scripts/code_cov_parse_info: better handle branch filtering Mauro Carvalho Chehab
2024-02-15 17:21   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 15/17] scripts/code_cov_parse_info: add support for filtering branches Mauro Carvalho Chehab
2024-02-15 18:41   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 16/17] scripts/code_cov_parse_info: fix files statistics Mauro Carvalho Chehab
2024-02-15 17:26   ` Kamil Konieczny
2024-02-15 10:27 ` [PATCH 17/17] scripts/code_cov_parse_info: better check if source file exists Mauro Carvalho Chehab
2024-02-15 17:24   ` Kamil Konieczny
2024-02-15 10:59 ` ✓ CI.xeBAT: success for Improve code_cov_parse_info tool Patchwork
2024-02-15 11:23 ` ✓ Fi.CI.BAT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.