RTTI hook type erasure is considered undefined behavior by clang

[alaviss]

Example

type
  X = object of RootObj

proc `=destroy`(x: var X) =
  discard

proc main() =
  var x: ref RootObj = (ref X)()

main()

Actual Output

Note that a recent clang version (>= 17) is required.

$ nim r --cc:clang --passC:-fsanitize=function --passL:-fsanitize=function test.nim

cache/nimskull/test_d/stdlib_system.nim.c:2046:3: runtime error: call to function eqdestroy___test_2 through pointer to incorrect function type 'void (*)(void *)'

Possible Solution

Generate generic thunks for hooks called via RTTI. For example

void eqdestroy_thunk___test_2(void* ptr) {
  eqdestroy___test_2((X*) ptr);
}

Alternatively type erase eqdestroy/eqtrace parameters, but that spells trouble if anyone uses =destroy/=trace as function pointers.

References

Many other projects are also dealing with the fallout:

• UBSan: Calling a function through pointer to incorrect function type is undefined behavior python/cpython#111178
• systemd fails miserably with clang-17's -fsanitize=function systemd/systemd#29972
• Undefined behavior around calls to generic functions like OPENSSL_LH_HASHFUNC openssl/openssl#22896

Apparently this new UB warning has to do with CFI (Control Flow Integrity) protections.

[zerbina]

Hm, that's annoying, but it does make sense, given that the current implementation violates the C standard.

Once the new RTTI handling is in place, this should be relatively easy to fix in an okay fashion; the creation logic for RTTI objects would simply request thunk procedures as needed.

Alternatively type erase eqdestroy/eqtrace parameters, but that spells trouble if anyone uses =destroy/=trace as function pointers.

In general, I think manually calling hook procedures (or taking their address) should be disallowed. Doing so is error prone, and it also - like you said - prevents the compiler from doing some internal adjustments to them.

[alaviss]

In general, I think manually calling hook procedures (or taking their address) should be disallowed. Doing so is error prone, and it also - like you said - prevents the compiler from doing some internal adjustments to them.

I don't think that will float really well, unless we rethink how hooks are defined.

Hooks are currently just procs shaped in a certain way, so I'd prefer that they continue to have the same capabilities as one.

[zerbina]

Hooks are currently just procs shaped in a certain way, so I'd prefer that they continue to have the same capabilities as one.

There are already restrictions placed on hook procedures (e.g., cannot rebind, must be in same module as type, must not raise), so I disagree that they're just procedures shaped in a certain way.