LTP cannot compile correctly due to duplicate object file names

[btashton]

When compiling LTP there are missing symbols because the Make.dep file is being built incorrectly.

For example see these entries that are all overriding each other:

❯ grep "12-1\.home" -A 2 ./Make.dep
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.c \
 /usr/include/stdc-predef.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_open/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/time.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/sem_wait/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/mmap/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_receive/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/sigqueue/12-1.c \
 /usr/include/stdc-predef.h \
--
12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
 ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c \
 /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \

This results in a lot of missing symbols, for example:

/usr/bin/ld: nuttx.rel:(.rodata+0xf52c): undefined reference to `ltp_timer_create_speculative_15_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf53c): undefined reference to `ltp_timer_create_speculative_2_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf54c): undefined reference to `ltp_timer_create_speculative_5_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf55c): undefined reference to `ltp_timer_delete_speculative_5_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf56c): undefined reference to `ltp_timer_delete_speculative_5_2_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf57c): undefined reference to `ltp_timer_getoverrun_speculative_6_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf58c): undefined reference to `ltp_timer_getoverrun_speculative_6_2_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf5ac): undefined reference to `ltp_timer_gettime_speculative_6_1_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf5bc): undefined reference to `ltp_timer_gettime_speculative_6_2_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf5cc): undefined reference to `ltp_timer_gettime_speculative_6_3_main'
/usr/bin/ld: nuttx.rel:(.rodata+0xf5ec): undefined reference to `ltp_timer_settime_speculative_12_1_main'

Additionally I had to make these to be able to support the large amount of command line arguments:
Changes to the OS:

diff --git a/tools/Config.mk b/tools/Config.mk
index 6adbebf21a..6400121281 100644
--- a/tools/Config.mk
+++ b/tools/Config.mk
@@ -328,9 +328,13 @@ endef
 #
 #   CONFIG_WINDOWS_NATIVE - Defined for a Windows native build
 
+define NL
+
+
+endef
+
 define ARCHIVE_ADD
-       @echo "AR (add): ${shell basename $(1)} $(2)"
-       $(Q) $(AR) $1 $(2)
+       $(Q) $(AR) $1 $(2) $(NL)
 endef
 
 # ARCHIVE - Same as above, but ensure the archive is
@@ -467,12 +471,13 @@ define CLEAN
        $(Q) if exist *$(LIBEXT) (del /f /q *$(LIBEXT))
        $(Q) if exist *~ (del /f /q *~)
        $(Q) if exist (del /f /q  .*.swp)
-       $(Q) if exist $(OBJS) (del /f /q $(OBJS))
+       $(Q) $(foreach OBJ, $(OBJS), $(if exist $(OBJS) (del /f /q $(OBJ))))
        $(Q) if exist $(BIN) (del /f /q  $(BIN))
 endef
 else
 define CLEAN
-       $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp $(OBJS) $(BIN)
+       $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp $(BIN)
+       $(Q) $(foreach OBJ, $(OBJS), $(rm -f $(OBJ)))
 endef
 endif

And these changes to apps:

diff --git a/Application.mk b/Application.mk
index d1dba985..f606398d 100644
--- a/Application.mk
+++ b/Application.mk
@@ -140,9 +140,9 @@ $(CXXOBJS): %$(SUFFIX)$(OBJEXT): %$(CXXEXT)
 
 archive:
 ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-       $(call ARCHIVE_ADD, "${shell cygpath -w $(BIN)}", $(OBJS))
+       $(foreach OBJ, $(OBJS), $(call ARCHIVE_ADD, "${shell cygpath -w $(BIN)}", $(OBJ)))
 else
-       $(call ARCHIVE_ADD, $(BIN), $(OBJS))
+       $(foreach OBJ, $(OBJS), $(call ARCHIVE_ADD, $(BIN) $(OBJ)))
 endif
 
 ifeq ($(BUILD_MODULE),y)

The defconfig for the sim is here that I used:

defconfig.txt

[btashton]

@xiaoxiang781216 This is very close to working, but I am a little unsure what to do with the VPATH issue.

[xiaoxiang781216]

@ttnie and @anchao please take a look and fix the issue reported by @btashton ASAP.

[protobits]

@btashton is the collision in Make.dep happening for files with same name on different places? Or is it really just one file that appears multiple times on Make.dep?

[protobits]

BTW, it seems I never applied the parallel dependency generation feature to apps (I thought I did). Maybe I can try to do that and deal with the VPATH issue while at it.

[protobits]

A potentially problematic line could be this one in Application.mk:
.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)

I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.

Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.

[btashton]

A potentially problematic line could be this one in Application.mk:
.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)

I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.

Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.

The issue here is that we need to use the full path and not just the filename and then the base path of the application directory. If I had

myapp/src/1.c
myapp/src/libs/1.c
myapp/src/2.c

I would have the same issue
With two entries like

1.full.app.dir:
1.full.app.dir:
2.full.app.dir:

[protobits]

Yeah, I'm not sure how to handle this since VPATH is actually a concatenation of paths. Maybe always using the last entry would be correct but it would still require a particular layout of external code (it would change depending on if it uses recursive make calls or if it builds all files in subdirectories from the top one).

[btashton]

I was able to make a small change to the mkdeps tool to allows the multiple entries, but the real issue here is that we are using replace in our calls to archive.
Take this example where we have to objects with the same object name:

apps/testing/ltp on  master [⇡$✘+?] took 25s 
❯ ar rcs  /home/bashton/nuttx/wrk/apps/libapps2.a ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o 
❯ ar rcs  /home/bashton/nuttx/wrk/apps/libapps2.a ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  
❯ nm /home/bashton/nuttx/wrk/apps/libapps2.a

12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
00000000 t a_thread_func
0000000a T ltp_interfaces_pthread_create_12_1_main
         U printf
         U pthread_create
         U strerror

We only end up with a single entry in the archive for the two different object files with the same name.

Removing the replace argument we no longer have this issue:

apps/testing/ltp on  master [⇡$✘+?] took 3s 
❯ ar cs  /home/bashton/nuttx/wrk/apps/libapps.a ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  
❯ ar cs  /home/bashton/nuttx/wrk/apps/libapps.a ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o 
❯ nm /home/bashton/nuttx/wrk/apps/libapps.a

12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
00000000 t a_thread_func
0000000a T ltp_interfaces_pthread_create_12_1_main
         U printf
         U pthread_create
         U strerror

12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
00000000 t a_thread_func
0000000a T ltp_interfaces_pthread_create_12_1_main
         U printf
         U pthread_create
         U strerror

I think making that change would break a lot of other things.

Another option would be to create a single object file that is then added to the libapp.a archive.

apps/testing/ltp on  master [⇡$✘+?] took 17s 
❯ ld -m elf_i386 -r ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  -o ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o
❯ ar rcs libapps.a ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o 
❯ nm libapps.a 

ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o:
00000013 t a_thread_func
00000518 t a_thread_func
00000004 b barrier
         U __errno
         U exit
         U getpid
00000000 b in_handler
00000000 t justreturn_handler
0000025a T ltp_interfaces_mq_timedsend_12_1_main
00000522 T ltp_interfaces_pthread_create_12_1_main
         U mq_open
         U mq_timedsend
00000000 d mq_timedsend_errno
         U mq_unlink
         U nanosleep
         U perror
         U printf
         U pthread_barrier_destroy
         U pthread_barrier_init
         U pthread_barrier_wait
         U pthread_create
         U pthread_exit
         U pthread_join
         U pthread_kill
         U sigaction
         U sigemptyset
         U sprintf
         U strerror
         U strlen
         U time

@xiaoxiang781216 Do you have any thoughts on this? I would be nice to be able to support this more generally without having this restriction of the symbols needing to be unique names across all apps. We are just asking to be hit with an unexpected bug.

[xiaoxiang781216]

We just hit the command line to large problem.

[protobits]

A potentially problematic line could be this one in Application.mk:
.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)
I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.
Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.

The issue here is that we need to use the full path and not just the filename and then the base path of the application directory. If I had

myapp/src/1.c
myapp/src/libs/1.c
myapp/src/2.c

I would have the same issue
With two entries like

1.full.app.dir:
1.full.app.dir:
2.full.app.dir:

I understand, but I was trying to come up with a fix that does this and at the same time does the right thing for NuttX's sources and couldn't think of a way. The line I highlighted uses VPATH in some way but I'm not entirely sure if it actually does anything useful.

[protobits]

BTW, the "replace" argument is probably not needed anymore. With the change I made sometime ago about how libraries are built, they are always built from scratch on each build (even without make clean). So it should be safe to not do any replace.

[btashton]

We just hit the command line to large problem.

I resolved the command line too large issue with my changes I commented on the top. Do you want me to put my fixes in draft PRs?

[btashton]

BTW, the "replace" argument is probably not needed anymore. With the change I made sometime ago about how libraries are built, they are always built from scratch on each build (even without make clean). So it should be safe to not do any replace.

That would make this simpler. I can see if that resolves this problem.