Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

No test cases for @custom sections #15

Description

@lars-t-hansen

I've found no test cases for @custom sections, meaning a large piece of the new feature - really the more complicated part - is not covered by tests. I realize that it's hard to test this because we would really be testing the structure of the binary resulting from parsing the text, but all the same it seems problematic not to have any tests. (Branch hinting has the same problem.)

Activity

  1. lars-t-hansen commented on Nov 9, 2021

    @lars-t-hansen
    Author

    For that matter, it doesn't look like the spec interpreter has any implementation of the @custom sections.

  2. rossberg commented on Nov 9, 2021

    @rossberg
    Member

    Yes, the spec interpreter does not interpret custom sections at all. By their definition of being custom, no confirming implementation needs to be able to handle them, so it would be wrong to require any such tests to pass in any interesting way.

    The same has always been true for the name section in the binary format. Not sure how to resolve that tension, we'd need a notion of "optional" tests, that an implementation can choose to want to pass.

  3. lars-t-hansen commented on Nov 9, 2021

    @lars-t-hansen
    Author

    Yes, the spec interpreter does not interpret custom sections at all. By their definition of being custom, no confirming implementation needs to be able to handle them, so it would be wrong to require any such tests to pass in any interesting way.

    I see the point, but this does not mean that the spec interpreter could not define a custom format of its own and could then have tests for that. Those tests could then test the (before S) etc clauses. Conforming implementations that do support some custom sections could perhaps create tests by adapting and augmenting those tests. It's not perfect but would be an improvement on the current situation. (I suspect this is pretty much what you're suggesting in your remark about "optional" tests.)

  4. rossberg commented on Nov 9, 2021

    @rossberg
    Member

    Yes, I recall that there was some earlier discussion about setting up infrastructure around the test suite for something like this, in the context of some other custom section proposal (can't find it right now).

    I'm very much in favour of doing this, but it's a fairly general problem and non-trivial infrastructure work, so that I would prefer to keep it separate from this proposal, especially since the situation isn't new but already exists in the MVP. Do you think that's justifiable?

  5. lars-t-hansen commented on Nov 9, 2021

    @lars-t-hansen
    Author

    This is certainly related: WebAssembly/branch-hinting#13

    I'm sure I would be inclined to let the issue go if there existed a burning need to ship this proposal, but is there? (Not intended as a rhetorical question; I'm curious about the urgency & utility of the proposal.)

  6. lars-t-hansen commented on Nov 9, 2021

    @lars-t-hansen
    Author

    Really WebAssembly/design#1424 I guess.

  7. alexcrichton commented on Nov 9, 2021

    @alexcrichton
    Contributor

    Reasonly-simple tests may be possible where the test looks something like:

    (assert_roundtrip (module (@custom "...")))

    which converts text-to-binary, then converts binary-to-text, then finally converts text-to-binary again, asserting that the two binary forms are equivalent. That would exercise a piece of functionality not used in the test suite today, though, a binary-to-text conversion. That being said I would expect that most engines already have one of those in the same manner as they've all got a text-to-binary to deal with the existing test suite.

  8. rossberg commented on Nov 9, 2021

    @rossberg
    Member

    @lars-t-hansen, fair question. Technically, there is no urgency, although annotations keep coming up in various places, and it be good to settle them sooner rather than later.

    @alexcrichton, well, the problem remains that we cannot require any implementation to pass such a test, since understanding custom sections is all optional. Moreover, the spec is pretty explicit that no implementation is allowed to reject malformed custom sections, so we cannot have any negative tests for them either.

    So, we really need infrastructure to keep running such "voluntary" tests separate from mandatory ones, and probably also have some special failure mode for negative tests. All in all, quite a bit of design and engineering is involved, I fear.

  9. yuri91 commented on Mar 23, 2022

    @yuri91
    Contributor

    I am trying to solve the testing issue for proposals like this and branch hinting.
    I opened an issue in the design repo since I would like to solve it generally, but in my proof of concept I added support in the interpreter for the @custom annotation.
    I am interested to know what you guys think about the approach.
    Design issue: WebAssembly/design#1445

  10. rossberg commented on Apr 25, 2023

    @rossberg
    Member

    Closing via #17.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions