Consistent crashes when navigating to some directories with the import file dialog

[jceipek]

Here, https://github.com/dfeneyrou/palanteer/blob/main/server/viewer/vwFileDialog.cpp#L203-L204

I consistently get a EXC_I386_GPFLT (general protection fault) on a.name.toChar() when navigating to certain directories. I can iterate and print the names of all elements in _fileEntries without issue, so I imagine that std::sort is doing something undesirable in conjunction with your string library. Unfortunately I'm not really sure how best to debug this issue, and I haven't found anything particularly special about the failing directories.

[dfeneyrou]

Do you confirm that it is also concerning the Mac OS port project?

If yes, the file sorting just alphabetically compares the strings (with strcasecmp) returned by the platform layer function osGetDirContent(, ).
You should double check the implementation of your implemented osGetDirContent() function for MacOS, it probably returns entries with invalid pointer (easy to check: just print them before the sorting)

[jceipek]

Yes it is. However, I copied the osGetDirContent from Linux and tried doing

    for (bsDirEntry *curr = &entries.front(); curr != &entries.back(); ++curr) {
        printf("%s\n", curr->name.toChar());
    }

at the end of osGetDirContent. Every pointer seems fine and I can see the names in directories printed as I navigate to them.

Everything works fine until the std::sort call in the vwFileDialog.cpp file triggers a general protection fault. I'm going to try to write a naive sort function and will report back whether it replicates the problem.

[jceipek]

Ok, using this extremely naive bubble sort, I no longer get crashes in the file dialog. I really don't know anything about C++, but I suspect that std::sort is violating some of your invariants:

                        {
                            Entry* head = _fileEntries.begin();
                            int count = _fileEntries.size();
                            bool didSwap = false;
                            do {
                                didSwap = false;
                                for (int i = 1; i < count; i++) {
                                    if (direction*strcasecmp((head+i)->name.toChar(), (head+i-1)->name.toChar()) < 0) {
                                        Entry temp = *(head+i);
                                        *(head+i) = *(head+i-1);
                                        *(head+i-1) = temp;
                                        didSwap = true;
                                    }
                                }
                            } while (didSwap);
                        }
                        
//                        std::sort(_fileEntries.begin(), _fileEntries.end(),
//                                  [direction](const Entry& a, const Entry& b)->bool { return direction*strcasecmp(a.name.toChar(), b.name.toChar())<=0; } );

[jceipek]

Weird. If I replace <= with < in your call to std::sort, I no longer get a crash.

However, when I run this, I get no output:

Entry* head = _fileEntries.begin();
int count = _fileEntries.size();
for (int a = 0; a < count; a++) {
    for (int b = 0; b < count; b++) {
        int res = strcasecmp((head+a)->name.toChar(), (head+b)->name.toChar());
        if (res == 0 && a != b) {
            printf("EQUAL: '%s' '%s'\n",(head+a)->name.toChar(), (head+b)->name.toChar());
        }
    }
}

[jceipek]

Also interesting: I sometimes get a pointer being freed was not allocated crash logged in lldb with the unmodified std::sort call. Looks like this happens in toChar's push_back call in reserve_ when doing delete[] oldArray

[dfeneyrou]

Weird. If I replace <= with < in your call to std::sort, I no longer get a crash.

In fact, this is the real fix.
I got this issue some time ago in another project. The comparison function shall follow the "strict weak ordering", and if it fails to do it, really bad things can happen (in this case, some duplicated item leading to memory issues)
See https://en.cppreference.com/w/cpp/named_req/Compare

I suppose that 2 elements were identical (for an unknown reason), which trigged the issue.
Thanks for the reporting!