Handle single-segment source mapping in source map header decoder

[osa1]

Single-segment mappings were already handled in readNextDebugLocation, but not in readSourceMapHeader.

Update readSourceMapHeader to handle single-segment mappings.

The code to handle single segments is copied from readNextDebugLocation.

[kripken]

Thanks! The code looks right here.

Please add a test for this - see #5906 for a PR that added similar functionality, so I am hoping the test can be similar to that (maybe extend it?).

[kripken]

(see also the error on CI, there is a minor indentation issue)

[osa1]

@kripken I've tried to add a unit test.. It's not ideal because I don't have access to the binary reader internals and can't check that the debug information is really parsed, but at least it tests that readSourceMapHeader does not crash on single-segment mappings.

Can we permanently allow testing this PR please?

[kripken]

I enabled testing now, but unfortunately I think github will not do that on further pushes to this branch. Github seems to require owners to approve testing each time, for the first PR to a repo. I'm not aware of a way to override that.

For a test, this might be good enough, but I was imagining something like

https://github.com/WebAssembly/binaryen/pull/5906/files#diff-580536145381d1bd50b499c1de9db09552b6f6cd50973190e2e8da955e2d4815

We can check in a wasm and a .wasm.map file alongside it, and the test can refer to them using %s.wasm / %s.wasm.map and then show that it prints out the debug info correctly.

Though, a gtest is also an elegant way to test this. If you prefer that (I'm ok either way) then let's do something like this test:

https://github.com/WebAssembly/binaryen/blob/0c62b402fa37e2777cf384677f604e24b78bbf71/test/gtest/printing.cpp

which uses text for the module input and not raw bytes, and does a test on output from the module.

[osa1]

I need to test WasmBinaryReader::readSourceMapHeader, so I need a binary module to be able to initialize WasmBinaryReader.

Are you suggesting writing a Wat module, then parse it as Module, then serialize it as binary, then using it to initialize WasmBinaryReader?

E.g.

#include "parser/wat-parser.h"
#include "wasm-binary.h"
#include "gtest/gtest.h"

using namespace wasm;

void parseWast(wasm::Module& wasm, const std::string& wast) {
  auto parsed = wasm::WATParser::parseModule(wasm, wast);
  if (auto* err = parsed.getErr()) {
    wasm::Fatal() << err->msg << "\n";
  }
}

// Check that debug location parsers can handle single-segment mappings.
TEST(BinaryReaderTest, SourceMappingSingleSegment) {
  auto moduleText = "(module)";
  Module module;
  parseWast(module, moduleText);

  BufferWithRandomAccess buffer;
  WasmBinaryWriter(&module, buffer, PassOptions());
  auto moduleBytes = buffer.getAsChars();

  // the rest is the same as before
}

(I can't figure out how to use parseWast from test/gtest/print-test.h, for some reason even with the header included I can't access it)

[kripken]

Ah, I think you need to copy the line

#include "print-test.h"

from the top of the example test I linked to.

If that's not it, what is the exact error you are seeing?

[osa1]

Ah, I think you need to copy the line

Thanks. A few more changes were needed but we figured it out. PTAL.

[kripken]

Looks good, though this doesn't test the output AFAICT? In my previous comment I suggested two ways in which to do that, but now that I think more about it, I guess there is no debug info to print - is that why you are not testing that? (If so, perhaps add a comment to the test like There is no debug info to print for such 1-segment entries, so all we test here is that we do not error)

(Also there is a minor CI error on the order of headers.)

[osa1]

@kripken a single-segment mapping only has the generated code location info (line/column) but not the source information, so if effectively disables mapping the region of the generated code to a source.

It's useful when you have something like: compiled user code, runtime system code (that doesn't correspond to any source code), compiled user code again. In these cases the code in the middle is not mapped, using a single-segment mapping.

Normally how I would test this is by checking that nextDebugLocationHasDebugInfo is false after reading the mapping, but that member is private.

If you have a better way to test lmk and I'll update.

Please also feel free to update my branch.

[kripken]

Makes sense, thanks. I added a comment.