[PATCH v2 0/7] linkage: better symbol aliasing

Mark Rutland posted 7 patches 4 years, 5 months ago
There is a newer version of this series
Documentation/asm-annotations.rst       | 11 ++--
arch/arm/lib/bitops.h                   |  8 +--
arch/arm64/include/asm/linkage.h        | 24 -------
arch/arm64/kvm/hyp/nvhe/cache.S         |  5 +-
arch/arm64/lib/clear_page.S             |  5 +-
arch/arm64/lib/copy_page.S              |  5 +-
arch/arm64/lib/memchr.S                 |  5 +-
arch/arm64/lib/memcmp.S                 |  6 +-
arch/arm64/lib/memcpy.S                 | 21 +++---
arch/arm64/lib/memset.S                 | 12 ++--
arch/arm64/lib/strchr.S                 |  6 +-
arch/arm64/lib/strcmp.S                 |  6 +-
arch/arm64/lib/strlen.S                 |  6 +-
arch/arm64/lib/strncmp.S                |  6 +-
arch/arm64/lib/strnlen.S                |  6 +-
arch/arm64/lib/strrchr.S                |  5 +-
arch/arm64/mm/cache.S                   | 35 ++++++----
arch/x86/boot/compressed/head_32.S      |  3 +-
arch/x86/boot/compressed/head_64.S      |  3 +-
arch/x86/crypto/aesni-intel_asm.S       |  4 +-
arch/x86/lib/memcpy_64.S                | 10 +--
arch/x86/lib/memmove_64.S               |  4 +-
arch/x86/lib/memset_64.S                |  6 +-
include/linux/linkage.h                 | 87 +++++++++++++++----------
tools/arch/x86/lib/memcpy_64.S          | 10 +--
tools/arch/x86/lib/memset_64.S          |  6 +-
tools/perf/util/include/linux/linkage.h | 80 +++++++++++++++--------
27 files changed, 209 insertions(+), 176 deletions(-)
[PATCH v2 0/7] linkage: better symbol aliasing
Posted by Mark Rutland 4 years, 5 months ago
This series aims to make symbol aliasing simpler and more consistent.
The basic idea is to replace SYM_FUNC_START_ALIAS(alias) and
SYM_FUNC_END_ALIAS(alias) with a new SYM_FUNC_ALIAS(alias, name), so
that e.g.

    SYM_FUNC_START(func)
    SYM_FUNC_START_ALIAS(alias1)
    SYM_FUNC_START_ALIAS(alias2)
        ... asm insns ...
    SYM_FUNC_END(func)
    SYM_FUNC_END_ALIAS(alias1)
    SYM_FUNC_END_ALIAS(alias2)
    EXPORT_SYMBOL(alias1)
    EXPORT_SYMBOL(alias2)

... can become:

    SYM_FUNC_START(name)
        ... asm insns ...
    SYM_FUNC_END(name)

    SYM_FUNC_ALIAS(alias1, func)
    EXPORT_SYMBOL(alias1)

    SYM_FUNC_ALIAS(alias2, func)
    EXPORT_SYMBOL(alias2)

This avoids repetition and hopefully make it easier to ensure
consistency (e.g. so each function has a single canonical name and
associated metadata).

I've build+boot tested arm64 defconfig without issues, and also build
tested arm/i386/x86_64 defconfig without issues. I've pushed the series
to my `linkage/alias-rework` branch on git.kernel.org, atop v5.17-rc1:

  git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git linkage/alias-rework
  https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=linkage/alias-rework

Since RFCv1 [1]:
* Drop arm64 dma alias removal (taken via arm64 for v5.17)
* Rename SYM_FUNC_LOCAL_ALIAS() -> SYM_FUNC_ALIAS_LOCAL()
* Update the tools/ copies of x86 routines
* Add preparatory fix for 32-bit arm
* Rebase to v5.17-rc1

[1] https://lore.kernel.org/r/20211206124715.4101571-1-mark.rutland@arm.com/

Thanks,
Mark.

Mark Rutland (7):
  arm: lib: remove leading whitespace in bitop macro
  linkage: add SYM_{ENTRY,START,END}_AT()
  linkage: add SYM_FUNC_ALIAS{,_LOCAL,_WEAK}()
  arm64: clean up symbol aliasing
  x86: clean up symbol aliasing
  linkage: remove SYM_FUNC_{START,END}_ALIAS()
  tools: update x86 string routines

 Documentation/asm-annotations.rst       | 11 ++--
 arch/arm/lib/bitops.h                   |  8 +--
 arch/arm64/include/asm/linkage.h        | 24 -------
 arch/arm64/kvm/hyp/nvhe/cache.S         |  5 +-
 arch/arm64/lib/clear_page.S             |  5 +-
 arch/arm64/lib/copy_page.S              |  5 +-
 arch/arm64/lib/memchr.S                 |  5 +-
 arch/arm64/lib/memcmp.S                 |  6 +-
 arch/arm64/lib/memcpy.S                 | 21 +++---
 arch/arm64/lib/memset.S                 | 12 ++--
 arch/arm64/lib/strchr.S                 |  6 +-
 arch/arm64/lib/strcmp.S                 |  6 +-
 arch/arm64/lib/strlen.S                 |  6 +-
 arch/arm64/lib/strncmp.S                |  6 +-
 arch/arm64/lib/strnlen.S                |  6 +-
 arch/arm64/lib/strrchr.S                |  5 +-
 arch/arm64/mm/cache.S                   | 35 ++++++----
 arch/x86/boot/compressed/head_32.S      |  3 +-
 arch/x86/boot/compressed/head_64.S      |  3 +-
 arch/x86/crypto/aesni-intel_asm.S       |  4 +-
 arch/x86/lib/memcpy_64.S                | 10 +--
 arch/x86/lib/memmove_64.S               |  4 +-
 arch/x86/lib/memset_64.S                |  6 +-
 include/linux/linkage.h                 | 87 +++++++++++++++----------
 tools/arch/x86/lib/memcpy_64.S          | 10 +--
 tools/arch/x86/lib/memset_64.S          |  6 +-
 tools/perf/util/include/linux/linkage.h | 80 +++++++++++++++--------
 27 files changed, 209 insertions(+), 176 deletions(-)

-- 
2.30.2

Re: [PATCH v2 0/7] linkage: better symbol aliasing
Posted by Ard Biesheuvel 4 years, 5 months ago
On Tue, 25 Jan 2022 at 12:32, Mark Rutland <mark.rutland@arm.com> wrote:
>
> This series aims to make symbol aliasing simpler and more consistent.
> The basic idea is to replace SYM_FUNC_START_ALIAS(alias) and
> SYM_FUNC_END_ALIAS(alias) with a new SYM_FUNC_ALIAS(alias, name), so
> that e.g.
>
>     SYM_FUNC_START(func)
>     SYM_FUNC_START_ALIAS(alias1)
>     SYM_FUNC_START_ALIAS(alias2)
>         ... asm insns ...
>     SYM_FUNC_END(func)
>     SYM_FUNC_END_ALIAS(alias1)
>     SYM_FUNC_END_ALIAS(alias2)
>     EXPORT_SYMBOL(alias1)
>     EXPORT_SYMBOL(alias2)
>
> ... can become:
>
>     SYM_FUNC_START(name)
>         ... asm insns ...
>     SYM_FUNC_END(name)
>
>     SYM_FUNC_ALIAS(alias1, func)
>     EXPORT_SYMBOL(alias1)
>
>     SYM_FUNC_ALIAS(alias2, func)
>     EXPORT_SYMBOL(alias2)
>
> This avoids repetition and hopefully make it easier to ensure
> consistency (e.g. so each function has a single canonical name and
> associated metadata).
>

I take it this affects the sizes of the alias ELF symbols? Does that matter?
Re: [PATCH v2 0/7] linkage: better symbol aliasing
Posted by Mark Rutland 4 years, 5 months ago
On Tue, Jan 25, 2022 at 04:28:11PM +0100, Ard Biesheuvel wrote:
> On Tue, 25 Jan 2022 at 12:32, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > This series aims to make symbol aliasing simpler and more consistent.
> > The basic idea is to replace SYM_FUNC_START_ALIAS(alias) and
> > SYM_FUNC_END_ALIAS(alias) with a new SYM_FUNC_ALIAS(alias, name), so
> > that e.g.
> >
> >     SYM_FUNC_START(func)
> >     SYM_FUNC_START_ALIAS(alias1)
> >     SYM_FUNC_START_ALIAS(alias2)
> >         ... asm insns ...
> >     SYM_FUNC_END(func)
> >     SYM_FUNC_END_ALIAS(alias1)
> >     SYM_FUNC_END_ALIAS(alias2)
> >     EXPORT_SYMBOL(alias1)
> >     EXPORT_SYMBOL(alias2)
> >
> > ... can become:
> >
> >     SYM_FUNC_START(name)
> >         ... asm insns ...
> >     SYM_FUNC_END(name)
> >
> >     SYM_FUNC_ALIAS(alias1, func)
> >     EXPORT_SYMBOL(alias1)
> >
> >     SYM_FUNC_ALIAS(alias2, func)
> >     EXPORT_SYMBOL(alias2)
> >
> > This avoids repetition and hopefully make it easier to ensure
> > consistency (e.g. so each function has a single canonical name and
> > associated metadata).
> >
> 
> I take it this affects the sizes of the alias ELF symbols? Does that matter?

The alias should be given the same size as the original symbol, unless I've
made an error. If you look at patch 3:

* In SYM_FUNC_START(name), via SYM_ENTRY_AT(name, ...), we create a local label
  for the start of the function: .L____sym_entry__##name

* In SYM_FUNC_END(name), via SYM_END_AT(name, ...), we create a local label for
  the end of the function: .L____sym_end__##name

* In SYM_FUNC_ALIAS*(alias,name), we define the start and end of the alias as
  the start and end of the original symbol using those local labels, e.g.

  | #define SYM_FUNC_ALIAS(alias, name)                                     \
  |         SYM_START_AT(alias, .L____sym_entry__##name, SYM_L_GLOBAL)      \
  |         SYM_END_AT(alias, .L____sym_end__##name, SYM_T_FUNC)

Note that:

* SYM_FUNC_START() uses SYM_START(), which uses SYM_ENTRY_AT()
* SYM_FUNC_END() uses SYM_END(), which uses SYM_END_AT()

... so the definition of tha alias is ultimately structurally identical to the
definition of the canoncial name, at least for now.

Thanks,
Mark.
Re: [PATCH v2 0/7] linkage: better symbol aliasing
Posted by Ard Biesheuvel 4 years, 5 months ago
On Tue, 25 Jan 2022 at 16:46, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Jan 25, 2022 at 04:28:11PM +0100, Ard Biesheuvel wrote:
> > On Tue, 25 Jan 2022 at 12:32, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > This series aims to make symbol aliasing simpler and more consistent.
> > > The basic idea is to replace SYM_FUNC_START_ALIAS(alias) and
> > > SYM_FUNC_END_ALIAS(alias) with a new SYM_FUNC_ALIAS(alias, name), so
> > > that e.g.
> > >
> > >     SYM_FUNC_START(func)
> > >     SYM_FUNC_START_ALIAS(alias1)
> > >     SYM_FUNC_START_ALIAS(alias2)
> > >         ... asm insns ...
> > >     SYM_FUNC_END(func)
> > >     SYM_FUNC_END_ALIAS(alias1)
> > >     SYM_FUNC_END_ALIAS(alias2)
> > >     EXPORT_SYMBOL(alias1)
> > >     EXPORT_SYMBOL(alias2)
> > >
> > > ... can become:
> > >
> > >     SYM_FUNC_START(name)
> > >         ... asm insns ...
> > >     SYM_FUNC_END(name)
> > >
> > >     SYM_FUNC_ALIAS(alias1, func)
> > >     EXPORT_SYMBOL(alias1)
> > >
> > >     SYM_FUNC_ALIAS(alias2, func)
> > >     EXPORT_SYMBOL(alias2)
> > >
> > > This avoids repetition and hopefully make it easier to ensure
> > > consistency (e.g. so each function has a single canonical name and
> > > associated metadata).
> > >
> >
> > I take it this affects the sizes of the alias ELF symbols? Does that matter?
>
> The alias should be given the same size as the original symbol, unless I've
> made an error. If you look at patch 3:
>
> * In SYM_FUNC_START(name), via SYM_ENTRY_AT(name, ...), we create a local label
>   for the start of the function: .L____sym_entry__##name
>
> * In SYM_FUNC_END(name), via SYM_END_AT(name, ...), we create a local label for
>   the end of the function: .L____sym_end__##name
>
> * In SYM_FUNC_ALIAS*(alias,name), we define the start and end of the alias as
>   the start and end of the original symbol using those local labels, e.g.
>
>   | #define SYM_FUNC_ALIAS(alias, name)                                     \
>   |         SYM_START_AT(alias, .L____sym_entry__##name, SYM_L_GLOBAL)      \
>   |         SYM_END_AT(alias, .L____sym_end__##name, SYM_T_FUNC)
>
> Note that:
>
> * SYM_FUNC_START() uses SYM_START(), which uses SYM_ENTRY_AT()
> * SYM_FUNC_END() uses SYM_END(), which uses SYM_END_AT()
>
> ... so the definition of tha alias is ultimately structurally identical to the
> definition of the canoncial name, at least for now.
>

Ah right, apologies for not looking more carefully - I assumed the
changed placement implied that the aliases had zero size.

And ultimately, I don't think there is an obviously correct answer
anyway, it's just the [apparently non-existent] change in behavior I
was curious about.
Re: [PATCH v2 0/7] linkage: better symbol aliasing
Posted by Mark Rutland 4 years, 5 months ago
On Tue, Jan 25, 2022 at 04:49:03PM +0100, Ard Biesheuvel wrote:
> On Tue, 25 Jan 2022 at 16:46, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Tue, Jan 25, 2022 at 04:28:11PM +0100, Ard Biesheuvel wrote:
> > > On Tue, 25 Jan 2022 at 12:32, Mark Rutland <mark.rutland@arm.com> wrote:
> > > >
> > > > This series aims to make symbol aliasing simpler and more consistent.
> > > > The basic idea is to replace SYM_FUNC_START_ALIAS(alias) and
> > > > SYM_FUNC_END_ALIAS(alias) with a new SYM_FUNC_ALIAS(alias, name), so
> > > > that e.g.
> > > >
> > > >     SYM_FUNC_START(func)
> > > >     SYM_FUNC_START_ALIAS(alias1)
> > > >     SYM_FUNC_START_ALIAS(alias2)
> > > >         ... asm insns ...
> > > >     SYM_FUNC_END(func)
> > > >     SYM_FUNC_END_ALIAS(alias1)
> > > >     SYM_FUNC_END_ALIAS(alias2)
> > > >     EXPORT_SYMBOL(alias1)
> > > >     EXPORT_SYMBOL(alias2)
> > > >
> > > > ... can become:
> > > >
> > > >     SYM_FUNC_START(name)
> > > >         ... asm insns ...
> > > >     SYM_FUNC_END(name)
> > > >
> > > >     SYM_FUNC_ALIAS(alias1, func)
> > > >     EXPORT_SYMBOL(alias1)
> > > >
> > > >     SYM_FUNC_ALIAS(alias2, func)
> > > >     EXPORT_SYMBOL(alias2)
> > > >
> > > > This avoids repetition and hopefully make it easier to ensure
> > > > consistency (e.g. so each function has a single canonical name and
> > > > associated metadata).
> > > >
> > >
> > > I take it this affects the sizes of the alias ELF symbols? Does that matter?
> >
> > The alias should be given the same size as the original symbol, unless I've
> > made an error. If you look at patch 3:
> >
> > * In SYM_FUNC_START(name), via SYM_ENTRY_AT(name, ...), we create a local label
> >   for the start of the function: .L____sym_entry__##name
> >
> > * In SYM_FUNC_END(name), via SYM_END_AT(name, ...), we create a local label for
> >   the end of the function: .L____sym_end__##name
> >
> > * In SYM_FUNC_ALIAS*(alias,name), we define the start and end of the alias as
> >   the start and end of the original symbol using those local labels, e.g.
> >
> >   | #define SYM_FUNC_ALIAS(alias, name)                                     \
> >   |         SYM_START_AT(alias, .L____sym_entry__##name, SYM_L_GLOBAL)      \
> >   |         SYM_END_AT(alias, .L____sym_end__##name, SYM_T_FUNC)
> >
> > Note that:
> >
> > * SYM_FUNC_START() uses SYM_START(), which uses SYM_ENTRY_AT()
> > * SYM_FUNC_END() uses SYM_END(), which uses SYM_END_AT()
> >
> > ... so the definition of tha alias is ultimately structurally identical to the
> > definition of the canoncial name, at least for now.
> >
> 
> Ah right, apologies for not looking more carefully - I assumed the
> changed placement implied that the aliases had zero size.

NP; I didn't make that clear in the cover letter, and now it's written up and
archived for future reference. :)

> And ultimately, I don't think there is an obviously correct answer
> anyway, it's just the [apparently non-existent] change in behavior I
> was curious about.

FWIW, I hadn't really considered whether we actually need that for the aliases;
it was jsut the path of least resistance implementation-wise, and I'd like to
be able to use the local labls for the bounds in future for other annotations
and sanity-checks.

Thanks,
Mark.