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

c compiler warnings silently ignored, giving wrong results/undefined behavior #11590

Open
timotheecour opened this issue Jun 25, 2019 · 0 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Jun 25, 2019

After codegen, c compiler warnings are silently ignored; this can give undefined behavior in which I ran into.

Example

# extracted from system/timers
proc mach_absolute_time(): int64 {.importc, header: "<mach/mach.h>".}
echo mach_absolute_time()

Current Output

the output is random stack garbage:

1158149750 # 1st run
-170359460 # 2nd run

Expected Output

356630733360154 # first run
356632381452769 # first run

Possible Solution

don't ignore warnings, and even error on warnings (except maybe a whitelist).
When compiling with --listCmd we get:

clang -c  -w  -I/Users/timothee/.choosenim/toolchains/nim-0.20.0/lib -I/Users/timothee/git_clone/nim/timn/tests/nim/all -o /tmp/d01bc/t04377.nim.c.o /tmp/d01bc/t04377.nim.c

if we remove -w we get:

/tmp/d01bc/t04377.nim.c:149:8: warning: implicit declaration of function 'mach_absolute_time' is invalid in C99 [-Wimplicit-function-declaration]
        T2_ = mach_absolute_time();

that warning currently gets ignored without error or any indication of which, and the generated code produces garbage.

The 2nd problem here is that the header is wrong (and also the return type is wrong): it should be:

proc mach_absolute_time(): uint64 {.importc, header: "<mach/mach_time.h>".}

with this, it gives the expected output.

Note: the reason system/timers actually works is by luck: it's because MachTimebaseInfoData itself uses the right header; but minor modifications would re-exhibit that bug, eg if we comment out an unrelated part of the code (eg mach_timebase_info(timeBaseInfo)), that correct header would get optimized out, and the undefined behavior would be there.

Note: this bug is obviously not specific to system/timers and can occur in many other unrelated settings

Additional Information

  • Was it working in the previous Nim releases?
    no
nim --version
Nim Compiler Version 0.20.0 [MacOSX: amd64]
Compiled at 2019-06-06
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: e7471cebae2a404f3e4239f199f5a0c422484aac
active boot switches: -d:release
timotheecour added a commit to timotheecour/Nim that referenced this issue Jun 25, 2019
@timotheecour timotheecour changed the title c compiler warnings silently ignored, giving undefined behavior c compiler warnings silently ignored, giving wrong results/undefined behavior Jun 27, 2019
timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 10, 2019
timotheecour added a commit to timotheecour/Nim that referenced this issue Jul 15, 2019
timotheecour added a commit to timotheecour/Nim that referenced this issue Dec 17, 2019
timotheecour added a commit to timotheecour/Nim that referenced this issue Jan 29, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 10, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 11, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Apr 12, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Apr 15, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Apr 16, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants