Thank you for spotting this and closing in on the cause. For information, another way to print the commands executed by make is to run V=1 make.

The problem is that when the compiler is configured with --enable-tsan, LDOPTS is set to the contents of contains -lm -lpthread -fsanitize=thread. We need to keep only the first two.

Removing (LDOPTS) doesn't seems to cause any errors on my local machine (linux) where (LDTOPS) expands to

-lzstd -fsanitize=thread -lm -lpthread

as you have mentioned @OlivierNicole

But in runner which seems to run only on an windows environment, couple of tests fail. The popular error seems to be

Fatal error: cannot load shared library dllunixbyt

In the runner, (LDOPTS) expands to

-ldopt -lws2_32 -ldopt -ladvapi32

Removing LDOPTS is almost certainly the wrong thing to do here. But there may be deeper issues:

• ocamlmklib must know about C libraries (-lxxx), their paths (-L/path/to/lib), and any flags that cc -shared needs to produce a shared object. It doesn't need to know about cc flags for static linking. I don't know in which category -fsanitize-thread belongs.
• ocamlmklib provides a -ldopt xxx flag to pass option xxx to cc -shared. However, quoting every word of LDFLAGS with -ldopt isn't going to work, because of a weird issue with -L/path/to/lib options, see pass LDFLAGS to ocamlmklib via -ldopt Zarith#124 (comment) .

I think we should try to improve ocamlmkliib rather than torture our makefiles even more. One possibility would be to silently pass any unrecognized option to cc -shared.

• ocamlmklib must know about C libraries (-lxxx), their paths (-L/path/to/lib), and any flags that cc -shared needs to produce a shared object. It doesn't need to know about cc flags for static linking. I don't know in which category -fsanitize-thread belongs.

I think that only executables needs to be linked with -fsanitize=thread, but I’m not entirely sure.

I am skeptical that it's correct to add -fsanitize=thread to an ldflags variable.s

Is there a dedicated variable for options that need to be passed when linking executables, but not libraries?

Yes, it also needs to be passed when compiling.

-fsanitize=thread seems to be required both at compile time (generating *.o files) and link time as shown here and here (for asan).

I believe the error message is printed by ocamlmklib over here and not the compiler or the linker.

In a private discussion with @Johan511 we agreed that the simplest option would be for ocamlmklib to ignore the flag -fsanitize=thread, rather than passing it down as in the current state of this PR. The reason for this is that ocamlc and ocamlopt already “know” how to compile and link C/object files with TSan support.

I can confirm this works to remove the spurious warning, but I will refrain from approving as I am biased as an internship mentor.

Then maybe @shindere would be available to review this?

After discussing this in private with @shindere, we agreed that -fsanitize=thread being added to LDFLAGS is a bit of a code smell, as CFLAGS ought to be included in LDFLAGS (edit: that’s not true. See next message for a correct summary). I will look into cleaning that and report on what it implies for this PR.

Sorry this is not correct. A more accurate summary is:

-fsanitize=thread should be part of CFLAGS. The CFLAGS should be passed to all the commands that need them. If this is done correctly, the flags should be passed to all commands that need them and no more; we should avoid the situation where part of these flags are wrongly passed to ocamlmklib and we are forced to remove them. In particular, it is fishy that in the current situation, -fsanitize=thread gets added to LDFLAGS.

Closing this PR, since the problem it attempted to fix has been addressed in #13201.

Although this problem has found a more principled solution, thanks @Johan511 for the initial fix proposal!