Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmpMem, copyMem, moveMem, and zeroMem create UB with nil pointer #22149

Open
tersec opened this issue Jun 24, 2023 · 0 comments
Open

cmpMem, copyMem, moveMem, and zeroMem create UB with nil pointer #22149

tersec opened this issue Jun 24, 2023 · 0 comments

Comments

@tersec
Copy link
Contributor

tersec commented Jun 24, 2023

Description

var p: pointer

doAssert p.isNil
discard cmpMem(p, p, 0)
copyMem(p, p, 0)
moveMem(p, p, 0)
zeroMem(p, 0)

All of cmpMem, copyMem, moveMem, and zeroMem state that

Like any procedure dealing with raw memory this is unsafe.
or
Like any procedure dealing with raw memory this is still unsafe, though.

However, it's even more unsafe than the obvious NULL pointer dereference.

https://gcc.gnu.org/gcc-4.9/porting_to.html notes that:

The pointers passed to memmove (and similar functions in <string.h>) must be non-null even when nbytes==0, so GCC can use that information to remove the check after the memmove call. Calling copy(p, NULL, 0) can therefore deference a null pointer and crash.

https://en.cppreference.com/w/cpp/string/byte/memcpy and https://en.cppreference.com/w/cpp/string/byte/memmove state similarly that

If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero.

This can come up practically because taking the address of an openArray of zero elements will be the NULL pointer. This is, of course, another unsafe operation, conceptually, so isn't a problem in isolation.

That the memcmp, memcpy, memmove, and memset C functions have the specifications and behavior they have, and to the extent cmpMem, copyMem, moveMem, and zeroMem are thin wrappers over such functions (but are not documented as such) means that this C UB quirk is exposed to Nim code.

This isn't just "unsafe" in the usual way pointers are unsafe. This is undefined behavior dependent on understanding how openArrays work and that these "unsafe" memory operations directly inherit all the behavior of the C versions, including undefined behavior.

It's reasonable enough to have such a set of <string.h> wrappers in the Nim stdlib in a way that doesn't add calling overhead for the many cases where Nim calls them for small, fixed numbers of bytes (sizeof(T), 2, 4, etc), but then they should be documented as such, at least for the C and C++ backends, so people using them can understand in a documented way exactly the way in which they're unsafe.

Nim Version

Nim Compiler Version 1.9.5 [Linux: amd64]
Compiled at 2023-06-24
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: c6c85f84db3bd7bd2d1c5823020c7df007f1bb69
active boot switches: -d:release
gcc (Debian 12.3.0-4) 12.3.0
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Debian clang version 14.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Current Output

$ nim c -r --cc:clang --passC="-fsanitize=undefined" --passL="-fsanitize=undefined" b
.cache/nim/b_d/@mb.nim.c:72:18: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:65:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:72:18 in
.cache/nim/b_d/@mb.nim.c:72:24: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:65:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:72:24 in
.cache/nim/b_d/@mb.nim.c:87:15: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:44:28: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:87:15 in
.cache/nim/b_d/@mb.nim.c:87:24: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:44:28: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:87:24 in
.cache/nim/b_d/@mb.nim.c:95:16: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:48:14: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:95:16 in
.cache/nim/b_d/@mb.nim.c:95:25: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:48:14: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:95:25 in
.cache/nim/b_d/@mb.nim.c:100:15: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:61:62: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior .cache/nim/b_d/@mb.nim.c:100:15 in
$ nim c -r --passC="-fsanitize=undefined" --passL="-fsanitize=undefined" b
.cache/nim/b_d/@mb.nim.c:72:11: runtime error: null pointer passed as argument 1, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:72:11: runtime error: null pointer passed as argument 2, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:87:8: runtime error: null pointer passed as argument 1, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:87:8: runtime error: null pointer passed as argument 2, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:95:8: runtime error: null pointer passed as argument 1, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:95:8: runtime error: null pointer passed as argument 2, which is declared to never be null
.cache/nim/b_d/@mb.nim.c:100:8: runtime error: null pointer passed as argument 1, which is declared to never be null

Expected Output

No response

Possible Solution

No response

Additional Information

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant