linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] objtool: fix build with future Make
@ 2018-03-25 22:32 Rasmus Villemoes
  2018-03-25 23:09 ` [PATCH 2/1] Kbuild: fix # escaping in .cmd files for " Rasmus Villemoes
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rasmus Villemoes @ 2018-03-25 22:32 UTC (permalink / raw)
  To: Josh Poimboeuf, Peter Zijlstra
  Cc: linux-kbuild, Masahiro Yamada, Michal Marek, Rasmus Villemoes,
	linux-kernel

I tried building using a freshly built Make (4.2.1-69-g8a731d1), but the
objtool build broke with

orc_dump.c: In function ‘orc_dump’:
orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
  if (elf_getshdrnum(elf, &nr_sections)) {

Turns out that with that new Make, the backslash was not removed, so cpp
didn't see a #include directive, grep found nothing, and
-DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.

Now, that new Make behaviour is documented in their NEWS file:

  * WARNING: Backward-incompatibility!
    Number signs (#) appearing inside a macro reference or function invocation
    no longer introduce comments and should not be escaped with backslashes:
    thus a call such as:
      foo := $(shell echo '#')
    is legal.  Previously the number sign needed to be escaped, for example:
      foo := $(shell echo '\#')
    Now this latter will resolve to "\#".  If you want to write makefiles
    portable to both versions, assign the number sign to a variable:
      C := \#
      foo := $(shell echo '$C')
    This was claimed to be fixed in 3.81, but wasn't, for some reason.
    To detect this change search for 'nocomment' in the .FEATURES variable.

There are likely other places in the tree that will need fixing, so
cc-ing Kbuild, but with this at least a x86-64 defconfig builds.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 tools/objtool/Makefile         | 2 +-
 tools/scripts/Makefile.include | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index e6acc281dd37..8ae824dbfca3 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -35,7 +35,7 @@ CFLAGS   += -Wall -Werror $(WARNINGS) -fomit-frame-pointer -O2 -g $(INCLUDES)
 LDFLAGS  += -lelf $(LIBSUBCMD)
 
 # Allow old libelf to be used:
-elfshdr := $(shell echo '\#include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
+elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
 CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
 
 AWK = awk
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index dd614463d4d6..495066bafbe3 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -120,3 +120,5 @@ ifneq ($(silent),1)
 	QUIET_UNINST   = @printf '  UNINST   %s\n' $1;
   endif
 endif
+
+pound := \#
-- 
2.15.1

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

* [PATCH 2/1] Kbuild: fix # escaping in .cmd files for future Make
  2018-03-25 22:32 [PATCH] objtool: fix build with future Make Rasmus Villemoes
@ 2018-03-25 23:09 ` Rasmus Villemoes
  2018-03-26 11:48   ` Masahiro Yamada
  2018-03-25 23:42 ` [PATCH] objtool: fix build with " Randy Dunlap
  2018-03-27  5:44 ` Masahiro Yamada
  2 siblings, 1 reply; 9+ messages in thread
From: Rasmus Villemoes @ 2018-03-25 23:09 UTC (permalink / raw)
  To: linux-kbuild, Masahiro Yamada, Douglas Anderson,
	Nick Desaulniers, Matthias Kaehlcke
  Cc: Michal Marek, Josh Poimboeuf, Peter Zijlstra, Rasmus Villemoes,
	linux-kernel

The latest official Make release is 4.2.1 from mid-2016, but the current
git release has this relevant note in the NEWS file:

  * WARNING: Backward-incompatibility!
    Number signs (#) appearing inside a macro reference or function invocation
    no longer introduce comments and should not be escaped with backslashes:
    thus a call such as:
      foo := $(shell echo '#')
    is legal.  Previously the number sign needed to be escaped, for example:
      foo := $(shell echo '\#')
    Now this latter will resolve to "\#".  If you want to write makefiles
    portable to both versions, assign the number sign to a variable:
      C := \#
      foo := $(shell echo '$C')
    This was claimed to be fixed in 3.81, but wasn't, for some reason.
    To detect this change search for 'nocomment' in the .FEATURES variable.

Prepare for whatever future Make release contains that change by fixing
up the .cmd file escaping - without this, make always thinks the command
string has changed and hence rebuilds everything.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
So the previous patch made everything build, but building again
revealed that this central place very much also needed fixing.

 scripts/Kbuild.include | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..7a926e4688f4 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -10,6 +10,7 @@ space   := $(empty) $(empty)
 space_escape := _-_SPACE_-_
 right_paren := )
 left_paren := (
+pound := \#
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
@@ -328,7 +329,7 @@ endif
 # (needed for make)
 # Replace >'< with >'\''< to be able to enclose the whole string in '...'
 # (needed for the shell)
-make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
+make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
 
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
-- 
2.15.1

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

* Re: [PATCH] objtool: fix build with future Make
  2018-03-25 22:32 [PATCH] objtool: fix build with future Make Rasmus Villemoes
  2018-03-25 23:09 ` [PATCH 2/1] Kbuild: fix # escaping in .cmd files for " Rasmus Villemoes
@ 2018-03-25 23:42 ` Randy Dunlap
  2018-03-27  5:44 ` Masahiro Yamada
  2 siblings, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2018-03-25 23:42 UTC (permalink / raw)
  To: Rasmus Villemoes, Josh Poimboeuf, Peter Zijlstra
  Cc: linux-kbuild, Masahiro Yamada, Michal Marek, linux-kernel

On 03/25/2018 03:32 PM, Rasmus Villemoes wrote:
> I tried building using a freshly built Make (4.2.1-69-g8a731d1), but the
> objtool build broke with
> 
> orc_dump.c: In function ‘orc_dump’:
> orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
>   if (elf_getshdrnum(elf, &nr_sections)) {
> 
> Turns out that with that new Make, the backslash was not removed, so cpp
> didn't see a #include directive, grep found nothing, and
> -DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.
> 
> Now, that new Make behaviour is documented in their NEWS file:
> 
>   * WARNING: Backward-incompatibility!
>     Number signs (#) appearing inside a macro reference or function invocation
>     no longer introduce comments and should not be escaped with backslashes:
>     thus a call such as:
>       foo := $(shell echo '#')
>     is legal.  Previously the number sign needed to be escaped, for example:
>       foo := $(shell echo '\#')
>     Now this latter will resolve to "\#".  If you want to write makefiles
>     portable to both versions, assign the number sign to a variable:
>       C := \#
>       foo := $(shell echo '$C')
>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>     To detect this change search for 'nocomment' in the .FEATURES variable.
> 
> There are likely other places in the tree that will need fixing, so
> cc-ing Kbuild, but with this at least a x86-64 defconfig builds.

Hi,

For one of these patches, can we say:
Fixes https://bugzilla.kernel.org/show_bug.cgi?id=197847
?

Thanks.

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  tools/objtool/Makefile         | 2 +-
>  tools/scripts/Makefile.include | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index e6acc281dd37..8ae824dbfca3 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -35,7 +35,7 @@ CFLAGS   += -Wall -Werror $(WARNINGS) -fomit-frame-pointer -O2 -g $(INCLUDES)
>  LDFLAGS  += -lelf $(LIBSUBCMD)
>  
>  # Allow old libelf to be used:
> -elfshdr := $(shell echo '\#include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
> +elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
>  CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
>  
>  AWK = awk
> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index dd614463d4d6..495066bafbe3 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -120,3 +120,5 @@ ifneq ($(silent),1)
>  	QUIET_UNINST   = @printf '  UNINST   %s\n' $1;
>    endif
>  endif
> +
> +pound := \#
> 


-- 
~Randy

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

* Re: [PATCH 2/1] Kbuild: fix # escaping in .cmd files for future Make
  2018-03-25 23:09 ` [PATCH 2/1] Kbuild: fix # escaping in .cmd files for " Rasmus Villemoes
@ 2018-03-26 11:48   ` Masahiro Yamada
  2018-04-05 21:43     ` Rasmus Villemoes
  0 siblings, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-03-26 11:48 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Linux Kbuild mailing list, Douglas Anderson, Nick Desaulniers,
	Matthias Kaehlcke, Michal Marek, Josh Poimboeuf, Peter Zijlstra,
	Linux Kernel Mailing List

2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
> The latest official Make release is 4.2.1 from mid-2016, but the current
> git release has this relevant note in the NEWS file:
>
>   * WARNING: Backward-incompatibility!
>     Number signs (#) appearing inside a macro reference or function invocation
>     no longer introduce comments and should not be escaped with backslashes:
>     thus a call such as:
>       foo := $(shell echo '#')
>     is legal.  Previously the number sign needed to be escaped, for example:
>       foo := $(shell echo '\#')
>     Now this latter will resolve to "\#".  If you want to write makefiles
>     portable to both versions, assign the number sign to a variable:
>       C := \#
>       foo := $(shell echo '$C')
>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>     To detect this change search for 'nocomment' in the .FEATURES variable.
>
> Prepare for whatever future Make release contains that change by fixing
> up the .cmd file escaping - without this, make always thinks the command
> string has changed and hence rebuilds everything.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> So the previous patch made everything build, but building again
> revealed that this central place very much also needed fixing.
>
>  scripts/Kbuild.include | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..7a926e4688f4 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -10,6 +10,7 @@ space   := $(empty) $(empty)
>  space_escape := _-_SPACE_-_
>  right_paren := )
>  left_paren := (
> +pound := \#
>
>  ###
>  # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
> @@ -328,7 +329,7 @@ endif
>  # (needed for make)
>  # Replace >'< with >'\''< to be able to enclose the whole string in '...'
>  # (needed for the shell)
> -make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
> +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
>

Thanks for the patch, but this changes the behavior.
With '#' replaced with $(pound), '\\' does not escape anything,
so it is treated as '\\'.


The following keeps the current behavior:

make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst
$$,$$$$,$(cmd_$(1)))))



But, I think the following is an even better fix:

make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst
$$,$$$$,$(cmd_$(1)))))



The following 4 test cases produce the same foo.txt,
but insane .cmd file is produced for some case.
Adjusting the number of back-slashes is not a perfect solution.

Replacing # with $(pound) works for all the cases.


[test code 1]
extra-y := foo.txt

quiet_cmd_foo = DEFINE   $@
      cmd_foo = echo '\#define FOO' >$@

$(obj)/foo.txt: FORCE
        $(call if_changed,foo)


[test code 2]
extra-y := foo.txt

quiet_cmd_foo = DEFINE   $@
      cmd_foo = echo \\\#define FOO >$@

$(obj)/foo.txt: FORCE
        $(call if_changed,foo)


[test code3]
extra-y := foo.txt

quiet_cmd_foo = DEFINE   $@
      cmd_foo = echo '$(pound)define FOO' >$@

$(obj)/foo.txt: FORCE
        $(call if_changed,foo)



[test code4]
extra-y := foo.txt

quiet_cmd_foo = DEFINE   $@
      cmd_foo = echo \$(pound)define FOO >$@

$(obj)/foo.txt: FORCE
        $(call if_changed,foo)





           | current | \$(pound) | \\$(pound) | $$(pound)
----------------------------------------------------------
test code1 |    OK   |    OK     |     NG     |   OK
test code2 |    NG   |    NG     |     OK     |   OK
test code3 |    OK   |    OK     |     NG     |   OK
test code4 |    NG   |    NG     |     OK     |   OK




-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH] objtool: fix build with future Make
  2018-03-25 22:32 [PATCH] objtool: fix build with future Make Rasmus Villemoes
  2018-03-25 23:09 ` [PATCH 2/1] Kbuild: fix # escaping in .cmd files for " Rasmus Villemoes
  2018-03-25 23:42 ` [PATCH] objtool: fix build with " Randy Dunlap
@ 2018-03-27  5:44 ` Masahiro Yamada
  2 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2018-03-27  5:44 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Josh Poimboeuf, Peter Zijlstra, Linux Kbuild mailing list,
	Michal Marek, Linux Kernel Mailing List

2018-03-26 7:32 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
> I tried building using a freshly built Make (4.2.1-69-g8a731d1), but the
> objtool build broke with
>
> orc_dump.c: In function ‘orc_dump’:
> orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
>   if (elf_getshdrnum(elf, &nr_sections)) {
>
> Turns out that with that new Make, the backslash was not removed, so cpp
> didn't see a #include directive, grep found nothing, and
> -DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.
>
> Now, that new Make behaviour is documented in their NEWS file:
>
>   * WARNING: Backward-incompatibility!
>     Number signs (#) appearing inside a macro reference or function invocation
>     no longer introduce comments and should not be escaped with backslashes:
>     thus a call such as:
>       foo := $(shell echo '#')
>     is legal.  Previously the number sign needed to be escaped, for example:
>       foo := $(shell echo '\#')
>     Now this latter will resolve to "\#".  If you want to write makefiles
>     portable to both versions, assign the number sign to a variable:
>       C := \#
>       foo := $(shell echo '$C')
>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>     To detect this change search for 'nocomment' in the .FEATURES variable.
>
> There are likely other places in the tree that will need fixing, so
> cc-ing Kbuild, but with this at least a x86-64 defconfig builds.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---

Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>




-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 2/1] Kbuild: fix # escaping in .cmd files for future Make
  2018-03-26 11:48   ` Masahiro Yamada
@ 2018-04-05 21:43     ` Rasmus Villemoes
  2018-04-06  3:53       ` Masahiro Yamada
  0 siblings, 1 reply; 9+ messages in thread
From: Rasmus Villemoes @ 2018-04-05 21:43 UTC (permalink / raw)
  To: Masahiro Yamada, Rasmus Villemoes
  Cc: Linux Kbuild mailing list, Douglas Anderson, Nick Desaulniers,
	Matthias Kaehlcke, Michal Marek, Josh Poimboeuf, Peter Zijlstra,
	Linux Kernel Mailing List

On 2018-03-26 13:48, Masahiro Yamada wrote:
> 2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
>> The latest official Make release is 4.2.1 from mid-2016, but the current
>> git release has this relevant note in the NEWS file:
>>
>>   * WARNING: Backward-incompatibility!
>>     Number signs (#) appearing inside a macro reference or function invocation
>>     no longer introduce comments and should not be escaped with backslashes:
>>     thus a call such as:
>>       foo := $(shell echo '#')
>>     is legal.  Previously the number sign needed to be escaped, for example:
>>       foo := $(shell echo '\#')
>>     Now this latter will resolve to "\#".  If you want to write makefiles
>>     portable to both versions, assign the number sign to a variable:
>>       C := \#
>>       foo := $(shell echo '$C')
>>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>>     To detect this change search for 'nocomment' in the .FEATURES variable.
>>
>> Prepare for whatever future Make release contains that change by fixing
>> up the .cmd file escaping - without this, make always thinks the command
>> string has changed and hence rebuilds everything.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>> So the previous patch made everything build, but building again
>> revealed that this central place very much also needed fixing.
>>
>>  scripts/Kbuild.include | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
>> index 065324a8046f..7a926e4688f4 100644
>> --- a/scripts/Kbuild.include
>> +++ b/scripts/Kbuild.include
>> @@ -10,6 +10,7 @@ space   := $(empty) $(empty)
>>  space_escape := _-_SPACE_-_
>>  right_paren := )
>>  left_paren := (
>> +pound := \#
>>
>>  ###
>>  # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
>> @@ -328,7 +329,7 @@ endif
>>  # (needed for make)
>>  # Replace >'< with >'\''< to be able to enclose the whole string in '...'
>>  # (needed for the shell)
>> -make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
>> +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
>>
> 
> Thanks for the patch, but this changes the behavior.
> With '#' replaced with $(pound), '\\' does not escape anything,
> so it is treated as '\\'.
> 
> 
> The following keeps the current behavior:
> 
> make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst
> $$,$$$$,$(cmd_$(1)))))
> 
> 
> 
> But, I think the following is an even better fix:
> 
> make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst
> $$,$$$$,$(cmd_$(1)))))

Makefile quoting has never been my strong suit. I actually thought I
tested my change by looking at some .o.cmd files containing \# before
and after, but I was fooled by tools/ containing and using their own
copy of make-cmd - so tools/build/Build.include will also need fixing.

Yes, writing $(pound) to the .cmd file seems like the safest choice. Do
you want me to respin or can/will you do the patch(es) yourself?

Rasmus

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

* Re: [PATCH 2/1] Kbuild: fix # escaping in .cmd files for future Make
  2018-04-05 21:43     ` Rasmus Villemoes
@ 2018-04-06  3:53       ` Masahiro Yamada
  2018-04-08 21:35         ` [PATCH v2] " Rasmus Villemoes
  0 siblings, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-04-06  3:53 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Linux Kbuild mailing list, Douglas Anderson, Nick Desaulniers,
	Matthias Kaehlcke, Michal Marek, Josh Poimboeuf, Peter Zijlstra,
	Linux Kernel Mailing List

2018-04-06 6:43 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
> On 2018-03-26 13:48, Masahiro Yamada wrote:
>> 2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
>>> The latest official Make release is 4.2.1 from mid-2016, but the current
>>> git release has this relevant note in the NEWS file:
>>>
>>>   * WARNING: Backward-incompatibility!
>>>     Number signs (#) appearing inside a macro reference or function invocation
>>>     no longer introduce comments and should not be escaped with backslashes:
>>>     thus a call such as:
>>>       foo := $(shell echo '#')
>>>     is legal.  Previously the number sign needed to be escaped, for example:
>>>       foo := $(shell echo '\#')
>>>     Now this latter will resolve to "\#".  If you want to write makefiles
>>>     portable to both versions, assign the number sign to a variable:
>>>       C := \#
>>>       foo := $(shell echo '$C')
>>>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>>>     To detect this change search for 'nocomment' in the .FEATURES variable.
>>>
>>> Prepare for whatever future Make release contains that change by fixing
>>> up the .cmd file escaping - without this, make always thinks the command
>>> string has changed and hence rebuilds everything.
>>>
>>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>> ---
>>> So the previous patch made everything build, but building again
>>> revealed that this central place very much also needed fixing.
>>>
>>>  scripts/Kbuild.include | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
>>> index 065324a8046f..7a926e4688f4 100644
>>> --- a/scripts/Kbuild.include
>>> +++ b/scripts/Kbuild.include
>>> @@ -10,6 +10,7 @@ space   := $(empty) $(empty)
>>>  space_escape := _-_SPACE_-_
>>>  right_paren := )
>>>  left_paren := (
>>> +pound := \#
>>>
>>>  ###
>>>  # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
>>> @@ -328,7 +329,7 @@ endif
>>>  # (needed for make)
>>>  # Replace >'< with >'\''< to be able to enclose the whole string in '...'
>>>  # (needed for the shell)
>>> -make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
>>> +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
>>>
>>
>> Thanks for the patch, but this changes the behavior.
>> With '#' replaced with $(pound), '\\' does not escape anything,
>> so it is treated as '\\'.
>>
>>
>> The following keeps the current behavior:
>>
>> make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst
>> $$,$$$$,$(cmd_$(1)))))
>>
>>
>>
>> But, I think the following is an even better fix:
>>
>> make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst
>> $$,$$$$,$(cmd_$(1)))))
>
> Makefile quoting has never been my strong suit. I actually thought I
> tested my change by looking at some .o.cmd files containing \# before
> and after, but I was fooled by tools/ containing and using their own
> copy of make-cmd - so tools/build/Build.include will also need fixing.
>
> Yes, writing $(pound) to the .cmd file seems like the safest choice. Do
> you want me to respin or can/will you do the patch(es) yourself?
>

Can you send v2 please?

I will pick it up shortly.



-- 
Best Regards
Masahiro Yamada

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

* [PATCH v2] Kbuild: fix # escaping in .cmd files for future Make
  2018-04-06  3:53       ` Masahiro Yamada
@ 2018-04-08 21:35         ` Rasmus Villemoes
  2018-04-09 13:31           ` Masahiro Yamada
  0 siblings, 1 reply; 9+ messages in thread
From: Rasmus Villemoes @ 2018-04-08 21:35 UTC (permalink / raw)
  To: Masahiro Yamada, Josh Poimboeuf, Peter Zijlstra
  Cc: Linux Kbuild mailing list, Rasmus Villemoes, Randy Dunlap, linux-kernel

I tried building using a freshly built Make (4.2.1-69-g8a731d1), but
already the objtool build broke with

orc_dump.c: In function ‘orc_dump’:
orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
  if (elf_getshdrnum(elf, &nr_sections)) {

Turns out that with that new Make, the backslash was not removed, so cpp
didn't see a #include directive, grep found nothing, and
-DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.

Now, that new Make behaviour is documented in their NEWS file:

  * WARNING: Backward-incompatibility!
    Number signs (#) appearing inside a macro reference or function invocation
    no longer introduce comments and should not be escaped with backslashes:
    thus a call such as:
      foo := $(shell echo '#')
    is legal.  Previously the number sign needed to be escaped, for example:
      foo := $(shell echo '\#')
    Now this latter will resolve to "\#".  If you want to write makefiles
    portable to both versions, assign the number sign to a variable:
      C := \#
      foo := $(shell echo '$C')
    This was claimed to be fixed in 3.81, but wasn't, for some reason.
    To detect this change search for 'nocomment' in the .FEATURES variable.

This also fixes up the two make-cmd instances to replace # with $(pound)
rather than with \#. There might very well be other places that need
similar fixup in preparation for whatever future Make release contains
the above change, but at least this builds an x86_64 defconfig with the
new make.

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=197847
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2:

- squash the original objtool-only patch with the patch fixing up
  make-cmd, to avoid repeating the reference to the Make NEWS.

- use $(pound) in .cmd files to avoid backslash-counting issues (Masahiro).

- add bugzilla link (Randy).

 scripts/Kbuild.include         | 5 +++--
 tools/build/Build.include      | 5 +++--
 tools/objtool/Makefile         | 2 +-
 tools/scripts/Makefile.include | 2 ++
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index cce31ee876b6..50cee534fd64 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -10,6 +10,7 @@ space   := $(empty) $(empty)
 space_escape := _-_SPACE_-_
 right_paren := )
 left_paren := (
+pound := \#
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
@@ -322,11 +323,11 @@ endif
 
 # Replace >$< with >$$< to preserve $ when reloading the .cmd file
 # (needed for make)
-# Replace >#< with >\#< to avoid starting a comment in the .cmd file
+# Replace >#< with >$(pound)< to avoid starting a comment in the .cmd file
 # (needed for make)
 # Replace >'< with >'\''< to be able to enclose the whole string in '...'
 # (needed for the shell)
-make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
+make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
 
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 418871d02ebf..a4bbb984941d 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -12,6 +12,7 @@
 # Convenient variables
 comma   := ,
 squote  := '
+pound   := \#
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
@@ -43,11 +44,11 @@ echo-cmd = $(if $($(quiet)cmd_$(1)),\
 ###
 # Replace >$< with >$$< to preserve $ when reloading the .cmd file
 # (needed for make)
-# Replace >#< with >\#< to avoid starting a comment in the .cmd file
+# Replace >#< with >$(pound)< to avoid starting a comment in the .cmd file
 # (needed for make)
 # Replace >'< with >'\''< to be able to enclose the whole string in '...'
 # (needed for the shell)
-make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
+make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
 
 ###
 # Find any prerequisites that is newer than target or that does not exist.
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index e6acc281dd37..8ae824dbfca3 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -35,7 +35,7 @@ CFLAGS   += -Wall -Werror $(WARNINGS) -fomit-frame-pointer -O2 -g $(INCLUDES)
 LDFLAGS  += -lelf $(LIBSUBCMD)
 
 # Allow old libelf to be used:
-elfshdr := $(shell echo '\#include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
+elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
 CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
 
 AWK = awk
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index dd614463d4d6..495066bafbe3 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -120,3 +120,5 @@ ifneq ($(silent),1)
 	QUIET_UNINST   = @printf '  UNINST   %s\n' $1;
   endif
 endif
+
+pound := \#
-- 
2.15.1

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

* Re: [PATCH v2] Kbuild: fix # escaping in .cmd files for future Make
  2018-04-08 21:35         ` [PATCH v2] " Rasmus Villemoes
@ 2018-04-09 13:31           ` Masahiro Yamada
  0 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2018-04-09 13:31 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Josh Poimboeuf, Peter Zijlstra, Linux Kbuild mailing list,
	Randy Dunlap, Linux Kernel Mailing List

2018-04-09 6:35 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>:
> I tried building using a freshly built Make (4.2.1-69-g8a731d1), but
> already the objtool build broke with
>
> orc_dump.c: In function ‘orc_dump’:
> orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
>   if (elf_getshdrnum(elf, &nr_sections)) {
>
> Turns out that with that new Make, the backslash was not removed, so cpp
> didn't see a #include directive, grep found nothing, and
> -DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.
>
> Now, that new Make behaviour is documented in their NEWS file:
>
>   * WARNING: Backward-incompatibility!
>     Number signs (#) appearing inside a macro reference or function invocation
>     no longer introduce comments and should not be escaped with backslashes:
>     thus a call such as:
>       foo := $(shell echo '#')
>     is legal.  Previously the number sign needed to be escaped, for example:
>       foo := $(shell echo '\#')
>     Now this latter will resolve to "\#".  If you want to write makefiles
>     portable to both versions, assign the number sign to a variable:
>       C := \#
>       foo := $(shell echo '$C')
>     This was claimed to be fixed in 3.81, but wasn't, for some reason.
>     To detect this change search for 'nocomment' in the .FEATURES variable.
>
> This also fixes up the two make-cmd instances to replace # with $(pound)
> rather than with \#. There might very well be other places that need
> similar fixup in preparation for whatever future Make release contains
> the above change, but at least this builds an x86_64 defconfig with the
> new make.
>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=197847
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---

Applied to linux-kbuild.  Thanks!


-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2018-04-09 13:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-25 22:32 [PATCH] objtool: fix build with future Make Rasmus Villemoes
2018-03-25 23:09 ` [PATCH 2/1] Kbuild: fix # escaping in .cmd files for " Rasmus Villemoes
2018-03-26 11:48   ` Masahiro Yamada
2018-04-05 21:43     ` Rasmus Villemoes
2018-04-06  3:53       ` Masahiro Yamada
2018-04-08 21:35         ` [PATCH v2] " Rasmus Villemoes
2018-04-09 13:31           ` Masahiro Yamada
2018-03-25 23:42 ` [PATCH] objtool: fix build with " Randy Dunlap
2018-03-27  5:44 ` 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).