Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Michal Marek]

On 2014-08-06 14:19, Konstantin Khlebnikov wrote:
> On Wed, Aug 6, 2014 at 3:45 PM, Michal Marek <mmarek@xxxxxxx> wrote:
>> On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
>>> This already has been fixed in commit c353acba28fb3fa1fd05fd
>>> ("kbuild: make: fix if_changed when command contains backslashes")
>>> but escaping still isn't perfect and triggers false-positive rebuilds.
>>>
>>> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
>>> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
>>> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
>>> Macro if_changed detects command change and rebuilds target again and again.
>>>
>>> Backslash escaping conflicts with other passes because it's used for escaping
>>> other symbols. To avoid that current macro handles only double backslashes.
>>> Obviously this doesn't work for \1 like above.
>>>
>>> This patch reorders passes. It doubles all backslashes before escaping # and '
>>>
>>> Visible effect in rebuilding x86/defconfig without changes, before patch:
>>>
>>> blind@zurg:~/src/linux$ make V=2
>>>   CHK     include/config/kernel.release
>>>   CHK     include/generated/uapi/linux/version.h
>>>   CHK     include/generated/utsrelease.h
>>>   CALL    scripts/checksyscalls.sh - due to target missing
>>>   CHK     include/generated/compile.h
>>>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
>>
>> With which make and shell version are you seeing this? While the patch
>> looks correct, I can't reproduce the error here:
> 
> /bin/sh points to dash (debian default setup).
> 
> I cannot reproduce this using bash. That explains why this bug is still here.

So the difference between the shells is that their 'echo' builtin treats
\<number> differently:
$ ./dash -c "echo '\2'"

$ ./dash -c "echo '\2'" | xxd
0000000: 020a                                     ..
$ /bin/bash -c "echo '\2'"
\2

For some reason we fail to escape the \ and dash treats \2 as \02. POSIX
says that this is implementation-defined, so none of the shells is wrong.

I need to look into this a bit further. I'll most likely end up applying
your patch, but I'd like to understand the fix in detail first.

Thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/