| Message ID | 20200730092637.487-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] semctl: Fix 32 bit build | expand |
Hi! > testcases/kernel/syscalls/ipc/semctl/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/ipc/semctl/Makefile b/testcases/kernel/syscalls/ipc/semctl/Makefile > index 99971a7db..2559b4c28 100644 > --- a/testcases/kernel/syscalls/ipc/semctl/Makefile > +++ b/testcases/kernel/syscalls/ipc/semctl/Makefile > @@ -8,6 +8,6 @@ LTPLIBS = ltpipc ltpnewipc > include $(top_srcdir)/include/mk/testcases.mk > > semctl01 semctl02 semctl03 semctl04 semctl05 semctl06 semctl07: LDLIBS += -lltpipc > -semctl08: LDLIBS += -lltpnewipc > +semctl08: LDLIBS = -lltpnewipc -lltp If nothing else this may break things if user passed something in LDLIBS, so it should be: LDLIBS = -lltpnewipc $(LDLIBS) And I guess the safest rule would be to add the -lltp* libraries first, because naturally none of the code in LTP but the test depends on these.
Hi Cyril, > Hi! > > testcases/kernel/syscalls/ipc/semctl/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/ipc/semctl/Makefile b/testcases/kernel/syscalls/ipc/semctl/Makefile > > index 99971a7db..2559b4c28 100644 > > --- a/testcases/kernel/syscalls/ipc/semctl/Makefile > > +++ b/testcases/kernel/syscalls/ipc/semctl/Makefile > > @@ -8,6 +8,6 @@ LTPLIBS = ltpipc ltpnewipc > > include $(top_srcdir)/include/mk/testcases.mk > > semctl01 semctl02 semctl03 semctl04 semctl05 semctl06 semctl07: LDLIBS += -lltpipc > > -semctl08: LDLIBS += -lltpnewipc > > +semctl08: LDLIBS = -lltpnewipc -lltp > If nothing else this may break things if user passed something in > LDLIBS, so it should be: > LDLIBS = -lltpnewipc $(LDLIBS) Thanks! I tried that before, but without ':' before '=': semctl08: LDLIBS = -lltpnewipc $(LDLIBS) Makefile:12: *** Recursive variable 'LDLIBS' references itself (eventually). Stop. Assigning as := fixes that: -semctl08: LDLIBS += -lltpnewipc +semctl08: LDLIBS := -lltpnewipc $(LDLIBS) Sorry for overlooking obvious error. > And I guess the safest rule would be to add the -lltp* libraries first, > because naturally none of the code in LTP but the test depends on these. Are you're going to fix by changing order somewhere in include/mk/? Or shell I push the fix with your ack? I'd prefer proper fix so commits like this or 22f510de8 ("Fix static linking with musl-fts") aren't needed any more. Kind regards, Petr
Hi! > > If nothing else this may break things if user passed something in > > LDLIBS, so it should be: > > > LDLIBS = -lltpnewipc $(LDLIBS) > > Thanks! I tried that before, but without ':' before '=': > semctl08: LDLIBS = -lltpnewipc $(LDLIBS) > Makefile:12: *** Recursive variable 'LDLIBS' references itself (eventually). Stop. > > Assigning as := fixes that: > -semctl08: LDLIBS += -lltpnewipc > +semctl08: LDLIBS := -lltpnewipc $(LDLIBS) > > Sorry for overlooking obvious error. > > > And I guess the safest rule would be to add the -lltp* libraries first, > > because naturally none of the code in LTP but the test depends on these. > Are you're going to fix by changing order somewhere in include/mk/? > Or shell I push the fix with your ack? > I'd prefer proper fix so commits like this or 22f510de8 ("Fix static linking > with musl-fts") aren't needed any more. I wonder what would be the easiest solution here. The main problem is that these flags are per-testcase defined and are not expanded before we enter rule to build a test. And as we are using implicit rules to compile C code we cannot easily change that. I guess that we can write down our rules and do whatever we want there though.
Hi Cyril, > > > And I guess the safest rule would be to add the -lltp* libraries first, > > > because naturally none of the code in LTP but the test depends on these. > > Are you're going to fix by changing order somewhere in include/mk/? > > Or shell I push the fix with your ack? > > I'd prefer proper fix so commits like this or 22f510de8 ("Fix static linking > > with musl-fts") aren't needed any more. > I wonder what would be the easiest solution here. > The main problem is that these flags are per-testcase defined and are > not expanded before we enter rule to build a test. And as we are using > implicit rules to compile C code we cannot easily change that. > I guess that we can write down our rules and do whatever we want there > though. Thanks for info. Well, I'll probably merge the original fix then. Kind regards, Petr
Hi! > > I wonder what would be the easiest solution here. > > > The main problem is that these flags are per-testcase defined and are > > not expanded before we enter rule to build a test. And as we are using > > implicit rules to compile C code we cannot easily change that. > > > I guess that we can write down our rules and do whatever we want there > > though. > Thanks for info. Well, I'll probably merge the original fix then. So I've been looking into the problem for a while and due to a make limitations the best bet for a solution would be adding a special variable for the LTP libraries as: diff --git a/include/mk/testcases.mk b/include/mk/testcases.mk index bb22be82e..03937516a 100644 --- a/include/mk/testcases.mk +++ b/include/mk/testcases.mk @@ -59,5 +59,8 @@ LDFLAGS += $(addprefix -L$(top_builddir)/libs/lib, $(LTPLIBS)) endif +%: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $< $(LTPLDLIBS) $(LDLIBS) -o $@ + $(LTPLIBS_DIRS) $(APICMDS_DIR) $(LIBLTP_DIR): %: mkdir -p "$@" diff --git a/testcases/kernel/syscalls/ipc/shmctl/Makefile b/testcases/kernel/syscalls/ipc/shmctl/Makefile index 0172bb495..2e0ed0ceb 100644 --- a/testcases/kernel/syscalls/ipc/shmctl/Makefile +++ b/testcases/kernel/syscalls/ipc/shmctl/Makefile @@ -10,7 +10,7 @@ shmctl05: LDLIBS += -lrt include $(top_srcdir)/include/mk/testcases.mk -shmctl01 shmctl02 shmctl03 shmctl04 shmctl05: LDLIBS += -lltpipc -shmctl06: LDLIBS += -lltpnewipc +shmctl01 shmctl02 shmctl03 shmctl04 shmctl05: LTPLDLIBS = -lltpipc +shmctl06: LTPLDLIBS = -lltpnewipc include $(top_srcdir)/include/mk/generic_leaf_target.mk
Hi Metan, > Hi! > > > I wonder what would be the easiest solution here. > > > The main problem is that these flags are per-testcase defined and are > > > not expanded before we enter rule to build a test. And as we are using > > > implicit rules to compile C code we cannot easily change that. > > > I guess that we can write down our rules and do whatever we want there > > > though. > > Thanks for info. Well, I'll probably merge the original fix then. > So I've been looking into the problem for a while and due to a make > limitations the best bet for a solution would be adding a special > variable for the LTP libraries as: > diff --git a/include/mk/testcases.mk b/include/mk/testcases.mk > index bb22be82e..03937516a 100644 > --- a/include/mk/testcases.mk > +++ b/include/mk/testcases.mk > @@ -59,5 +59,8 @@ LDFLAGS += $(addprefix -L$(top_builddir)/libs/lib, $(LTPLIBS)) > endif > +%: %.c > + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $< $(LTPLDLIBS) $(LDLIBS) -o $@ > + > $(LTPLIBS_DIRS) $(APICMDS_DIR) $(LIBLTP_DIR): %: > mkdir -p "$@" > diff --git a/testcases/kernel/syscalls/ipc/shmctl/Makefile b/testcases/kernel/syscalls/ipc/shmctl/Makefile > index 0172bb495..2e0ed0ceb 100644 > --- a/testcases/kernel/syscalls/ipc/shmctl/Makefile > +++ b/testcases/kernel/syscalls/ipc/shmctl/Makefile > @@ -10,7 +10,7 @@ shmctl05: LDLIBS += -lrt > include $(top_srcdir)/include/mk/testcases.mk > -shmctl01 shmctl02 shmctl03 shmctl04 shmctl05: LDLIBS += -lltpipc > -shmctl06: LDLIBS += -lltpnewipc > +shmctl01 shmctl02 shmctl03 shmctl04 shmctl05: LTPLDLIBS = -lltpipc > +shmctl06: LTPLDLIBS = -lltpnewipc > include $(top_srcdir)/include/mk/generic_leaf_target.mk Nice, it fixes problem here and in testcases/kernel/syscalls/ipc/semctl/Makefile (the original problem, if I also change to LTPLDLIBS). But various other dependencies across LTP fail: https://travis-ci.org/github/pevik/ltp/builds/713370406 e.g. https://travis-ci.org/github/pevik/ltp/jobs/713370415 gcc -L/usr/src/ltp/utils/sctp/func_tests/../lib -L/usr/src/ltp/utils/sctp/func_tests/../testlib -L../../../lib test_fragments_v6.o -lltp -lsctputil -lsctp -lpthread -o test_fragments_v6 /usr/lib/gcc/x86_64-alpine-linux-musl/9.3.0/../../../../x86_64-alpine-linux-musl/bin/ld: /tmp/ccBpdLjB.o: in function `setup': /usr/src/ltp/testcases/kernel/syscalls/bpf/bpf_map01.c:133: undefined reference to `rlimit_bump_memlock' /usr/lib/gcc/x86_64-alpine-linux-musl/9.3.0/../../../../x86_64-alpine-linux-musl/bin/ld: /tmp/ccBpdLjB.o: in function `run': /usr/src/ltp/testcases/kernel/syscalls/bpf/bpf_map01.c:53: undefined reference to `bpf_map_create' collect2: error: ld returned 1 exit status https://travis-ci.org/github/pevik/ltp/jobs/713370407 /usr/bin/ld: /tmp/ccaGt1eJ.o: in function `main': /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1012: undefined reference to `configureClient' /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1015: undefined reference to `getConfiguration' /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1008: undefined reference to `configureServer' /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1009: undefined reference to `setupClients' Obviously using LTPLDLIBS doesn't apply here. But I don't see what is missing / wrong. Kind regards, Petr
Hi! > https://travis-ci.org/github/pevik/ltp/jobs/713370407 > /usr/bin/ld: /tmp/ccaGt1eJ.o: in function `main': > /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1012: undefined reference to `configureClient' > /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1015: undefined reference to `getConfiguration' > /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1008: undefined reference to `configureServer' > /usr/bin/ld: /usr/src/ltp/testcases/network/nfsv4/locks/locktests.c:1009: undefined reference to `setupClients' > > Obviously using LTPLDLIBS doesn't apply here. But I don't see what is missing / > wrong. Well it matched our explicit rule instead of (different) implicit one, looks like we have to define a full set, I will send a patchset later on.
diff --git a/testcases/kernel/syscalls/ipc/semctl/Makefile b/testcases/kernel/syscalls/ipc/semctl/Makefile index 99971a7db..2559b4c28 100644 --- a/testcases/kernel/syscalls/ipc/semctl/Makefile +++ b/testcases/kernel/syscalls/ipc/semctl/Makefile @@ -8,6 +8,6 @@ LTPLIBS = ltpipc ltpnewipc include $(top_srcdir)/include/mk/testcases.mk semctl01 semctl02 semctl03 semctl04 semctl05 semctl06 semctl07: LDLIBS += -lltpipc -semctl08: LDLIBS += -lltpnewipc +semctl08: LDLIBS = -lltpnewipc -lltp include $(top_srcdir)/include/mk/generic_leaf_target.mk
Since dfe45f3cf libltpnewipc started to depend on safe macros from libltp.a, thus order for linking must be -lltpnewipc -lltp. Default order (where -lltp goes first) caused build failures on 32 bit: cd testcases/kernel/syscalls/ipc/semctl gcc -m32 -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=return-type -fno-common -g -O2 -fno-strict-aliasing -pipe -Wall -W -Wold-style-definition -I../../../../../include -I../../../../../include -I../../../../../include/old/ -m32 -L../../../../../libs/libltpipc -L../../../../../libs/libltpnewipc -L../../../../../lib semctl08.c -lltp -lltpnewipc -o semctl08 /usr/bin/ld: ../../../../../libs/libltpnewipc/libltpnewipc.a(libnewipc.o): in function `probe_free_addr': libs/libltpnewipc/libnewipc.c:79: undefined reference to `safe_shmget' /usr/bin/ld: libs/libltpnewipc/libnewipc.c:81: undefined reference to `safe_shmat' /usr/bin/ld: libs/libltpnewipc/libnewipc.c:82: undefined reference to `safe_shmdt' /usr/bin/ld: libs/libltpnewipc/libnewipc.c:83: undefined reference to `safe_shmctl' NOTE: other uses of -lltpnewipc (msg{ctl,get,snd}, shm{at,ctl}) are not affected. Fixes: dfe45f3cf ("libs/libltpnewipc: Use safe macros") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, Reason why I don't push this fix directly is that I wonder why msg{ctl,get,snd}, shm{at,ctl} aren't affected and whether we can avoid directly specifying -lltp. Kind regards, Petr testcases/kernel/syscalls/ipc/semctl/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)