Segmentation fault SIGSEGV, what can it be?

Gerd

Hi,
looks like the pointer is changed anywhere.
From the given lines of code i assume that "rfPower4x1" is a public variable, so it may be changed from everywhere.
What you can do to find the error:

search for "rfPower4x1" in the project and check every assignment to that
make "rfPower4x1" privat to your class and implement some function for accessing the data, so the pointer self can only be changed from within the class
use a data-breakpoint in the debugger

Regards
gerd

SGaist

Hi,

Out of curiosity, why are you instantiating a new QByteArray ? That's an unusual use of that class

StephanWoebbeking

Hi Gerd, I have searched through all access to rfPower4x1, there is no contradicting access to it at all. I only assign it in the constructor of the device, I do "clear" every now and then, so I could understand if its empty, but the instance should still exist?!? Especially, there is no assignment at all where a null pointer could be the result. So from the first access there should be a valid object behind... Also the device object is used extensively, so this can not have been destroyed yet from my point of view.

I have made the data public, because it's not that huge project overall. And this device object is bound quite closely to the mainwindow anyway. So I didn't see extensive need for data hiding, even I know from the pure architectural point of view it would be the right thing. But as the mainwindow would be able to access the data via methods anyway and the device itself doesn't really do much with the data but only stores it I didn't see much use from it...

Obviously I have used breakpoints a lot already, but it didn't get me anywhere. I didn't use a data-breakpoint yet, I have to check that out first I must admit...

Stephan

StephanWoebbeking

@SGaist said:

Out of curiosity, why are you instantiating a new QByteArray ? That's an unusual use of that class

I worked with the instance directly before, but I hit the same issue. And because I had no more ideas I changed over to a pointer. I think I will change it back, doesn't make it worse I hope. I will tell you if it gets me anywhere.

I must admit, still I have no bl***y idea, where the problem could be. This seems to be quite a standard way of doing things, I use the QByteArray a lot, I use objects in general, I don't really have much problem with either both of it. It's just this one single point where I just can't work out what the problem is... I suppose it's something really silly and its right afront my eyes, I just don't see it....

Stephan

StephanWoebbeking

One more bit of information...

I am just about to change to an instance instead of a pointer... then it crossed me, that the "device" is also a pointer to the instance. I don't really see any issue there, but maybe someone else does?

Stephan

StephanWoebbeking

Changed back to use the instance directly and NOT the pointer - the same problem, but already when I do "clear" on the array. Funny is, that the debugger visualizes the QByteArray, its empty (which is ok), but on first access I get the SIGSEGV... Obviously I can't instantiate it then via "new" then...

Stephan

jsulm

Do you mean it crashes at device->rfPower4x1.clear();?
Does device point to a valid instance?

StephanWoebbeking

Yes, that's exactly where I get the segmentation fault. Device is accessible, I use it a lot. The debugger shows the content of rfPower4x1 to be "" / empty, that's fine...

Any ideas?
Stephan

StephanWoebbeking

One more thing. Now I have added "rfPower4x1.clear();" right in the constructor of the device - works perfectly, no problem but "device->rfPower4x1.clear();" crashes shortly after. No destroying access coded as far as I can see it.

Actually, there are some subobjects to the device, five to be exact. All implement the rfPower4x1 field as well, but its never accessed for the subobjects, only in the father object. That's not great design, I was up to consider to change it, but then this fault holds me up and I would like to sort that out before. Partly because I need it right like this but mainly because I want to understand what's going on...

Stephan

jsulm

What happens if you remove device->rfPower4x1.clear();?

StephanWoebbeking

Then, the error occurs consequently at "device->rfPower4x1.append( data );"...

SGaist

Silly question, but did you properly initialize device ? Is it deleted somewhere ?

StephanWoebbeking

Hi SGaist, no, it's not a silly question, but the answer could, potentially, lead to a silly answer. ;)

The device was dynamically initialized with "new" and works fine to all other aspects. So private data is happily accessed in read and write operations as well as method calls. The device is used without problems before and after the questionable calls to the rfPower4x1 field. Also the subobjects are working fine, its just this QByteArray...

Stephan

StephanWoebbeking

Hi, back to the multithreading again. I figured, that indeed the data I am processing comes back asynchronously. So the data is transferred using "emit" into a signal / function. But from there, I just access the device object which works fine for all other data. Now, all threads of my application share the same memory map, don't they? And device as wenn as the rfPower4x1 are dynamically reserved, so they are on the heap? That's why I don't see a problem in there, am I right with my suggestion?

Then, maybe I can get some closer to a solution wondering would be the situation if I don't use dynamic memory but us the objects directly instead? Aren't they then placed on the stack? How do different threads then find where the object is, without getting confused by various heights that the stack can have?

Regards, Stephan

SGaist

Can you share your code ?

Where are you accessing device ? Where is it allocated ?

StephanWoebbeking

Sure, its only rather complex already and I don't really see how to strip it down as it involves communication with an external device...

The definition for the device pointer is in the mainwindow.h:

private:
...    wbstlDevice *device;
    QByteArray tmpBuffer, rfPower4x1;
    QString fl7TextBuffer;

The instantiation is in the constructor for the mainwindow.cpp:

MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWindow) {
...
    device  = new wbstlDevice( this );

I use dynamic instantiation to allow the device having access back to the window via the pointer.

Then wbstlDevice is defined like this:

#ifndef WBSTLDEVICE_H
#define WBSTLDEVICE_H

#include <QObject>
#include <QByteArray>
#include <QString>

class wbstlDevice {
public:
    wbstlDevice( QObject *parent = 0 );
    virtual ~wbstlDevice();
    void reset();
    bool convert2IdTo3Id( unsigned char devketId, unsigned char rfId );
    bool isInitialized();
    bool initDevice();
    bool supportsFpSync();
    bool supportsClockCorrection();
    bool usesTestTone();
    bool usesTestMode();
    bool needsTestMode4TransmissionTest();
    bool canUseTestTone();

    virtual void addRFPower4X1( QByteArray data );
    virtual unsigned int convertSvcCmdIdToCmdInt( unsigned int svcCmd );
    virtual unsigned int getChannelFreqMHz( unsigned int channel );
    virtual unsigned int rfPower4x1Length(); // returns length in indices! One index contains 6 bytes / 12 nibbles
    bool isRfPower4x1Valid();
    QString convertSvcCmdIdToCmdStr( unsigned int svcCmd );
    unsigned int convertCmdIdToSvcCmd( unsigned int rawCmd );
    unsigned int convertCmdStrToSvcCmd( QString cmdStr );
    QString getSWVersionStr();
    void setSWVersion( QByteArray data );
    char getAdaptivePower();

    bool deviceNameChanged, serialIDChanged, autoLockChanged, pairingChanged, identifyChanged, walktestChanged, debugScreenChanged;
    bool initialized, testMode, validEEVersion, needsDectModeCorrection;
    char deviceId, marketId, rfId, devketId, fpSync;
    char swVersion[ 3 ];
    QString errorString;
    int freqStart, freqStep, freqCount;
    wbstlDevice *currentDevice, *deviceSRX1G9, *deviceSRX2G4, *devicePRX1G9, *devicePTX1G9, *devicePTX2G4;
    QByteArray rfPower4x1;
};
#endif // WBSTLDEVICE_H

As I said for simplification I do access the rfPower4x1 field directly (public). The method "isRfPower4x1Valid" is defined like this:

bool wbstlDevice::isRfPower4x1Valid() {
    bool ret = true;
    int idx1, idx2, sum, expLength;

    expLength = ( currentDevice->rfPower4x1Length() * WbstlConst::RF_POWER_4X1_INDEX_LENGTH );
    if ( rfPower4x1.length() != expLength )
        ret = false;
    idx1 = 0;
    while ( idx1 < expLength ) {
        sum = 0;
        for ( idx2 = 0; idx2 < WbstlConst::RF_POWER_4X1_INDEX_LENGTH; idx1++, idx2++ )
            sum += rfPower4x1.at( idx1 );
        if ( ( sum == 0x00 ) || ( sum == 0xFF ) )
            ret = false;
    }

    return ret;
}

And in the mainwindow is did like this:

        device->rfPower4x1.clear();
        device->rfPower4x1.append( data );
        appendLog( ants::printHex( device->rfPower4x1 ) );
        appendLog( QString( "%1" ).arg( device->rfPower4x1.length() ) );
        if ( device->isRfPower4x1Valid() )
            appendLog( "RF power 4x1:          ok" );
        else
            appendLog( QString( "RF power 4x1 contains some errors: %1" ).arg( ants::printHex( data ) ) );

Even rfPower4x1 is statically instantiated, I get the SIGSEGV only when I clear the array... any more ideas?

Regards,
Stephan

jsulm

"I use dynamic instantiation to allow the device having access back to the window via the pointer" - no need for dynamic allocation. Do:

MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWindow), wbstlDevice( this )
{}

In general, Device should not know anything about MainWindow - else it is bad design. If you need to communicate from Device to MainWindow you should emit signals in Device which MainWindow connects to own slots. This way Device does not care about MainWindow implementation (if you change something in MainWindow you do not have to change Device).

StephanWoebbeking

@jsulm Ok, I am aware that there is some redesign suitable. Also with respect to the device and the sub-class structure. But does this helps us with regard to the segmentation fault? I was planning to do that in a refactoring session some time down the line, but if you think that it is essential for the function I would have to reschedule...?

Stephan

jsulm

You could try to change wbstlDevice to an instance variable instead of pointer and see whether is still crashes.

Gerd

The few lines of code you shown so far didn't seem to have any error.
As you where talking about asynchronus data filling this error may be thread-related.
How is the method in MainWindow called that deals with device ?