Re: [PATCH 7/8] gdb: add make-init-c script

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-05-18 4:23 p.m., Simon Marchi wrote:
> I removed the use of stamp-init because... I am not sure if it's still
> required.  I can't remember why it's needed in the first place.  If it
> is still needed, please remind me why and I'll add it back.

I can now answer myself for this.  Because of the move-if-changed, if a
dependency (e.g. infcmd.c) is more recent than init.c, the rule will
run.  move-if-changed will see that the content has not changed, so
won't update the output file init.c (and its timestamp).  Running make
again and again will run the rule again and again, for nothing.  So if
everything's built, you'll still get:

    $ make
      GEN    init.c
    make[1]: Entering directory '/home/simark/build/binutils-gdb/gdb'
    make[2]: Entering directory '/home/simark/build/binutils-gdb/gdb/doc'
    make[2]: Nothing to be done for 'all'.
    make[2]: Leaving directory '/home/simark/build/binutils-gdb/gdb/doc'
    make[2]: Entering directory '/home/simark/build/binutils-gdb/gdb/data-directory'
    make[2]: Nothing to be done for 'all'.
    make[2]: Leaving directory '/home/simark/build/binutils-gdb/gdb/data-directory'
    make[1]: Leaving directory '/home/simark/build/binutils-gdb/gdb'

I re-introduced stamp-init, here's the updated patch:


From 568ae14c5989385babf26ef87511339fa8ffeaeb Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Tue, 18 May 2021 14:55:56 -0400
Subject: [PATCH] gdb: add make-init-c script

I would like to modify how the init.c file is generated (its content).
But as it is, a bash script with multiple sed invocations in a Makefile
target, it's not very maintainable.  Replace that with a bash script
that does the same, but in a more readable way.

The Makefile rule uses the "-" prefix in front of the for loop, I
presume to ignore any error coming from the fact that xml-builtin.c and
cp-name-parser.c are not found in the srcdir (they are generated source
files).  I prefer not to blindly ignore errors, so filter these files
out of INIT_FILES instead (we already filter out other files).

There are no expected meaningful changes to the generated init.c file.
Just the _initialize_all_file declaration that is moved down and "void"
in parenthesis that is removed.

The new regular expression is a bit tighter than the existing one, it
requires the init function to be followed by exactly ` ()`.  Update
bpf-tdep.c accordingly.

gdb/ChangeLog:

	* Makefile.in (INIT_FILES_FILTER_OUT): New.
	(INIT_FILES): Use INIT_FILES_FILTER_OUT.
	(stamp-init): Use make-initc.
	(clean mostlyclean): Remove stamp-init.
	* bpf-tdep.c (_initialize_bpf_tdep): Remove "void".
	* silent-rules.mk (ECHO_INIT_C): Change.
	* make-init-c: New file.

Change-Id: I6d6b12cbccf24ab79d1219bff05df01624c684f9
---
 gdb/Makefile.in     | 49 ++++++++++++++++++++++-----------------------
 gdb/bpf-tdep.c      |  2 +-
 gdb/make-init-c     | 49 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/silent-rules.mk |  2 +-
 4 files changed, 75 insertions(+), 27 deletions(-)
 create mode 100755 gdb/make-init-c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a9fade803b0d..31c72b559ccb 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1837,8 +1837,15 @@ test-cp-name-parser$(EXEEXT): test-cp-name-parser.o $(LIBIBERTY)
 # maybe we could just require every .o file to have an initialization routine
 # of a given name (top.o -> _initialize_top, etc.).
 #
-# Formatting conventions:  The name of the _initialize_* routines must start
-# in column zero, and must not be inside #if.
+# Formatting conventions:  The name of the initialization routines must begin
+# with `_initialize_`, must start in column zero, and be followed with exactly
+# ` ()`.  For example:
+#
+# void
+# _initialize_foo ()
+# {
+#   ...
+# }
 #
 # Note that the set of files with init functions might change, or the names
 # of the functions might change, so this files needs to depend on all the
@@ -1846,33 +1853,25 @@ test-cp-name-parser$(EXEEXT): test-cp-name-parser.o $(LIBIBERTY)
 # this Makefile has generally been written, we do this indirectly, by
 # computing the list of source files from the list of object files.
 
+INIT_FILES_FILTER_OUT = \
+	cp-name-parser.o \
+	init.o \
+	version.o \
+	xml-builtin.o \
+	%_S.o \
+	%_U.o
+
 INIT_FILES = \
 	$(patsubst %.o,%.c, \
 	  $(patsubst %-exp.o,%-exp.y, \
-	    $(filter-out init.o version.o %_S.o %_U.o,\
-	      $(COMMON_OBS))))
+	    $(filter-out $(INIT_FILES_FILTER_OUT), $(COMMON_OBS))))
 
 init.c: stamp-init; @true
-stamp-init: $(INIT_FILES) config.status
-	@$(ECHO_INIT_C) echo "Making init.c"
-	@rm -f init.c-tmp init.l-tmp
-	@touch init.c-tmp
-	@-for f in $(INIT_FILES); do \
-	    sed -n -e 's/^_initialize_\([a-z_0-9A-Z]*\).*/\1/p' \
-	        $(srcdir)/$$f 2>/dev/null; \
-	done > init.l-tmp
-	@echo '/* Do not modify this file.  */' >>init.c-tmp
-	@echo '/* It is created automatically by the Makefile.  */'>>init.c-tmp
-	@echo '#include "defs.h"      /* For initialize_file_ftype.  */' >>init.c-tmp
-	@echo 'extern void initialize_all_files(void);' >>init.c-tmp
-	@sed -e 's/\(.*\)/extern initialize_file_ftype _initialize_\1;/' <init.l-tmp >>init.c-tmp
-	@echo 'void' >>init.c-tmp
-	@echo 'initialize_all_files (void)' >>init.c-tmp
-	@echo '{' >>init.c-tmp
-	@sed -e 's/\(.*\)/  _initialize_\1 ();/' <init.l-tmp >>init.c-tmp
-	@echo '}' >>init.c-tmp
-	@$(SHELL) $(srcdir)/../move-if-change init.c-tmp init.c
-	@echo stamp > stamp-init
+stamp-init: $(INIT_FILES) config.status $(srcdir)/make-init-c
+	$(ECHO_INIT_C)
+	$(SILENCE) $(srcdir)/make-init-c $(addprefix $(srcdir)/,$(INIT_FILES)) > init.c-tmp
+	$(SILENCE) $(SHELL) $(srcdir)/../move-if-change init.c-tmp init.c
+	$(SILENCE) echo stamp > stamp-init
 
 .PRECIOUS: init.c
 
@@ -1933,7 +1932,7 @@ tags: TAGS
 clean mostlyclean: $(CONFIG_CLEAN)
 	@$(MAKE) $(FLAGS_TO_PASS) DO=clean "DODIRS=$(CLEANDIRS)" subdir_do
 	rm -f *.o *.a *~ init.c-tmp init.l-tmp version.c-tmp
-	rm -f init.c stamp-init version.c stamp-version
+	rm -f init.c version.c stamp-version
 	rm -f gdb$(EXEEXT) core make.log
 	rm -f gdb[0-9]$(EXEEXT)
 	rm -f test-cp-name-parser$(EXEEXT)
diff --git a/gdb/bpf-tdep.c b/gdb/bpf-tdep.c
index 352a22198e96..a29cd90ba387 100644
--- a/gdb/bpf-tdep.c
+++ b/gdb/bpf-tdep.c
@@ -370,7 +370,7 @@ bpf_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
 void _initialize_bpf_tdep ();
 void
-_initialize_bpf_tdep (void)
+_initialize_bpf_tdep ()
 {
   register_gdbarch_init (bfd_arch_bpf, bpf_gdbarch_init);
 
diff --git a/gdb/make-init-c b/gdb/make-init-c
new file mode 100755
index 000000000000..227895f69078
--- /dev/null
+++ b/gdb/make-init-c
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2013-2021 Free Software Foundation, Inc.
+#
+# This file is part of GDB.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Usage:
+#
+#    ./make-init-c source-files > init.c-tmp
+#
+# Where SOURCE-FILES is the list of source files to extract init functions from.
+
+# Abort on command error.
+set -e
+
+init_files=("$@")
+functions=($(sed -n -e 's/^\(_initialize_[a-zA-Z0-9_]*\) ()$/\1/p' "${init_files[@]}"))
+
+echo "/* Do not modify this file.  */"
+echo "/* It is created automatically by the Makefile.  */"
+echo "#include \"defs.h\"      /* For initialize_file_ftype.  */"
+
+for f in "${functions[@]}"; do
+  echo "extern initialize_file_ftype $f;"
+done
+
+echo "void initialize_all_files ();"
+echo "void"
+echo "initialize_all_files ()"
+echo "{"
+
+for f in "${functions[@]}"; do
+  echo "  $f ();"
+done
+
+echo "}"
diff --git a/gdb/silent-rules.mk b/gdb/silent-rules.mk
index 7ed73a767c68..f7b959f8390c 100644
--- a/gdb/silent-rules.mk
+++ b/gdb/silent-rules.mk
@@ -9,7 +9,7 @@ ECHO_GEN_XML_BUILTIN = \
               @echo "  GEN    xml-builtin.c";
 ECHO_GEN_XML_BUILTIN_GENERATED = \
               @echo "  GEN    xml-builtin-generated.c";
-ECHO_INIT_C =  echo "  GEN    init.c" ||
+ECHO_INIT_C = @echo "  GEN    init.c"
 ECHO_SIGN =   @echo "  SIGN   gdb";
 ECHO_YACC =   @echo "  YACC   $@";
 ECHO_LEX  =   @echo "  LEX    $@";
-- 
2.31.1