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

[WebAssembly] Sink wasm section headers #295

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
23 changes: 19 additions & 4 deletions src/webassembly.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,27 +194,36 @@ void ReadNames(const Section& section, IndexedNames* func_names,
kElemSegment = 8,
kDataSegment = 9
};

string_view header = section.data;
string_view data = section.contents;

while (!data.empty()) {
NameType type = static_cast<NameType>(ReadVarUInt7(&data));
uint32_t size = ReadVarUInt32(&data);
sink->AddFileRange("wasm_overhead", "[Wasm Section Headers]",
StrictSubstr(header, 0, data.data() - header.data()));

string_view section = ReadPiece(size, &data);

if (type == NameType::kFunction || type == NameType::kDataSegment) {
if (type == NameType::kFunction || type == NameType::kDataSegment
|| type == NameType::kGlobal) {
uint32_t count = ReadVarUInt32(&section);
for (uint32_t i = 0; i < count; i++) {
string_view entry = section;
uint32_t index = ReadVarUInt32(&section);
uint32_t name_len = ReadVarUInt32(&section);
string_view name = ReadPiece(name_len, &section);
entry = StrictSubstr(entry, 0, name.data() - entry.data() + name.size());
sink->AddFileRange("wasm_funcname", name, entry);
sink->AddFileRange("wasm_name", name, entry);

// Global names aren't really functions or data, we don't track them yet.
if (type == NameType::kGlobal)
continue;
IndexedNames *names = (type == NameType::kFunction ? func_names : dataseg_names);
(*names)[index] = std::string(name);
}
}
header = data;
}
}

Expand Down Expand Up @@ -287,8 +296,9 @@ uint32_t GetNumFunctionImports(const Section& section) {
void ReadCodeSection(const Section& section, const IndexedNames& names,
uint32_t num_imports, RangeSink* sink) {
string_view data = section.contents;

uint32_t count = ReadVarUInt32(&data);
sink->AddFileRange("wasm_overhead", "[Wasm Section Headers]",
StrictSubstr(section.data, 0, data.data() - section.data.data()));

for (uint32_t i = 0; i < count; i++) {
string_view func = data;
Expand All @@ -304,6 +314,7 @@ void ReadCodeSection(const Section& section, const IndexedNames& names,
std::string name = "func[" + std::to_string(i) + "]";
sink->AddFileRange("wasm_function", name, func);
} else {
printf("function %d, %s\n", i, iter->second.c_str());
sink->AddFileRange("wasm_function", ItaniumDemangle(iter->second, sink->data_source()), func);
}
}
Expand All @@ -313,6 +324,9 @@ void ReadDataSection(const Section& section, const IndexedNames& names,
RangeSink* sink) {
string_view data = section.contents;
uint32_t count = ReadVarUInt32(&data);
printf("Section.data: %p, contents: %p\n", section.data.data(), section.contents.data());
sink->AddFileRange("wasm_overhead", "[Wasm Section Headers]",
StrictSubstr(section.data, 0, data.data() - section.data.data()));

for (uint32_t i = 0; i < count; i++) {
string_view segment = data;
Expand All @@ -338,6 +352,7 @@ void ReadDataSection(const Section& section, const IndexedNames& names,
std::string name = "data[" + std::to_string(i) + "]";
sink->AddFileRange("wasm_data", name, segment);
} else {
printf("data %d, %s\n", i, iter->second.c_str());
sink->AddFileRange("wasm_data", iter->second, segment);
}
}
Expand Down
18 changes: 15 additions & 3 deletions tests/wasm/symbol_test.test
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Sections:

# Data section
# FIXME: This is wrong, should be the data section header
# CHECK!!!!: 1c6-1c9 3 [Wasm Section Headers]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something very strange is going on here. When I run bloaty with -vv I can see in the log:

[shortsymbols, wasm_function] AddFileRange(main, 1b6, 10)
[shortsymbols, wasm_overhead] AddFileRange([Wasm Section Headers], 1c6, 3)

(and no unexpected AddFileRange calls for "main").
But the file map output has

1b6-1c6	         16		main
1c6-1c9	          3		main

i.e. the name from the entry starting at 1b6 entry has overridden the 1c6 entry. "main" is the last entry in the code section, and the section header entry is the first one in the data section. A similar overflow seems to happen between the data and name sections below.
Any idea what might be happening?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think you might be running into a heuristic that has gone wrong:

https://github.com/google/bloaty/blob/master/src/range_map.cc#L294-L300

https://github.com/google/bloaty/blob/master/src/range_map.h#L170-L173

This is intended to fill in padding gaps between functions. But it's gone wrong here, because your [Wasm Section Headers] is an accurate/precious label, not a fallback.

We should probably change that combining Compress() pass to run prior to AddCatchAll(), and look for truly empty ranges rather than fallback ranges.

# BUG: 1c6-1c9 3 main
# CHECK: 1c9-1cf 6 .data.global2
# CHECK: 1cf-1d5 6 .data.global3
Expand All @@ -193,16 +194,27 @@ Sections:
# Name section
# FIXME: Name section header and subsection header is 1db-1e6
# BUG: 1db-1e6 11 .data.global4
# CHECK!!: 1db-1e6 11 [Wasm Section Headers]
# Function names
# CHECK: 1e6-1f9 19 __wasm_call_ctors
# CHECK: 1f9-20a 17 __wasm_init_tls
# CHECK: 20a-21e 20 __wasm_init_memory
# CHECK: 21e-225 7 func1
# CHECK: 225-22c 7 func2
# CHECK: 22c-23d 17 __original_main
# FIXME: Subsection headers and global names
# BUG: 23d-243 6 main
# BUG: 243-27f 60 [section name]

# FIXME: this is the following:
# BUG: 23d-246 9 main
# But it should be:
# CHECK!!: 23d-243 6 main



# Global names
# CHECK: 246-257 17 __stack_pointer
# CHECK: 257-263 12 __tls_base
# CHECK: 263-26f 12 __tls_size
# CHECK: 26f-27f 16 __tls_align
# Data names
# CHECK: 27f-28e 15 .data.global2
# CHECK: 28e-29d 15 .data.global3
Expand Down