Re: [Xen-devel] [PATCH v2] build: provide option to disambiguate symbol names

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Nov 08, 2019 at 12:18:40PM +0100, Jan Beulich wrote:
> The .file assembler directives generated by the compiler do not include
> any path components (gcc) or just the ones specified on the command line
> (clang, at least version 5), and hence multiple identically named source
> files (in different directories) may produce identically named static
> symbols (in their kallsyms representation). The binary diffing algorithm
> used by xen-livepatch, however, depends on having unique symbols.
> 
> Make the ENFORCE_UNIQUE_SYMBOLS Kconfig option control the (build)
> behavior, and if enabled use objcopy to prepend the (relative to the
> xen/ subdirectory) path to the compiler invoked STT_FILE symbols. Note
> that this build option is made no longer depend on LIVEPATCH, but merely
> defaults to its setting now.
> 
> Conditionalize explicit .file directive insertion in C files where it
> exists just to disambiguate names in a less generic manner; note that
> at the same time the redundant emission of STT_FILE symbols gets
> suppressed for clang. Assembler files as well as multiply compiled C
> ones using __OBJECT_FILE__ are left alone for the time being.
> 
> Since we now expect there not to be any duplicates anymore, also don't
> force the selection of the option to 'n' anymore in allrandom.config.
> Similarly COVERAGE no longer suppresses duplicate symbol warnings if
> enforcement is in effect, which in turn allows
> SUPPRESS_DUPLICATE_SYMBOL_WARNINGS to simply depend on
> !ENFORCE_UNIQUE_SYMBOLS.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

> ---
> v2: Re-base. Conditionalize COVERAGE's select.
> 
> The clang behavior may require further tweaking if different versions
> behave differently. Alternatively we could pass two --redefine-sym
> arguments to objcopy.
> 
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -38,7 +38,7 @@ config FRAME_POINTER
>  config COVERAGE
>       bool "Code coverage support"
>       depends on !LIVEPATCH
> -     select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS
> +     select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS if !ENFORCE_UNIQUE_SYMBOLS
>       ---help---
>         Enable code coverage support.
>  
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -194,12 +194,24 @@ FORCE:
>  
>  .PHONY: clean
>  clean:: $(addprefix _clean_, $(subdir-all))
> -     rm -f *.o *~ core $(DEPS_RM)
> +     rm -f *.o .*.o.tmp *~ core $(DEPS_RM)
>  _clean_%/: FORCE
>       $(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
>  
> +SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
> +
>  %.o: %.c Makefile
> +ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
> +     $(CC) $(CFLAGS) -c $< -o $(@D)/.$(@F).tmp
> +ifeq ($(clang),y)
> +     $(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
> +else
> +     $(OBJCOPY) --redefine-sym $(<F)=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
> +endif
> +     rm -f $(@D)/.$(@F).tmp
> +else
>       $(CC) $(CFLAGS) -c $< -o $@
> +endif
>  
>  %.o: %.S Makefile
>       $(CC) $(AFLAGS) -c $< -o $@
> --- a/xen/arch/x86/x86_64/compat.c
> +++ b/xen/arch/x86/x86_64/compat.c
> @@ -2,7 +2,7 @@
>   * compat.c
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/hypercall.h>
>  #include <compat/xen.h>
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -16,7 +16,7 @@
>   * with this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/lib.h>
>  #include <xen/init.h>
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -2,7 +2,7 @@
>   * physdev.c
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/types.h>
>  #include <xen/guest_access.h>
> --- a/xen/arch/x86/x86_64/platform_hypercall.c
> +++ b/xen/arch/x86/x86_64/platform_hypercall.c
> @@ -2,7 +2,7 @@
>   * platform_hypercall.c
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/lib.h>
>  #include <compat/platform.h>
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -373,8 +373,7 @@ config FAST_SYMBOL_LOOKUP
>  
>  config ENFORCE_UNIQUE_SYMBOLS
>       bool "Enforce unique symbols"
> -     default y
> -     depends on LIVEPATCH
> +     default LIVEPATCH
>       ---help---
>         Multiple symbols with the same name aren't generally a problem
>         unless livepatching is to be used.
> @@ -387,8 +386,8 @@ config ENFORCE_UNIQUE_SYMBOLS
>         livepatch build and apply correctly.
>  
>  config SUPPRESS_DUPLICATE_SYMBOL_WARNINGS
> -     bool "Suppress duplicate symbol warnings" if !ENFORCE_UNIQUE_SYMBOLS
> -     default y if !ENFORCE_UNIQUE_SYMBOLS
> +     bool "Suppress duplicate symbol warnings"
> +     depends on !ENFORCE_UNIQUE_SYMBOLS
>       ---help---
>         Multiple symbols with the same name aren't generally a problem
>         unless Live patching is to be used, so these warnings can be
> --- a/xen/common/compat/domain.c
> +++ b/xen/common/compat/domain.c
> @@ -3,7 +3,7 @@
>   *
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/lib.h>
>  #include <xen/sched.h>
> --- a/xen/common/compat/kernel.c
> +++ b/xen/common/compat/kernel.c
> @@ -2,7 +2,7 @@
>   * kernel.c
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/init.h>
>  #include <xen/lib.h>
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -1,4 +1,4 @@
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/types.h>
>  #include <xen/hypercall.h>
> --- a/xen/common/compat/multicall.c
> +++ b/xen/common/compat/multicall.c
> @@ -2,7 +2,7 @@
>   * multicall.c
>   */
>  
> -asm(".file \"" __FILE__ "\"");
> +EMIT_FILE;
>  
>  #include <xen/types.h>
>  #include <xen/multicall.h>
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -11,7 +11,15 @@
>  
>  #ifndef __ASSEMBLY__
>  #include <xen/compiler.h>
> +
> +#if defined(CONFIG_ENFORCE_UNIQUE_SYMBOLS) || defined(__clang__)
> +# define EMIT_FILE asm ( "" )
> +#else
> +# define EMIT_FILE asm ( ".file \"" __FILE__ "\"" )
> +#endif
> +
>  #endif
> +
>  #include <asm/config.h>
>  
>  #define EXPORT_SYMBOL(var)
> --- a/xen/tools/kconfig/allrandom.config
> +++ b/xen/tools/kconfig/allrandom.config
> @@ -2,4 +2,3 @@
>  
>  CONFIG_GCOV_FORMAT_AUTODETECT=y
>  CONFIG_UBSAN=n
> -CONFIG_ENFORCE_UNIQUE_SYMBOLS=n

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel