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

[wasm-objdump] set function types for correct params+locals indexing #2353

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

rvagg
Copy link
Contributor

@rvagg rvagg commented Dec 13, 2023

wasm-objdump --disassemble has different treatment of locals than --details because it doesn't collect function types so it can properly work out how many params the function has and therefore the index number of the local.

test/dump/noncanon-leb128-opcode.txt demonstrates this nicely as can be seen in here. The --details of that test module looks like this:

noncanon-leb128-opcode.0.wasm:     file format wasm 0x1

Section Details:

Type[2]:
 - type[0] () -> i32
 - type[1] (i32) -> i32
Function[1]:
 - func[0] sig=1 <main>
Table[1]:
 - table[0] type=funcref initial=1 max=9
Export[1]:
 - func[0] <main> -> "main"
Code[1]:
 - func[0] size=37 <main>

so "main" has sig=1, i.e. it takes 1 parameter, (i32) -> i32. But because --disassemble mode doesn't collect function types, it defaults all functions to sig=0, so calculation of locals[X] always looks at the first type definition to figure out the number of parameters. In this case it's zero parameters but this is arbitrary and depends on what the first type is. I have other examples if that helps.


Further details of how this happens:

  • Enter a function and figure out the initial local_index_ based on the signature look-up to find the number of params:
    Result BinaryReaderObjdumpDisassemble::BeginFunctionBody(Index index,
    Offset size) {
    printf("%06" PRIzx " func[%" PRIindex "]", GetPrintOffset(state->offset),
    index);
    auto name = GetFunctionName(index);
    if (!name.empty()) {
    printf(" <" PRIstringview ">", WABT_PRINTF_STRING_VIEW_ARG(name));
    }
    printf(":\n");
    last_opcode_end = 0;
    in_function_body = true;
    current_function_index = index;
    auto type_index = objdump_state_->function_types[index];
    local_index_ = objdump_state_->function_param_counts[type_index];
    return Result::Ok;
    }
  • local[%d] uses, and increments local_index_:
    Result BinaryReaderObjdumpDisassemble::OnLocalDecl(Index decl_index,
    Index count,
    Type type) {
    if (!in_function_body) {
    return Result::Ok;
    }
    Offset offset = current_opcode_offset;
    size_t data_size = state->offset - offset;
    printf(" %06" PRIzx ":", GetPrintOffset(offset));
    for (size_t i = 0; i < data_size && i < IMMEDIATE_OCTET_COUNT;
    i++, offset++) {
    printf(" %02x", data_[offset]);
    }
    for (size_t i = data_size; i < IMMEDIATE_OCTET_COUNT; i++) {
    printf(" ");
    }
    printf(" | local[%" PRIindex, local_index_);
    if (count != 1) {
    printf("..%" PRIindex "", local_index_ + count - 1);
    }
    local_index_ += count;
    printf("] type=%s\n", type.GetName().c_str());
    last_opcode_end = current_opcode_offset + data_size;
    current_opcode_offset = last_opcode_end;
    return Result::Ok;
    }

@rvagg rvagg force-pushed the rvagg/disassemble-fixes branch from 04d4a7c to 7529afc Compare December 13, 2023 23:43
Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

lgtm.. but I wonder why the output of only one test changed here?

@rvagg
Copy link
Contributor Author

rvagg commented Dec 14, 2023

That was a surprise to me too! I'm building tests from --disassemble output from the core spec tests and this changes a bunch of those. I think that the reason this only changes one test output file is the simplicity of the cases and the ~random chance that type 0 is mostly a zero-param function; this one happened to hit one where type 0 was something else.

@sbc100 sbc100 merged commit 4e043bb into WebAssembly:main Dec 14, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants