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

[Michal Marek <mmarek@suse.cz>]

Dne 9.6.2014 18:23, Sam Ravnborg napsal(a):
> From b92cb72f08908a718da05318975545e988bfdaf9 Mon Sep 17 00:00:00 2001
> From: Sam Ravnborg <sam@ravnborg.org>
> 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