diff options
author | Patric Stout <truebrain@openttd.org> | 2020-12-04 22:12:39 +0100 |
---|---|---|
committer | Patric Stout <github@truebrain.nl> | 2020-12-06 16:10:58 +0100 |
commit | c98717cb4518d80ccccb2ae1677d0a394542f129 (patch) | |
tree | a1c585edd2b1fa672de3fb4e88be8f8efb08f6c1 | |
parent | c558936ec3223f95677e182da1325717f8163ef6 (diff) | |
download | openttd-c98717cb4518d80ccccb2ae1677d0a394542f129.tar.xz |
Fix: do not add an offset to a nullptr
This is, by specs, undefined behaviour. See
https://reviews.llvm.org/D67122
In cases where this is done, optimizations done by LLVM can
generate code that causes crashes.
GetVariableAddress() had two (legit) ways this could happen:
- For SaveLoad set to global
- For SaveLoad set to SLE_VAR_NULL, where sld->address is always
a nullptr, and object could or could not be a nullptr.
-rw-r--r-- | src/saveload/saveload.h | 13 |
1 files changed, 12 insertions, 1 deletions
diff --git a/src/saveload/saveload.h b/src/saveload/saveload.h index abd58ea04..3581a5594 100644 --- a/src/saveload/saveload.h +++ b/src/saveload/saveload.h @@ -873,7 +873,18 @@ static inline bool IsNumericType(VarType conv) */ static inline void *GetVariableAddress(const void *object, const SaveLoad *sld) { - return const_cast<byte *>((const byte*)(sld->global ? nullptr : object) + (ptrdiff_t)sld->address); + /* Entry is a global address. */ + if (sld->global) return sld->address; + + /* Entry is a null-variable, mostly used to read old savegames etc. */ + if (GetVarMemType(sld->conv) == SLE_VAR_NULL) { + assert(sld->address == nullptr); + return nullptr; + } + + /* Everything else should be a non-null pointer. */ + assert(object != nullptr); + return const_cast<byte *>((const byte *)object + (ptrdiff_t)sld->address); } int64 ReadValue(const void *ptr, VarType conv); |