linux-kernel - Re: [PATCH] selftests: sync: missing CFLAGS while compiling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 16 Jan 2018 12:00:45 +0100
From:   Anders Roxell <anders.roxell@...aro.org>
To:     shuah@...nel.org
Cc:     linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org, tglx@...utronix.de,
        Kate Stewart <kstewart@...uxfoundation.org>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Lei.Yang@...driver.com, Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH] selftests: sync: missing CFLAGS while compiling

On 10 January 2018 at 01:05, Shuah Khan <shuah@...nel.org> wrote:
> On 01/05/2018 09:33 AM, Anders Roxell wrote:
>> Based on patch: https://patchwork.kernel.org/patch/10042045/
>>
>> arch64-linux-gnu-gcc -c sync.c -o sync/sync.o
>> sync.c:42:29: fatal error: linux/sync_file.h: No such file or directory
>>  #include <linux/sync_file.h>
>>                              ^
>> CFLAGS is not used during the compile step, so the system instead of
>> kernel headers are used.  Fix this by using lib.mk's compile rules and
>> remove CFLAGS from the linking step.
>
> Hmm. The changes don't match the change log. It odes more than just
> removing LDFLAGS from compile step,

oh, you are correct. I'm sorry.
I will re-spin the patch and make the changelog reflect the actual change
next time.

>
>>
>> Reported-by: Lei Yang <Lei.Yang@...driver.com>
>> Signed-off-by: Anders Roxell <anders.roxell@...aro.org>
>> ---
>>  tools/testing/selftests/sync/Makefile | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
>> index b3c8ba3cb668..58b9336d6c84 100644
>> --- a/tools/testing/selftests/sync/Makefile
>> +++ b/tools/testing/selftests/sync/Makefile
>> @@ -27,12 +27,6 @@ OBJS := $(patsubst %,$(OUTPUT)/%,$(OBJS))
>>  TESTS := $(patsubst %,$(OUTPUT)/%,$(TESTS))
>>
>>  $(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS)
>> -     $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS)
>
> So why not just delete $(LDFLAGS)??
> $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS)
>
>> -
>> -$(OBJS): $(OUTPUT)/%.o: %.c
>> -     $(CC) -c $^ -o $@
>> -
>> -$(TESTS): $(OUTPUT)/%.o: %.c
>> -     $(CC) -c $^ -o $@
>> +     $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(LDFLAGS)
>>
>>  EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS)
>>
>
> I can't take this patch the way it is. It breaks the following
> use-case:
>
> make O=/tmp/kselftest TARGETS=sync kselftest

Oh, I've missed that use-case. Will fix that in the re-spin of the patch.

To fix this build issue when cross compiling

--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -30,7 +30,7 @@ $(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS)
        $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS)

 $(OBJS): $(OUTPUT)/%.o: %.c
-       $(CC) -c $^ -o $@
+       $(CC) -c $^ -o $@ $(CFLAGS)

 $(TESTS): $(OUTPUT)/%.o: %.c
        $(CC) -c $^ -o $@


The other issue that needs to be addressed somehow would be make the
build system do
headers_install or if that should be a prerequisite.
I've seen ways to install the headers: "make -C ../../../..
headers_install" if the headers
aren't there, in
tools/testing/selftests/gpio/Makefile
tools/testing/selftests/vm/Makefile

Is that the preferred way or should an update in
Documentation/dev-tools/kselftest.rst be done to say do "make
headers_install" before
building kselftests?


Cheers,
Anders

>
> thanks,
> -- Shuah