[WebAssembly] MC: Fix for outputing wasm object to /dev/null

[efriedma]

lib/MC/WasmObjectWriter.cpp

405 ↗

(On Diff #184382)

It seems unsafe to assume all streams that don't support tell() are /dev/null.

[sbc100]

lib/MC/WasmObjectWriter.cpp

405 ↗

(On Diff #184382)

This code relies on OS.tell() working... if OS.tell() doesn't work should we abort instead? Then we would break the /dev/null. Are there other raw_pwrite_stream's that don't support OS.tell()? It seems like the ability to pwrite would normally come with the ability to tell().

[sbc100]

lib/MC/WasmObjectWriter.cpp

405 ↗

(On Diff #184382)

There is precedence at raw_ostream.h:352 for handling a zero result from tell() to mean /dev/null.

[efriedma]

lib/MC/WasmObjectWriter.cpp

405 ↗

(On Diff #184382)

Can we handle this in some more consistent way?

I guess it's an issue with the way we detect whether a device supports seeking. For /dev/null and friends, lstat() with any offset technically succeeds, but it doesn't actually seek anywhere: the resulting offset is 0. We should probably fix our detection code to recognize that and set supportsSeeking() to false, instead of scattering weird special cases across LLVM. Maybe just reuse the Windows codepath in raw_fd_ostream::raw_fd_ostream that checks the fd refers to a file.

I guess that means we would end up buffering the entire file in some cases, due to supportsSeeking() checks; not sure how important that is.

If we do decide to go with this patch, you probably don't want to change the type of the "Size" variable; you're breaking the overflow check.

[sbc100]

• Fix type of Size

[sbc100]

lib/MC/WasmObjectWriter.cpp

405 ↗

(On Diff #184382)

I guess technically it doesn't support seeking so we should probably report supportsSeeking() correctly. However, I'd prefer to land this change than then follow up if thats OK?

[efriedma]

LGTM

lib/MC/WasmObjectWriter.cpp

408 ↗

(On Diff #184581)

"Size &&" doesn't seem useful here? I mean, if Size == 0, the overflow test will pass.

405 ↗

(On Diff #184382)

Sure.

llvm/trunk/lib/MC/WasmObjectWriter.cpp

void WasmObjectWriter::startCustomSection(SectionBookkeeping &Section,
 
   // The position where the custom section starts.
   Section.ContentsOffset = W.OS.tell();
 }
 
 // Now that the section is complete and we know how big it is, patch up the
 // section size field at the start of the section.
 void WasmObjectWriter::endSection(SectionBookkeeping &Section) {
-  uint64_t Size = W.OS.tell() - Section.PayloadOffset;
+  uint64_t Size = W.OS.tell();
+  // /dev/null doesn't support seek/tell and can report offset of 0.
+  // Simply skip this patching in that case.
+  if (!Size)
+    return;
+
+  Size -= Section.PayloadOffset;
   if (uint32_t(Size) != Size)
     report_fatal_error("section size does not fit in a uint32_t");
 
   LLVM_DEBUG(dbgs() << "endSection size=" << Size << "\n");
 
   // Write the final section size to the payload_len field, which follows
   // the section id byte.
   WritePatchableLEB(static_cast<raw_pwrite_stream &>(W.OS), Size,

llvm/trunk/test/MC/WebAssembly/null-output.s

+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o /dev/null < %s
+
+    .text
+    .section .text.main,"",@
+    .type    main,@function
+main:
+    .functype   main (i32, i32) -> (i32)
+    end_function
+.Lfunc_end0:
+    .size main, .Lfunc_end0-main