Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Name section parsing #268

Closed
mikevoronov opened this issue Mar 20, 2019 · 5 comments
Closed

Name section parsing #268

mikevoronov opened this issue Mar 20, 2019 · 5 comments

Comments

@mikevoronov
Copy link
Contributor

mikevoronov commented Mar 20, 2019

Hi!

As i understand correctly, It seems that parsing of name section is incomplete. According to the spec it follow that: name section lies in custom section and it can has some (not one) subsections of three possible types (ModuleNameSection, FunctionNameSection, LocalNameSection). So pseudo-code for its deserialization could be as such:

1. read custom header with name and size payload_len
2. while current_readed < payload_len:
  3. read name subsection with size name_payload_len
  4. current_readed += name_payload_len

But now deserialization includes only 1 and 3 steps and parses only the first section. And names_section returns not a sequence of subsections but just an enum that could be one of these subsection types. So if a module has all these three subsection types, it is possible to get only the first one (and also serialize only one).

@folex
Copy link

folex commented Mar 26, 2019

Also, tests on name section deserialization do not check that deserialization is correct: https://github.com/paritytech/parity-wasm/blob/master/src/elements/name_section.rs#L235-L243

And that leads to a bug where deserialization incorrectly reads length twice. E.g., for NAME_TYPE_MODULE it reads here and here

Btw, we already have a fix in fluencelabs' fork of parity-wasm, you can see diff here. But we can't make tests pass by ourselves because there are some binary Wasm modules in tests that would deserialize only with a "broken" deserialization.

What's the best way to make this PR?

@folex
Copy link

folex commented Mar 26, 2019

@folex
Copy link

folex commented Mar 26, 2019

cc @svyatonik :)

@svyatonik
Copy link
Contributor

I worked on interpreter mostly, so I'd ping @NikVolf here instead :) Nikolay, could you, please, take a look? :)

@svyatonik
Copy link
Contributor

Think the reason is WebAssembly/design#984

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants