[RFC] kbuild: move cc_c_o logic to a shell script

Commit Message

Sam Ravnborg June 9, 2014, 4:23 p.m. UTC

From b92cb72f08908a718da05318975545e988bfdaf9 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[e-mail]>
Date: Mon, 9 Jun 2014 18:03:22 +0200
Subject: [PATCH] kbuild: move cc_c_o logic to a shell script

The amount of makefile magic required when building a
.o file from a .c file had grown far too much.
Move all the logic to a shell script where it is much
easier to follow the sequence and thus easier to debug/enhance.

This transition has following impact on the current functionality:
- We no longer rebuild the kernel if the source for recordmount is changed
  The source filehas not changed for a couple of years
- A shortcut to build symtypes files is lost. It was never
  supported by the top-level Makefile anyway

Signed-off-by: Sam Ravnborg <[e-mail]>
---

The final link of vmlinux was moved out to a shell script
resulting in some much more readable code.
Try to do the same for .c => .o rule.

I would like someone with more shell-foo than me to review the
shell script.

	Sam


 scripts/Makefile.build | 115 ++++--------------------------
 scripts/kcc.sh         | 187 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+), 100 deletions(-)
 create mode 100644 scripts/kcc.sh

Comments

Michal Marek July 3, 2014, 9:49 p.m. UTC | #1

Dne 9.6.2014 18:23, Sam Ravnborg napsal(a):
> From b92cb72f08908a718da05318975545e988bfdaf9 Mon Sep 17 00:00:00 2001
> From: Sam Ravnborg <[e-mail]>
> Date: Mon, 9 Jun 2014 18:03:22 +0200
> Subject: [PATCH] kbuild: move cc_c_o logic to a shell script
> 
> The amount of makefile magic required when building a
> .o file from a .c file had grown far too much.
> Move all the logic to a shell script where it is much
> easier to follow the sequence and thus easier to debug/enhance.

That's a nice cleanup! Some review below, but I'll have to do one more pass.


> This transition has following impact on the current functionality:
> - We no longer rebuild the kernel if the source for recordmount is changed
>   The source filehas not changed for a couple of years

Yeah, that should not be an issue.


> - A shortcut to build symtypes files is lost. It was never
>   supported by the top-level Makefile anyway

There is a rule for that:
%.symtypes: %.c prepare scripts FORCE
        $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)

But it could be added to the script, e.g. if $outfile ends in .symtypes,
do just the genksyms run.


> diff --git a/scripts/kcc.sh b/scripts/kcc.sh
> new file mode 100644
> index 0000000..2b0bad6
> --- /dev/null
> +++ b/scripts/kcc.sh
> @@ -0,0 +1,187 @@
> +#!/bin/sh
> +# kernel wrapper for cc
> +# In the simple case we just call cc with the supplied arguments.
> +# But we may check the source with sparse before we call cc.
> +# And we may postprocess the source or the .o file to support
> +# module versioning and function tracing.
> +
> +# Error out on error
> +set -e
> +
> +# Print command and execute
> +# ${quiet} determine which command to print (if any)
> +run()
> +{
> +	case "${quiet}" in
> +		"silent_")
> +			shift
> +			;;
> +		"quiet_")
> +			printf "  %s \n" "$1 ${why}"
> +			shift
> +			;;
> +		*)
> +			shift
> +			echo "$*"

While there is no -n or -e option to gcc, I'd use printf just to be on
the safe side:
                        printf '%s\n' "$*"

> +	esac
> +
> +	# Run the command
> +	$*
> +}
> +
> +# Calculate symbol versions on the preprocessed source
> +# using genksyms and postprocess the output in a way
> +# that it is usable as a linker script.
> +# Then generate the .o file with the __crc_exported_symbol replaced.
> +gen_sym_types()
> +{
> +	if [ "${KBUILD_SYMTYPES}" = "y" ]; then
> +		dump_types="-T $1.symtypes"
> +	fi
> +
> +	if [ "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" = "y" ]; then
> +		mod_prefix="-s _"
> +	fi
> +
> +	if [ "${KBUILD_PRESERVE}" != "" ]; then
> +		preserve="-p";
> +		if [ -f $1.symref ]; then
> +			reference="-r $1.symref"
> +		fi
> +	fi

The -r <file>.symref should not depend on KBUILD_PRESERVE


> +
> +	out=$1.ver
> +	shift
> +
> +	${CPP} -D__GENKSYMS__ $* ${infile} | \
> +	    ${GENKSYMS} ${dump_types} ${mod_prefix} ${preserve} ${reference} > ${out}
> +
> +	# Generate <file>.o from .tmp_<file>.o using the linker to
> +	# replace the unresolved symbols __crc_exported_symbol with
> +	# the actual value of the checksum generated by genksyms
> +	${LD} ${LDFLAGS} -r -o ${outfile} ${tmpfile}.o -T ${out}
> +	rm -f ${tmpfile}.o ${out}
> +}
> +
> +# Create __mcount_loc section with pointers to all calls to mcount
> +record_mcount()
> +{
> +	# Due to recursion, we must skip empty.o.
> +	# The empty.o file is created in the make process in order to determine
> +	# the target endianness and word size. It is made before all other C
> +	# files, including recordmcount.
> +
> +	if [ "$(basename ${outfile})" = "empty.o" ]; then
> +		return
> +	fi
> +
> +	if [ "${BUILD_C_RECORDMCOUNT}" != "" ]; then
> +		if [ "${RECORDMCOUNT_WARN}" != "" ]; then

In the Makefile, there was a check for RECORDMCOUNT_WARN coming from the
make command line, but that's not a big deal. Just document it in the
changelog.


> +# Do we have to run record_mcount?
> +if [ "${CONFIG_FTRACE_MCOUNT_RECORD}" != "" ]; then
> +	if [ $(echo $* | grep -c -e "-pg") -gt 0 ]; then

case " $* " in
" -pg ")
    record_mcount
esac

This has the advantage over the Makefile version that it checks a whole
word "-pg". Also, it saves some forking.

Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 003bc26..3497a7c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -158,14 +158,9 @@  __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
 	@:
 
 # Linus' kernel sanity checking tool
-ifneq ($(KBUILD_CHECKSRC),0)
-  ifeq ($(KBUILD_CHECKSRC),2)
-    quiet_cmd_force_checksrc = CHECK   $<
-          cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
-  else
-      quiet_cmd_checksrc     = CHECK   $<
-            cmd_checksrc     = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
-  endif
+ifeq ($(KBUILD_CHECKSRC),2)
+quiet_cmd_force_checksrc = CHECK   $<
+      cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
 endif
 
 # Do section mismatch analysis for each module/built-in.o
@@ -219,108 +214,28 @@  cmd_cc_i_c       = $(CPP) $(c_flags)   -o $@ $<
 $(obj)/%.i: $(src)/%.c FORCE
 	$(call if_changed_dep,cc_i_c)
 
-cmd_gensymtypes =                                                           \
-    $(CPP) -D__GENKSYMS__ $(c_flags) $< |                                   \
-    $(GENKSYMS) $(if $(1), -T $(2))                                         \
-     $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))             \
-     $(if $(KBUILD_PRESERVE),-p)                                            \
-     -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
-
-quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
-cmd_cc_symtypes_c =                                                         \
-    set -e;                                                                 \
-    $(call cmd_gensymtypes,true,$@) >/dev/null;                             \
-    test -s $@ || rm -f $@
-
-$(obj)/%.symtypes : $(src)/%.c FORCE
-	$(call cmd,cc_symtypes_c)
-
-# C (.c) files
-# The C file is compiled and updated dependency information is generated.
-# (See cmd_cc_o_c + relevant part of rule_cc_o_c)
-
-quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
-
-ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
-
-else
-# When module versioning is enabled the following steps are executed:
-# o compile a .tmp_<file>.o from <file>.c
-# o if .tmp_<file>.o doesn't contain a __ksymtab version, i.e. does
-#   not export symbols, we just rename .tmp_<file>.o to <file>.o and
-#   are done.
-# o otherwise, we calculate symbol versions using the good old
-#   genksyms on the preprocessed source and postprocess them in a way
-#   that they are usable as a linker script
-# o generate <file>.o from .tmp_<file>.o using the linker to
-#   replace the unresolved symbols __crc_exported_symbol with
-#   the actual value of the checksum generated by genksyms
-
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
-cmd_modversions =								\
-	if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then		\
-		$(call cmd_gensymtypes,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $(@D)/.tmp_$(@F:.o=.ver);					\
-										\
-		$(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F) 			\
-			-T $(@D)/.tmp_$(@F:.o=.ver);				\
-		rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);		\
-	else									\
-		mv -f $(@D)/.tmp_$(@F) $@;					\
-	fi;
-endif
-
-ifdef CONFIG_FTRACE_MCOUNT_RECORD
-ifdef BUILD_C_RECORDMCOUNT
-ifeq ("$(origin RECORDMCOUNT_WARN)", "command line")
-  RECORDMCOUNT_FLAGS = -w
-endif
-# Due to recursion, we must skip empty.o.
-# The empty.o file is created in the make process in order to determine
-#  the target endianness and word size. It is made before all other C
-#  files, including recordmcount.
-sub_cmd_record_mcount =					\
-	if [ $(@) != "scripts/mod/empty.o" ]; then	\
-		$(objtree)/scripts/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)";	\
-	fi;
-recordmcount_source := $(srctree)/scripts/recordmcount.c \
-		    $(srctree)/scripts/recordmcount.h
-else
-sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
-	"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
-	"$(if $(CONFIG_64BIT),64,32)" \
-	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CFLAGS)" \
-	"$(LD)" "$(NM)" "$(RM)" "$(MV)" \
-	"$(if $(part-of-module),1,0)" "$(@)";
-recordmcount_source := $(srctree)/scripts/recordmcount.pl
-endif
-cmd_record_mcount = 						\
-	if [ "$(findstring -pg,$(_c_flags))" = "-pg" ]; then	\
-		$(sub_cmd_record_mcount)			\
-	fi;
-endif
-
 define rule_cc_o_c
-	$(call echo-cmd,checksrc) $(cmd_checksrc)			  \
-	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
-	$(cmd_modversions)						  \
-	$(call echo-cmd,record_mcount)					  \
-	$(cmd_record_mcount)						  \
-	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
-	                                              $(dot-target).tmp;  \
-	rm -f $(depfile);						  \
+	$(CONFIG_SHELL) $(srctree)/scripts/kcc.sh			\
+	    $< $@ $(depfile)						\
+	    $(if $(part-of-module),m,b)					\
+	    "$(echo-why)"						\
+	    $(c_flags);							\
+	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >	\
+	                                           $(dot-target).tmp;	\
+	rm -f $(depfile);						\
 	mv -f $(dot-target).tmp $(dot-target).cmd
 endef
+# cmd_cc_o_c is required by if_changed_rule to detect changes in options
+cmd_cc_o_c = $(CC) $(c_flags)
 
 # Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
+$(obj)/%.o: $(src)/%.c FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)
diff --git a/scripts/kcc.sh b/scripts/kcc.sh
new file mode 100644
index 0000000..2b0bad6
--- /dev/null
+++ b/scripts/kcc.sh
@@ -0,0 +1,187 @@ 
+#!/bin/sh
+# kernel wrapper for cc
+# In the simple case we just call cc with the supplied arguments.
+# But we may check the source with sparse before we call cc.
+# And we may postprocess the source or the .o file to support
+# module versioning and function tracing.
+
+# Error out on error
+set -e
+
+# Print command and execute
+# ${quiet} determine which command to print (if any)
+run()
+{
+	case "${quiet}" in
+		"silent_")
+			shift
+			;;
+		"quiet_")
+			printf "  %s \n" "$1 ${why}"
+			shift
+			;;
+		*)
+			shift
+			echo "$*"
+	esac
+
+	# Run the command
+	$*
+}
+
+# Calculate symbol versions on the preprocessed source
+# using genksyms and postprocess the output in a way
+# that it is usable as a linker script.
+# Then generate the .o file with the __crc_exported_symbol replaced.
+gen_sym_types()
+{
+	if [ "${KBUILD_SYMTYPES}" = "y" ]; then
+		dump_types="-T $1.symtypes"
+	fi
+
+	if [ "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" = "y" ]; then
+		mod_prefix="-s _"
+	fi
+
+	if [ "${KBUILD_PRESERVE}" != "" ]; then
+		preserve="-p";
+		if [ -f $1.symref ]; then
+			reference="-r $1.symref"
+		fi
+	fi
+
+	out=$1.ver
+	shift
+
+	${CPP} -D__GENKSYMS__ $* ${infile} | \
+	    ${GENKSYMS} ${dump_types} ${mod_prefix} ${preserve} ${reference} > ${out}
+
+	# Generate <file>.o from .tmp_<file>.o using the linker to
+	# replace the unresolved symbols __crc_exported_symbol with
+	# the actual value of the checksum generated by genksyms
+	${LD} ${LDFLAGS} -r -o ${outfile} ${tmpfile}.o -T ${out}
+	rm -f ${tmpfile}.o ${out}
+}
+
+# Create __mcount_loc section with pointers to all calls to mcount
+record_mcount()
+{
+	# Due to recursion, we must skip empty.o.
+	# The empty.o file is created in the make process in order to determine
+	# the target endianness and word size. It is made before all other C
+	# files, including recordmcount.
+
+	if [ "$(basename ${outfile})" = "empty.o" ]; then
+		return
+	fi
+
+	if [ "${BUILD_C_RECORDMCOUNT}" != "" ]; then
+		if [ "${RECORDMCOUNT_WARN}" != "" ]; then
+			warn_on_notrace="-w"
+		fi
+
+                ${objtree}/scripts/recordmcount ${warn_on_notrace} ${outfile}
+	else
+
+		mcount=${srctree}/scripts/recordmcount.pl
+		if [ "${CONFIG_CPU_BIG_ENDIAN}" != "" ]; then
+			endian=big
+		else
+			endian=little
+		fi
+		if [ "${CONFIG_64BIT}" != "" ]; then
+			bits=64
+		else
+			bits=32
+		fi
+		if [ "${module}" = "m" ]; then
+			is_module=1
+		else
+			is_module=0
+		fi
+
+		${mcount} ${ARCH} ${endian} ${bits} "${OBJDUMP}" "${OBJCOPY}" \
+		    "${CC} ${KBUILD_CFLAGS}" "${LD}" "${NM}" "${RM}" "${MV}"  \
+		    ${is_module} ${outfile}
+	fi
+}
+
+# Delete output files in case of error
+trap cleanup SIGHUP SIGINT SIGQUIT SIGTERM ERR
+cleanup()
+{
+        rm -f ${outfile}
+}
+
+#
+#
+# kcc.sh infile outfile depfile {b,m} why {options to cc}
+#  where b == built-in, m == module
+
+infile=$1
+shift
+outfile=$1
+shift
+depfile=$1
+shift
+
+# Print [M] for modules
+if [ $1 = m ]; then
+	module=m
+	modtag="[M]"
+else
+	modtag="   "
+fi
+shift
+
+why=$1
+shift
+
+# We need access to CONFIG_ symbols
+case "${KCONFIG_CONFIG}" in
+*/*)
+        . "${KCONFIG_CONFIG}"
+        ;;
+*)
+        # Force using a file from the current directory
+        . "./${KCONFIG_CONFIG}"
+esac
+
+#set -x
+
+# Run sparse (or another checker) on the input file?
+if [ "${KBUILD_CHECKSRC}" = "1" ]; then
+	cmd="${CHECK} ${CHECKFLAGS} $* ${infile}"
+	run "CHECK   ${infile}" ${cmd}
+fi
+
+# Have we enabled module versioning?
+if [ "${CONFIG_MODVERSIONS}" = "" ]; then
+	cccmd="${CC} $* -c -o ${outfile} ${infile}"
+	run "CC ${modtag}  ${outfile}" ${cccmd}
+else
+	# Module versioning is enabled
+	# If the .o file contains exported symbol calculate symbol versions
+	dir=$(dirname ${outfile})
+	file=$(basename ${outfile})
+	tmpfile=${dir}/.tmp_${file}
+
+	cccmd="${CC} $* -c -o ${tmpfile}.o ${infile}"
+	run "CC ${modtag}  ${outfile}" ${cccmd}
+
+        if ${OBJDUMP} -h ${tmpfile}.o | grep -q __ksymtab; then
+		# The .o file contains exported symbols
+		# (contains __ksymtab section)
+		# Calculate symbol versions using genksyms
+		gen_sym_types ${tmpfile} $*
+        else
+                mv -f ${tmpfile}.o ${outfile}
+        fi
+fi
+
+# Do we have to run record_mcount?
+if [ "${CONFIG_FTRACE_MCOUNT_RECORD}" != "" ]; then
+	if [ $(echo $* | grep -c -e "-pg") -gt 0 ]; then
+		record_mcount
+	fi
+fi