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

Segfault because of unsanitazed function parameters #37

Open
isovic opened this issue Feb 4, 2015 · 5 comments
Open

Segfault because of unsanitazed function parameters #37

isovic opened this issue Feb 4, 2015 · 5 comments

Comments

@isovic
Copy link

isovic commented Feb 4, 2015

Just to mention, this happens in an older version (not sure if the bug is present in the most recent one).
This is the GDB's output from running my program:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ff2f5399700 (LWP 9563)]
0x0000000000410f8d in obtainAlignment (maxNumBlocks=3, queryLength=178, targetLength=-13, W=14, bestScore=-1,
position=-1, alignData=0x0, alignment=0x7ff2f5397f60, alignmentLength=0x7ff2f5397f5c)
at src/alignment/myers.cpp:765

At the beginning of the obtainAlignment function, there is an allocation without checking the values of input parameters.
Also, it would be good that after every malloc/new there is a check if the pointer is NULL (but please continue handling things in C-like manner, throwing exceptions is just awful :-) ).

@Martinsos
Copy link
Owner

Ah yes, good catch, thanks! I didnt do validation of input parameters yet, but I should certainly do it.
I usually do not check if malloc pointer is NULL, how likely is that to happen? Is that something that is usually handled?

@Martinsos
Copy link
Owner

This one will be handled by #38

@isovic
Copy link
Author

isovic commented Feb 8, 2015

I usually do not check if malloc pointer is NULL, how likely is that to happen?

I think it is always a good practice, though if you are allocating small chunks of memory it is very unlikely you won't be able to actually get it. But since your library could potentially consume huge amounts of space, I think it would be prudent.

E.g. As from one of the previous examples I've sent you, even on relatively small input sizes (e.g. query sequence of length 30k) the memory consumption gets up to 100MB if alignment reconstruction was requested. Imagine if I was to run it with even a larger query sequence, and perform that in parallel on several cores. I feel you could get in the ball park of available free memory on a normal (not very-high end) laptop.

@Martinsos
Copy link
Owner

That is true! Ok, I will certainly takecare of that at some moment soon,
thanks
On Feb 8, 2015 8:41 AM, "Ivan Sovic" notifications@github.com wrote:

I usually do not check if malloc pointer is NULL, how likely is that to
happen?

I think it is always a good practice, though if you are allocating small
chunks of memory it is very unlikely you won't be able to actually get it.
But since your library could potentially consume huge amounts of space, I
think it would be prudent.

E.g. As from one of the previous examples I've sent you, even on
relatively small input sizes (e.g. query sequence of length 30k) the
memory consumption gets up to 100MB if alignment reconstruction was
requested. Imagine if I was to run it with even a larger query sequence,
and perform that in parallel on several cores. I feel you could get in the
ball park of available free memory on a normal (not very-high end) laptop.


Reply to this email directly or view it on GitHub
#37 (comment).

@Martinsos
Copy link
Owner

Note: checking if memory was allocated is not such a big issue anymore, since we do not consumer large amount of memory any more (thanks to hirschbergs algorithm).

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

2 participants