The stringification operator, `#`, in the preprocessor escapes strings.
For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
they treat section names that contain \".
The portable solution is to not use a string literal with the
preprocessor stringification operator.
In this case, since __section unconditionally uses the stringification
operator, we actually want the more verbose
__attribute__((__section__())).
Link: https://bugs.llvm.org/show_bug.cgi?id=42950
Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
Signed-off-by: Nick Desaulniers <[email protected]>
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 92ef163a7479..ac45f6d40d39 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __section("___kentry" "+" #sym ) \
+ __attribute__((__section__("___kentry+" #sym))) \
= (unsigned long)&sym;
#endif
--
2.28.0.709.gb0816b6eb0-goog
On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> they treat section names that contain \".
>
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
>
> In this case, since __section unconditionally uses the stringification
> operator, we actually want the more verbose
> __attribute__((__section__())).
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> include/linux/compiler.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 92ef163a7479..ac45f6d40d39 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> extern typeof(sym) sym; \
> static const unsigned long __kentry_##sym \
> __used \
> - __section("___kentry" "+" #sym ) \
> + __attribute__((__section__("___kentry+" #sym))) \
> = (unsigned long)&sym;
> #endif
>
> --
> 2.28.0.709.gb0816b6eb0-goog
>
There was this previous mini-thread:
https://lore.kernel.org/lkml/[email protected]/
and this older one:
https://lore.kernel.org/lkml/[email protected]/
Just for my own curiosity: how does KENTRY actually get used? grep
doesn't show any hits, and the thread from 2019 was actually going to
drop it if I read it right, and also just remove stringification from
the __section macro.
There are still other instances that need to be fixed, right?
Thanks.
On Tue, Sep 29, 2020 at 04:08:01PM -0400, Arvind Sankar wrote:
> On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> > The stringification operator, `#`, in the preprocessor escapes strings.
> > For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> > they treat section names that contain \".
> >
> > The portable solution is to not use a string literal with the
> > preprocessor stringification operator.
> >
> > In this case, since __section unconditionally uses the stringification
> > operator, we actually want the more verbose
> > __attribute__((__section__())).
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > include/linux/compiler.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 92ef163a7479..ac45f6d40d39 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > extern typeof(sym) sym; \
> > static const unsigned long __kentry_##sym \
> > __used \
> > - __section("___kentry" "+" #sym ) \
> > + __attribute__((__section__("___kentry+" #sym))) \
> > = (unsigned long)&sym;
> > #endif
> >
> > --
> > 2.28.0.709.gb0816b6eb0-goog
> >
>
> There was this previous mini-thread:
> https://lore.kernel.org/lkml/[email protected]/
> and this older one:
> https://lore.kernel.org/lkml/[email protected]/
>
> Just for my own curiosity: how does KENTRY actually get used? grep
> doesn't show any hits, and the thread from 2019 was actually going to
> drop it if I read it right, and also just remove stringification from
> the __section macro.
>
> There are still other instances that need to be fixed, right?
>
> Thanks.
Ignore the last question, I see you have separate patches for the rest.
On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> > The stringification operator, `#`, in the preprocessor escapes strings.
> > For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> > they treat section names that contain \".
> >
> > The portable solution is to not use a string literal with the
> > preprocessor stringification operator.
> >
> > In this case, since __section unconditionally uses the stringification
> > operator, we actually want the more verbose
> > __attribute__((__section__())).
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > include/linux/compiler.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 92ef163a7479..ac45f6d40d39 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > extern typeof(sym) sym; \
> > static const unsigned long __kentry_##sym \
> > __used \
> > - __section("___kentry" "+" #sym ) \
> > + __attribute__((__section__("___kentry+" #sym))) \
> > = (unsigned long)&sym;
> > #endif
> >
> > --
> > 2.28.0.709.gb0816b6eb0-goog
> >
>
> There was this previous mini-thread:
> https://lore.kernel.org/lkml/[email protected]/
> and this older one:
> https://lore.kernel.org/lkml/[email protected]/
>
> Just for my own curiosity: how does KENTRY actually get used? grep
> doesn't show any hits, and the thread from 2019 was actually going to
> drop it if I read it right, and also just remove stringification from
> the __section macro.
Oh, sorry I didn't respond on that thread; I could have sworn I ran a
grep for KENTRY back then.
$ git log -S KENTRY
Doesn't seem to get any hits, so I'm not sure what I should use for a
proper Fixes tag in the event we just remove it. Let me grab lunch,
then I'll send a v2 that just removes the KENTRY block. Thanks for
the reminder!
And I don't remember what ever happened to Joe's script for treewide
conversion of __section.
--
Thanks,
~Nick Desaulniers
On Tue, Sep 29, 2020 at 1:25 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <[email protected]> wrote:
> >
> > On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> > > they treat section names that contain \".
> > >
> > > The portable solution is to not use a string literal with the
> > > preprocessor stringification operator.
> > >
> > > In this case, since __section unconditionally uses the stringification
> > > operator, we actually want the more verbose
> > > __attribute__((__section__())).
> > >
> > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > ---
> > > include/linux/compiler.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > > index 92ef163a7479..ac45f6d40d39 100644
> > > --- a/include/linux/compiler.h
> > > +++ b/include/linux/compiler.h
> > > @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > > extern typeof(sym) sym; \
> > > static const unsigned long __kentry_##sym \
> > > __used \
> > > - __section("___kentry" "+" #sym ) \
> > > + __attribute__((__section__("___kentry+" #sym))) \
> > > = (unsigned long)&sym;
> > > #endif
> > >
> > > --
> > > 2.28.0.709.gb0816b6eb0-goog
> > >
> >
> > There was this previous mini-thread:
> > https://lore.kernel.org/lkml/[email protected]/
> > and this older one:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Just for my own curiosity: how does KENTRY actually get used? grep
> > doesn't show any hits, and the thread from 2019 was actually going to
> > drop it if I read it right, and also just remove stringification from
> > the __section macro.
>
> Oh, sorry I didn't respond on that thread; I could have sworn I ran a
> grep for KENTRY back then.
>
> $ git log -S KENTRY
Added by
b67067f1176df6ee727450546b58704e4b588563 ?
>
> Doesn't seem to get any hits, so I'm not sure what I should use for a
> proper Fixes tag in the event we just remove it. Let me grab lunch,
> then I'll send a v2 that just removes the KENTRY block. Thanks for
> the reminder!
>
> And I don't remember what ever happened to Joe's script for treewide
> conversion of __section.
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Tue, Sep 29, 2020 at 01:30:22PM -0700, Nick Desaulniers wrote:
> On Tue, Sep 29, 2020 at 1:25 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <[email protected]> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> > > > they treat section names that contain \".
> > > >
> > > > The portable solution is to not use a string literal with the
> > > > preprocessor stringification operator.
> > > >
> > > > In this case, since __section unconditionally uses the stringification
> > > > operator, we actually want the more verbose
> > > > __attribute__((__section__())).
> > > >
> > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > ---
> > > > include/linux/compiler.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > > > index 92ef163a7479..ac45f6d40d39 100644
> > > > --- a/include/linux/compiler.h
> > > > +++ b/include/linux/compiler.h
> > > > @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > > > extern typeof(sym) sym; \
> > > > static const unsigned long __kentry_##sym \
> > > > __used \
> > > > - __section("___kentry" "+" #sym ) \
> > > > + __attribute__((__section__("___kentry+" #sym))) \
> > > > = (unsigned long)&sym;
> > > > #endif
> > > >
> > > > --
> > > > 2.28.0.709.gb0816b6eb0-goog
> > > >
> > >
> > > There was this previous mini-thread:
> > > https://lore.kernel.org/lkml/[email protected]/
> > > and this older one:
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Just for my own curiosity: how does KENTRY actually get used? grep
> > > doesn't show any hits, and the thread from 2019 was actually going to
> > > drop it if I read it right, and also just remove stringification from
> > > the __section macro.
> >
> > Oh, sorry I didn't respond on that thread; I could have sworn I ran a
> > grep for KENTRY back then.
> >
> > $ git log -S KENTRY
>
> Added by
> b67067f1176df6ee727450546b58704e4b588563 ?
>
Yeah but nothing seems to have used it. I assume for LTO we use some
other technique to mark functions as used?
On Tue, Sep 29, 2020 at 1:47 PM Arvind Sankar <[email protected]> wrote:
>
> On Tue, Sep 29, 2020 at 01:30:22PM -0700, Nick Desaulniers wrote:
> > On Tue, Sep 29, 2020 at 1:25 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <[email protected]> wrote:
> > > >
> > > > On Tue, Sep 29, 2020 at 12:43:18PM -0700, Nick Desaulniers wrote:
> > > > > The stringification operator, `#`, in the preprocessor escapes strings.
> > > > > For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> > > > > they treat section names that contain \".
> > > > >
> > > > > The portable solution is to not use a string literal with the
> > > > > preprocessor stringification operator.
> > > > >
> > > > > In this case, since __section unconditionally uses the stringification
> > > > > operator, we actually want the more verbose
> > > > > __attribute__((__section__())).
> > > > >
> > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> > > > > Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> > > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > > ---
> > > > > include/linux/compiler.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > > > > index 92ef163a7479..ac45f6d40d39 100644
> > > > > --- a/include/linux/compiler.h
> > > > > +++ b/include/linux/compiler.h
> > > > > @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > > > > extern typeof(sym) sym; \
> > > > > static const unsigned long __kentry_##sym \
> > > > > __used \
> > > > > - __section("___kentry" "+" #sym ) \
> > > > > + __attribute__((__section__("___kentry+" #sym))) \
> > > > > = (unsigned long)&sym;
> > > > > #endif
> > > > >
> > > > > --
> > > > > 2.28.0.709.gb0816b6eb0-goog
> > > > >
> > > >
> > > > There was this previous mini-thread:
> > > > https://lore.kernel.org/lkml/[email protected]/
> > > > and this older one:
> > > > https://lore.kernel.org/lkml/[email protected]/
> > > >
> > > > Just for my own curiosity: how does KENTRY actually get used? grep
> > > > doesn't show any hits, and the thread from 2019 was actually going to
> > > > drop it if I read it right, and also just remove stringification from
> > > > the __section macro.
> > >
> > > Oh, sorry I didn't respond on that thread; I could have sworn I ran a
> > > grep for KENTRY back then.
> > >
> > > $ git log -S KENTRY
> >
> > Added by
> > b67067f1176df6ee727450546b58704e4b588563 ?
> >
>
> Yeah but nothing seems to have used it. I assume for LTO we use some
> other technique to mark functions as used?
Nicholas, do you recall why KENTRY was added in
b67067f1176df6ee727450546b58704e4b588563? May I remove that and the
addition to INIT_DATA from that commit?
--
Thanks,
~Nick Desaulniers
Hi Nick,
On Tue, Sep 29, 2020 at 9:43 PM Nick Desaulniers
<[email protected]> wrote:
>
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> they treat section names that contain \".
>
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
>
> In this case, since __section unconditionally uses the stringification
> operator, we actually want the more verbose
> __attribute__((__section__())).
Let's add a comment about this in the code -- otherwise we/someone
will convert it back without noticing. Also we could add another on
`__section` itself warning about this.
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
Is there a link / have we opened a bug on GCC's side too?
Thanks!
Cheers,
Miguel
From: Nick Desaulniers
> Sent: 29 September 2020 20:43
>
> The stringification operator, `#`, in the preprocessor escapes strings.
> For example, `# "foo"` becomes `"\"foo\""`. GCC and Clang differ in how
> they treat section names that contain \".
>
> The portable solution is to not use a string literal with the
> preprocessor stringification operator.
>
> In this case, since __section unconditionally uses the stringification
> operator, we actually want the more verbose
> __attribute__((__section__())).
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Fixes: commit e04462fb82f8 ("Compiler Attributes: remove uses of __attribute__ from compiler.h")
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> include/linux/compiler.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 92ef163a7479..ac45f6d40d39 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -155,7 +155,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> extern typeof(sym) sym; \
> static const unsigned long __kentry_##sym \
> __used \
> - __section("___kentry" "+" #sym ) \
> + __attribute__((__section__("___kentry+" #sym))) \
> = (unsigned long)&sym;
> #endif
I guess what this really wants is:
__section(__kentry+##sym)
but that generates an error because you can only use ## between
variable names.
Perhaps someone shouldn't have tries to be clever and not put
an unusual character in the section name.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, 2020-09-29 at 13:25 -0700, Nick Desaulniers wrote:
> And I don't remember what ever happened to Joe's script for treewide
> conversion of __section.
Nor do I but here (attached) is the script.
My recollection is there was some problem with mscros
with ## concatenation in some converted uses.
On Wed, 2020-09-30 at 08:40 -0700, Joe Perches wrote:
> On Tue, 2020-09-29 at 13:25 -0700, Nick Desaulniers wrote:
> > And I don't remember what ever happened to Joe's script for treewide
> > conversion of __section.
>
> Nor do I but here (attached) is the script.
>
> My recollection is there was some problem with mscros
> with ## concatenation in some converted uses.
I believe I have it sorted now and I've attached
a new version of the script.
It runs against -next (or any other tree) and
produces a single commit.
It converts all the various uses of
__attribute__((section(<foo>)))
to
__section("<foo>")
changes the various macros with token pasting uses
I believe appropriately as well.
Please give it a spin.
There were 4 problems as below.
With these 4 items fixed, the build works (seems to at least)
1: compiler_attributes.h needed to unquote the __section__(#S)
the old automated patch didn't apply as the file had changed
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attri>
index ea7b756b1c8f..b6fef9033c0b 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -254,7 +254,7 @@
* gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#i>
* clang: https://clang.llvm.org/docs/AttributeReference.html#section-declspec>
*/
-#define __section(S) __attribute__((__section__(#S)))
+#define __section(section) __attribute__((__section__(section)))
2: The script needed to use different token pasting for
__section(foo##bar##baz)
The script converted this to
__section("foo" ## bar ## "baz")
instead this needed to be
__section("foo" #bar "baz")
3: scripts/mod/modpost.c needed quoting of its internal __section uses:
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 69341b36f271..f882ce0d9327 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2254,7 +2254,7 @@ static void add_header(struct buffer *b, struct module *mod)
buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
buf_printf(b, "\n");
buf_printf(b, "__visible struct module __this_module\n");
- buf_printf(b, "__section(.gnu.linkonce.this_module) = {\n");
+ buf_printf(b, "__section(\".gnu.linkonce.this_module\") = {\n");
buf_printf(b, "\t.name = KBUILD_MODNAME,\n");
if (mod->has_init)
buf_printf(b, "\t.init = init_module,\n");
@@ -2308,7 +2308,7 @@ static int add_versions(struct buffer *b, struct module *mod)
buf_printf(b, "\n");
buf_printf(b, "static const struct modversion_info ____versions[]\n");
- buf_printf(b, "__used __section(__versions) = {\n");
+ buf_printf(b, "__used __section(\"__versions\") = {\n");
4: tools/ was excluded, but now needs conversion
tools/include/linux/objtool.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/include/linux/objtool.h b/tools/include/linux/objtool.h
index ab82c793c897..577f51436cf9 100644
--- a/tools/include/linux/objtool.h
+++ b/tools/include/linux/objtool.h
@@ -60,7 +60,7 @@ struct unwind_hint {
* For more information, see tools/objtool/Documentation/stack-validation.txt.
*/
#define STACK_FRAME_NON_STANDARD(func) \
- static void __used __section(.discard.func_stack_frame_non_standard) \
+ static void __used __section(".discard.func_stack_frame_non_standard") \
*__func_stack_frame_non_standard_##func = func
#else /* __ASSEMBLY__ */
Here's a new version that does not update arch/powerpc/boot/ files
to avoid the defective conversions that Nick found in powerpc.