Re: [RFC PATCH] Mini-OS: explicitly mark symbols to be visible for apps

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, le ven. 24 nov. 2023 14:59:03 +0100, a ecrit:
> Add an EXPORT_SYMBOL() macro to explicitly mark a symbol to be visible
> for an app or library linked with Mini-OS. This enables to hide all
> other symbols from external components, avoiding any problems with
> duplicate symbol names.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

> ---
> Based on top of my previous series "Mini-OS: hide mini-os internal
> symbols".
> This is a RFC as an alternative to the map file containing all
> visible symbol names. It would add more code churn, but it has the
> large benefit of declaring visibility of a symbol at the place
> where it is defined, reducing the chance to miss a proper symbol
> visibility definition.
> This patch is including an example for the do_exit() function in
> order to prove the viability of the approach.
> In case this idea is approved, I'd setup a patch series replacing
> the second patch of above series with this patch (minus the do_exit()
> example) and the scattered EXPORT_SYMBOL() instances.

I agree on the rationale, indeed.

> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  Makefile      | 3 ++-
>  include/lib.h | 6 ++++++
>  kernel.c      | 1 +
>  mini-os.map   | 1 -
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d4768110..5dbad8ce 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -166,7 +166,8 @@ $(OBJ_DIR)/arch/x86/minios-x86%.lds:  
> arch/x86/minios-x86.lds.S
>  
>  $(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib 
> $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds mini-os.map
>       $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@
> -     $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* --keep-global-symbols=mini-os.map $@ 
> $@
> +     $(OBJCOPY) --dump-section .export_symbol=syms mini-os-kern.o
> +     $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* --keep-global-symbols=mini-os.map 
> --keep-global-symbols=syms --remove-section=.export_symbol $@ $@
>  
>  $(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O)
>       $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o 
> $@.o
> diff --git a/include/lib.h b/include/lib.h
> index dd68985a..24fd0186 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -64,6 +64,12 @@
>  #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>  #endif
>  
> +#define EXPORT_SYMBOL(sym)                    \
> +    extern typeof(sym) sym;                   \
> +    asm(".section \".export_symbol\",\"a\"\n" \
> +        "\t.ascii \""#sym"\n\"\n"             \
> +        ".previous")
> +
>  #ifdef HAVE_LIBC
>  #include <sys/queue.h>
>  #include <sys/stat.h>
> diff --git a/kernel.c b/kernel.c
> index 1f97d8dd..29eed535 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -199,3 +199,4 @@ void do_exit(void)
>          HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
>      }
>  }
> +EXPORT_SYMBOL(do_exit);
> diff --git a/mini-os.map b/mini-os.map
> index 58a3a0ee..90f02edf 100644
> --- a/mini-os.map
> +++ b/mini-os.map
> @@ -69,7 +69,6 @@ close
>  closedir
>  closelog
>  connect
> -do_exit
>  dup
>  dup2
>  err
> -- 
> 2.35.3
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.