QFile::close() leads to SIGILL

Cobra91151

Hello!

Have you tried the WinDbg to track down this issue?

SeDi

@Cobra91151 Thanks for the idea - I have never heard of it before! As I understand, that'd be for Windows, but my problem occurs on an Android device.

That brought me to just go and try the same code on my windows machine. Runs without a problem there.
So it's an "Android only" problem? I mean.. QFile...?

I'm puzzled.

Cobra91151

@SeDi

Additionally, you can enable the Developer mode on your Android device, connect to Win 10 machine and debug it live inside of Qt Creator. Maybe it could help you find this issue.

JonB

@SeDi
Your code looks good to me. Why not start by removing your while loop entirely? Then, just read one line, do nothing with it?

SeDi

@Cobra91151 , now I see what you mean! Of course, I am doing exactly that. It crashes on this very line. The stack trace just doesn't help:

1 CsvTableModel::readDataFromFile          csvtablemodel.cpp 114 0xcc41ab00 
2 main                                     main.cpp          18  0xcc4188e8 
3 startQtApplication(_JNIEnv *, _jclass *)                       0xcc5e483a 
4 ??                                                             0xe988767a

Unfortunately, I am still lost :-(

Does the code look ok for you?
The csv is about 40kB heavy, but I can't figure how that would have any effect.
Do I have to somehow disconnect the QTextStream or wait for something to clean up internally?

Christian Ehrlicher

Please show your main.cpp. Where is exactly line 114?

Cobra91151

@SeDi

Add this code to main.cpp:

#ifdef QT_DEBUG
    qputenv("QT_FATAL_WARNINGS", "1");
    qputenv("QT_MESSAGE_PATTERN",
            "Type: %{type}\nProduct Name: %{appname}\nFile: %{file}\nLine: %{line}\nMethod: %{function}\nThreadID: %{threadid}\nThreadPtr: %{qthreadptr}\nMessage: %{message}");
 #endif

Then, try to debug your app.

SeDi

@Christian-Ehrlicher Line 114 of CsvTableModel is "file.close()" The crash occurs exactly when I step-proceed over file.close(). I don't actually use my model up to this point. I try to just get the file-reading process ready first.

My main.cpp is this (I have used an example file from QZXing as a starting point):

#include <QGuiApplication>
#include <QQmlApplicationEngine>
#include <QQmlContext>

#include <QDebug>

#include <Qt>
#include "QZXing.h"
#include "application.h"
#include "csvtablemodel.h"

int main(int argc, char *argv[])
{
  QCoreApplication::setAttribute(Qt::AA_UseHighDpiPixmaps);
    QGuiApplication app(argc, argv);

    CsvTableModel csvModel;
    csvModel.readDataFromFile();

    QZXing::registerQMLTypes();

    Application customApp;
    customApp.checkPermissions();

    return app.exec();
}

The model's constructor is empty:

CsvTableModel::CsvTableModel() {}

@Cobra91151 : Adding the qputenv lines didn't change a thing, neither was the crash any different, nor the ApplicationOutput (I've made a diff).

I appreciate your help, guys!

JonB

@SeDi
Like I said, why not remove the while(!csvIn.atEnd()) loop? Keep removing stuff till you get the minimum to make the file.close() crash.

SeDi

@JonB, I am sorry - i didn't see your answer, probably because I was answering to @Cobra91151 while you wrote it.

The problem is solved. file.close() was a red herring, as well as the difference between Android and Windows. The crash hat a reason so profane that I am ashamed to mention it.

The method ist

bool CsvTableModel::readDataFromFile() {

but I did not return true.

Facepalm. My apologies and gratitude to y'all. Turns out reduction actually was the key, as it even crashed when the method was empty. One of these moments. Thanks!

JonB

@SeDi

Didn't your compiler warn about bool function with no return at end?
You don't use the return result from your call in main().
Regardless of these, how does omitting a return result lead to a SIGILL?

Christian Ehrlicher

@JonB said in QFile::close() leads to SIGILL:

Regardless of these, how does omitting a return result lead to a SIGILL?

It's undefined behavior.

JonB

@Christian-Ehrlicher
Interesting! Then I'm even more surprised the OP's compiler didn't issue any warning for this code....

Do you have a reference for "It's undefined behavior"? Of course I know if you test the result you don't know what you'll get, but I'd like to read up on what it has to say if you fail to return any result, leading to possible crash.

Many, many years ago I used a C compiler on some unmentionable home computer system. I think it was a 6502 CPU. There, the compiler used the hardware stack to return the (two bytes) of any function result. What this meant was: I had to go through every line of code, working on all other C systems (program was cross-platform), so that if any function returned a value you had to assign that to a variable (or e.g. pass it as a value to another function, etc.). Failure to do so left a number on the stack after the function call, leading to later crash. What this meant was, for example, I had to find & change every single printf(...) in existing code to dummy = printf(), and similar for the hundreds of other functions which happened to return some value we don't care about...! :( And I still don't believe that behaviour was ever "allowed" in a C compiler.

Christian Ehrlicher

@JonB said in QFile::close() leads to SIGILL:

Do you have a reference for "It's undefined behavior"?

Don't return anything when the function should is undefined behavior by default :)

leading to possible crash.

It may also work, but it may also eat kittens. It depends on the compiler, the optimization level, the moon phase, ...

I know that one compiler did not warn about this kind of stuff, gcc 7.5 at least prints a warning (but compilation does not fail)

JonB

@Christian-Ehrlicher
Saying that "no return result" leads to "undefined behaviour" on the return result is one thing. That's not what I'm asking. Saying that the "undefined behaviour" could lead to "crash" (e.g. on exiting function) is quite another. I need a reference here! I'm going to have a look....

P.S.
What is that you & @Chris-Kawa have about threatening my fluffy kitten the whole time? She is now getting quite alarmed about these threats....

Christian Ehrlicher

@JonB said in QFile::close() leads to SIGILL:

Saying that the "undefined behaviour" could lead to "crash" (e.g. on exiting function) is quite another.

Undefined behavior is ... well undefined. anything can happen. My compiler returns '6' for this piece of code:

int foo()
{
	printf("Hello\n");
}

int main(int argc, char *argv[])
{
    printf("foo: %d\n", foo());
    return 0;
}

and '2116800' when I remove the printf() statement.

JonB

@Christian-Ehrlicher
Yes, but you know that is not the issue we are discussing! You know we are not talking about what the return result number is, we are talking about "crashing" on the } of foo() (or at the caller) because foo() does not have a return statement.

The following also has "undefined behaviour" (in what it prints), but I don't expect it to "crash":

int z;
printf("%d\n", z);

Christian Ehrlicher

Again: it's undefined - anything can happen, in this case it crashed.

JonB

@Christian-Ehrlicher
For the record, I have read what I can from various stackoverflow posts, etc. I do accept (now) that non-void no-return => UB => "anything may happen including crash". I was not aware of this, interesting, and thank you. Note however that there was no mention of damage to nearby kittens, either in C++ standard or in posts, so I am calming mine down about this (apart from, I have assured her this is one error I have never made)!

SGaist

Hi,

@JonB said in QFile::close() leads to SIGILL:

The following also has "undefined behaviour" (in what it prints), but I don't expect it to "crash":
int z;
printf("%d\n", z);

Strictly speaking it's not undefined behaviour. You will print a random value from an uninitialized int variable.

As @Christian-Ehrlicher already wrote undefined behaviour can be anything from funky values to a crash.

I had once to debug a strange crash that in the end was due to a missing return statement for a QString and the return value wasn't even used. For the fun of experimenting, I changed the return type to int and no crash anymore. That's what undefined behaviour is. That took me quite a while to find because the original warning was swamped in a tons of other warnings (words of the original developer: don't care these are just warnings), the return value being ignored, I did not take notice immediately that the return statement was missing and the code was so involved that tracing the crash correctly was pretty hard as is also required external hardware (