Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exporting/importing debug location information from .wast/.asm.js/.s formats #1017

Merged
merged 4 commits into from
Jun 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions auto_update_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
print ' '.join(cmd)
actual = run_command(cmd)
with open(os.path.join('test', wasm), 'w') as o: o.write(actual)
if 'debugInfo' in asm:
cmd += ['--source-map', os.path.join('test', wasm + '.map'), '-o', 'a.wasm']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a.wasm the right place for this? it should be stored where check.py looks for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code above generates proper data for test;a.wasm is just a file we can delete -- we only need source map from this run.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. Sorry for my confusion.

run_command(cmd)


for dot_s_dir in ['dot_s', 'llvm_autogenerated']:
for s in sorted(os.listdir(os.path.join('test', dot_s_dir))):
Expand Down
27 changes: 27 additions & 0 deletions check.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,31 @@ def do_asm2wasm_test():
fail_with_error('wasm interpreter error: ' + err) # failed to pretty-print
fail_with_error('wasm interpreter error')

# verify debug info
if 'debugInfo' in asm:
jsmap = 'a.wasm.map'
cmd += ['--source-map', jsmap,
'--source-map-url', 'http://example.org/' + jsmap,
'-o', 'a.wasm']
run_command(cmd)
if not os.path.isfile(jsmap):
fail_with_error('Debug info map not created: %s' % jsmap)
with open(wasm + '.map', 'rb') as expected:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this uses some new test output files in the test dir. please also update auto_update_tests.py which should generate all the test output files in the test folder

with open(jsmap, 'rb') as actual:
fail_if_not_identical(actual.read(), expected.read())
with open('a.wasm', 'rb') as binary:
url_section_name = bytearray([16]) + bytearray('sourceMappingURL')
payload = 'http://example.org/' + jsmap
assert len(payload) < 256, 'name too long'
url_section_contents = bytearray([len(payload)]) + bytearray(payload)
print url_section_name
binary_contents = bytearray(binary.read())
if url_section_name not in binary_contents:
fail_with_error('source map url section not found in binary')
if url_section_contents not in binary_contents[binary_contents.index(url_section_name):]:
fail_with_error('source map url not found in url section')


print '\n[ checking asm2wasm binary reading/writing... ]\n'

asmjs = os.path.join(options.binaryen_test, 'hello_world.asm.js')
Expand Down Expand Up @@ -241,6 +266,8 @@ def do_asm2wasm_test():
print '..', t
t = os.path.join(options.binaryen_test, t)
cmd = WASM_DIS + [t]
if os.path.isfile(t + '.map'): cmd += ['--source-map', t + '.map']

actual = run_command(cmd)

with open(t + '.fromBinary') as f:
Expand Down
4 changes: 2 additions & 2 deletions src/asm2wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1382,13 +1382,13 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
while (i < expressionStack.size()) {
exp = expressionStack[i];
if (debugLocations.count(exp) == 0) {
debugLocations[exp] = { fileIndex, lineNumber };
debugLocations[exp] = { fileIndex, lineNumber, 0 };
break;
}
i++;
}
} else {
debugLocations[exp] = { fileIndex, lineNumber };
debugLocations[exp] = { fileIndex, lineNumber, 0 };
}
break;
}
Expand Down
19 changes: 19 additions & 0 deletions src/parsing.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,25 @@ struct ParseException {
}
};

struct MapParseException {
std::string text;

MapParseException() : text("unknown parse error") {}
MapParseException(std::string text) : text(text) {}

void dump(std::ostream& o) {
Colors::magenta(o);
o << "[";
Colors::red(o);
o << "map parse exception: ";
Colors::green(o);
o << text;
Colors::magenta(o);
o << "]";
Colors::normal(o);
}
};

// Helper for parsers that may not have unique label names. This transforms
// the names into unique ones, as required by Binaryen IR.
struct UniqueNameMapper {
Expand Down
9 changes: 7 additions & 2 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {

Module* currModule = nullptr;
Function* currFunction = nullptr;
Function::DebugLocation lastPrintedLocation;

PrintSExpression(std::ostream& o) : o(o) {
setMinify(false);
Expand All @@ -58,8 +59,11 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
auto iter = debugLocations.find(curr);
if (iter != debugLocations.end()) {
auto fileName = currModule->debugInfoFileNames[iter->second.fileIndex];
o << ";; " << fileName << ":" << iter->second.lineNumber << '\n';
doIndent(o, indent);
if (lastPrintedLocation != iter->second) {
lastPrintedLocation = iter->second;
o << ";;@ " << fileName << ":" << iter->second.lineNumber << ":" << iter->second.columnNumber << '\n';
doIndent(o, indent);
}
}
}
Visitor<PrintSExpression>::visit(curr);
Expand Down Expand Up @@ -599,6 +603,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
}
void visitFunction(Function *curr) {
currFunction = curr;
lastPrintedLocation = { 0, 0, 0 };
printOpening(o, "func ", true);
printName(curr->name);
if (curr->type.is()) {
Expand Down
27 changes: 21 additions & 6 deletions src/s2wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class S2WasmBuilder {
MixedArena* allocator;
LinkerObject* linkerObj;
std::unique_ptr<LinkerObject::SymbolInfo> symbolInfo;
std::unordered_map<uint32_t, uint32_t> fileIndexMap;

public:
S2WasmBuilder(const char* input, bool debug)
Expand Down Expand Up @@ -601,7 +602,9 @@ class S2WasmBuilder {
size_t fileId = getInt();
skipWhitespace();
auto quoted = getQuoted();
WASM_UNUSED(fileId); WASM_UNUSED(quoted); // TODO: use the fileId and quoted
uint32_t index = wasm->debugInfoFileNames.size();
fileIndexMap[fileId] = index;
wasm->debugInfoFileNames.push_back(std::string(quoted.begin(), quoted.end()));
s = strchr(s, '\n');
return;
}
Expand Down Expand Up @@ -665,22 +668,31 @@ class S2WasmBuilder {

mustMatch(":");

Function::DebugLocation debugLocation = { 0, 0, 0 };
bool useDebugLocation = false;
auto recordFile = [&]() {
if (debug) dump("file");
size_t fileId = getInt();
skipWhitespace();
auto quoted = getQuoted();
WASM_UNUSED(fileId); WASM_UNUSED(quoted); // TODO: use the fileId and quoted
uint32_t index = wasm->debugInfoFileNames.size();
fileIndexMap[fileId] = index;
wasm->debugInfoFileNames.push_back(std::string(quoted.begin(), quoted.end()));
s = strchr(s, '\n');
};
auto recordLoc = [&]() {
if (debug) dump("loc");
size_t fileId = getInt();
skipWhitespace();
size_t row = getInt();
uint32_t row = getInt();
skipWhitespace();
size_t column = getInt();
WASM_UNUSED(fileId); WASM_UNUSED(row); WASM_UNUSED(column); // TODO: use the fileId, row and column
uint32_t column = getInt();
auto iter = fileIndexMap.find(fileId);
if (iter == fileIndexMap.end()) {
abort_on("idx");
}
useDebugLocation = true;
debugLocation = { iter->second, row, column };
s = strchr(s, '\n');
};
auto recordLabel = [&]() {
Expand Down Expand Up @@ -746,7 +758,10 @@ class S2WasmBuilder {
// parse body
func->body = allocator->alloc<Block>();
std::vector<Expression*> bstack;
auto addToBlock = [&bstack](Expression* curr) {
auto addToBlock = [&](Expression* curr) {
if (useDebugLocation) {
func->debugLocations[curr] = debugLocation;
}
Expression* last = bstack.back();
if (last->is<Loop>()) {
last = last->cast<Loop>()->body;
Expand Down
19 changes: 16 additions & 3 deletions src/tools/asm2wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ int main(int argc, const char *argv[]) {
bool legalizeJavaScriptFFI = true;
Asm2WasmBuilder::TrapMode trapMode = Asm2WasmBuilder::TrapMode::JS;
bool wasmOnly = false;
std::string sourceMapFilename;
std::string sourceMapUrl;
std::string symbolMap;
bool emitBinary = true;

Expand Down Expand Up @@ -99,9 +101,15 @@ int main(int argc, const char *argv[]) {
[&legalizeJavaScriptFFI](Options *o, const std::string &) {
legalizeJavaScriptFFI = false;
})
.add("--debuginfo", "-g", "Emit names section and debug info (for debug info you must emit text, -S, for this to work)",
.add("--debuginfo", "-g", "Emit names section in wasm binary (or full debuginfo in wast)",
Options::Arguments::Zero,
[&](Options *o, const std::string &arguments) { options.passOptions.debugInfo = true; })
.add("--source-map", "-sm", "Emit source map (if using binary output) to the specified file",
Options::Arguments::One,
[&sourceMapFilename](Options *o, const std::string &argument) { sourceMapFilename = argument; })
.add("--source-map-url", "-su", "Use specified string as source map URL",
Options::Arguments::One,
[&sourceMapUrl](Options *o, const std::string &argument) { sourceMapUrl = argument; })
.add("--symbolmap", "-s", "Emit a symbol map (indexes => names)",
Options::Arguments::One,
[&](Options *o, const std::string &argument) { symbolMap = argument; })
Expand Down Expand Up @@ -136,8 +144,9 @@ int main(int argc, const char *argv[]) {
}

Asm2WasmPreProcessor pre;
// wasm binaries can contain a names section, but not full debug info
pre.debugInfo = options.passOptions.debugInfo && !emitBinary;
// wasm binaries can contain a names section, but not full debug info --
// debug info is disabled if a map file is not specified with wasm binary
pre.debugInfo = options.passOptions.debugInfo && (!emitBinary || sourceMapFilename.size());
auto input(
read_file<std::vector<char>>(options.extra["infile"], Flags::Text, options.debug ? Flags::Debug : Flags::Release));
char *start = pre.process(input.data());
Expand Down Expand Up @@ -204,6 +213,10 @@ int main(int argc, const char *argv[]) {
writer.setDebugInfo(options.passOptions.debugInfo);
writer.setSymbolMap(symbolMap);
writer.setBinary(emitBinary);
if (emitBinary) {
writer.setSourceMapFilename(sourceMapFilename);
writer.setSourceMapUrl(sourceMapUrl);
}
writer.write(wasm, options.extra["output"]);

if (options.debug) std::cerr << "done." << std::endl;
Expand Down
20 changes: 19 additions & 1 deletion src/tools/wasm-as.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ using namespace wasm;
int main(int argc, const char *argv[]) {
bool debugInfo = false;
std::string symbolMap;
std::string sourceMapFilename;
std::string sourceMapUrl;
Options options("wasm-as", "Assemble a .wast (WebAssembly text format) into a .wasm (WebAssembly binary format)");
options.extra["validate"] = "wasm";
options
Expand All @@ -51,6 +53,12 @@ int main(int argc, const char *argv[]) {
.add("--debuginfo", "-g", "Emit names section and debug info",
Options::Arguments::Zero,
[&](Options *o, const std::string &arguments) { debugInfo = true; })
.add("--source-map", "-sm", "Emit source map to the specified file",
Options::Arguments::One,
[&sourceMapFilename](Options *o, const std::string &argument) { sourceMapFilename = argument; })
.add("--source-map-url", "-su", "Use specified string as source map URL",
Options::Arguments::One,
[&sourceMapUrl](Options *o, const std::string &argument) { sourceMapUrl = argument; })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are duplicated in two files. i am currently getting rid of most of the existing duplication in #1023. it would be good to do the same thing for these options, however, i understand if you want to land this first and leave the refactoring to followup work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather move this out of scope this PR -- --source-map and --source-map-url are not really optimization options and have different meaning in some cases, e.g. for asm2wasm --source-map means output file, but for wasm-dis -- input file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point, we'll need more refactoring here. Definitely let's leave for later.

.add("--symbolmap", "-s", "Emit a symbol map (indexes => names)",
Options::Arguments::One,
[&](Options *o, const std::string &argument) { symbolMap = argument; })
Expand Down Expand Up @@ -86,13 +94,23 @@ int main(int argc, const char *argv[]) {
if (options.debug) std::cerr << "binarification..." << std::endl;
BufferWithRandomAccess buffer(options.debug);
WasmBinaryWriter writer(&wasm, buffer, options.debug);
writer.setDebugInfo(debugInfo);
// if debug info is used, then we want to emit the names section
writer.setNamesSection(debugInfo);
std::unique_ptr<std::ofstream> sourceMapStream = nullptr;
if (sourceMapFilename.size()) {
sourceMapStream = make_unique<std::ofstream>();
sourceMapStream->open(sourceMapFilename);
writer.setSourceMap(sourceMapStream.get(), sourceMapUrl);
}
if (symbolMap.size() > 0) writer.setSymbolMap(symbolMap);
writer.write();

if (options.debug) std::cerr << "writing to output..." << std::endl;
Output output(options.extra["output"], Flags::Binary, options.debug ? Flags::Debug : Flags::Release);
buffer.writeTo(output);
if (sourceMapStream) {
sourceMapStream->close();
}

if (options.debug) std::cerr << "Done." << std::endl;
}
16 changes: 16 additions & 0 deletions src/tools/wasm-dis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,17 @@ using namespace cashew;
using namespace wasm;

int main(int argc, const char *argv[]) {
std::string sourceMapFilename;
Options options("wasm-dis", "Un-assemble a .wasm (WebAssembly binary format) into a .wast (WebAssembly text format)");
options.add("--output", "-o", "Output file (stdout if not specified)",
Options::Arguments::One,
[](Options *o, const std::string &argument) {
o->extra["output"] = argument;
Colors::disable();
})
.add("--source-map", "-sm", "Consume source map from the specified file to add location information",
Options::Arguments::One,
[&sourceMapFilename](Options *o, const std::string &argument) { sourceMapFilename = argument; })
.add_positional("INFILE", Options::Arguments::One,
[](Options *o, const std::string &argument) {
o->extra["infile"] = argument;
Expand All @@ -46,11 +50,23 @@ int main(int argc, const char *argv[]) {
if (options.debug) std::cerr << "parsing binary..." << std::endl;
Module wasm;
try {
std::unique_ptr<std::ifstream> sourceMapStream;
WasmBinaryBuilder parser(wasm, input, options.debug);
if (sourceMapFilename.size()) {
sourceMapStream = make_unique<std::ifstream>();
sourceMapStream->open(sourceMapFilename);
parser.setDebugLocations(sourceMapStream.get());
}
parser.read();
if (sourceMapStream) {
sourceMapStream->close();
}
} catch (ParseException& p) {
p.dump(std::cerr);
Fatal() << "error in parsing wasm binary";
} catch (MapParseException& p) {
p.dump(std::cerr);
Fatal() << "error in parsing wasm source mapping";
}

if (options.debug) std::cerr << "Printing..." << std::endl;
Expand Down
Loading