| Message ID | 20200929194318.548707-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | compiler.h: avoid escaped section names | expand |
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 <ndesaulniers@google.com> > --- > 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/20200629205448.GA1474367@rani.riverdale.lan/ and this older one: https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ 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 <ndesaulniers@google.com> > > --- > > 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/20200629205448.GA1474367@rani.riverdale.lan/ > and this older one: > https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ > > 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 <nivedita@alum.mit.edu> 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 <ndesaulniers@google.com> > > --- > > 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/20200629205448.GA1474367@rani.riverdale.lan/ > and this older one: > https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ > > 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.
On Tue, Sep 29, 2020 at 1:25 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <nivedita@alum.mit.edu> 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 <ndesaulniers@google.com> > > > --- > > > 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/20200629205448.GA1474367@rani.riverdale.lan/ > > and this older one: > > https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ > > > > 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
On Tue, Sep 29, 2020 at 01:30:22PM -0700, Nick Desaulniers wrote: > On Tue, Sep 29, 2020 at 1:25 PM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <nivedita@alum.mit.edu> 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 <ndesaulniers@google.com> > > > > --- > > > > 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/20200629205448.GA1474367@rani.riverdale.lan/ > > > and this older one: > > > https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ > > > > > > 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 <nivedita@alum.mit.edu> 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 > > <ndesaulniers@google.com> wrote: > > > > > > On Tue, Sep 29, 2020 at 1:08 PM Arvind Sankar <nivedita@alum.mit.edu> 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 <ndesaulniers@google.com> > > > > > --- > > > > > 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/20200629205448.GA1474367@rani.riverdale.lan/ > > > > and this older one: > > > > https://lore.kernel.org/lkml/20190904181740.GA19688@gmail.com/ > > > > > > > > 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?
Hi Nick, On Tue, Sep 29, 2020 at 9:43 PM Nick Desaulniers <ndesaulniers@google.com> 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 <ndesaulniers@google.com> > --- > 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.
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
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 <ndesaulniers@google.com> --- include/linux/compiler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)