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

Nim & C disagree on type size #6860

Closed
kvinwang opened this issue Dec 2, 2017 · 6 comments

Comments

@kvinwang
Copy link
Contributor

commented Dec 2, 2017

By turn on checkabi, many errors in stdlib show up.
These errors are very dangerous, that can cause undefined runtime behaviors. See #6496

$ nim c -d:checkabi test.nim
In file included from csources/c/i386/stdlib_system.c:10:0:
/home/loong/osc/nim/lib/nimbase.h:509:3: error: static assertion failed: "Nim & C disagree on type size"
   _Static_assert(sizeof(typ) == sz, "Nim & C disagree on type size")

A list of errors show up in my app (It's only a small part of whole stdlib):

 NIM_CHECK_SIZE(struct stat, 4);/*  Stat @ ["lib/system/sysio.nim", 323, 6] */
 NIM_CHECK_SIZE(dev_t, 4);/*  Dev @ ["lib/posix/posix_other.nim", 139, 2] */
 NIM_CHECK_SIZE(off_t, 8);/*  Off @ ["lib/posix/posix_other.nim", 148, 2] */
 NIM_CHECK_SIZE(struct stat, 68);/*  Stat @ ["lib/posix/posix_other.nim", 202, 2] */
 NIM_CHECK_SIZE(struct tm, 36);/*  StructTM @ ["lib/pure/times.nim", 1169, 6] */
 NIM_CHECK_SIZE(union epoll_data, 4);/*  epoll_data @ ["lib/posix/epoll.nim", 38, 2] */
 NIM_CHECK_SIZE(struct epoll_event, 8);/*  epoll_event @ ["lib/posix/epoll.nim", 47, 2] */
 NIM_CHECK_SIZE(sa_family_t, 4);/*  TSa_Family @ ["lib/posix/posix_other.nim", 407, 2] */
 NIM_CHECK_SIZE(struct sockaddr_storage, 4);/*  Sockaddr_storage @ ["lib/posix/posix_other.nim", 419, 2] */
 NIM_CHECK_SIZE(struct sockaddr, 260);/*  SockAddr @ ["lib/posix/posix_other.nim", 409, 2] */
 NIM_CHECK_SIZE(struct sockaddr_in, 12);/*  Sockaddr_in @ ["lib/posix/posix_other.nim", 467, 2] */
 NIM_CHECK_SIZE(struct sockaddr_in6, 32);/*  Sockaddr_in6 @ ["lib/posix/posix_other.nim", 477, 2] */
 NIM_CHECK_SIZE(struct sockaddr_un, 112);/*  Sockaddr_un @ ["lib/posix/posix_other.nim", 414, 2] */
sizeof(              struct stat) ==  88 != 4
sizeof(                    dev_t) ==   8 != 4
sizeof(                    off_t) ==   4 != 8
sizeof(              struct stat) ==  88 != 68
sizeof(                struct tm) ==  44 != 36
sizeof(         union epoll_data) ==   8 != 4
sizeof(       struct epoll_event) ==  12 != 8
sizeof(              sa_family_t) ==   2 != 4
sizeof(  struct sockaddr_storage) == 128 != 4
sizeof(          struct sockaddr) ==  16 != 260
sizeof(       struct sockaddr_in) ==  16 != 12
sizeof(      struct sockaddr_in6) ==  28 != 32
sizeof(       struct sockaddr_un) == 110 != 112
@kvinwang

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2017

I think the -d:checkabi should be turned on by default. Just like this one:
typedef int Nim_and_C_compiler_disagree_on_target_architecture[sizeof(NI) == sizeof(void*) && NIM_INTBITS == sizeof(NI)*8 ? 1 : -1];

@Araq

This comment has been minimized.

Copy link
Member

commented Dec 4, 2017

They are only dangerous if sizeof(T) is used. When T is an object the compiler doesn't allow that.

@kvinwang

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2017

They are only dangerous if sizeof(T) is used. When T is an object the compiler doesn't allow that.

Not exactly. This is a simple example on macOS:

import posix
var ids: seq[Uid] = @[1, 2, 3, 4]
echo "ids:", ids   # ids:@[1, 0, 2, 0]
@krux02

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

What is the test.nim that you used?

When I run the last example I get the following error:

/home/arne/.cache/nim/scratch_d/stdlib_dollars.c:52:1: error: static_assert failed "Nim & C disagree on type size"
NIM_CHECK_SIZE(TFrame, -3);

Which means that -d:checkabi is incorrect now, because -3 (szUnknownSize) means that Nim doesn't know the size.

@kvinwang

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

When I run the last example I get the following error:

/home/arne/.cache/nim/scratch_d/stdlib_dollars.c:52:1: error: static_assert failed "Nim & C disagree on type size"
NIM_CHECK_SIZE(TFrame, -3);

Seems like thats a bug in -d:checkabi. It works fine with Nim 0.19.2.
I get this error:

/Users/loong/.cache/nim/hi_d/stdlib_posix.c:42:1: error: static_assert failed "Nim & C disagree on type size"
NIM_CHECK_SIZE(uid_t, 8);
^~~~~~~~~~~~~~~~~~~~~~~~
@krux02

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

I just fixed checkabi (not merged yet). The problem was that the constant -3 for the size of a type is used to tag the type as: nim doesn't know the size of that type and should not even try to guess. The switch checkabi didn't know about this magic constant, because it is older than that. -3 is the correct value here. The problem is that the check is generated even though in this case it should not be generated.

Araq added a commit that referenced this issue Jul 5, 2019

@Araq Araq closed this in 02b9af2 Jul 6, 2019

narimiran added a commit that referenced this issue Jul 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.