This fairly simple program will fail, and I cannot find my bug:
git clone https://github.com/pb-cdunn/nim-help.git
cd nim-help
make
# nim c -r t_raptor_db.nim
Traceback (most recent call last)
t_raptor_db.nim(181) t_raptor_db
raptor_db.nim(271) get_length_cutoff
/raptor_db.nim(254) load_rdb
Nim/lib/system/assign.nim(111) genericAssign
Nim/lib/system/assign.nim(100) genericAssignAux
Nim/lib/system/assign.nim(24) genericAssignAux
Nim/lib/system/assign.nim(20) genericAssignAux
Nim/lib/system/assign.nim(54) genericAssignAux
Nim/lib/system/gc.nim(435) newObjNoInit
Nim/lib/system/gc.nim decRef
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
I hoped I could debug this by using --newruntime and valgrind, but I am not able to build with newruntime.
raptor_db.nim(178, 38) Error: region needs to be an object type
I would be very excited to see "newruntime" working, precisely to help me with this sort of memory bug. But that's in the distant future.
If no-one can help me fix this, I might have to give up on Nim completely within my company.
I've pushed a commit to master which completely drops all use of the unittest module. It still fails in GC, so it's not a result of any unittest-macro magic.
There must be something fundamental that I'm doing wrong.
Hi cdunn
change your strlen wrapper in raptor_db.nim to
proc strlen(a: var Headroom): int =
let n = strlen(cast[cstring](a[0].addr))
echo " calc'd:", n
return n
: unhandled exception: /data/Projects/nim-help/t_raptor_db.nim(156, 16) `"m1/3005/0_5852" == db.seqs[0].header` [AssertionError]Switching to your wrapper makes no difference for me.
Yes, if you make small changes the code can crash in different places, though I haven't been able to reproduce your assertion failure.
Do you think you've the actual bug? Could you point me to it?
Can you try changing line 236 with var sr = SequenceRecord() ?
I'm not sure but maybe what you're doing is scope-escaping memory? That var sr: SequenceRecord only available in the stack and it got destroyed after you copy (or move?) to result.seqs.add(sr)
I cannot run it, currently don't have available linux box currently.
- var sr: SequenceRecord
+ var sr = SequenceRecord()
I am not over-stating the problem here. If we cannot fix this, probably the entire Bioinformatics community will be forced to give up on Nim. (A colleague is already fed up with Nim multi-threading, though I've been willing simply to avoid the threadpool library.) I will still re-adopt Nim when newruntime is available, but I will have zero credibility on Nim with my colleagues. We need a solution for weird memory bugs like this, a simple way to discover them, even if it turns out to be programmer error. I am not a terrible coder.
I thought I'd try -d:useMalloc:
nim c --gc:none -d:useMalloc -d:useSysAssert -d:useGcAssert --debugger:native -d:nimBurnFree -d:corruption -r t_raptor_db.nim
/pbi/flash/cdunn/gh/Nim/lib/system/mmdisp.nim(443, 25) Error: pragmas are only allowed in the header of a proc; redefinition of 'deallocOsPages' from /pbi/flash/cdunn/gh/Nim/lib/system/threads.nim(111, 8)
You're right that returning Db instead of ref Db "fixes" the problem, but is that really a solution? Returning a "ref" simply means that load_rdb() is really a Db constructor. That should work.
And no, an alias for DbRef = ref Db does _not fix it. Same for DB = ref object. Still crashes....
... Oh! Your clue helped me find the problem.
let scanned = sscanf(line.cstring, v_frmt.cstring,
addr result.version)
I really, really want the "newruntime". This kind of mistake is way too easy.
I really, really want the "newruntime". This kind of mistake is way too easy.
This mistake is easy, but you really should reconsider mixing such low-level code with your high-level scripts. Any use of addr, ptr, cast should be very deliberate, the analogy to Rust's unsafe is very apt here. I'm sure that even the "newruntime" will have escape hatches which will allow you to ignore the compiler, this is what you're effectively doing here.
A good solution here would be to write a Nim native sscanf or just use the stdlib's parseutils or strscans.
I wasn't sure whether you actually fixed your code with that post now, but I was already looking at your code when I saw the post, so I continued.
I fixed the code differently, by just removing the c types you used. I trust that the test case works, because I'm not sure if I broke something. :)
Yes, the suggestion to use parseutils and strscans is not a bad one, and thanks for the code example. But I did consider both of those before proceeding.
First, I couldn't figure out how parseBiggestInt works. It is very important to specify the size of integer. I don't want a machine-dependent solution. It drives me crazy that I cannot specify the size that the input format provides.
Second, I saw that strscans provides a weird $w, which is just the wrong thing. I want any non-whitespace. There is an inversion matcher, but I wasn't sure how to use it. I couldn't figure out the "user-definable matchers". Basically, I just had much more confidence in the API for sscanf than for strscans. Maybe examples would help.
+({'a'..'z', 'A'..'Z', '_', '0'..'9', '/'} -> buf0.add($_)),
Third, I have trouble telling when memory allocations will occur. That's a big problem with Nim strings around to the parsers. Just look at the disaster in streams.FileStream, which is incredibly slow. (And don't get me started on threadpool). I need to know what is going on at a pretty low level.
Finally, on avoiding low-level code, I think people in this forum misunderstand the niche for Nim. There are tons of good languages these days. I need something specifically for:
Python is better at (1). Python is almost as good at (2), but I want a language with static-typing and fast compilation, instead of an interpreted language. (3) is where Python fails. Static-typing is absolutely required for C interfaces because so many things can go wrong.
I included in my broken code only the minimum needed for debugging. It's actually a lot of code, growing all the time, and calling various C and C++ libraries. That's not even up for discussion. Without the C/C++ interfacing, I would use a different language. And what I'm saying is that I still need to be able to debug. I will have low-level bugs in my code. I need ways to detect them. The "newruntime" seems to be critical for that. Araq is right.
Oh, I should also mention that I am parsing GBs of data. In bioinformatics, we deal with TBs.
On strscans, apparently $+ is the way to parse non-whitespace into a sting. That was not at all obvious to me. But even that feature is not enough. The failure to specify integer sizes is really a HUGE problem.
I agree, generic int parsing would be a very nice addition to strutils/parseutils.
Somehow binary, hex and octal got generic int parsing before plain number stored in strings: https://github.com/nim-lang/Nim/pull/11107/files.
And haha good catch on sscanf, didn't check that one after strlen.
Unfortunately I don't see how Nim newruntime can help detecting buffer overflow.
One thing that can be done is that when sysAssert and/or gcAssert flag are defined, all seq/string/ref get an additional field canary field, initialized with a pattern say 0x42424242, and the pattern is checked regularly to make sure no wild C proc or unsafe cast/addr usage overwrote it.
Yes, a "canary field" would be a good debug-mode for reference-counting. That's a very good idea.
Buffer-overflow is hard to detect, but when you switch to malloc/free there is a very good chance that valgrind will notice something. Maybe that's wishful thinking on my part.
Anyway, your help saved the day. I'm expanding our use of Nim internally.
I still have a lot of concerns about performance for simple things, but Nim has so much upside that I think we'll be ok.
Unfortunately, it is also as slow as C if you use strutils for naive string processing due to plenty of intermediate allocations, and the code that tends to loop over and over on the same string.
A way to fix that would be to have a library in the spirit of zero-functional but specialized for strings. Temporary immutable allocations could use alloca to avoid stressing the GC (or newruntime could do that optimization).
I see multiple potential ways to implement that in an efficient and extensible manner if someone wants to explore it: