Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Freeing LanguagePack with wrong size. #8899

Merged
merged 1 commit into from Apr 2, 2021

Conversation

Milek7
Copy link
Contributor

@Milek7 Milek7 commented Mar 27, 2021

Motivation / Problem

AddressSanitizer complains:

==1707680==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x7f369345e800 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   168159 bytes;
  size of the deallocated type: 572 bytes.
    #0 0x7f369b8a9cc9 in operator delete(void*, unsigned long) /home/milek7/gcc-git/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x55e29b86ce1b in std::default_delete<LanguagePack>::operator()(LanguagePack*) const /usr/include/c++/11.0.1/bits/unique_ptr.h:85
    #2 0x55e29b86bda4 in std::unique_ptr<LanguagePack, std::default_delete<LanguagePack> >::~unique_ptr() /usr/include/c++/11.0.1/bits/unique_ptr.h:361
    #3 0x55e29b87161b in LoadedLanguagePack::~LoadedLanguagePack() (/home/milek7/ottd2/build/openttd+0xb62861b)
    #4 0x7f3698ce7696 in __run_exit_handlers (/usr/lib/libc.so.6+0x3f696)
    #5 0x7f3698ce783d in exit (/usr/lib/libc.so.6+0x3f83d)
    #6 0x7f3698ccfb2b in __libc_start_main (/usr/lib/libc.so.6+0x27b2b)
    #7 0x55e29aafc08d in _start (/home/milek7/ottd2/build/openttd+0xa8b308d)

0x7f369345e800 is located 0 bytes inside of 168159-byte region [0x7f369345e800,0x7f36934878df)
allocated by thread T0 here:
    #0 0x7f369b8a8b69 in operator new(unsigned long) /home/milek7/gcc-git/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x55e29b287305 in ReadFileToMem(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned long&, unsigned long) /home/milek7/ottd2/src/fileio.cpp:1274
    #2 0x55e29b867459 in ReadLanguagePack(LanguageMetadata const*) /home/milek7/ottd2/src/strings.cpp:1716
    #3 0x55e29b869444 in InitializeLanguagePacks() /home/milek7/ottd2/src/strings.cpp:1971
    #4 0x55e29b6052a8 in openttd_main(int, char**) /home/milek7/ottd2/src/openttd.cpp:723
    #5 0x55e29af5d2c4 in main /home/milek7/ottd2/src/os/unix/unix.cpp:265
    #6 0x7f3698ccfb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

SUMMARY: AddressSanitizer: new-delete-type-mismatch /home/milek7/gcc-git/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172 in operator delete(void*, unsigned long)

Description

Currently ReadFileToMem uses operator new directly to create object with given size. Afterwards it is reinterpreted to LanguagePack, and it is freed as such, but it is incorrect to free object with mismatched size. This makes AddressSanitizer complain.
This PR removes hackery from ReadFileToMem and makes it return regular char[]. On side of language pack code, custom deleter is used in unique_ptr to free reinterpreted pointer properly.

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')

@LordAro LordAro requested a review from michicc Mar 28, 2021
michicc
michicc approved these changes Apr 2, 2021
@michicc michicc merged commit 295f34a into OpenTTD:master Apr 2, 2021
11 checks passed
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.

None yet

2 participants