Skip to content

Conversation

@victl
Copy link

@victl victl commented Jan 7, 2023

I've successfully builded GoldenDict on macOS with AppleSilicon chips (the M2).

I hope this would help many macOS users who crane for the ability to run GoldenDict on their shiny Macs.

Build steps are updated in the README.md file

Copy link
Owner

@vedgy vedgy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can Qt WebKit be compiled on macOS Ventura? If so, this pull request does not directly depend on Qt WebEngine and should be created in the upstream repository https://github.com/goldendict/goldendict/

In any case, compatibility with older macOS versions must be preserved if at all possible.


Note: To compile with `libhunspell` older than 1.5 pass `"CONFIG+=old_hunspell"` to `qmake`.
## How to build on macOS Ventura 13.1 (succeeded on a Macbook Air M2)
The old `*.dylib` files included in the `maclibs` directory are outdated (for x86), so they've been removed.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these old files work with older architectures or Mac versions? If so, can the maclibs/ directory stay? Depending on the detected Mac version or on a new CONFIG variable, goldendict.pro could either use maclibs/ or rely on homebrew.

vedgy added a commit that referenced this pull request Jun 10, 2023
ThreadSanitizer reports a data race:
  Read of size 4 at 0x7b040003edb0 by thread T19:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    #1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    #2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    #3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    #4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    #5 <null> <null> (libQt5Core.so.5+0xe8710)

  Previous write of size 4 at 0x7b040003edb0 by thread T18:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    #1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    #2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    #3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    #4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    #5 <null> <null> (libQt5Core.so.5+0xe8710)

dict.mddResources is shared and accessed concurrently by several
separate MddResourceRequest objects. The line
`sptr< IndexedMdd > mddResource = *i;` contains a data race, because
sptr is not thread-safe as sptr_base::count is not atomic.

MdxDictionary::mddResources and its elements are not modified after the
initialization in MdxDictionary::doDeferredInit(). Simply dereference
the pointer instead of copying a sptr element of mddResources to
eliminate the data race.

The crash reported in goldendict#1653 may be caused by this data race.

Apply the same fix to duplicate code in
MdxDictionary::getCachedFileName().
btrkeks pushed a commit to btrkeks/goldendict that referenced this pull request Jun 15, 2023
ThreadSanitizer reports a data race:
  Read of size 4 at 0x7b040003edb0 by thread T19:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    vedgy#1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    vedgy#2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    vedgy#3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    vedgy#4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    vedgy#5 <null> <null> (libQt5Core.so.5+0xe8710)

  Previous write of size 4 at 0x7b040003edb0 by thread T18:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    vedgy#1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    vedgy#2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    vedgy#3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    vedgy#4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    vedgy#5 <null> <null> (libQt5Core.so.5+0xe8710)

dict.mddResources is shared and accessed concurrently by several
separate MddResourceRequest objects. The line
`sptr< IndexedMdd > mddResource = *i;` contains a data race, because
sptr is not thread-safe as sptr_base::count is not atomic.

MdxDictionary::mddResources and its elements are not modified after the
initialization in MdxDictionary::doDeferredInit(). Simply dereference
the pointer instead of copying a sptr element of mddResources to
eliminate the data race.

The crash reported in goldendict#1653 may be caused by this data race.

Apply the same fix to duplicate code in
MdxDictionary::getCachedFileName().
vedgy added a commit that referenced this pull request Jun 27, 2023
ThreadSanitizer reports a data race:
  Read of size 4 at 0x7b040003edb0 by thread T19:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    #1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    #2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    #3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    #4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    #5 <null> <null> (libQt5Core.so.5+0xe8710)

  Previous write of size 4 at 0x7b040003edb0 by thread T18:
    #0 sptr_base<Mdx::IndexedMdd>::increment() ../sptr.hh:23
    #1 sptr_base<Mdx::IndexedMdd>::sptr_base(sptr_base<Mdx::IndexedMdd> const&) ../sptr.hh:35
    #2 sptr<Mdx::IndexedMdd>::sptr(sptr<Mdx::IndexedMdd> const&) ../sptr.hh:93
    #3 Mdx::MddResourceRequest::run() ../mdx.cc:849
    #4 Mdx::MddResourceRequestRunnable::run() ../mdx.cc:791
    #5 <null> <null> (libQt5Core.so.5+0xe8710)

dict.mddResources is shared and accessed concurrently by several
separate MddResourceRequest objects. The line
`sptr< IndexedMdd > mddResource = *i;` contains a data race, because
sptr is not thread-safe as sptr_base::count is not atomic.

MdxDictionary::mddResources and its elements are not modified after the
initialization in MdxDictionary::doDeferredInit(). Simply dereference
the pointer instead of copying a sptr element of mddResources to
eliminate the data race.

The crash reported in goldendict#1653 may be caused by this data race.

Apply the same fix to duplicate code in
MdxDictionary::getCachedFileName().
vedgy added a commit that referenced this pull request Sep 30, 2023
Comparison operators for MdictParser::RecordIndex and qint64 use
RecordIndex's data members shadowStartPos and shadowEndPos. The binary
search in MdictParser::RecordIndex::bsearch() uses these comparison
operators to find a qint64 in an ordered RecordIndex collection.
The condition for a bsearch() in MdictParser::readRecordBlock() uses
RecordIndex's data member endPos in place of shadowEndPos by mistake.

Assignments a few lines below in readRecordBlock() confirm that this is
a mistake:
    RecordIndex const & recordIndex = recordBlockInfos_[idx];
...
      recordSize = recordIndex.shadowEndPos - i->first;
...
    recordInfo.recordSize = recordSize;
MdictParser::RecordInfo::recordSize must be non-negative because it is
used as a size. For example, in
IndexedMdd::loadFile(gd::wstring const &, std::vector<char> & result):
    result.resize( indexEntry.recordSize );
std::vector::resize() takes an unsigned size_type argument. Passing a
negative integer to it would attempt to resize to a huge size close to
the maximum value of std::vector::size_type.

There are two practical consequences of this mistake:
1. An affected image is never displayed in the article view.
2. GoldenDict occasionally crashes while loading an affected article.

I can consistently reproduce the crash by running a Debug build of
GoldenDict via GDB and loading the single article of the affected
dictionary attached to goldendict#1672. The backtrace:
    Thread 7 "Thread (pooled)" received signal SIGSEGV, Segmentation fault.
    #0  in  () at /usr/lib/libc.so.6
    #1  in Mdx::IndexedMdd::loadFile(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&, std::vector<char, std::allocator<char> >&)
        (this=0x55555639d5d0, name=L"\\OPEU4_0002.png", result=std::vector of length 1993913, capacity 1993913 = {...}) at ../mdx.cc:184
    #2  in Mdx::MddResourceRequest::run() (this=0x555556c15f90)
        at ../mdx.cc:849
    #3  in Mdx::MddResourceRequestRunnable::run()
        (this=0x555556c163d0) at ../mdx.cc:791
    #4  in  () at /usr/lib/libQt5Core.so.5
    #5  in  () at /usr/lib/libQt5Core.so.5
    #6  in  () at /usr/lib/libc.so.6
    goldendict#7  in  () at /usr/lib/libc.so.6

Correct the mistake in readRecordBlock() and increment the Mdict format
version, because the crash goes away only after reindexing an affected
dictionary.

qint64 RecordIndex::endPos is used only in
MdictParser::readRecordBlockInfos() now. So this data member can be
replaced with a function-local variable. Keep the unnecessary data
member in case it needs to be used elsewhere and because its counterpart
qint64 RecordIndex::startPos is still used in readRecordBlock().
All existing RecordInfo instances are stack variables, so removing
endPos wouldn't make noticeable performance impact.

Fixes goldendict#1672
vedgy added a commit that referenced this pull request Sep 30, 2023
Comparison operators for MdictParser::RecordIndex and qint64 use
RecordIndex's data members shadowStartPos and shadowEndPos. The binary
search in MdictParser::RecordIndex::bsearch() uses these comparison
operators to find a qint64 in an ordered RecordIndex collection.
The condition for a bsearch() in MdictParser::readRecordBlock() uses
RecordIndex's data member endPos in place of shadowEndPos by mistake.

Assignments a few lines below in readRecordBlock() confirm that this is
a mistake:
    RecordIndex const & recordIndex = recordBlockInfos_[idx];
...
      recordSize = recordIndex.shadowEndPos - i->first;
...
    recordInfo.recordSize = recordSize;
MdictParser::RecordInfo::recordSize must be non-negative because it is
used as a size. For example, in
IndexedMdd::loadFile(gd::wstring const &, std::vector<char> & result):
    result.resize( indexEntry.recordSize );
std::vector::resize() takes an unsigned size_type argument. Passing a
negative integer to it would attempt to resize to a huge size close to
the maximum value of std::vector::size_type.

There are two practical consequences of this mistake:
1. An affected image is never displayed in the article view.
2. GoldenDict occasionally crashes while loading an affected article.

I can consistently reproduce the crash by running a Debug build of
GoldenDict via GDB and loading the single article of the affected
dictionary attached to goldendict#1672. The backtrace:
    Thread 7 "Thread (pooled)" received signal SIGSEGV, Segmentation fault.
    #0  in  () at /usr/lib/libc.so.6
    #1  in Mdx::IndexedMdd::loadFile(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&, std::vector<char, std::allocator<char> >&)
        (this=0x55555639d5d0, name=L"\\OPEU4_0002.png", result=std::vector of length 1993913, capacity 1993913 = {...}) at ../mdx.cc:184
    #2  in Mdx::MddResourceRequest::run() (this=0x555556c15f90)
        at ../mdx.cc:849
    #3  in Mdx::MddResourceRequestRunnable::run()
        (this=0x555556c163d0) at ../mdx.cc:791
    #4  in  () at /usr/lib/libQt5Core.so.5
    #5  in  () at /usr/lib/libQt5Core.so.5
    #6  in  () at /usr/lib/libc.so.6
    goldendict#7  in  () at /usr/lib/libc.so.6

Correct the mistake in readRecordBlock() and increment the Mdict format
version, because the crash goes away only after reindexing an affected
dictionary.

Fixes goldendict#1672
vedgy added a commit that referenced this pull request Jan 8, 2024
Comparison operators for MdictParser::RecordIndex and qint64 use
RecordIndex's data members shadowStartPos and shadowEndPos. The binary
search in MdictParser::RecordIndex::bsearch() uses these comparison
operators to find a qint64 in an ordered RecordIndex collection.
The condition for a bsearch() in MdictParser::readRecordBlock() uses
RecordIndex's data member endPos in place of shadowEndPos by mistake.

Assignments a few lines below in readRecordBlock() confirm that this is
a mistake:
    RecordIndex const & recordIndex = recordBlockInfos_[idx];
...
      recordSize = recordIndex.shadowEndPos - i->first;
...
    recordInfo.recordSize = recordSize;
MdictParser::RecordInfo::recordSize must be non-negative because it is
used as a size. For example, in
IndexedMdd::loadFile(gd::wstring const &, std::vector<char> & result):
    result.resize( indexEntry.recordSize );
std::vector::resize() takes an unsigned size_type argument. Passing a
negative integer to it would attempt to resize to a huge size close to
the maximum value of std::vector::size_type.

There are two practical consequences of this mistake:
1. An affected image is never displayed in the article view.
2. GoldenDict occasionally crashes while loading an affected article.

I can consistently reproduce the crash by running a Debug build of
GoldenDict via GDB and loading the single article of the affected
dictionary attached to goldendict#1672. The backtrace:
    Thread 7 "Thread (pooled)" received signal SIGSEGV, Segmentation fault.
    #0  in  () at /usr/lib/libc.so.6
    #1  in Mdx::IndexedMdd::loadFile(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&, std::vector<char, std::allocator<char> >&)
        (this=0x55555639d5d0, name=L"\\OPEU4_0002.png", result=std::vector of length 1993913, capacity 1993913 = {...}) at ../mdx.cc:184
    #2  in Mdx::MddResourceRequest::run() (this=0x555556c15f90)
        at ../mdx.cc:849
    #3  in Mdx::MddResourceRequestRunnable::run()
        (this=0x555556c163d0) at ../mdx.cc:791
    #4  in  () at /usr/lib/libQt5Core.so.5
    #5  in  () at /usr/lib/libQt5Core.so.5
    #6  in  () at /usr/lib/libc.so.6
    goldendict#7  in  () at /usr/lib/libc.so.6

Correct the mistake in readRecordBlock() and increment the Mdict format
version, because the crash goes away only after reindexing an affected
dictionary.

Fixes goldendict#1672
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

Successfully merging this pull request may close these issues.

2 participants