netdev - Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code

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]

Message-ID: <CAEf4BzbQ3JF9s_LsOEeVUoXDR57USdNFvQ4E=5vUzQ=sAPuUaQ@mail.gmail.com>
Date:   Wed, 31 Jul 2019 10:04:20 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Ilya Leoshkevich <iii@...ux.ibm.com>
Cc:     Stanislav Fomichev <sdf@...ichev.me>,
        Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be
 compiled as C code

On Wed, Jul 31, 2019 at 6:21 AM Ilya Leoshkevich <iii@...ux.ibm.com> wrote:
>
> > Am 27.07.2019 um 20:53 schrieb Andrii Nakryiko <andrii.nakryiko@...il.com>:
> >
> > On Fri, Jul 26, 2019 at 3:01 PM Stanislav Fomichev <sdf@...ichev.me> wrote:
> >>
> >> On 07/26, Andrii Nakryiko wrote:
> >>> On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@...ichev.me> wrote:
> >>>>
> >>>> On 07/26, Andrii Nakryiko wrote:
> >>>>> Apprently listing header as a normal dependency for a binary output
> >>>>> makes it go through compilation as if it was C code. This currently
> >>>>> works without a problem, but in subsequent commits causes problems for
> >>>>> differently generated test.h for test_progs. Marking those headers as
> >>>>> order-only dependency solves the issue.
> >>>> Are you sure it will not result in a situation where
> >>>> test_progs/test_maps is not regenerated if tests.h is updated.
> >>>>
> >>>> If I read the following doc correctly, order deps make sense for
> >>>> directories only:
> >>>> https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
> >>>>
> >>>> Can you maybe double check it with:
> >>>> * make
> >>>> * add new prog_tests/test_something.c
> >>>> * make
> >>>> to see if the binary is regenerated with test_something.c?
> >>>
> >>> Yeah, tested that, it triggers test_progs rebuild.
> >>>
> >>> Ordering is still preserved, because test.h is dependency of
> >>> test_progs.c, which is dependency of test_progs binary, so that's why
> >>> it works.
> >>>
> >>> As to why .h file is compiled as C file, I have no idea and ideally
> >>> that should be fixed somehow.
> >> I guess that's because it's a prerequisite and we have a target that
> >> puts all prerequisites when calling CC:
> >>
> >> test_progs: a.c b.c tests.h
> >>        gcc a.c b.c tests.h -o test_progs
> >>
> >> So gcc compiles each input file.
> >
> > If that's really a definition of the rule, then it seems not exactly
> > correct. What if some of the prerequisites are some object files,
> > directories, etc. I'd say the rule should only include .c files. I'll
> > add it to my TODO list to go and check how all this is defined
> > somewhere, but for now I'm leaving everything as is in this patch.
> >
>
> I believe it’s an implicit built-in rule, which is defined by make
> itself here:
>
> https://git.savannah.gnu.org/cgit/make.git/tree/default.c?h=4.2.1#n131
>
> The observed behavior arises because these rules use $^ all over the
> place. My 2c is that it would be better to make the rule explicit,
> because it would cost just an extra line, but it would be immediately
> obvious why the code is correct w.r.t. rebuilds.

Thanks for pointing this out, Ilya! I agree, I'd rather have a simple
explicit rule in Makefile that having to guess where this is coming
from and why it doesn't work as you'd expect it to. If no one else
adds that first, I'll eventually get to this.

>
> Best regards,
> Ilya