opcodes for known sections

[Krovatkin]

Per WebAssembly/design#740
this adds a few bits to support opcodes for known sections
fyi @commonlisp @arunetm

@cellulle @MikeHolman homeless PR looks for a nice review and branch to be adopted into

---

This change is

[commonlisp]

Can we use a simpler test case for this?

[commonlisp]

Looks like we have to rebase this one.

[commonlisp]

@Cellule Just want to put this on your radar since other changes (globals and table/elements) will depend on it.

[Cellule]

Reviewed 1 of 4 files at r1, 9 of 11 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

lib/WasmReader/WasmBinaryReader.cpp, line 177 at r2 (raw file):

    else
    {
        sectionName = (char*)(m_pc); //including "name" section

You are capturing the pointer too soon, you must read the length first then do sectionName = m_pc.
Right now, the name string will include the leb128 encoded length

---

lib/WasmReader/WasmBinaryReader.cpp, line 179 at r2 (raw file):

        sectionName = (char*)(m_pc); //including "name" section
        nameLength = LEB128(len);
        m_pc += nameLength; //skip section name for now

Should add a check here CheckBytesLeft(nameLength); before moving the pointer.

---

lib/WasmReader/WasmBinaryReader.cpp, line 182 at r2 (raw file):

    }

    sectionSize = LEB128(len);

According to Section Header
The sectionSize comes before the name and after the id.
Also, the end is calculated with payload_len - sizeof(name) - sizeof(name_len)

---

lib/WasmReader/WasmBinaryReader.cpp, line 187 at r2 (raw file):

#if ENABLE_DEBUG_CONFIG_OPTIONS
    Assert(nameLength < 64);

While your at it, instead of an assert, could you make this a check and just print Unknown section name if the length is greater than 64.

---

test/rlexedirs.xml, line 376 at r2 (raw file):

<dir>
  <default>
    <files>WasmSpec</files>

This is going to conflict with George's change.
I suggest to wait until he merges it than resolve this file using his version since I prefer to comment out these tests for the time being rather than deleting it.

---

Comments from Reviewable

[Krovatkin]

@Cellule that's amazing how much I managed to mess up in like 10 lines of code :-)))))) Regarding removing the tests George and I weren't quite sure which change goes first. Apparently his does :-) I'll back out that part and fix the rest.

[Krovatkin]

@Cellule updated the PR.

[Cellule]

Reviewed 9 of 10 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

lib/WasmReader/WasmBinaryReader.cpp, line 178 at r3 (raw file):

    }
    else
    {

What happens to header.code in this scenario ?
I assume it keeps it default value, should we have a value that means user section ?

---

Comments from Reviewable

[Krovatkin]

lib/WasmReader/WasmBinaryReader.cpp, line 178 at r3 (raw file):

Previously, Cellule (Michael Ferris) wrote…

What happens to header.code in this scenario ?
I assume it keeps it default value, should we have a value that means user section ?

hhmmm, it is unlikely that it gets a default value, since its parent structure starts out as an uninitialized struct :-( I think adding an explicit user section is a really great idea. I updated the PR

---

Comments from Reviewable

[Cellule]

---

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[Krovatkin]

@arunetm Could you please merge this when you have a mnt?

[arunetm-zz]

Merged to WebAssembly branch.