Re: [PATCH v2 2/5] compat: enforce distinguishable file names in symbol table

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-10-26 at 05:50 -0600, Jan Beulich wrote:
> To make it possible to tell apart the static symbols in files built a
> second for compat guest support, arrange for their source file names to

        ^ time ?

> be prefixed by a suitable path. We can't do this without explicit .file
> directives, since gcc has always been stripping paths from file names
> handed to the internally generated .file directive. However, we can
> leverage __FILE__ if we make sure the second instance gets compiled out
> of other than the very directory the wrapper sits in.
> 
> Where suitable, remove the long redundant explicit inclusions of
> xen/config.h at once.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -86,8 +86,7 @@ AFLAGS-$(clang)         += -no-integrate
>  ALL_OBJS := $(ALL_OBJS-y)
>  
>  # Get gcc to generate the dependencies for us.
> -CFLAGS-y += -MMD -MF .$(@F).d
> -DEPS = .*.d
> +CFLAGS-y += -MMD -MF $(@D)/.$(@F).d
>  
>  CFLAGS += $(CFLAGS-y)
>  
> @@ -103,6 +102,14 @@ LDFLAGS += $(LDFLAGS-y)
>  
>  include Makefile
>  
> +DEPS = .*.d
> +define gendep
> +    ifneq ($(1),$(subst /,:,$(1)))
> +        DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
> +    endif
> +endef
> +$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o))))

Is this generating a .subdir.file.d for each subdir/file.o in obj-y?

This is as a consequence of now building subdir/file.o from the parent
directory instead of recursing for some subset of files?

It seems quite inconsistent to me to have xen/arch/x86/x86_64/Makefile
building some files directly and xen/arch/x86/Makefile to be building
another subset of those files via x86_64/FOO.o. Even more so that other
than compat.o I can't see what makes many affected files (e.g. mm.o or
platform_hypercall.o) special in this regard.

Does all of that fall out from a desire to reuse __FILE__? If so I'm
inclined to suggest that -DBUILD_FILENAME_PREFIX="compat/" or whatever
would seem likely to me to end up less strange (but maybe you tried that
and it was worse?).

> +
>  # Ensure each subdirectory has exactly one trailing slash.
>  subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
>  subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -13,7 +13,7 @@ obj-y += bitops.o
>  obj-bin-y += bzimage.init.o
>  obj-bin-y += clear_page.o
>  obj-bin-y += copy_page.o
> -obj-y += compat.o
> +obj-y += compat.o x86_64/compat.o
>  obj-$(CONFIG_KEXEC) += crash.o
>  obj-y += debug.o
>  obj-y += delay.o
> @@ -25,7 +25,6 @@ obj-y += domain_page.o
>  obj-y += e820.o
>  obj-y += extable.o
>  obj-y += flushtlb.o
> -obj-y += platform_hypercall.o
>  obj-y += i387.o
>  obj-y += i8259.o
>  obj-y += io_apic.o
> @@ -37,14 +36,15 @@ obj-y += microcode_amd.o
>  obj-y += microcode_intel.o
>  # This must come after the vendor specific files.
>  obj-y += microcode.o
> -obj-y += mm.o
> +obj-y += mm.o x86_64/mm.o
>  obj-y += monitor.o
>  obj-y += mpparse.o
>  obj-y += nmi.o
>  obj-y += numa.o
>  obj-y += pci.o
>  obj-y += percpu.o
> -obj-y += physdev.o
> +obj-y += physdev.o x86_64/physdev.o
> +obj-y += platform_hypercall.o x86_64/platform_hypercall.o
>  obj-y += psr.o
>  obj-y += setup.o
>  obj-y += shutdown.o
> --- a/xen/arch/x86/x86_64/Makefile
> +++ b/xen/arch/x86/x86_64/Makefile
> @@ -2,7 +2,6 @@ subdir-y += compat
>  
>  obj-bin-y += entry.o
>  obj-bin-y += gpr_switch.o
> -obj-y += mm.o
>  obj-y += traps.o
>  obj-y += machine_kexec.o
>  obj-y += pci.o
> @@ -10,10 +9,7 @@ obj-y += acpi_mmcfg.o
>  obj-y += mmconf-fam10h.o
>  obj-y += mmconfig_64.o
>  obj-y += mmconfig-shared.o
> -obj-y += compat.o
>  obj-y += domain.o
> -obj-y += physdev.o
> -obj-y += platform_hypercall.o
>  obj-y += cpu_idle.o
>  obj-y += cpufreq.o
>  obj-bin-y += kexec_reloc.o
> --- a/xen/arch/x86/x86_64/compat.c
> +++ b/xen/arch/x86/x86_64/compat.c
> @@ -2,7 +2,8 @@
>   * compat.c
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/hypercall.h>
>  #include <compat/xen.h>
>  #include <compat/physdev.h>
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -16,7 +16,8 @@
>   * with this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/lib.h>
>  #include <xen/init.h>
>  #include <xen/mm.h>
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -2,7 +2,8 @@
>   * physdev.c
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/types.h>
>  #include <xen/guest_access.h>
>  #include <compat/xen.h>
> --- a/xen/arch/x86/x86_64/platform_hypercall.c
> +++ b/xen/arch/x86/x86_64/platform_hypercall.c
> @@ -2,7 +2,8 @@
>   * platform_hypercall.c
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/lib.h>
>  #include <compat/platform.h>
>  
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -63,7 +63,7 @@ obj-$(perfc)       += perfc.o
>  obj-$(crash_debug) += gdbstub.o
>  obj-$(xenoprof)    += xenoprof.o
>  
> -subdir-$(CONFIG_COMPAT) += compat
> +obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o
> multicall.o tmem_xen.o xlat.o)
>  
>  subdir-$(x86_64) += hvm
>  
> --- a/xen/common/compat/Makefile
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -obj-y += domain.o
> -obj-y += kernel.o
> -obj-y += memory.o
> -obj-y += multicall.o
> -obj-y += xlat.o
> -obj-y += tmem_xen.o
> --- a/xen/common/compat/domain.c
> +++ b/xen/common/compat/domain.c
> @@ -3,7 +3,8 @@
>   *
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/lib.h>
>  #include <xen/sched.h>
>  #include <xen/domain.h>
> --- a/xen/common/compat/kernel.c
> +++ b/xen/common/compat/kernel.c
> @@ -2,7 +2,8 @@
>   * kernel.c
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  #include <xen/errno.h>
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -1,4 +1,5 @@
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/types.h>
>  #include <xen/hypercall.h>
>  #include <xen/guest_access.h>
> --- a/xen/common/compat/multicall.c
> +++ b/xen/common/compat/multicall.c
> @@ -2,7 +2,8 @@
>   * multicall.c
>   */
>  
> -#include <xen/config.h>
> +asm(".file \"" __FILE__ "\"");
> +
>  #include <xen/types.h>
>  #include <xen/multicall.h>
>  #include <xen/trace.h>
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -58,7 +58,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI
>  	mv -f $@.new $@
>  
>  compat/%.i: compat/%.c Makefile
> -	$(CPP) $(filter-out -M% .%.d -include
> %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
> +	$(CPP) $(filter-out -M% %.d -include
> %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
>  
>  compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build
> -source.py
>  	mkdir -p $(@D)
> 
>