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
