Generated C code (refc) uses &NULL->field (undefined behavior)

[iacore]

Description

I only discovered this bug with zig cc. I don't know why it works with clang.

First, install zigcc from https://github.com/enthus1ast/zigcc

Then, set zigcc as c compiler & linker

nim.cfg

--cc:clang
--clang.exe="zigcc"
--clang.linkerexe="zigcc"

this code will then SIGILL

othertest.nim

type Foo = object
  timers*: seq[int]

let foo = new(Foo)
# if you break here, foo.timers is `null`
foo.timers.setLen(0) # error

Relevant functions

@m..@s..@[email protected]@[email protected]@[email protected], new(Foo) in Nim
it simply zeros the memory of Foo, but foo.timers is not initialized.

N_LIB_PRIVATE N_NIMCALL(tyObject_Foo__ALbn9ajynE9aaHaFZevtuvfQ*, new__othertest_3)(void) {
	tyObject_Foo__ALbn9ajynE9aaHaFZevtuvfQ* result;
	tyObject_Foo__ALbn9ajynE9aaHaFZevtuvfQ* r;
	nimfr_("new", "/home/user/.choosenim/toolchains/nim-1.6.10/lib/system.nim");
{	result = NIM_NIL;
	r = NIM_NIL;
	nimln_(906, "/home/user/.choosenim/toolchains/nim-1.6.10/lib/system.nim");
	r = (tyObject_Foo__ALbn9ajynE9aaHaFZevtuvfQ*) newObj((&NTIreffoo__K4pVu8IrAnEsmQQDNObn5Q_), sizeof(tyObject_Foo__ALbn9ajynE9aaHaFZevtuvfQ));
	nimln_(907, "/home/user/.choosenim/toolchains/nim-1.6.10/lib/system.nim");
	result = r;
	goto BeforeRet_;
	}BeforeRet_: ;
	popFrame();
	return result;
}

after that, (*foo__othertest_30).timers)->Sup is used when (*foo__othertest_30).timers) is still NULL

@mothertest.nim.c:

N_LIB_PRIVATE N_NIMCALL(void, NimMainModule)(void) {
{
	TFrame FR_; FR_.len = 0;

	nimRegisterGlobalMarker(TM__2CAWbMbLhiVM79c9cCxoxJpg_2);

}/* preInitProc end */
{
	nimfr_("othertest", "/home/user/computing/debug/nim-async-heap/othertest.nim");
	nimln_(4, "/home/user/computing/debug/nim-async-heap/othertest.nim");
	asgnRef((void**) (&foo__othertest_30), new__othertest_3());
	nimln_(5, "/home/user/computing/debug/nim-async-heap/othertest.nim");
	asgnRef((void**) (&(*foo__othertest_30).timers), (tySequence__qwqHTkRvwhrRyENtudHQ7g*) setLengthSeqV2(&((*foo__othertest_30).timers)->Sup, (&NTIseqLintT__qwqHTkRvwhrRyENtudHQ7g_), ((NI) 0)));
	popFrame();
}
}

Nim Version

Nim Compiler Version 1.6.10 [Linux: amd64]

Current Output

❯ nim r metrotest.nim
Hint: used config file '/home/user/.choosenim/toolchains/nim-1.6.10/config/nim.cfg' [Conf]
Hint: used config file '/home/user/.choosenim/toolchains/nim-1.6.10/config/config.nims' [Conf]
Hint: used config file '/home/user/.config/nim/nim.cfg' [Conf]
Hint: used config file '/home/user/computing/debug/nim-async-heap/nim.cfg' [Conf]
Hint: gc: refc; opt: none (DEBUG BUILD, `-d:release` generates faster code)
9921 lines; 0.028s; 8.656MiB peakmem; proj: /home/user/computing/debug/nim-async-heap/metrotest.nim; out: /home/user/.cache/nim/metrotest_d/metrotest_80921195266A94664E263A582C34B4157AAA4BC3 [SuccessX]
Hint: /home/user/.cache/nim/metrotest_d/metrotest_80921195266A94664E263A582C34B4157AAA4BC3  [Exec]
Traceback (most recent call last)
/home/user/computing/debug/nim-async-heap/metrotest.nim(6) metrotest
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/asyncmacro.nim(232) testmain
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/asyncmacro.nim(28) testmainNimAsyncContinue
/home/user/computing/debug/nim-async-heap/metrotest.nim(4) testmainIter
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/asyncdispatch.nim(1863) sleepAsync
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/asyncdispatch.nim(1181) getGlobalDispatcher
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/asyncdispatch.nim(1168) newDispatcher
/home/user/.choosenim/toolchains/nim-1.6.10/lib/pure/collections/heapqueue.nim(239) clear
SIGILL: Illegal operation.
Error: execution of an external program failed: '/home/user/.cache/nim/metrotest_d/metrotest_80921195266A94664E263A582C34B4157AAA4BC3 '

Expected Output

It should not crash

Possible Solution

In codegen, use the ISO C function offsetof.

Also, this bug only appears with --mm:refc. --mm:orc doesn't have this problem.

Additional Information

both clang and gcc by default allows accessing address of field of NULL pointer to struct.

// obj == NULL (* struct Foo)
&obj->field

I think this is undefined behavior in ISO C. gcc and clang treat this as taking the address of field. Zig has UBSan on by default, so this UB is caught.

https://en.cppreference.com/w/c/language/operator_member_access

Dereferencing a null pointer, a pointer to an object outside of its lifetime (a dangling pointer), a misaligned pointer, or a pointer with indeterminate value is undefined behavior, except when the dereference operator is nullified by applying the address-of operator to its result, as in &*E.

P.S. I did not find the extension in https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/

[iacore]

It might be good to run the tests with zigcc to find more UB.

I got this failure when running ./koch test. This is one of the many failures.

FAIL: tests/collections/tseq.nim c --mm:refc                       (11.42 sec)
Test "tests/collections/tseq.nim" in category "collections"
Failure: reExitcodesDiffer
Expected:
exitcode: 0

Gotten:
exitcode: 1

Output:
Traceback (most recent call last)
tseq.nim(69)             tseq
tseq.nim(59)             nested
assign.nim(179)          genericSeqAssign
assign.nim(143)          genericAssign
assign.nim(99)           genericAssignAux
assign.nim(75)           genericAssignAux
gc.nim(482)              newObjNoInit
SIGILL: Illegal operation.


Renamed /tmp/diffStrings_a_NarVlDz2 to /tmp/diffStrings_b_iutV0c0g
/tmp/diffStrings_b_iutV0c0g --- Text
1 exitcode: 0                            1 exitcode: 1
.                                        2 
.                                        3 Output:
.                                        4 Traceback (most recent call last)
.                                        5 tseq.nim(69)             tseq
.                                        6 tseq.nim(59)             nested
.                                        7 assign.nim(179)          genericSeqAs
.                                        . sign
.                                        8 assign.nim(143)          genericAssig
.                                        . n
.                                        9 assign.nim(99)           genericAssig
.                                        . nAux
.                                       10 assign.nim(75)           genericAssig
.                                       .. nAux
.                                       11 gc.nim(482)              newObjNoInit
.                                       12 SIGILL: Illegal operation.


FAIL: tests/collections/tseq.nim c --mm:refc                       (11.42 sec)

[ghost]

I only discovered this bug with zig cc. I don't know why it works with clang.

That's simply because zig cc has -fsanitize=undefined enabled by default, you can do the same with clang if you enable that option. SIGILL is specifically thrown when hitting a sanitizer.

Also, as far as I know refc isn't exactly friendly to that sanitizer, ORC works much better in that regard as you said.

[ghost]

Also likely a duplicate of #20961

[iacore]

Also likely a duplicate of #20961

Not duplicate.

[tersec]

#20294 #20747 #20795 have more discussion of this pattern Nim has had of using a faux-offsetof invoking UB (fixed in that case, and appears distinct in a couple of ways from this instance).

https://reviews.llvm.org/D67122 describes LLVM/clang's view of this particular C rule.