[PATCH v6 1/5] export_report: Rehabilitate script

Matthew Maurer posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v6 1/5] export_report: Rehabilitate script
Posted by Matthew Maurer 1 month, 1 week ago
The `export_report.pl` script was broken [1] a while back due to a code
cleanup causing the regex to no longer match. Additionally, it assumes a
`modules.order` file containing `.ko` in a build directory with `.mod.c`
files. I cannot find when this would have been the case in the history,
as normally `.ko` files only appear in `modules.order` in installed
modules directories, and those do not contain `.mod.c` files.
This patch makes it able to report symbol usage counts for a build tree
with modules and MODVERSIONS.

Since the rest of this series will change the format of `.mod.c`, this
change fixes the script to work correctly against a current build tree.
Given that the regex no longer matches the format used in `.mod.c`, it
cannot have worked since 2019, so updating this script is purely out of
an abundance of caution. I am unsure who uses this script or for what
purpose.

* modules.order in a build directory uses .o, not .ko files. Allow .o
  files when parsing modules.order.
* The .mod.c format changed [1] how it expressed the section attribute,
  leading to a regex mismatch. Update it to match modpost.c

[1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 scripts/export_report.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index feb3d5542a62..30b5f7819086 100755
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -55,6 +55,7 @@ sub collectcfiles {
     open my $fh, '< modules.order' or die "cannot open modules.order: $!\n";
     while (<$fh>) {
 	s/\.ko$/.mod.c/;
+	s/\.o$/.mod.c/;
 	push (@file, $_)
     }
     close($fh);
@@ -120,7 +121,7 @@ foreach my $thismod (@allcfiles) {
 			next;
 		}
 		if ($state == 1) {
-			$state = 2 if ($_ =~ /__attribute__\(\(section\("__versions"\)\)\)/);
+			$state = 2 if ($_ =~ /__used __section\("__versions"\)/);
 			next;
 		}
 		if ($state == 2) {
-- 
2.47.0.rc1.288.g06298d1525-goog
Re: [PATCH v6 1/5] export_report: Rehabilitate script
Posted by Masahiro Yamada 4 weeks, 1 day ago
On Wed, Oct 16, 2024 at 1:19 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> The `export_report.pl` script was broken [1] a while back due to a code
> cleanup causing the regex to no longer match.

Instead of the link to lore, you can refer to
commit a3d0cb04f7df ("modpost: use __section in the output to *.mod.c")



> Additionally, it assumes a
> `modules.order` file containing `.ko` in a build directory with `.mod.c`
> files. I cannot find when this would have been the case in the history,
> as normally `.ko` files only appear in `modules.order` in installed
> modules directories, and those do not contain `.mod.c` files.

If necessary, you can refer to
commit f65a486821cf ("kbuild: change module.order to list *.o instead of *.ko")


As suggested, I vote for the removal since it has been broken for 5 years
since a3d0cb04f7df.



-- 
Best Regards
Masahiro Yamada
Re: [PATCH v6 1/5] export_report: Rehabilitate script
Posted by Sami Tolvanen 1 month, 1 week ago
Hi,

On Tue, Oct 15, 2024 at 4:19 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> The `export_report.pl` script was broken [1] a while back due to a code
> cleanup causing the regex to no longer match. Additionally, it assumes a
> `modules.order` file containing `.ko` in a build directory with `.mod.c`
> files. I cannot find when this would have been the case in the history,
> as normally `.ko` files only appear in `modules.order` in installed
> modules directories, and those do not contain `.mod.c` files.
> This patch makes it able to report symbol usage counts for a build tree
> with modules and MODVERSIONS.
>
> Since the rest of this series will change the format of `.mod.c`, this
> change fixes the script to work correctly against a current build tree.
> Given that the regex no longer matches the format used in `.mod.c`, it
> cannot have worked since 2019, so updating this script is purely out of
> an abundance of caution. I am unsure who uses this script or for what
> purpose.
>
> * modules.order in a build directory uses .o, not .ko files. Allow .o
>   files when parsing modules.order.
> * The .mod.c format changed [1] how it expressed the section attribute,
>   leading to a regex mismatch. Update it to match modpost.c
>
> [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/

If this script has been broken for half a decade and nobody noticed,
does anyone actually use it? If this is dead code, I would prefer to
just delete it.

Sami
Re: [PATCH v6 1/5] export_report: Rehabilitate script
Posted by Luis Chamberlain 1 month ago
On Wed, Oct 16, 2024 at 05:29:21PM -0700, Sami Tolvanen wrote:
> Hi,
> 
> On Tue, Oct 15, 2024 at 4:19 PM Matthew Maurer <mmaurer@google.com> wrote:
> >
> > The `export_report.pl` script was broken [1] a while back due to a code
> > cleanup causing the regex to no longer match. Additionally, it assumes a
> > `modules.order` file containing `.ko` in a build directory with `.mod.c`
> > files. I cannot find when this would have been the case in the history,
> > as normally `.ko` files only appear in `modules.order` in installed
> > modules directories, and those do not contain `.mod.c` files.
> > This patch makes it able to report symbol usage counts for a build tree
> > with modules and MODVERSIONS.
> >
> > Since the rest of this series will change the format of `.mod.c`, this
> > change fixes the script to work correctly against a current build tree.
> > Given that the regex no longer matches the format used in `.mod.c`, it
> > cannot have worked since 2019, so updating this script is purely out of
> > an abundance of caution. I am unsure who uses this script or for what
> > purpose.
> >
> > * modules.order in a build directory uses .o, not .ko files. Allow .o
> >   files when parsing modules.order.
> > * The .mod.c format changed [1] how it expressed the section attribute,
> >   leading to a regex mismatch. Update it to match modpost.c
> >
> > [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/
> 
> If this script has been broken for half a decade and nobody noticed,
> does anyone actually use it? If this is dead code, I would prefer to
> just delete it.

I'm in full agreement, please trace back the history down to why the
heck we have this, otherwise chuck it.

  Luis