Re: [PATCH 3/3] x86/entry: Make intra-funciton symbols properly local

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.01.2024 19:17, Andrew Cooper wrote:
> Each of these symbols are local to their main function.  By not having them
> globally visible, livepatch's binary diffing logic can reason about the
> functions properly.

I'm not happy with this, and not only because of the way you're putting
things: "globally visible" is misleading - the symbols aren't global.
What you're doing is even eliminate the local symbols from the symbol
table. Which in turn yields less easy to follow disassembly. I could
perhaps be talked into accepting this as long as we agree to not go as
far as converting labels which can be jumped to from other functions as
well. Yet even then ...

> @@ -859,9 +859,9 @@ handle_exception_saved:
>  #endif
>  
>  /* No special register assumptions. */
> -exception_with_ints_disabled:
> +.L_exception_with_ints_disabled:
>          testb $3,UREGS_cs(%rsp)         # interrupts disabled outside Xen?
> -        jnz   FATAL_exception_with_ints_disabled
> +        jnz   .L_FATAL_exception_with_ints_disabled
>          movq  %rsp,%rdi
>          call  search_pre_exception_table
>          testq %rax,%rax                 # no fixup code for faulting EIP?
> @@ -891,7 +891,7 @@ exception_with_ints_disabled:
>          jmp   restore_all_xen           # return to fixup code
>  
>  /* No special register assumptions. */
> -FATAL_exception_with_ints_disabled:
> +.L_FATAL_exception_with_ints_disabled:

... perhaps with the exception of this one label I'd like to ask that
.L not be followed by an underscore. That carries no useful information,
while prefix and name are already properly separated by the difference
in case (see e.g. .Lcompat_bounce_exception).

To suggest a possible middle ground: Can we perhaps have the .L prefixes
added only conditionally upon LIVEPATCH=y (by way of a suitable macro)?

Jan