[PATCH] D87258: [WebAssembly, LowerTypeTests] Fix control-flow integrity support

[Dominic Chen via Phabricator via llvm-commits]

ddcc added a comment.

In D87258#2261190 <https://reviews.llvm.org/D87258#2261190>, @dschuff wrote:

> One thing that's happened since you did all of this originally is multiple-table support; the reference-types proposal includes multiple tables, has been standardized, and will soon be (or is already?) available in all the browsers. IIRC you originally did both a version with and without multiple-tables, but I forget how much of that was in s2wasm vs in LLVM. IMO we should just make CFI require multiple tables, and then we can dedicate as many tables for whatever purposes we like.
> I assume that we'll still need the core functionality of this CL, which is having the table layout be determined at IPO time rather than being assigned by the objectWriter/linker?
>
> Can you remind me of what the state was for multiple-table support when you originally wrote all this?

This CL is still using a single table, so it relies on the LowerTypeTests pass pre-assigning table indices into !wasm.index metadata, then generating arithmetic checks at each call site based on the valid index range for each type. Later, the ASMPrinter and MCSymbol components propagate and assign those table indices during object generation.

The multiple table implementation was in D22246 <https://reviews.llvm.org/D22246>, which wasn't merged. I recall the design was more complicated and experimental because there were a couple of remaining issues:

1. Depending on how we assign table indices, false negatives may occur with certain CFI checks. Suppose we put function "void F1 <https://reviews.llvm.org/F1>(struct Foo*)" at index 1 in table A, function "void F2 <https://reviews.llvm.org/F2>(struct void *)" at index 1 in table B, and there's an indirect call site CS that dispatches using the latter type, but may call either F1 <https://reviews.llvm.org/F1> or F2 <https://reviews.llvm.org/F2>. Normally, the CFI pass would only allow CS to call F2 <https://reviews.llvm.org/F2> (because they have the same type), and not F1 <https://reviews.llvm.org/F1> (because it has a different type, even if this may be considered a false positive). But if the backend generates indices that could alias across tables, then we won't detect this and allow both to go through. The WebAssembly runtime signature check on indirect calls doesn't help, because both functions will have the same lowered type as a pointer is just an i64.
2. It was difficult to keep track of which table should be used for which call site in LLVM IR. The Clang front-end generates calls to the type test intrinsic (which contains the type metadata) somewhere between e.g. a function pointer load and its indirect call, but there's no direct link to the indirect call in the IR. But, the LowerTypeTests pass needs to somehow find and tag all the call sites and valid call targets of the same type with a unique identifier. Both the single and multiple table design are using metadata for tagging things, but this probably isn't fully correct since metadata may be removed by intermediate optimizations?

In general, with the multiple table approach, LowerTypeTests would need to do the tagging and then remove all the type test intrinsics since we don't need additional arithmetic checks. Later, somewhere in the backend, we would need to place and generate (table, index) pairs for all valid call targets of the same type, and fixup each indirect call to refer to the correct table. I don't see support for e.g. a table reference on the call_indirect instruction, so it looks like all of this backend machinery would need to be implemented as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87258/new/

https://reviews.llvm.org/D87258