Simplify and clean up TSan annotations

[OlivierNicole]

While working on removing the last known data races in the runtime, I had to update a number of TSan-related annotations in the code. The two primary reasons are that #12681 made some of them redundant, and that the distinction between CAMLno_tsan and CAMLreally_no_tsan is confusing and—as I discovered—not really useful in practice.

This PR is a collection of arguably independent commits, but on the other hand all of them are very small.

This PR is best reviewed commit by commit:

• Initially, we introduced CAMLreally_no_tsan as a complement to CAMLno_tsan. The idea was that CAMLno_tsan should be used to de-instrument functions that we don’t want instrumented with --enable-tsan, while CAMLreally_no_tsan de-instruments them in all cases, including when, e.g., -fsanitize=thread is passed through the CFLAGS. However, experience shows that it is vastly more convenient to chase data races in the runtime using --enable-tsan than by modifying CFLAGS. As a consequence, CAMLreally_no_tsan is not really relevant anymore. I replace the pair CAMLno_tsan / CAMLreally_no_tsan with CAMLno_tsan / CAMLno_tsan_for_perf. Functions marked CAMLno_tsan are never instrumented, whereas functions marked CAMLno_tsan_for_perf are not instrumented to save performance, except when TSAN_INSTRUMENT_ALL is defined. Defining TSAN_INSTRUMENT_ALL ensure maximum coverage when looking for data races in the runtime.
  As a consequence, I update the Inria TSan CI to define TSAN_INSTRUMENT_ALL.
• A number of TSan false positives related to volatile writes were removed by the merge of Fix TSan false positives due to volatile write handling #12681, and we no longer need to silence the corresponding reports.
• For maximum coverage, rather than marking the whole do_some_marking function with CAMLno_tsan, I un-instrumented only the instruction that could cause a false positive.
• I removed TSan annotations on functions on which I can no longer trigger data race reports.
• On the other hand, some functions started to cause data races reports again—they probably were previously silenced indirectly by the silencing directives in __tsan_default_suppressions. All of them races are already known (ThreadSanitizer issues #11040). I added two CAMLno_tsan annotations to silence those reports until we fixed those races, so that they don’t clog the CI logs.

[fabbing]

Well, I didn't coauthored this one but it looks good anyway!

[OlivierNicole]

Thank you for your reviews. I put back the CAMLno_tsan on try_update_object_header because I just realized that otherwise we will get TSan alarms on the first race of #12737 (at least, so long as that PR is not merged).

[gasche]

Approved on @fabbing and @dustanddreams' behalf.

[gasche]

Needs a Change entry.

[gasche]

(It looks like this needs a rebase again?)

[OlivierNicole]

Sorry, rebased now.

[OlivierNicole]

I deleted the branch by mistake again…

[OlivierNicole]

In the triaging meeting of 13 Dec, 2023 it was mentioned that this PR would be nice to have in 5.2, I believe. (It does make life easier to reproduce data race reports.)
Is something more required for this PR to be merged?

[gasche]

I don't remember anything from this PR but I approved above and would be happy to merge a version if the conflicts are fixed. It looks like whenever I look at it, it needs a rebase (maybe you should do the trick of inserting your Changes entry in a somewhat sorted position, instead of at the end?). Let's merge in 5.2 as well, indeed.

[OlivierNicole]

(maybe you should do the trick of inserting your Changes entry in a somewhat sorted position, instead of at the end?)

That’s a nice trick!

Rebased again.

[gasche]

@Octachron these refactoring changes were reviewed, agreed-upon and approved a month ago, can you confirm that including them in 5.2, the first TSan release, is the correct move?

[gasche]

(I'm adding the milestone here not to force inclusion, but so that we do not forget to discuss this again.)

[Octachron]

Cleaning up the TSAN annotations before the first release is indeed a good idea. Let's cherry-pick those simplifications to 5.2 .

[gasche]

Done in fb7aa7e, thanks!