[PATCH] D141103: [WebAssembly] Ensure 'end_function' in functions

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added a reviewer: sbc100.
Herald added subscribers: pmatos, asb, wingo, ecnelises, sunfish, hiraditya, jgravelle-google, dschuff.
Herald added a project: All.
aheejin requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Local info is supposed to be emitted in the start of every function.
When there are locals, `.local` section should be present, and we emit
local info according to the section.

If there is no locals, empty local info should be emitted. This empty
local info is emitted whenever a first instruction is emitted within a
function without encountering a `.local` section. If there is no
instruction, `end_function` pseudo instruction should be present and the
empty local info will be emitted when parsing the pseudo instruction.

The following assembly is malformed because the function `test` doesn't
have an `end_function` at the end, and the parser doesn't end up
emitting the empty local info needed. But currently we don't error out
and silently produce an invalid binary.

  .functype test () -> ()
  test:

This patch adds one extra state to the Wasm assembly parser,
`FunctionLabel` to detect whether a function label is parsed but not
ended properly when the next function starts or the file ends.

It is somewhat tricky to distinguish `FunctionLabel` and
`FunctionStart`, because it is not always possible to ensure the state
goes from `FunctionLabel` -> `FunctionStart`. `.functype` directive does
not seem to be mandated before a function label, in which case we don't
know if the label is a function at the time of parsing. But when we do
know the label is function, we would like to ensure it ends with an
`end_function` properly. Also we would like to error out when it does
not.

For example,

  .functype test() -> ()
  test:

We should error out for this because we know `test` is a function and it
doesn't end with an `end_function`. This PR fixes this.

  test:

We don't error out for this because there is no info that `test` is a
function, so we don't know whether there should be an `end_function` or
not.

  test:
  .functype test() -> ()

We error out for this currently already, because we currently switch to
`FunctionStart` state when we first see `.functype` directive after its
label definition.

Fixes https://github.com/llvm/llvm-project/issues/57427.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141103

Files:
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/test/MC/WebAssembly/func-end-errors.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141103.486720.patch
Type: text/x-patch
Size: 3908 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230106/d84089e6/attachment.bin>