[01/30] gcc-plugins: fix build condition of SANCOV plugin
diff mbox

Message ID 1523595999-27433-2-git-send-email-yamada.masahiro@socionext.com
State New
Headers show

Commit Message

Masahiro Yamada April 13, 2018, 5:06 a.m. UTC
Since commit d677a4d60193 ("Makefile: support flag
-fsanitizer-coverage=trace-cmp"), you miss to build the SANCOV
plugin under some circumstances.

  CONFIG_KCOV=y
  CONFIG_KCOV_ENABLE_COMPARISONS=y
  Your compiler does not support -fsanitize-coverage=trace-pc
  Your compiler does not support -fsanitize-coverage=trace-cmp

Under this condition, $(CFLAGS_KCOV) is not empty but contains a
space, so the following ifeq-conditional is false.

    ifeq ($(CFLAGS_KCOV),)

Then, scripts/Makefile.gcc-plugins misses to add sancov_plugin.so to
gcc-plugin-y while the SANCOV plugin is necessary as an alternative
means.

Fixes: d677a4d60193 ("Makefile: support flag -fsanitizer-coverage=trace-cmp")
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3:
  - newly added

Changes in v2: None

 scripts/Makefile.gcc-plugins | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Masahiro Yamada May 4, 2018, 2:21 p.m. UTC | #1
Hi Kees,


2018-04-13 14:06 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> Since commit d677a4d60193 ("Makefile: support flag
> -fsanitizer-coverage=trace-cmp"), you miss to build the SANCOV
> plugin under some circumstances.
>
>   CONFIG_KCOV=y
>   CONFIG_KCOV_ENABLE_COMPARISONS=y
>   Your compiler does not support -fsanitize-coverage=trace-pc
>   Your compiler does not support -fsanitize-coverage=trace-cmp
>
> Under this condition, $(CFLAGS_KCOV) is not empty but contains a
> space, so the following ifeq-conditional is false.
>
>     ifeq ($(CFLAGS_KCOV),)
>
> Then, scripts/Makefile.gcc-plugins misses to add sancov_plugin.so to
> gcc-plugin-y while the SANCOV plugin is necessary as an alternative
> means.
>
> Fixes: d677a4d60193 ("Makefile: support flag -fsanitizer-coverage=trace-cmp")
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---


I am planning to queue this up to the fixes branch
since this is a bug fix.

Do you have any comment on this?





> Changes in v3:
>   - newly added
>
> Changes in v2: None
>
>  scripts/Makefile.gcc-plugins | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index b2a95af..7f5c862 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -14,7 +14,7 @@ ifdef CONFIG_GCC_PLUGINS
>    endif
>
>    ifdef CONFIG_GCC_PLUGIN_SANCOV
> -    ifeq ($(CFLAGS_KCOV),)
> +    ifeq ($(strip $(CFLAGS_KCOV)),)
>        # It is needed because of the gcc-plugin.sh and gcc version checks.
>        gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)           += sancov_plugin.so
>
> --
> 2.7.4
>
Kees Cook May 4, 2018, 4:21 p.m. UTC | #2
On Fri, May 4, 2018 at 7:21 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Kees,
>
>
> 2018-04-13 14:06 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>> Since commit d677a4d60193 ("Makefile: support flag
>> -fsanitizer-coverage=trace-cmp"), you miss to build the SANCOV
>> plugin under some circumstances.
>>
>>   CONFIG_KCOV=y
>>   CONFIG_KCOV_ENABLE_COMPARISONS=y
>>   Your compiler does not support -fsanitize-coverage=trace-pc
>>   Your compiler does not support -fsanitize-coverage=trace-cmp
>>
>> Under this condition, $(CFLAGS_KCOV) is not empty but contains a
>> space, so the following ifeq-conditional is false.
>>
>>     ifeq ($(CFLAGS_KCOV),)
>>
>> Then, scripts/Makefile.gcc-plugins misses to add sancov_plugin.so to
>> gcc-plugin-y while the SANCOV plugin is necessary as an alternative
>> means.
>>
>> Fixes: d677a4d60193 ("Makefile: support flag -fsanitizer-coverage=trace-cmp")
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>
>
> I am planning to queue this up to the fixes branch
> since this is a bug fix.
>
> Do you have any comment on this?

Looks fine to me; thanks!

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

>
>
>
>
>
>> Changes in v3:
>>   - newly added
>>
>> Changes in v2: None
>>
>>  scripts/Makefile.gcc-plugins | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
>> index b2a95af..7f5c862 100644
>> --- a/scripts/Makefile.gcc-plugins
>> +++ b/scripts/Makefile.gcc-plugins
>> @@ -14,7 +14,7 @@ ifdef CONFIG_GCC_PLUGINS
>>    endif
>>
>>    ifdef CONFIG_GCC_PLUGIN_SANCOV
>> -    ifeq ($(CFLAGS_KCOV),)
>> +    ifeq ($(strip $(CFLAGS_KCOV)),)
>>        # It is needed because of the gcc-plugin.sh and gcc version checks.
>>        gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)           += sancov_plugin.so
>>
>> --
>> 2.7.4
>>
>
>
>
> --
> Best Regards
> Masahiro Yamada
Masahiro Yamada May 5, 2018, 1:35 a.m. UTC | #3
2018-05-05 1:21 GMT+09:00 Kees Cook <keescook@chromium.org>:
> On Fri, May 4, 2018 at 7:21 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Hi Kees,
>>
>>
>> 2018-04-13 14:06 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>>> Since commit d677a4d60193 ("Makefile: support flag
>>> -fsanitizer-coverage=trace-cmp"), you miss to build the SANCOV
>>> plugin under some circumstances.
>>>
>>>   CONFIG_KCOV=y
>>>   CONFIG_KCOV_ENABLE_COMPARISONS=y
>>>   Your compiler does not support -fsanitize-coverage=trace-pc
>>>   Your compiler does not support -fsanitize-coverage=trace-cmp
>>>
>>> Under this condition, $(CFLAGS_KCOV) is not empty but contains a
>>> space, so the following ifeq-conditional is false.
>>>
>>>     ifeq ($(CFLAGS_KCOV),)
>>>
>>> Then, scripts/Makefile.gcc-plugins misses to add sancov_plugin.so to
>>> gcc-plugin-y while the SANCOV plugin is necessary as an alternative
>>> means.
>>>
>>> Fixes: d677a4d60193 ("Makefile: support flag -fsanitizer-coverage=trace-cmp")
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>
>>
>> I am planning to queue this up to the fixes branch
>> since this is a bug fix.
>>
>> Do you have any comment on this?
>
> Looks fine to me; thanks!
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> -Kees
>
>>



Applied to linux-kbuild/fixes.

Patch
diff mbox

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index b2a95af..7f5c862 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -14,7 +14,7 @@  ifdef CONFIG_GCC_PLUGINS
   endif
 
   ifdef CONFIG_GCC_PLUGIN_SANCOV
-    ifeq ($(CFLAGS_KCOV),)
+    ifeq ($(strip $(CFLAGS_KCOV)),)
       # It is needed because of the gcc-plugin.sh and gcc version checks.
       gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)           += sancov_plugin.so