Invalid codegen when block ends with lent

[etan-status]

When a block ends with an inlined call to a lent returning function, C code exhibiting a use-after-free bug is generated.

Example

test.nim

type Foo = object
  a, b, c, d: uint64

proc c(i: uint64): Foo =
  Foo(a: i, b: i, c: i, d: i)

func x(f: Foo): lent Foo {.inline.} =
  f

proc m() =
  let f = block:
    let i = c(42)
    x(i)

  echo $f.a

m()

Build with -O3 to force aggressive optimizations.

rm -rf nimcache test test.dSYM && nim c -d:release -r test.nim

Current Output

% ./test                                                        
4403969664

nimcache/@mtest.nim.c

N_LIB_PRIVATE N_NIMCALL(void, m__test_32)(void) {
	tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ f;
	tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ* T1_;
	tyArray__nHXaesL0DJZHyVS07ARPRA T3_;
	nimZeroMem((void*)(&f), sizeof(tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ));
	T1_ = (tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ*)0;
	{
		tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ i;
		nimZeroMem((void*)(&i), sizeof(tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ));
		c__test_8(42ULL, (&i));
		T1_ = x__test_20((&i));  // Note: This returns a pointer to `i` (`lent`)
	}
	f = (*T1_);  // Note: T1 points to `i` which has been freed (no longer in scope)
	nimZeroMem((void*)T3_, sizeof(tyArray__nHXaesL0DJZHyVS07ARPRA));

	T3_[0] = dollar___systemZdollars_9(f.a);
	echoBinSafe(T3_, 1);
}

Expected Output

% ./test                                                        
42

Possible Solution

• Use Nim 1.2 instead of Nim 1.6 to avoid lent.

Beside that, there are a few workarounds for individual occurrences. The challenge is to find all of them (maybe there are similar issues outside block contexts).

• Do not end a block with a function calling into a lent.
• Do not use lent (especially prevalent in options.nim > get).
• Ensure the called function that returns a lent is not inlined (noinline, or putting logs inside).
• Disabling LTO to prevent aggressive inlining can also reduce the risk.

Additional Information

Discovered as part of status-im/nimbus-eth2#3907 on GitHub Actions (macos-11 and macos-12) and reproduced on macOS 12.5 with Xcode 13.4.1 on Nim compiler 1.6 (5f61f15) on an Intel MacBook Pro (16-inch, 2019).

% nim -v
Nim Compiler Version 1.6.7 [MacOSX: amd64]
Compiled at 2022-07-28
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 5f61f1594d0c11caf0fb650e3d3bc32cf8f38890
active boot switches: -d:release

[Menduist]

@ringabout that's not specific to ARC/ORC (the example doesn't use ARC/ORC) and I don't think it's OS / Arch specific either (also happens on my linux), it's just codegen
However, it's a showstopper imho

[etan-status]

Does it work on Windows by accident, or is the code still technically undefined behaviour? On macOS, Clang may start deleting surrounding code with -O3, as it optimizes more and more sections away.

[ringabout]

Yeah, it is an undefined behaviour in c, optimizations may free the stack variables when out of the scope.

https://stackoverflow.com/questions/13888268/what-happens-when-a-variable-goes-out-of-scope

[ringabout]

I checked the code generated by if expr (which works). Another possible solution is to deref the pointer in the block scope like below:

{
tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ i;
nimZeroMem((void*)(&i), sizeof(tyObject_Foo__9bOqVyAHO8e6D7f036GENHQ));
c__test_8(42ULL, (&i));
T1_ = x__test_20((&i));  // Note: This returns a pointer to `i` (`lent`)
f = (*T1_);
}

[etan-status]

@Araq The fix is incomplete and leads to compiler crashes with --gc:orc on Nim commit 16f6dc05fdb2e1ca3086936c15c2ad3de542d991. Same example as given in this issue.

• When using --gc:orc, nkHiddenTryStmt and nkStmtListExpr wrappers are injected into the AST, but those wrappers have typ == nil.
• Therefore, when derefBlock moves the nkHiddenDeref down a layer, it may next wrap one of those injected typ == nil nodes.
• When genDeref is later called again on this moved derefBlock, it will call let mt = mapType(p.config, e[0].typ, mapTypeChooser(e[0])).
• This leads to a SIGSEGV as e[0].typ == nil, and mapType accesses typ.kind on typ == nil in its initial case statement.

The problem can be visualized, by adding this helper function to ccgexprs.nim:

proc d(e: PNode, l = 0) =
  var s = ""
  for i in 0 ..< l:
    s = s & "  "
  echo s, "- len=", $e.safeLen, " typ=", $e.typ, " ", $(cast[int](e.typ)), " kind=", $e.kind
  for i in 0 ..< e.safeLen:
    e[i].d(l + 1)

And then, in derefBlock, call e.d() and n.d() before the expr p, n, d line to see the transformation.

Compiling with --gc:orc injects:

• nkStmtListExpr with typ == nil when compiling the minimal example given in issue description.
• additionally, nkHiddenTryStmt when compiling @cheatfate's asyncproc branch of status-im/nim-chronos (nim c -r --gc:orc tests/testall)

Both of these cases are problematic.