get_abi.pl: add support for ABI valitation in runtime
cover get_abi.pl: add support for ABI valitation in runtime
Hi Greg,
I was talking today with Jonathan Cameron today about how to ensure that the ABI is not missing something.
While it would be doable to validate the ABI by searching __ATTR and similar macros around the driver, this would probably be very complex and would take a while to parse.
Yet, there's one way that should be quick and easier to implement:
Read the symbols from the current system in runtime, and check if everything is declared under Documentation/ABI.
As you know, scripts/get_abi.pl has already a search command, that would allow seeking for a symbol inside the ABI. Using a logic similar to that, but checking for all symbols under /sys is not hard to implemenent. That's what patch 1 does.
Right now, the results aren't exaustive (I opted this way for the RFC version, as otherwise there will be too many symbols that won't match the regexes generated from the What: fields).
It basically reports results where the sysfs nodename matches one or more What, but doesn't match the regex.
This implementation runs very quick on my desktop: it takes less than 2 seconds to run. So, it sounds a good start to help identifying what's missing.
One of the problems with the ABI definitions is how to define wildcards there. Different ABI declarations use different notations. For this first RFC version, it all the above as wildcards[1]:
<foo>
{foo}
[foo]
/.../
*and convert them into:
.*
[1] perhaps the better would be to just use regex on What:, as this would avoid extra heuristics at get_abi.pl, but this is somewhat OOT from this patch.
One of the first results is that some /sys symbols that are present on *lots* of sysfs nodes, but they aren't properly defined at ABI:
/sys/.*/(initstate|bind|unbind)
(there are definitions, but those aren't covering all occurrences)
Another problem it caught is that slab definitions are like:
/sys/kernel/slab/cache/alloc_calls
Instead of using a wildcard, like:
/sys/kernel/slab/*/alloc_calls
or:
/sys/kernel/slab/<cache>/alloc_calls
So, they don't match the actual symbols found at the system.
What do you think?
Regards,
Mauro
Mauro Carvalho Chehab (1):
get_abi.pl: Check for missing symbols at the ABI specs
scripts/get_abi.pl | 72 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-)
Signature · 1 line
2.31.1
RFC PATCH 1/1 get_abi.pl: Check for missing symbols at the ABI specs
Check for the symbols that exists under /sys but aren't defined at Documentation/ABI.
--- scripts/get_abi.pl | 72 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-)
scripts/get_abi.pl +70 -2
@@ -14,6 +14,7 @@ my $man = 0; my $debug = 0; my $enable_lineno = 0; my $prefix="Documentation/ABI"; +my $sysfs_prefix="/sys"; # # If true, assumes that the description is formatted with ReST
@@ -36,7 +37,7 @@ pod2usage(2) if (scalar @ARGV < 1 || @ARGV > 2); my ($cmd, $arg) = @ARGV; -pod2usage(2) if ($cmd ne "search" && $cmd ne "rest" && $cmd ne "validate"); +pod2usage(2) if ($cmd ne "search" && $cmd ne "rest" && $cmd ne "validate" && $cmd ne "undefined"); pod2usage(2) if ($cmd eq "search" && !$arg); require Data::Dumper if ($debug);
@@ -521,6 +522,68 @@ sub search_symbols { } } +my %leaf; + +my $escape_symbols = qr { ([\x01-\x08\x0e-\x1f\x21-\x29\x2b-\x2d\x3a-\x40\x7b-\xff]) }x; +sub parse_existing_sysfs { + my $file = $File::Find::name; + + my $mode = (stat($file))[2]; + return if ($mode & S_IFDIR); + + # /sys/.*/<foo> + return if ($file =~ m/(initstate|bind|unbind)/); + + my $leave = $file; + $leave =~ s,.*/,,; + + if (defined($leaf{$leave})) { + # FIXME: need to check if the path makes sense + my $what = $leaf{$leave}; + + $what =~ s/,/ /g; + + $what =~ s/\<[^\>]+\>/.*/g; + $what =~ s/\{[^\}]+\}/.*/g; + $what =~ s/\[[^\]]+\]/.*/g; + $what =~ s,/\.\.\./,/.*/,g; + $what =~ s,/\*/,/.*/,g; + + $what =~ s/\s+/ /g; + + # Escape all other symbols + $what =~ s/$escape_symbols/\\$1/g; + + foreach my $i (split / /,$what) { + if ($file =~ m#^$i$#) { +# print "$file: $i: OK!\n"; + return; + } + } + + print "$file: $leave is defined at $what\n"; + + return; + } + +# print "$file not found.\n"; +} + +sub undefined_symbols { + foreach my $what (sort keys %data) { + my $leave = $what; + $leave =~ s,.*/,,; + + if (defined($leaf{$leave})) { + $leaf{$leave} .= " " . $what; + } else { + $leaf{$leave} = $what; + } + } + + find({wanted =>\&parse_existing_sysfs, no_chdir => 1}, $sysfs_prefix); +} + # Ensure that the prefix will always end with a slash # While this is not needed for find, it makes the patch nicer # with --enable-lineno
@@ -536,7 +599,9 @@ print STDERR Data::Dumper->Dump([\%data], [qw(*data)]) if ($debug); # # Handles the command # -if ($cmd eq "search") { +if ($cmd eq "undefined") { + undefined_symbols; +} elsif ($cmd eq "search") { search_symbols; } else { if ($cmd eq "rest") {
@@ -575,6 +640,9 @@ B<rest> - output the ABI in ReST markup language B<validate> - validate the ABI contents +B<undefined> - existing symbols at the system that aren't + defined at Documentation/ABI + =back =head1 OPTIONS
Signature · 1 line
2.31.1
Quoted 5 lines · Mauro Carvalho Chehab
Hi Greg,
I was talking today with Jonathan Cameron today about how to ensure that the ABI is not missing something.
Quoted 8 lines
While it would be doable to validate the ABI by searching
__ATTRand similar macros around the driver, this would probably be very complex and would take a while to parse.Yet, there's one way that should be quick and easier to implement:
Read the symbols from the current system in runtime, and check if everything is declared under
Documentation/ABI.
Nice!
Quoted 34 lines
As you know,
scripts/get_abi.plhas already a search command, that would allow seeking for a symbol inside the ABI. Using a logic similar to that, but checking for all symbols under /sys is not hard to implemenent. That's what patch 1 does.Right now, the results aren't exaustive (I opted this way for the RFC version, as otherwise there will be too many symbols that won't match the regexes generated from the What: fields).
It basically reports results where the sysfs nodename matches one or more What, but doesn't match the regex.
This implementation runs very quick on my desktop: it takes less than 2 seconds to run. So, it sounds a good start to help identifying what's missing.
One of the problems with the ABI definitions is how to define wildcards there. Different ABI declarations use different notations. For this first RFC version, it all the above as wildcards[1]:
<foo> {foo} [foo] /.../ *and convert them into:
.*[1] perhaps the better would be to just use regex on What:, as this would avoid extra heuristics at get_abi.pl, but this is somewhat OOT from this patch.
Quoted 6 lines
One of the first results is that some /sys symbols that are present on *lots* of sysfs nodes, but they aren't properly defined at ABI:
/sys/.*/(initstate|bind|unbind)(there are definitions, but those aren't covering all occurrences)
We should fix that up.
Quoted 9 lines
Another problem it caught is that slab definitions are like:
/sys/kernel/slab/cache/alloc_callsInstead of using a wildcard, like:
/sys/kernel/slab/*/alloc_calls
or:
/sys/kernel/slab/<cache>/alloc_callsSo, they don't match the actual symbols found at the system.
Then we should also fix those up.
Quoted 1 line
What do you think?
I like this, thanks for doing this. We should fix up the text files to match what we have in a format that we can actually test for things. That will be very helpful to run on some devices so that I can go yell at driver developers :)
thanks,
greg k-h