Add support for compilation-hints proposal

[Lukasdoe]

Title says it all, this PR adds support for compilation hints.

[sbc100]

CC @kripken who has worked on this in binaryen. Does this approach look reasonable to you?

[sbc100]

Can you just do hint.instruction_frequency = instruction_frequency here instead of the placement new?

[Lukasdoe]

Since hint.instruction_frequency is not a valid C++ object, we're not allowed to use its assignment copy operator. In practice, using it should work as expected, but it is still undefined behavior nevertheless (afaik). That's why I opted for just writing over it, which should definitely be safe.

[sbc100]

Can you explain why more? Why can't we using the assignment operator here? Surely the LHS doesn't matter here since its being clobbered by the RHS.

Does the compiler complain if you do hint.instruction_frequency = instruction_frequency?

[Lukasdoe]

While the compiler does not catch the undefined behavior, ASAN does:

  +AddressSanitizer:DEADLYSIGNAL
  +=================================================================
  +==720746==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x55d1e4e29fd8 bp 0x000000000002 sp 0x7fffd147b960 T0)
  +==720746==The signal is caused by a READ memory access.
  +==720746==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
  +    #0 0x55d1e4e29fd8 in atomic_compare_exchange_strong<__sanitizer::atomic_uint8_t> /home/abuild/rpmbuild/BUILD/llvm20-20.1.8-build/llvm-20.1.8.src/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_atomic_clang.h:84:10
  +    #1 0x55d1e4e29fd8 in AtomicallySetQuarantineFlagIfAllocated /home/abuild/rpmbuild/BUILD/llvm20-20.1.8-build/llvm-20.1.8.src/projects/compiler-rt/lib/asan/asan_allocator.cpp:669:10
  +    #2 0x55d1e4e29fd8 in __asan::Allocator::Deallocate(void*, unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType) /home/abuild/rpmbuild/BUILD/llvm20-20.1.8-build/llvm-20.1.8.src/projects/compiler-rt/lib/asan/asan_allocator.cpp:733:10
  +    #3 0x55d1e4f12b9b in operator delete(void*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm20-20.1.8-build/llvm-20.1.8.src/projects/compiler-rt/lib/asan/asan_new_delete.cpp:155:3
  +    #4 0x55d1e4f1c4a4 in std::__new_allocator<unsigned char>::deallocate(unsigned char*, unsigned long) /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/new_allocator.h:172:2
  +    #5 0x55d1e4f1c43b in std::allocator_traits<std::allocator<unsigned char>>::deallocate(std::allocator<unsigned char>&, unsigned char*, unsigned long) /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/alloc_traits.h:649:13
  +    #6 0x55d1e4f1c43b in std::_Vector_base<unsigned char, std::allocator<unsigned char>>::_M_deallocate(unsigned char*, unsigned long) /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/stl_vector.h:396:4
  +    #7 0x55d1e4f1c3ce in std::_Vector_base<unsigned char, std::allocator<unsigned char>>::~_Vector_base() /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/stl_vector.h:375:2
  +    #8 0x55d1e4f18230 in std::vector<unsigned char, std::allocator<unsigned char>>::~vector() /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/stl_vector.h:805:7
  +    #9 0x55d1e503d839 in std::vector<unsigned char, std::allocator<unsigned char>>::_M_move_assign(std::vector<unsigned char, std::allocator<unsigned char>>&&, std::integral_constant<bool, true>) /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/stl_vector.h:2266:7
  +    #10 0x55d1e50280f4 in std::vector<unsigned char, std::allocator<unsigned char>>::operator=(std::vector<unsigned char, std::allocator<unsigned char>>&&) /usr/bin/../lib64/gcc/x86_64-suse-linux/15/../../../../include/c++/15/bits/stl_vector.h:838:2
  +    #11 0x55d1e505984a in wabt::CodeMetadataExpr::CodeMetadataExpr(std::basic_string_view<char, std::char_traits<char>>, std::vector<unsigned char, std::allocator<unsigned char>>, wabt::Location const&) /home/wabt/include/wabt/ir.h:723:15

When using the assignment operator of the LHS, its implementation (in this case of std::vector) wrongfully assumes a valid object state of the LHS and, therefore, accesses uninitialized memory. That's why we're only allowed to call the assignment operator on objects in a proper state, which the LHS is not. Therefore, we use the placement new operator to fully overwrite the memory associated with the (non-existent) LHS object.

[sbc100]

Can this be an anonymous union?

[Lukasdoe]

Since we need to overwrite the constructor, we need to name the union, afaik. But please correct me if there's a way to define a destructor for an anonymous union in C++.

[zherczeg]

Would it be better to use a pointer to these members? This way the union would be a union of pointer types. We could also allocate memory enough for the structure, and not the largest structure size (union can be large if one member is large). (Note: I am not a reviewer, this is just my thoughts)

[zherczeg]

Another option is using derived classes, each class contains one type from the union, and provide a create() method instead of constructors. The constructors become private.

[Lukasdoe]

@sbc100 the failing test is just a timeout on an unaffected test. It does not seem to be related to this PR.