build failure caused by RUNTIME_CONST()

build failure caused by RUNTIME_CONST()

[Oleg Nesterov]

make bzImage results in

	undefined reference to `__start_runtime_shift_d_hash_shift'
	undefined reference to `__stop_runtime_shift_d_hash_shift'
	undefined reference to `__start_runtime_ptr_dentry_hashtable'
	undefined reference to `__stop_runtime_ptr_dentry_hashtable'

The patch below seems to fix the problem, but I didn't find any report on lkml,
so perhaps I am the only one which hits this problem? And perhaps this is because
my gcc 5.3.1 is quite old?

OTOH, I know nothing about lds magic, so I fail to understand where these
__start/stop_runtime_xxx can come from without something like the change below...

Oleg.
---

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ad6afc5c4918..6846fa6bdd81 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -913,10 +913,12 @@
 
 #define RUNTIME_NAME(t,x) runtime_##t##_##x
 
-#define RUNTIME_CONST(t,x)						\
+#define RUNTIME_CONST(t,x) _RUNTIME_CONST(RUNTIME_NAME(t,x))
+
+#define _RUNTIME_CONST(name)						\
 	. = ALIGN(8);							\
-	RUNTIME_NAME(t,x) : AT(ADDR(RUNTIME_NAME(t,x)) - LOAD_OFFSET) {	\
-		*(RUNTIME_NAME(t,x));					\
+	name : AT(ADDR(name) - LOAD_OFFSET) {	\
+		BOUNDED_SECTION_PRE_LABEL(name, name, __start_, __stop_) \
 	}
 
 /* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */

Re: build failure caused by RUNTIME_CONST()

[Linus Torvalds]

On Fri, 2 Aug 2024 at 04:45, Oleg Nesterov <oleg@redhat.com> wrote:
>
> make bzImage results in
>
>         undefined reference to `__start_runtime_shift_d_hash_shift'
>         undefined reference to `__stop_runtime_shift_d_hash_shift'
>         undefined reference to `__start_runtime_ptr_dentry_hashtable'
>         undefined reference to `__stop_runtime_ptr_dentry_hashtable'

Grr.

> The patch below seems to fix the problem, but I didn't find any report on lkml,
> so perhaps I am the only one which hits this problem? And perhaps this is because
> my gcc 5.3.1 is quite old?

It's not your gcc. It must be your linker that is old and decrepit.

> OTOH, I know nothing about lds magic, so I fail to understand where these
> __start/stop_runtime_xxx can come from without something like the change below...

So it comes from this:

    https://sourceware.org/binutils/docs/ld/Input-Section-Example.html

and in there you will find

   "If an output section’s name is the same as the input section’s name
    and is representable as a C identifier, then the linker will
    automatically see PROVIDE two symbols: __start_SECNAME and
    __stop_SECNAME, where SECNAME is the name of the section. These
    indicate the start address and end address of the output section
    respectively"

but apparently your linker doesn't do that.

I guess we'll have to go with doing this manually as in your patch,
but can you say what your linker version is so that I can curse it in
private and document it in public?

And yes, I think you must be one of the very few users of it, because
I too am not finding any other reports on lore (or with a wider google
search),

                  Linus

Re: build failure caused by RUNTIME_CONST()

[Oleg Nesterov]

On 08/02, Linus Torvalds wrote:
>
> > The patch below seems to fix the problem, but I didn't find any report on lkml,
> > so perhaps I am the only one which hits this problem? And perhaps this is because
> > my gcc 5.3.1 is quite old?
>
> It's not your gcc. It must be your linker that is old and decrepit.

Ah, sorry. Of course I understood that this must ba a linker issue, sorry for
confusion!

	$ ld -v
	GNU ld version 2.25-17.fc23

> > OTOH, I know nothing about lds magic, so I fail to understand where these
> > __start/stop_runtime_xxx can come from without something like the change below...
>
> So it comes from this:
>
>     https://sourceware.org/binutils/docs/ld/Input-Section-Example.html
>
> and in there you will find
>
>    "If an output section’s name is the same as the input section’s name
>     and is representable as a C identifier, then the linker will
>     automatically see PROVIDE two symbols: __start_SECNAME and
>     __stop_SECNAME, where SECNAME is the name of the section. These
>     indicate the start address and end address of the output section
>     respectively"

Thanks!

> but apparently your linker doesn't do that.

apparently yes,

> I guess we'll have to go with doing this manually as in your patch,

OK... so if you think it makes sense to support the old toolchains,
perhaps you can commit the "proper" patch?

I can try to polish my hack, but as I said I don't understand this magic,
and I can only test the build on x86.

Oleg.

Re: build failure caused by RUNTIME_CONST()

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

On Fri, 2 Aug 2024 at 15:10, Oleg Nesterov <oleg@redhat.com> wrote:
>
>         $ ld -v
>         GNU ld version 2.25-17.fc23

Yeah, we document that we support building with ld-2.25.  And I went
and looked into the binutils-gdb repo, and it looks like this whole
automatic start/stop symbol thing was introduced in 2.29.

> I can try to polish my hack, but as I said I don't understand this magic,
> and I can only test the build on x86.

Your hack doesn't look horrific to me - or at least no more horrific
than lds files always look.

I ended up with a slightly different approach, only because I'm
(probably entirely in vain) hoping that we might aim to use this
"standard" format of start/stop symbols, so I introduced it as some
kind of simple "NAMED_SECTION()" macro instead.

So this patch seems to work for me, and looks somewhat reasonable (if
people actually start using this and want to use different alignments,
we might have to make that alignment an argument in the future, but
let's go for a really simple macro interface for now).

Does it build and work for you with your old linker too?

                  Linus

[-- Attachment #2: 0001-runtime-constants-deal-with-old-decrepit-linkers.patch --]
[-- Type: text/x-patch, Size: 2735 bytes --]

From eb4ad08d935b5081f4c998543623fabfd2162e07 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Fri, 2 Aug 2024 18:12:06 -0700
Subject: [PATCH] runtime constants: deal with old decrepit linkers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The runtime constants linker script depended on documented linker
behavior [1]:

 "If an output section’s name is the same as the input section’s name
  and is representable as a C identifier, then the linker will
  automatically PROVIDE two symbols: __start_SECNAME and __stop_SECNAME,
  where SECNAME is the name of the section. These indicate the start
  address and end address of the output section respectively"

to just automatically define the symbol names for the bounds of the
runtime constant arrays.

It turns out that this isn't actually something we can rely on, with old
linkers not generating these automatic symbols.  It looks to have been
introduced in binutils-2.29 back in 2017, and we still support building
with versions all the way back to binutils-2.25 (from 2015).

And yes, Oleg actually seems to be using such ancient versions of
binutils.

So instead of depending on the implicit symbols from "section names
match and are representable C identifiers", just do this all manually.
It's not like it causes us any extra pain, we already have to do that
for all the other sections that we use that often have special
characters in them.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Link: https://sourceware.org/binutils/docs/ld/Input-Section-Example.html [1]
Link: https://lore.kernel.org/all/20240802114518.GA20924@redhat.com/
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 include/asm-generic/vmlinux.lds.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ad6afc5c4918..f6a99e5ab439 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -911,13 +911,11 @@
 #define CON_INITCALL							\
 	BOUNDED_SECTION_POST_LABEL(.con_initcall.init, __con_initcall, _start, _end)
 
-#define RUNTIME_NAME(t,x) runtime_##t##_##x
+#define NAMED_SECTION(name) \
+	name : AT(ADDR(name) - LOAD_OFFSET) ALIGN(8) \
+	{ BOUNDED_SECTION_PRE_LABEL(name, name, __start_, __stop_) }
 
-#define RUNTIME_CONST(t,x)						\
-	. = ALIGN(8);							\
-	RUNTIME_NAME(t,x) : AT(ADDR(RUNTIME_NAME(t,x)) - LOAD_OFFSET) {	\
-		*(RUNTIME_NAME(t,x));					\
-	}
+#define RUNTIME_CONST(t,x) NAMED_SECTION(runtime_##t##_##x)
 
 /* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
 #define KUNIT_TABLE()							\
-- 
2.46.0.rc1.5.g1eba42a021

Re: build failure caused by RUNTIME_CONST()

[Oleg Nesterov]

On 08/02, Linus Torvalds wrote:
>
> On Fri, 2 Aug 2024 at 15:10, Oleg Nesterov <oleg@redhat.com> wrote:
> >
> >         $ ld -v
> >         GNU ld version 2.25-17.fc23
>
> Yeah, we document that we support building with ld-2.25.

Where? I tried to grep Documentation/ but didn't find anything...

> Reported-by: Oleg Nesterov <oleg@redhat.com>
> Link: https://sourceware.org/binutils/docs/ld/Input-Section-Example.html [1]
> Link: https://lore.kernel.org/all/20240802114518.GA20924@redhat.com/
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Tested-by: Oleg Nesterov <oleg@redhat.com>

But,

> -#define RUNTIME_NAME(t,x) runtime_##t##_##x
> +#define NAMED_SECTION(name) \
> +	name : AT(ADDR(name) - LOAD_OFFSET) ALIGN(8) \
                                            ^^^^^^^^
it seems that all other section do

	. = ALIGN(x); name : ...

was this change intentional?

Oleg.

Re: build failure caused by RUNTIME_CONST()

[Linus Torvalds]

On Sat, 3 Aug 2024 at 05:01, Oleg Nesterov <oleg@redhat.com> wrote:
>
> it seems that all other section do
>
>         . = ALIGN(x); name : ...
>
> was this change intentional?

It was intentional, but you're right, I should probably just do the
ugly ". = ALIGN(8)" syntax to match everything else.

The ALIGN in the section definition aligns the output section, not the
input section. But I think that's redundant, with the "AT()" thing
specifying where in the output it goes. I should have used SUBALIGN()
to align the input sections.

And the ". = ALIGN()" at the head should be unnecessary too, since the
alignment should come from the input section itself, and we should
probably not specify that in the linker script, but in the section
definition.

Honestly, our linker scripts are basically voodoo, and the ALIGN
pattern we use is part of that voodoo. Part of it is that linker
script syntax is just horrendous, and part of it is that writing
linker scripts is so rare that people just mostly do it with the
"monkey see, monkey do" model of programming, ie cutting and pasting
things that they don't understand and modifying them so that it
"works".

IOW, the right thing to do is really to not have any ALIGN directive
at all, and have the alignment come from the input sections by
specifying it in the source when the section is built.

Except we don't do that either, unless it sometimes happens almost by
accident (ie when we tell the compiler to use specific section names
and alignment, then the compiler will actually do it for us).

I'll update my fix to be the minimal "don't rock the boat" fix. And
we'll continue with the nasty linker script voodoo approach.

Oh well.

          Linus

Re: build failure caused by RUNTIME_CONST()

[Linus Torvalds]

On Sat, 3 Aug 2024 at 05:01, Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/02, Linus Torvalds wrote:
> >
> > Yeah, we document that we support building with ld-2.25.
>
> Where? I tried to grep Documentation/ but didn't find anything...

It's not hugely obvious, but you can find the line

    binutils               2.25             ld -v

in Documentation/process/changes.rst under the "Current Minimal
Requirements" header.

                Linus

Re: build failure caused by RUNTIME_CONST()

[Rasmus Villemoes]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 2 Aug 2024 at 15:10, Oleg Nesterov <oleg@redhat.com> wrote:
>>
>>         $ ld -v
>>         GNU ld version 2.25-17.fc23
>
> Yeah, we document that we support building with ld-2.25.  And I went
> and looked into the binutils-gdb repo, and it looks like this whole
> automatic start/stop symbol thing was introduced in 2.29.
>

Well, the basics of it goes much further back. ld/NEWS says

Changes in version 2.6:

  * When an ELF section name is representable as a C identifier (this is not true
  of most ELF section names), the linker will automatically define symbols
  __start_SECNAME and __stop_SECNAME, where SECNAME is the section name, at the
  beginning and the end of the section.  This is used by glibc.

but then also goes on with

    Addendum: Current versions of the linker (at least for version 2.18 onwards
  and possibly much earlier as well) place two restrictions on this feature:  The
  symbols are only implemented for orphaned sections, not for explicitly placed
  sections and they are PROVIDEd rather than being defined.

and I think what was changed in 2.29 is that some of that was being
relaxed again with commit cbd0eecf261c ("Always define referenced
__start_SECNAME/__stop_SECNAME"), which is what you find running 'git
blame' on the current doc text. But yes, it does seem that the 2.29
semantics are what are needed to avoid explicitly defining the symbols
in the linker script.

Rasmus