Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Function alignment #22369

Unanswered
ethanaobrien asked this question in Q&A
Aug 14, 2024 · 1 comments · 12 replies
Discussion options

I'm working on compiling a certain emulator via emscripten, where the lowest bit of every code pointer needs to be zero.

Setting -falign-functions=2. Though this appears to be ignored (with no warnings from the compiler)

https://github.com/libretro/picodrive/blob/d6f625a1251c78caf6f2dc81c1ffdb724587bb24/Makefile#L26

Is there a hacky way to get this done with a modern build of emscripten?

(P.S. The "EMULATED_FUNCTION_POINTERS" option seems to have disappeared without a trace)

You must be logged in to vote

Replies: 1 comment · 12 replies

Comment options

Yes, this looks like a clang bug. Testcase:

void foo() {}
void bar() {}

int main(int argc, char **argv) {
  if (argc == 17) return (int)&foo;
  else return (int)&bar;
}

Compiling with or without -falign-functions=2 makes no impact on the layout of the table. They are assigned adjacent indexes.

@sbc100 is this a bug in wasm-ld specifically perhaps, I think the table is laid out there?

You must be logged in to vote
12 replies
@ethanaobrien
Comment options

Ah, that does seem relatively simple.

I've modified the function to the below, assuming my logic is correct, it should align all functions by 2:

void ElemSection::addEntry(FunctionSymbol *sym) {
  if (sym->hasTableIndex() || sym->isStub)
    return;

  uint64_t alignment = 2;

  uint64_t index = config->tableBase + (indirectFunctions.size() * alignment);

  sym->setTableIndex(index);
  indirectFunctions.emplace_back(sym);
}

However, linking with O2 or O3 results in wasm-opt failing

wasm-opt: /build/emscripten/src/binaryen/src/mixed_arena.h:188: T& ArenaVectorBase<SubType, T>::operator[](size_t) const [with SubType = ArenaVector<wasm::Expression*>; T = wasm::Expression*; size_t = long unsigned int]: Assertion `index < usedElements' failed.
emcc: error: '/usr/bin/wasm-opt --strip-target-features --post-emscripten -O3 --low-memory-unused --asyncify --pass-arg=asyncify-propagate-addlist --pass-arg=asyncify-imports@env.invoke_*,env.__asyncjs__*,*.fd_sync,*.emscripten_promise_await,*.emscripten_idb_load,*.emscripten_idb_store,*.emscripten_idb_delete,*.emscripten_idb_exists,*.emscripten_idb_clear,*.emscripten_idb_load_blob,*.emscripten_idb_store_blob,*.emscripten_sleep,*.emscripten_wget_data,*.emscripten_scan_registers,*.emscripten_lazy_load_code,*._load_secondary_module,*.emscripten_fiber_swap,*.SDL_Delay --zero-filled-memory --pass-arg=directize-initial-contents-immutable picodrive_libretro.wasm -o picodrive_libretro.wasm --mvp-features --enable-multivalue --enable-mutable-globals --enable-reference-types --enable-sign-ext' failed (received SIGABRT (-6))
make: *** [Makefile.emscripten:236: picodrive_libretro.js] Error 1

Building with O1 results in an RuntimeError: index out of bounds error.

Perhaps it may not be as simple as modifying that function? Or my math is wrong

@sbc100
Comment options

I think likely the setTableIndex argument has to match the index into indirectFunctions. Do you would likely need to push a one of more nullptr values onto indirectFunctions array.

@ethanaobrien
Comment options

Ah that would make sense. It's pretty unclean but this works.

void ElemSection::addEntry(FunctionSymbol *sym) {
  // Don't add stub functions to the wasm table.  The address of all stub
  // functions should be zero and they should they don't appear in the table.
  // They only exist so that the calls to missing functions can validate.
  uint32_t padding = 1;
  uint64_t alignment = 2;

  if (indirectFunctions.size() == 0 && padding > 0) {
    for (uint32_t i=0; i<padding; i++) {
      indirectFunctions.push_back(nullptr);
    }
  }

  uint64_t index = config->tableBase + indirectFunctions.size();

  if (sym->hasTableIndex() || sym->isStub)
    return;
  sym->setTableIndex(index);
  indirectFunctions.emplace_back(sym);

  if (alignment > 1) {
    for (uint32_t i=0; i<alignment-1; i++) {
      indirectFunctions.push_back(nullptr);
    }
  }
}

I also had mo modify ElemSection::writeBody to handle a nullptr

    if (sym == nullptr) {
      (void) tableIndex;
      writeUleb128(os, 0, "function index");
      ++tableIndex;
      continue;
    }
@sbc100
Comment options

Great! Would you like to send a PR and we can discuss further there?

@ethanaobrien
Comment options

Opened here: llvm/llvm-project#105532

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
🙏
Q&A
Labels
None yet
3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.