Fix a crash bug caused by a nested call of parallelForEach.

[ruiu]

Summary

parallelForEach is not reentrant. We use parallelForEach to call
each section's writeTo(), so calling the same function within writeTo()
is not safe.

Fixes https://bugs.llvm.org/show_bug.cgi?id=41508

[andrewrk]

I tested this and confirm that it solves the bug (which I reported).

[ruiu]

I will keep this patch for a day to give sbc100 to take a look.

This patch should be included in 8.0.1 once landed.

[sbc100]

So IIUC, this moves the parallelism to per-input-file, rather per-chuck-per-input-file.

@ruiu presumably we want to restore more parallelism if we land this change? Having said that perhaps this won't be much of regression for reasonable sized projects since the projects that benefit from high parallelism will most likely have many input files. One concern is LTO, where there can be essentially one giant object file, in which case I think we would loose all our parallelism. However I imagine in that case that the link time would massively dominated by the compile time?

Otherwise LGTM.

Also, it makes me sad that parallelForEach can't detect this behaviour, especially since it leads to bugs that are hard to reliably reproduce. Should we try to get parallelForEach fixed?

[sbc100]

In D60757#1468973, @sbc100 wrote:

So IIUC, this moves the parallelism to per-input-file, rather per-chuck-per-input-file.

@ruiu presumably we want to restore more parallelism if we land this change? Having said that perhaps this won't be much of regression for reasonable sized projects since the projects that benefit from high parallelism will most likely have many input files. One concern is LTO, where there can be essentially one giant object file, in which case I think we would loose all our parallelism. However I imagine in that case that the link time would massively dominated by the compile time?

Otherwise LGTM.

Also, it makes me sad that parallelForEach can't detect this behaviour, especially since it leads to bugs that are hard to reliably reproduce. Should we try to get parallelForEach fixed?

Sorry, I just saw https://reviews.llvm.org/D60758. lgtm

[ruiu]

There should not be a fundamental reason for parallelForEach to not be reentrant, so we eventually want to fix it so that we can call the function without worrying the nested condition. For the time being, we should be able to detect the error condition more easily by adding the assertions.

wasm/OutputSections.cpp

void CodeSection::writeTo(uint8_t *Buf) {
   // Write section header
   memcpy(Buf, Header.data(), Header.size());
   Buf += Header.size();
 
   // Write code section headers
   memcpy(Buf, CodeSectionHeader.data(), CodeSectionHeader.size());
 
   // Write code section bodies
-  parallelForEach(Functions,
-                  [&](const InputChunk *Chunk) { Chunk->writeTo(Buf); });
+  for (const InputChunk *Chunk : Functions)
+    Chunk->writeTo(Buf);
 }
 
 uint32_t CodeSection::numRelocations() const {
   uint32_t Count = 0;
   for (const InputChunk *Func : Functions)
     Count += Func->NumRelocations();
   return Count;
 }
void DataSection::writeTo(uint8_t *Buf) {
 
   // Write section header
   memcpy(Buf, Header.data(), Header.size());
   Buf += Header.size();
 
   // Write data section headers
   memcpy(Buf, DataSectionHeader.data(), DataSectionHeader.size());
 
-  parallelForEach(Segments, [&](const OutputSegment *Segment) {
+  for (const OutputSegment *Segment : Segments) {
     // Write data segment header
     uint8_t *SegStart = Buf + Segment->SectionOffset;
     memcpy(SegStart, Segment->Header.data(), Segment->Header.size());
 
     // Write segment data payload
     for (const InputChunk *Chunk : Segment->InputSegments)
       Chunk->writeTo(Buf);
-  });
+  }
 }
 
 uint32_t DataSection::numRelocations() const {
   uint32_t Count = 0;
   for (const OutputSegment *Seg : Segments)
     for (const InputChunk *InputSeg : Seg->InputSegments)
       Count += InputSeg->NumRelocations();
   return Count;
void CustomSection::writeTo(uint8_t *Buf) {
 
   // Write section header
   memcpy(Buf, Header.data(), Header.size());
   Buf += Header.size();
   memcpy(Buf, NameData.data(), NameData.size());
   Buf += NameData.size();
 
   // Write custom sections payload
-  parallelForEach(InputSections,
-                  [&](const InputSection *Section) { Section->writeTo(Buf); });
+  for (const InputSection *Section : InputSections)
+    Section->writeTo(Buf);
 }
 
 uint32_t CustomSection::numRelocations() const {
   uint32_t Count = 0;
   for (const InputSection *InputSect : InputSections)
     Count += InputSect->NumRelocations();
   return Count;
 }
 
 void CustomSection::writeRelocations(raw_ostream &OS) const {
   for (const InputSection *S : InputSections)
     S->writeRelocations(OS);
 }