| Message ID | 20190827204007.201890-1-ndesaulniers@google.com |
|---|---|
| Headers | show |
| Series | treewide: prefer __section from compiler_attributes.h | expand |
On Tue, 2019-08-27 at 13:39 -0700, Nick Desaulniers wrote: > GCC unescapes escaped string section names while Clang does not. Because > __section uses the `#` stringification operator for the section name, it > doesn't need to be escaped. > > This fixes an Oops observed in distro's that use systemd and not > net.core.bpf_jit_enable=1, when their kernels are compiled with Clang. > > Instead, we should: > 1. Prefer __section(.section_name_no_quotes). > 2. Only use __attribute__((__section(".section"))) when creating the Please use __ before and after section i.e. __attribute__((__section__("<section_name>")))
On Tue, Aug 27, 2019 at 10:40 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > GCC unescapes escaped string section names while Clang does not. Because > __section uses the `#` stringification operator for the section name, it > doesn't need to be escaped. > > This fixes an Oops observed in distro's that use systemd and not > net.core.bpf_jit_enable=1, when their kernels are compiled with Clang. > > Instead, we should: > 1. Prefer __section(.section_name_no_quotes). > 2. Only use __attribute__((__section(".section"))) when creating the > section name via C preprocessor (see the definition of __define_initcall > in arch/um/include/shared/init.h). > > This antipattern was found with: > $ grep -e __section\(\" -e __section__\(\" -r > Hi Nick, thanks for the v2 of your patch-series. I just checked v2 on top of Linux v5.3-rc6... sdi@iniza:~/src/linux-kernel/linux$ grep -e __section\(\" -e __section__\(\" -r include/linux/compiler_attributes.h: * __section(".foo") include/linux/compiler_attributes.h: * verbose __attribute__((__section__(".foo" x))) should be preferred. OK: Description of the problem ^^ arch/sh/include/asm/cache.h:#define __read_mostly __attribute__((__section__(".data..read_mostly"))) PATCH next-20190827 ^^ sh: prefer __section from compiler_attributes.h https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190827&id=baf58858e8b6d853a7a8308901fcdd438e92a522 arch/arm64/kernel/smp_spin_table.c:volatile unsigned long __section(".mmuoff.data.read") arch/arm64/include/asm/cache.h:#define __read_mostly __attribute__((__section__(".data..read_mostly"))) PATCH next-20190827 ^^ arm64: prefer __section from compiler_attributes.h https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190827&id=80d838122643a09a9f99824adea4b4261e4451e6 arch/um/include/shared/init.h: __attribute__((__section__(".initcall" level ".init"))) = fn ??? ^^ > See the discussions in: > https://bugs.llvm.org/show_bug.cgi?id=42950 > https://marc.info/?l=linux-netdev&m=156412960619946&w=2 > List CBL issue tracker to discussions: https://github.com/ClangBuiltLinux/linux/issues/619 - Sedat - > Changes V1 -> V2: > * drop arm64, arc, and sh patches as they were picked up by their > maintainers. > * Split the previous V1 hunk from include/linux that touched > include/linux/compiler.h off into its own patch for inclusion in > stable, as it fixes a user visible issue. > * Collect Acks and Tested by tags. > > Nick Desaulniers (14): > s390/boot: prefer __section from compiler_attributes.h > include/linux/compiler.h: prefer __section from compiler_attributes.h > parisc: prefer __section from compiler_attributes.h > um: prefer __section from compiler_attributes.h > ia64: prefer __section from compiler_attributes.h > arm: prefer __section from compiler_attributes.h > mips: prefer __section from compiler_attributes.h > sparc: prefer __section from compiler_attributes.h > powerpc: prefer __section and __printf from compiler_attributes.h > x86: prefer __section from compiler_attributes.h > include/asm-generic: prefer __section from compiler_attributes.h > include/linux: prefer __section from compiler_attributes.h > include/linux/compiler.h: remove unused KENTRY macro > compiler_attributes.h: add note about __section > > arch/arm/include/asm/cache.h | 2 +- > arch/arm/include/asm/mach/arch.h | 4 ++-- > arch/arm/include/asm/setup.h | 2 +- > arch/ia64/include/asm/cache.h | 2 +- > arch/mips/include/asm/cache.h | 2 +- > arch/parisc/include/asm/cache.h | 2 +- > arch/parisc/include/asm/ldcw.h | 2 +- > arch/powerpc/boot/main.c | 3 +-- > arch/powerpc/boot/ps3.c | 6 ++---- > arch/powerpc/include/asm/cache.h | 2 +- > arch/powerpc/kernel/btext.c | 2 +- > arch/s390/boot/startup.c | 2 +- > arch/sparc/include/asm/cache.h | 2 +- > arch/sparc/kernel/btext.c | 2 +- > arch/um/kernel/um_arch.c | 6 +++--- > arch/x86/include/asm/cache.h | 2 +- > arch/x86/include/asm/intel-mid.h | 2 +- > arch/x86/include/asm/iommu_table.h | 5 ++--- > arch/x86/include/asm/irqflags.h | 2 +- > arch/x86/include/asm/mem_encrypt.h | 2 +- > arch/x86/kernel/cpu/cpu.h | 3 +-- > include/asm-generic/error-injection.h | 2 +- > include/asm-generic/kprobes.h | 5 ++--- > include/linux/cache.h | 6 +++--- > include/linux/compiler.h | 31 ++++----------------------- > include/linux/compiler_attributes.h | 10 +++++++++ > include/linux/cpu.h | 2 +- > include/linux/export.h | 2 +- > include/linux/init_task.h | 4 ++-- > include/linux/interrupt.h | 5 ++--- > include/linux/sched/debug.h | 2 +- > include/linux/srcutree.h | 2 +- > 32 files changed, 54 insertions(+), 74 deletions(-) > > -- > 2.23.0.187.g17f5b7556c-goog >
On Wed, Aug 28, 2019 at 3:00 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Tue, Aug 27, 2019 at 10:40 PM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > GCC unescapes escaped string section names while Clang does not. Because > > __section uses the `#` stringification operator for the section name, it > > doesn't need to be escaped. > > > > This fixes an Oops observed in distro's that use systemd and not > > net.core.bpf_jit_enable=1, when their kernels are compiled with Clang. > > > > Instead, we should: > > 1. Prefer __section(.section_name_no_quotes). > > 2. Only use __attribute__((__section(".section"))) when creating the > > section name via C preprocessor (see the definition of __define_initcall > > in arch/um/include/shared/init.h). Case 2 referenced below. > > > > This antipattern was found with: > > $ grep -e __section\(\" -e __section__\(\" -r > > > > Hi Nick, > > thanks for the v2 of your patch-series. > > I just checked v2 on top of Linux v5.3-rc6... > arch/um/include/shared/init.h: __attribute__((__section__(".initcall" > level ".init"))) = fn > > ??? ^^ Right, thanks for checking. That case is a section dynamically built via preprocessor, so that's the case I'm referring to in case 2. > > > See the discussions in: > > https://bugs.llvm.org/show_bug.cgi?id=42950 > > https://marc.info/?l=linux-netdev&m=156412960619946&w=2 > > > > List CBL issue tracker to discussions: > https://github.com/ClangBuiltLinux/linux/issues/619 Will do in v3! -- Thanks, ~Nick Desaulniers
On Tue, Aug 27, 2019 at 7:47 PM Joe Perches <joe@perches.com> wrote: > > On Tue, 2019-08-27 at 13:39 -0700, Nick Desaulniers wrote: > > GCC unescapes escaped string section names while Clang does not. Because > > __section uses the `#` stringification operator for the section name, it > > doesn't need to be escaped. > > > > This fixes an Oops observed in distro's that use systemd and not > > net.core.bpf_jit_enable=1, when their kernels are compiled with Clang. > > > > Instead, we should: > > 1. Prefer __section(.section_name_no_quotes). > > 2. Only use __attribute__((__section(".section"))) when creating the > > Please use __ before and after section > > i.e. __attribute__((__section__("<section_name>"))) > > *explitive*!!! -- Thanks, ~Nick Desaulniers
> $ grep -e __section\(\" -e __section__\(\" -r > > arch/um/include/shared/init.h: __attribute__((__section__(".initcall" > level ".init"))) = fn > 2. Only use __attribute__((__section__(".section"))) when creating the section name via C preprocessor (see the definition of __define_initcall in arch/um/include/shared/init.h). Who reads commit messages? /me hides - Sedat -
GCC unescapes escaped string section names while Clang does not. Because __section uses the `#` stringification operator for the section name, it doesn't need to be escaped. This fixes an Oops observed in distro's that use systemd and not net.core.bpf_jit_enable=1, when their kernels are compiled with Clang. Instead, we should: 1. Prefer __section(.section_name_no_quotes). 2. Only use __attribute__((__section(".section"))) when creating the section name via C preprocessor (see the definition of __define_initcall in arch/um/include/shared/init.h). This antipattern was found with: $ grep -e __section\(\" -e __section__\(\" -r See the discussions in: https://bugs.llvm.org/show_bug.cgi?id=42950 https://marc.info/?l=linux-netdev&m=156412960619946&w=2 Changes V1 -> V2: * drop arm64, arc, and sh patches as they were picked up by their maintainers. * Split the previous V1 hunk from include/linux that touched include/linux/compiler.h off into its own patch for inclusion in stable, as it fixes a user visible issue. * Collect Acks and Tested by tags. Nick Desaulniers (14): s390/boot: prefer __section from compiler_attributes.h include/linux/compiler.h: prefer __section from compiler_attributes.h parisc: prefer __section from compiler_attributes.h um: prefer __section from compiler_attributes.h ia64: prefer __section from compiler_attributes.h arm: prefer __section from compiler_attributes.h mips: prefer __section from compiler_attributes.h sparc: prefer __section from compiler_attributes.h powerpc: prefer __section and __printf from compiler_attributes.h x86: prefer __section from compiler_attributes.h include/asm-generic: prefer __section from compiler_attributes.h include/linux: prefer __section from compiler_attributes.h include/linux/compiler.h: remove unused KENTRY macro compiler_attributes.h: add note about __section arch/arm/include/asm/cache.h | 2 +- arch/arm/include/asm/mach/arch.h | 4 ++-- arch/arm/include/asm/setup.h | 2 +- arch/ia64/include/asm/cache.h | 2 +- arch/mips/include/asm/cache.h | 2 +- arch/parisc/include/asm/cache.h | 2 +- arch/parisc/include/asm/ldcw.h | 2 +- arch/powerpc/boot/main.c | 3 +-- arch/powerpc/boot/ps3.c | 6 ++---- arch/powerpc/include/asm/cache.h | 2 +- arch/powerpc/kernel/btext.c | 2 +- arch/s390/boot/startup.c | 2 +- arch/sparc/include/asm/cache.h | 2 +- arch/sparc/kernel/btext.c | 2 +- arch/um/kernel/um_arch.c | 6 +++--- arch/x86/include/asm/cache.h | 2 +- arch/x86/include/asm/intel-mid.h | 2 +- arch/x86/include/asm/iommu_table.h | 5 ++--- arch/x86/include/asm/irqflags.h | 2 +- arch/x86/include/asm/mem_encrypt.h | 2 +- arch/x86/kernel/cpu/cpu.h | 3 +-- include/asm-generic/error-injection.h | 2 +- include/asm-generic/kprobes.h | 5 ++--- include/linux/cache.h | 6 +++--- include/linux/compiler.h | 31 ++++----------------------- include/linux/compiler_attributes.h | 10 +++++++++ include/linux/cpu.h | 2 +- include/linux/export.h | 2 +- include/linux/init_task.h | 4 ++-- include/linux/interrupt.h | 5 ++--- include/linux/sched/debug.h | 2 +- include/linux/srcutree.h | 2 +- 32 files changed, 54 insertions(+), 74 deletions(-) -- 2.23.0.187.g17f5b7556c-goog