include/zone/export.h may not get created if GNU make uses --shuffle mode

[Kumba42]

Hi, I am investigating a bug in Gentoo, #936119, which is happening in an automated tinderbox build where the include/zone/export.h file isn't getting created during the compilation phase if GNU make employs its --shuffle option, which will cause it to "simulate non-deterministic build order in parallel makefiles".

I suspect the tinderbox build does this as a build fuzzer to find obscure failure modes, but it got me looking at the nature of this one file, and I'm puzzled why it's being created at build time by the Makefile, when there doesn't seem to be anything conditional about it. The file should always get created if the normal flow of make targets happen (all -> libzone.a -> EXPORT_HEADER). Nor is the #include line in include/zone.h that pulls in zone/export.h itself conditional. So I feel like this file should always exist and the specific target that creates it in the Makefile logic be removed, but I wanted to see if there's some undocumented reasoning why it's set up this way first.

[k0ekk0ek]

Hi @Kumba42! The has to with how simdzone is included in NSD. If built as a standalone project via CMake, the export header is generated using GenerateExportHeader so that dllexport/dllimport on Windows and is automatically correct (dll vs. lib). On other platforms it takes care of the visibility attribute, though it's not as important to actually use the library. We chose to include simdzone in NSD as a submodule and link it statically for convenience. Of course, in that scenario we don't care about visibility and so I simply added a Makefile rule to create a dummy header. Looking at the Makefile, I think making the *OBJECTS targets depend on the header rather than libzone.a should solve the problem?

[Kumba42]

Looking at the Makefile, I think making the *OBJECTS targets depend on the header rather than libzone.a should solve the problem?

I gave this a test and used the same shuffle seed in the failure log I have, and that doesn't appear to work. It failed compiling on a different object file with that change (src/haswell, specifically). What does appear to work with that seed and other random seeds is making $(EXPORT_HEADER) the first dependency of the 'all' target, before 'libzone.a':

--- a/simdzone/Makefile.in.orig
+++ b/simdzone/Makefile.in
@@ -33,7 +33,7 @@ EXPORT_HEADER = include/zone/export.h

 .PHONY: all clean

-all: libzone.a
+all: $(EXPORT_HEADER) libzone.a

 clean:
    @rm -f .depend
@@ -45,7 +45,7 @@ distclean: clean
 realclean: distclean
    @rm -rf autom4te*

-libzone.a: $(EXPORT_HEADER) $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)
+libzone.a: $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)
    $(AR) rcs libzone.a $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)

 $(EXPORT_HEADER):

Not sure if this would impact other build targets, though. I have a FreeBSD system that I tested this change on, and seems to be okay with it.

[k0ekk0ek]

I think if you put it as a dependency of all it doesn't signal that it's needed to compile the objects(?)

Can you give this a try?

diff --git a/Makefile.in b/Makefile.in
index 5cf81a7..4754591 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -50,22 +50,22 @@ maintainer-clean: realclean
 devclean:
        @rm -rf config.h.in configure

-libzone.a: $(EXPORT_HEADER) $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)
+libzone.a: $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)
        $(AR) rcs libzone.a $(OBJECTS) $($(WESTMERE)_OBJECTS) $($(HASWELL)_OBJECTS)

 $(EXPORT_HEADER):
        @mkdir -p include/zone
        @echo "#define ZONE_EXPORT" > include/zone/export.h

-$(WESTMERE_OBJECTS): .depend
+$(WESTMERE_OBJECTS): $(EXPORT_HEADER) .depend
        @mkdir -p src/westmere
        $(CC) $(DEPFLAGS) $(CPPFLAGS) $(CFLAGS) -march=westmere -o $@ -c $(SOURCE)/$(@:.o=.c)

-$(HASWELL_OBJECTS): .depend
+$(HASWELL_OBJECTS): $(EXPORT_HEADER) .depend
        @mkdir -p src/haswell
        $(CC) $(DEPFLAGS) $(CPPFLAGS) $(CFLAGS) -march=haswell -o $@ -c $(SOURCE)/$(@:.o=.c)

-$(OBJECTS): .depend
+$(OBJECTS): $(EXPORT_HEADER) .depend
        @mkdir -p src/fallback
        $(CC) $(DEPFLAGS) $(CPPFLAGS) $(CFLAGS) -o $@ -c $(SOURCE)/$(@:.o=.c)
        @touch $@

[k0ekk0ek]

@Kumba42, I issued a PR for this. I believe it'd be the fix for the issue, but if I build just simdzone with the --shuffle flag it doesn't trigger. I'll test by building as part of NSD and report back.

[k0ekk0ek]

A quick test locally didn't trigger the issue. I'll merge #218 to the main branch. Please let me know if it works for you, it'd be good to have this fixed in the upcoming 4.10.1 NSD release.

[Kumba42]

I think if you put it as a dependency of all it doesn't signal that it's needed to compile the objects(?)

Can you give this a try?

This one works. My original idea actually didn't -- forgot to start w/ a clean directory when doing my tests.

[k0ekk0ek]

Happy to hear it's fixed. Thanks for taking the time to report :+1: