[SOLVED] Qt Programm crashes after calling a function.
-
Hi to every body!
I have writen a small programm to read data with UART protocol from a usb dongle for CAN BUS.
I have writen a class which reads a databank sqlite file with the names of the CANIDs and signals.
Every can frame has 1~8 bytes of information and a DLC 4-bit signature that means Data length Code.
when DLC is 2 we have 2 bytes when is 8 we have 8 bytes etc.When the dongle send a frame with 8 bytes everything is working perfect, but when DLC is lower than 8 the programm crasses.
Especially i call @ results = candatabank->CANSignalList(B.id,CANDataFrameAs64bitUint,B.DLC);
results is QVector<QStringList>@
If the DLC is lower than 8 the softaware crasses on this call, only the call is enough to destroy the programm. Why? i cannot understand that.
!http://i61.tinypic.com/n2g6mh.png(screen shot)!
SEE the screenshot for this error and at https://github.com/ntosis/openCANalysis is the whole of code!! -
Hi,
One thing I can see is that you don't check in BitsToUINT16 that bitstart and bitend are within the range 0-63 so you might be trying to access the array at a wrong index
-
Hi,
Looking at the variables window, I see that the this pointer is null. Somehow you called that MainWindow::addTreeChild() function on a null MainWindow pointer. candatabank->CANSignalList() is the first line in that function where the this pointer is called, therefore it crushes there.
You should find out how you end up using a null MainWindow pointer.
PS: A call like
@candatabank->CANSignalList()@is actually
@this->candatabank->CANSignalList()@ -
[quote author="SGaist" date="1420665508"]Hi,
One thing I can see is that you don't check in BitsToUINT16 that bitstart and bitend are within the range 0-63 so you might be trying to access the array at a wrong index[/quote]
i was thinking about it but i made this change to code!
@int CANdatabank::CANSignalList()
{
return 1;
}
@
i deleted everything and with the call
int results = candatabank->CANSignalList();
The program crashes also!! i think it is not in function but something else in call structure -
[quote author="xmaze" date="1420667424"]
[quote author="SGaist" date="1420665508"]Hi,One thing I can see is that you don't check in BitsToUINT16 that bitstart and bitend are within the range 0-63 so you might be trying to access the array at a wrong index[/quote]
i was thinking about it but i made this change to code!
@int CANdatabank::CANSignalList()
{
return 1;
}
@
i deleted everything and with the call
int results = candatabank->CANSignalList();
The program crashes also!! i think it is not in function but something else in call structure[/quote]Since the * this * pointer is null, a call to CANSignalList() will crash the program regardless of the body of that function.
-
What's puzzling is if "this" is null, how can it even go so far ?
On a completely unrelated note, slots should not return values. Use Q_INVOKABLE if needed and you have several memory leaks.
-
[quote author="ckakman" date="1420668650"]
Since the * this * pointer is null, a call to CANSignalList() will crash the program regardless of the body of that function.
[/quote]Now i understand what do you mean but i cannot find the reason!!
-
[quote author="SGaist" date="1420669005"]What's puzzling is if "this" is null, how can it even go so far ?[/quote]
Since "this" is just a parameter passed to member functions, you can go until you use the "this" pointer.
-
[quote author="ckakman" date="1420670165"]
Since "this" is just a parameter passed to member functions, you can go until you use the "this" pointer.[/quote]Do you have an idea how to find the solution?
When the DLC is 8 why the program runs completely without malfunction? -
[quote author="ckakman" date="1420670165"]
[quote author="SGaist" date="1420669005"]What's puzzling is if "this" is null, how can it even go so far ?[/quote]Since "this" is just a parameter passed to member functions, you can go until you use the "this" pointer.[/quote]
I guess you meant how it could go that deep in the call stack if this were null all along. That's puzzling indeed.
I wonder how that file is compiled because line 382 in the screenshot doesn't look like valid c++. B.DLC is a member variable of the struct. The compiler couldn't have determined the array size statically.
The inner and outer loops always iterate over the same ranges since s is sizeof(B.Data). However bits has varying size depending on B.DLC.
Here comes my conspricy theory: If bits is not large enough, an out-of-bounds access occurs and maybe "this" pointer variable in the stack frame gets overwritten, hence the crush.
-
@for(int i=0; i<s; ++i){
for(int b=0; b<8; ++b) {
int f=i*8+b; bool bit = (B.Data[i]&(1<<(7-b)));
if(bit) bits[f]=1; else bits[f]=0;
}}@in this loop when f is equal to 34 then <<like in screen shots>> the pointer "parent" is NULL and after some loops, when f is 44 or 43 i dont remeber exactly, the this pointer is NULL. why??
-
The screenshot is taken on Gnome, so the compiler is probably GCC. If I am not mistaken GCC has this nonstandard feature where arrays can have their sizes be specified during runtime. I think this was a C feature.
xmaze noted in the issue description that the crash occurs when DLC is less than 8. So bits is a smaller array if DLC is smaller, but the outer for loop always iterates 8 times (Data is an array of size 8). Inner loop also iterates 8 times, so bits must always be of size 64 but it is not when DLC is less than 8. So out of bounds array access happens for sure.
It seems that the "this" pointer is rather close to bits in the stack frame so it gets overwritten when out if array access occurs.
-
You should declare bits as an array of size 64 not as B.DLC * 8.
-
but i tried to workaround this problem before few days, in function SortTheFrameToStructure() i put all the arrived frames from the usb dongle in Vector as a Structure!
@for (int i=0; i<R.DLC;i++) {R.Data[i] = list[5+i].toUInt(&ok,16);}
if(R.DLC<8) {for(int k=R.DLC;k<8;k++) R.Data[k]=0;} //Work around NEED to TESTED@when the DLC is lower i try to put some data to eliminate the overflow.
Do u think is that a mess? it is a bad idea or what? -
[quote author="xmaze" date="1420674425"]but i tried to workaround this problem before few days, in function SortTheFrameToStructure() i put all the arrived frames from the usb dongle in Vector as a Structure!
@for (int i=0; i<R.DLC;i++) {R.Data[i] = list[5+i].toUInt(&ok,16);}
if(R.DLC<8) {for(int k=R.DLC;k<8;k++) R.Data[k]=0;} //Work around NEED to TESTED@when the DLC is lower i try to put some data to eliminate the overflow.
Do u think is that a mess? it is a bad idea or what? [/quote]The problem is not in Data. What you are doing there is initializing unused bytes. That is OK.
You need to make sure that the bits array in add child function is of size 64. Otherwise you will have out of bounds access since your loops always iterate 64 times in total.
-
Thank you very much!!
Solution found!!
i have and some more questions for this programm, need i make new thread or can i ask in this thread?
-
[quote author="ckakman" date="1420668650"]
Since the * this * pointer is null, a call to CANSignalList() will crash the program regardless of the body of that function.
[/quote]Not quite true. A call to a memberfunction of a null or deleted object may succeed if that function doesn't access any members. Not that I recommend calling functions on those "shells of" objects, but still...
[quote author="SGaist" date="1420669005"]What's puzzling is if "this" is null, how can it even go so far ?
On a completely unrelated note, slots should not return values. Use Q_INVOKABLE if needed and you have several memory leaks.[/quote]
Sorry, also need to disagree on that. It is perfectly fine for slots to have return values. Don't expect them to be used in cases where the slot is used as slot, but it is possible and may be valid. Q_INVOKABLE is there for methods that you want to be callable from for instance QtScript but that are not slots.[quote author="xmaze" date="1420705632"]Thank you very much!!
Solution found!!
i have and some more questions for this programm, need i make new thread or can i ask in this thread?[/quote]
If they are not related to this problem, but only to this program, please use new threads. And for each problem, please use the search here, the documentation and the general internet first of course. -
[quote author="Andre" date="1420707497"]
[quote author="ckakman" date="1420668650"]
Since the * this * pointer is null, a call to CANSignalList() will crash the program regardless of the body of that function.
[/quote]Not quite true. A call to a memberfunction of a null or deleted object may succeed if that function doesn't access any members. Not that I recommend calling functions on those "shells of" objects, but still...
[/quote]I am aware of that possibility and I myself explained it to the OP. But the situation here is different:
Note that CANSignalList() is not a member function of a null or deleted object. Instead, that function is a member function of a member variable of the null object. Therefore it is called via the null this pointer. See the line 398 in the screenshot. -
Ah, right. Yes, you are right. I stand corrected. I did not look at the code carefully enough.
-
[quote author="Andre" date="1420707497"]
/snip
[quote author="SGaist" date="1420669005"]What's puzzling is if "this" is null, how can it even go so far ?On a completely unrelated note, slots should not return values. Use Q_INVOKABLE if needed and you have several memory leaks.[/quote]
Sorry, also need to disagree on that. It is perfectly fine for slots to have return values. Don't expect them to be used in cases where the slot is used as slot, but it is possible and may be valid. Q_INVOKABLE is there for methods that you want to be callable from for instance QtScript but that are not slots.
/snip again
[/quote]I completely agree with you they can. However I've already seen cases where people where using that in a multithreaded environment and there's no need to tell you what happened. That's why I recommend against that use and rather use Q_INVOKABLE also as a mean to make the code clearer about what it can do and how it can be properly used.
