linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
@ 2017-07-26 12:46 Nicholas Piggin
  2017-08-08  1:53 ` Masahiro Yamada
  2017-08-09 16:03 ` Masahiro Yamada
  0 siblings, 2 replies; 7+ messages in thread
From: Nicholas Piggin @ 2017-07-26 12:46 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nicholas Piggin, Russell King - ARM Linux, Michal Marek,
	linux-arm-kernel, linux-kbuild, stable

The .data and .bss sections were modified in the generic linker script to
pull in sections named .data.<C identifier>, which are generated by gcc with
-ffunction-sections and -fdata-sections options.

The problem with this pattern is it can also match section names that Linux
defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
get moved into the wrong place.

The way to avoid this is to use ".." separators for explicit section names
(the dot character is valid in a section name but not a C identifier).
However currently there are sections which don't follow this rule, so for
now just disable the wild card by default.

Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2

Cc: <stable@vger.kernel.org> # 4.9
Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index da0be9a8d1de..9623d78f8494 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -60,6 +60,22 @@
 #define ALIGN_FUNCTION()  . = ALIGN(8)
 
 /*
+ * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
+ * generates .data.identifier sections, which need to be pulled in with
+ * .data. We don't want to pull in .data..other sections, which Linux
+ * has defined. Same for text and bss.
+ */
+#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
+#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
+#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
+#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
+#else
+#define TEXT_MAIN .text
+#define DATA_MAIN .data
+#define BSS_MAIN .bss
+#endif
+
+/*
  * Align to a 32 byte boundary equal to the
  * alignment gcc 4.5 uses for a struct
  */
@@ -198,12 +214,9 @@
 
 /*
  * .data section
- * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections generates
- * .data.identifier which needs to be pulled in with .data, but don't want to
- * pull in .data..stuff which has its own requirements. Same for bss.
  */
 #define DATA_DATA							\
-	*(.data .data.[0-9a-zA-Z_]*)					\
+	*(DATA_MAIN)							\
 	*(.ref.data)							\
 	*(.data..shared_aligned) /* percpu related */			\
 	MEM_KEEP(init.data)						\
@@ -434,16 +447,17 @@
 		VMLINUX_SYMBOL(__security_initcall_end) = .;		\
 	}
 
-/* .text section. Map to function alignment to avoid address changes
+/*
+ * .text section. Map to function alignment to avoid address changes
  * during second ld run in second ld pass when generating System.map
- * LD_DEAD_CODE_DATA_ELIMINATION option enables -ffunction-sections generates
- * .text.identifier which needs to be pulled in with .text , but some
- * architectures define .text.foo which is not intended to be pulled in here.
- * Those enabling LD_DEAD_CODE_DATA_ELIMINATION must ensure they don't have
- * conflicting section names, and must pull in .text.[0-9a-zA-Z_]* */
+ *
+ * TEXT_MAIN here will match .text.fixup and .text.unlikely if dead
+ * code elimination is enabled, so these sections should be converted
+ * to use ".." first.
+ */
 #define TEXT_TEXT							\
 		ALIGN_FUNCTION();					\
-		*(.text.hot .text .text.fixup .text.unlikely)		\
+		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely)	\
 		*(.ref.text)						\
 	MEM_KEEP(init.text)						\
 	MEM_KEEP(exit.text)						\
@@ -613,7 +627,7 @@
 		BSS_FIRST_SECTIONS					\
 		*(.bss..page_aligned)					\
 		*(.dynbss)						\
-		*(.bss .bss.[0-9a-zA-Z_]*)				\
+		*(BSS_MAIN)						\
 		*(COMMON)						\
 	}
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-07-26 12:46 [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured Nicholas Piggin
@ 2017-08-08  1:53 ` Masahiro Yamada
  2017-08-08  3:19   ` Nicholas Piggin
  2017-08-09 16:03 ` Masahiro Yamada
  1 sibling, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2017-08-08  1:53 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

Hi Nicholas,

2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> The .data and .bss sections were modified in the generic linker script to
> pull in sections named .data.<C identifier>, which are generated by gcc with
> -ffunction-sections and -fdata-sections options.
>
> The problem with this pattern is it can also match section names that Linux
> defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
> get moved into the wrong place.
>
> The way to avoid this is to use ".." separators for explicit section names
> (the dot character is valid in a section name but not a C identifier).
> However currently there are sections which don't follow this rule, so for
> now just disable the wild card by default.
>
> Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
>
> Cc: <stable@vger.kernel.org> # 4.9
> Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index da0be9a8d1de..9623d78f8494 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -60,6 +60,22 @@
>  #define ALIGN_FUNCTION()  . = ALIGN(8)
>
>  /*
> + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
> + * generates .data.identifier sections, which need to be pulled in with
> + * .data. We don't want to pull in .data..other sections, which Linux
> + * has defined. Same for text and bss.
> + */
> +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
> +#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
> +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
> +#else
> +#define TEXT_MAIN .text
> +#define DATA_MAIN .data
> +#define BSS_MAIN .bss
> +#endif


From the git-log, I think you are planning to rename
  .data.unlikely   ->   .data..unlikey
then the problem will be fixed.

("git grep data.unlikely" gave me only 7 instances.)


Why is this patch necessary?


You are trying to migrate some architectures to DCDE,
so this #ifdef does not solve the problem, I think.






-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-08-08  1:53 ` Masahiro Yamada
@ 2017-08-08  3:19   ` Nicholas Piggin
  2017-08-08  3:42     ` Masahiro Yamada
  0 siblings, 1 reply; 7+ messages in thread
From: Nicholas Piggin @ 2017-08-08  3:19 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

On Tue, 8 Aug 2017 10:53:56 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Nicholas,
> 
> 2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> > The .data and .bss sections were modified in the generic linker script to
> > pull in sections named .data.<C identifier>, which are generated by gcc with
> > -ffunction-sections and -fdata-sections options.
> >
> > The problem with this pattern is it can also match section names that Linux
> > defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
> > get moved into the wrong place.
> >
> > The way to avoid this is to use ".." separators for explicit section names
> > (the dot character is valid in a section name but not a C identifier).
> > However currently there are sections which don't follow this rule, so for
> > now just disable the wild card by default.
> >
> > Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
> >
> > Cc: <stable@vger.kernel.org> # 4.9
> > Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
> >  1 file changed, 26 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index da0be9a8d1de..9623d78f8494 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -60,6 +60,22 @@
> >  #define ALIGN_FUNCTION()  . = ALIGN(8)
> >
> >  /*
> > + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
> > + * generates .data.identifier sections, which need to be pulled in with
> > + * .data. We don't want to pull in .data..other sections, which Linux
> > + * has defined. Same for text and bss.
> > + */
> > +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> > +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
> > +#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
> > +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
> > +#else
> > +#define TEXT_MAIN .text
> > +#define DATA_MAIN .data
> > +#define BSS_MAIN .bss
> > +#endif  
> 
> 
> From the git-log, I think you are planning to rename
>   .data.unlikely   ->   .data..unlikey
> then the problem will be fixed.
> 
> ("git grep data.unlikely" gave me only 7 instances.)
> 
> 
> Why is this patch necessary?
> 
> 
> You are trying to migrate some architectures to DCDE,
> so this #ifdef does not solve the problem, I think.

Well basically I want to preserve previous behaviour and avoid
regressions as far as possible. Patches to rename existing
sections I think would not be so suitable for @stable. And
there could be other things we've missed.

Yes I would like to ensure all our sections use .. separators
which (hopefully). Until then I think this is a minimal fix.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-08-08  3:19   ` Nicholas Piggin
@ 2017-08-08  3:42     ` Masahiro Yamada
  2017-08-08  4:26       ` Nicholas Piggin
  0 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2017-08-08  3:42 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

Hi Nicholas,

2017-08-08 12:19 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> On Tue, 8 Aug 2017 10:53:56 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Hi Nicholas,
>>
>> 2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
>> > The .data and .bss sections were modified in the generic linker script to
>> > pull in sections named .data.<C identifier>, which are generated by gcc with
>> > -ffunction-sections and -fdata-sections options.
>> >
>> > The problem with this pattern is it can also match section names that Linux
>> > defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
>> > get moved into the wrong place.
>> >
>> > The way to avoid this is to use ".." separators for explicit section names
>> > (the dot character is valid in a section name but not a C identifier).
>> > However currently there are sections which don't follow this rule, so for
>> > now just disable the wild card by default.
>> >
>> > Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
>> >
>> > Cc: <stable@vger.kernel.org> # 4.9
>> > Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
>> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> > ---
>> >  include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
>> >  1 file changed, 26 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> > index da0be9a8d1de..9623d78f8494 100644
>> > --- a/include/asm-generic/vmlinux.lds.h
>> > +++ b/include/asm-generic/vmlinux.lds.h
>> > @@ -60,6 +60,22 @@
>> >  #define ALIGN_FUNCTION()  . = ALIGN(8)
>> >
>> >  /*
>> > + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
>> > + * generates .data.identifier sections, which need to be pulled in with
>> > + * .data. We don't want to pull in .data..other sections, which Linux
>> > + * has defined. Same for text and bss.
>> > + */
>> > +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>> > +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
>> > +#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
>> > +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
>> > +#else
>> > +#define TEXT_MAIN .text
>> > +#define DATA_MAIN .data
>> > +#define BSS_MAIN .bss
>> > +#endif
>>
>>
>> From the git-log, I think you are planning to rename
>>   .data.unlikely   ->   .data..unlikey
>> then the problem will be fixed.
>>
>> ("git grep data.unlikely" gave me only 7 instances.)
>>
>>
>> Why is this patch necessary?
>>
>>
>> You are trying to migrate some architectures to DCDE,
>> so this #ifdef does not solve the problem, I think.
>
> Well basically I want to preserve previous behaviour and avoid
> regressions as far as possible. Patches to rename existing
> sections I think would not be so suitable for @stable. And
> there could be other things we've missed.
>
> Yes I would like to ensure all our sections use .. separators
> which (hopefully). Until then I think this is a minimal fix.
>


After you rename the sections, will you revert this patch?

I think the #ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
should not be permanent.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-08-08  3:42     ` Masahiro Yamada
@ 2017-08-08  4:26       ` Nicholas Piggin
  2017-08-08  4:31         ` Masahiro Yamada
  0 siblings, 1 reply; 7+ messages in thread
From: Nicholas Piggin @ 2017-08-08  4:26 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

On Tue, 8 Aug 2017 12:42:18 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi Nicholas,
> 
> 2017-08-08 12:19 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> > On Tue, 8 Aug 2017 10:53:56 +0900
> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> >  
> >> Hi Nicholas,
> >>
> >> 2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:  
> >> > The .data and .bss sections were modified in the generic linker script to
> >> > pull in sections named .data.<C identifier>, which are generated by gcc with
> >> > -ffunction-sections and -fdata-sections options.
> >> >
> >> > The problem with this pattern is it can also match section names that Linux
> >> > defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
> >> > get moved into the wrong place.
> >> >
> >> > The way to avoid this is to use ".." separators for explicit section names
> >> > (the dot character is valid in a section name but not a C identifier).
> >> > However currently there are sections which don't follow this rule, so for
> >> > now just disable the wild card by default.
> >> >
> >> > Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
> >> >
> >> > Cc: <stable@vger.kernel.org> # 4.9
> >> > Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
> >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> > ---
> >> >  include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
> >> >  1 file changed, 26 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> >> > index da0be9a8d1de..9623d78f8494 100644
> >> > --- a/include/asm-generic/vmlinux.lds.h
> >> > +++ b/include/asm-generic/vmlinux.lds.h
> >> > @@ -60,6 +60,22 @@
> >> >  #define ALIGN_FUNCTION()  . = ALIGN(8)
> >> >
> >> >  /*
> >> > + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
> >> > + * generates .data.identifier sections, which need to be pulled in with
> >> > + * .data. We don't want to pull in .data..other sections, which Linux
> >> > + * has defined. Same for text and bss.
> >> > + */
> >> > +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> >> > +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
> >> > +#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
> >> > +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
> >> > +#else
> >> > +#define TEXT_MAIN .text
> >> > +#define DATA_MAIN .data
> >> > +#define BSS_MAIN .bss
> >> > +#endif  
> >>
> >>
> >> From the git-log, I think you are planning to rename
> >>   .data.unlikely   ->   .data..unlikey
> >> then the problem will be fixed.
> >>
> >> ("git grep data.unlikely" gave me only 7 instances.)
> >>
> >>
> >> Why is this patch necessary?
> >>
> >>
> >> You are trying to migrate some architectures to DCDE,
> >> so this #ifdef does not solve the problem, I think.  
> >
> > Well basically I want to preserve previous behaviour and avoid
> > regressions as far as possible. Patches to rename existing
> > sections I think would not be so suitable for @stable. And
> > there could be other things we've missed.
> >
> > Yes I would like to ensure all our sections use .. separators
> > which (hopefully). Until then I think this is a minimal fix.
> >  
> 
> 
> After you rename the sections, will you revert this patch?
> 
> I think the #ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> should not be permanent.

Once we have the code reasonably well used on a number of archs,
I think we should remove the ifdefs, yes we definitely should be
moving toward sharing common code wherever possible.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-08-08  4:26       ` Nicholas Piggin
@ 2017-08-08  4:31         ` Masahiro Yamada
  0 siblings, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2017-08-08  4:31 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

2017-08-08 13:26 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> On Tue, 8 Aug 2017 12:42:18 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
>> Hi Nicholas,
>>
>> 2017-08-08 12:19 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
>> > On Tue, 8 Aug 2017 10:53:56 +0900
>> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>> >
>> >> Hi Nicholas,
>> >>
>> >> 2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
>> >> > The .data and .bss sections were modified in the generic linker script to
>> >> > pull in sections named .data.<C identifier>, which are generated by gcc with
>> >> > -ffunction-sections and -fdata-sections options.
>> >> >
>> >> > The problem with this pattern is it can also match section names that Linux
>> >> > defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
>> >> > get moved into the wrong place.
>> >> >
>> >> > The way to avoid this is to use ".." separators for explicit section names
>> >> > (the dot character is valid in a section name but not a C identifier).
>> >> > However currently there are sections which don't follow this rule, so for
>> >> > now just disable the wild card by default.
>> >> >
>> >> > Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
>> >> >
>> >> > Cc: <stable@vger.kernel.org> # 4.9
>> >> > Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
>> >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> >> > ---
>> >> >  include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------
>> >> >  1 file changed, 26 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> >> > index da0be9a8d1de..9623d78f8494 100644
>> >> > --- a/include/asm-generic/vmlinux.lds.h
>> >> > +++ b/include/asm-generic/vmlinux.lds.h
>> >> > @@ -60,6 +60,22 @@
>> >> >  #define ALIGN_FUNCTION()  . = ALIGN(8)
>> >> >
>> >> >  /*
>> >> > + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which
>> >> > + * generates .data.identifier sections, which need to be pulled in with
>> >> > + * .data. We don't want to pull in .data..other sections, which Linux
>> >> > + * has defined. Same for text and bss.
>> >> > + */
>> >> > +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>> >> > +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
>> >> > +#define DATA_MAIN .data .data.[0-9a-zA-Z_]*
>> >> > +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]*
>> >> > +#else
>> >> > +#define TEXT_MAIN .text
>> >> > +#define DATA_MAIN .data
>> >> > +#define BSS_MAIN .bss
>> >> > +#endif
>> >>
>> >>
>> >> From the git-log, I think you are planning to rename
>> >>   .data.unlikely   ->   .data..unlikey
>> >> then the problem will be fixed.
>> >>
>> >> ("git grep data.unlikely" gave me only 7 instances.)
>> >>
>> >>
>> >> Why is this patch necessary?
>> >>
>> >>
>> >> You are trying to migrate some architectures to DCDE,
>> >> so this #ifdef does not solve the problem, I think.
>> >
>> > Well basically I want to preserve previous behaviour and avoid
>> > regressions as far as possible. Patches to rename existing
>> > sections I think would not be so suitable for @stable. And
>> > there could be other things we've missed.
>> >
>> > Yes I would like to ensure all our sections use .. separators
>> > which (hopefully). Until then I think this is a minimal fix.
>> >
>>
>>
>> After you rename the sections, will you revert this patch?
>>
>> I think the #ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>> should not be permanent.
>
> Once we have the code reasonably well used on a number of archs,
> I think we should remove the ifdefs, yes we definitely should be
> moving toward sharing common code wherever possible.
>


OK, I will pick up this for fixes.
Thanks.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured
  2017-07-26 12:46 [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured Nicholas Piggin
  2017-08-08  1:53 ` Masahiro Yamada
@ 2017-08-09 16:03 ` Masahiro Yamada
  1 sibling, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2017-08-09 16:03 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Russell King - ARM Linux, Michal Marek, linux-arm-kernel,
	Linux Kbuild mailing list, stable

2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> The .data and .bss sections were modified in the generic linker script to
> pull in sections named .data.<C identifier>, which are generated by gcc with
> -ffunction-sections and -fdata-sections options.
>
> The problem with this pattern is it can also match section names that Linux
> defines explicitly, e.g., .data.unlikely. This can cause Linux sections to
> get moved into the wrong place.
>
> The way to avoid this is to use ".." separators for explicit section names
> (the dot character is valid in a section name but not a C identifier).
> However currently there are sections which don't follow this rule, so for
> now just disable the wild card by default.
>
> Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2
>
> Cc: <stable@vger.kernel.org> # 4.9
> Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to linux-kbuild/fixes.  Thanks!

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-08-09 16:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 12:46 [PATCH] kbuild: linker script do not match C names unless LD_DEAD_CODE_DATA_ELIMINATION is configured Nicholas Piggin
2017-08-08  1:53 ` Masahiro Yamada
2017-08-08  3:19   ` Nicholas Piggin
2017-08-08  3:42     ` Masahiro Yamada
2017-08-08  4:26       ` Nicholas Piggin
2017-08-08  4:31         ` Masahiro Yamada
2017-08-09 16:03 ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).