Instead of keeping track of a permanently empty `CSnapshot` object in client and server separately, add `CSnapshot::EmptySnapshot` to access a singleton empty `CSnapshot`.
Mark pointer parameters of snapshot functions as `const` when possible.
Cast `int`s to `unsigned` before subtracting to ensure that integer wrapping is being used instead of causing undefined behavior. Same as in `UndiffItem`.
```
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/master/src/base/math.h:16:40 in
src/master/src/engine/shared/snapshot.cpp:206:21: runtime error: signed integer overflow: 256 - -2147483648 cannot be represented in type 'int'
0 0x7650b7 in CSnapshotDelta::DiffItem(int const*, int const*, int*, int) src/master/src/engine/shared/snapshot.cpp:206:21
1 0x765cea in CSnapshotDelta::CreateDelta(CSnapshot*, CSnapshot*, void*) src/master/src/engine/shared/snapshot.cpp:323:7
2 0x51a0e2 in CServer::DoSnapshot() src/master/src/engine/server/server.cpp:964:36
3 0x537486 in CServer::Run() src/master/src/engine/server/server.cpp:2818:6
4 0x4feeb7 in main src/master/src/engine/server/main.cpp:190:21
5 0x7fc51ec27d09 in __libc_start_main csu/../csu/libc-start.c:308:16
6 0x4c3819 in _start (servers/DDNet-Server-ubsan+0x4c3819)
src/master/src/engine/shared/snapshot.cpp:206:21: runtime error: signed integer overflow: 1645289600 - -2139062144 cannot be represented in type 'int'
0 0x7650b7 in CSnapshotDelta::DiffItem(int const*, int const*, int*, int) src/master/src/engine/shared/snapshot.cpp:206:21
1 0x765cea in CSnapshotDelta::CreateDelta(CSnapshot*, CSnapshot*, void*) src/master/src/engine/shared/snapshot.cpp:323:7
2 0x51a0e2 in CServer::DoSnapshot() src/master/src/engine/server/server.cpp:964:36
3 0x537486 in CServer::Run() src/master/src/engine/server/server.cpp:2818:6
4 0x4feeb7 in main src/master/src/engine/server/main.cpp:190:21
5 0x7efd50c4ed09 in __libc_start_main csu/../csu/libc-start.c:308:16
6 0x4c3819 in _start (servers/DDNet-Server-ubsan+0x4c3819)
```
See #6650.
Use `bytes_be_to_uint` and `uint_to_bytes_be` instead.
As casting between `int` and `unsigned` preserves the bit representation of the value, it's not necessary to apply additional tricks to convert between `char` arrays and `int`.
As the integer overflow/underflow in `UndiffItem` can happen during normal gameplay, we should in this case neither ignore the snapshot delta nor show an error message.
Instead of depending on the particular compiler doing integer wrapping, when integer overflows or underflows occur, we make it part of the design, by casting to `unsigned`, which ensures that integer wrapping is being used.
Use a different error code for every return statement, so it's easier to determine why unpacking a delta failed.
The codes are grouped by the first digit of the error code:
- `-1xx`: not enough data to read
- `-2xx`: value is invalid
- `-3xx`: could not build snapshot item
```
src/engine/shared/snapshot.cpp:219:18: runtime error: signed integer overflow: -2011501152 + -1594687485 cannot be represented in type 'int'
0 0x5593cbc3534c in CSnapshotDelta::UndiffItem(int const*, int*, int*, int, int*) src/engine/shared/snapshot.cpp:219
1 0x5593cbc3852d in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:442
2 0x5593cbb881a6 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:624
3 0x5593cbb9a907 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1016
4 0x5593ca9f44da in CClient::Update() src/engine/client/client.cpp:2628
5 0x5593caa199dd in CClient::Run() src/engine/client/client.cpp:3220
6 0x5593caa970f3 in main src/engine/client/client.cpp:4717
7 0x7fe086d04d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
8 0x7fe086d04e3f in __libc_start_main_impl ../csu/libc-start.c:392
9 0x5593ca55e6f4 in _start (build-demofuzz/DDNet+0x24936f4)
```
```
src/engine/shared/snapshot.cpp:693:28: runtime error: left shift of negative value -1
0 0x55cae1608071 in CSnapshotBuilder::NewItem(int, int, int) src/engine/shared/snapshot.cpp:686
1 0x55cae1603fe0 in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:390
2 0x55cae15544c6 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:624
3 0x55cae1566c27 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1016
4 0x55cae03c07fa in CClient::Update() src/engine/client/client.cpp:2628
5 0x55cae03e5cfd in CClient::Run() src/engine/client/client.cpp:3220
6 0x55cae0463413 in main src/engine/client/client.cpp:4717
7 0x7fbc7d855d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
8 0x7fbc7d855e3f in __libc_start_main_impl ../csu/libc-start.c:392
9 0x55cadff2aa14 in _start (build-demofuzz/DDNet+0x2493a14)
```
According to cppcheck's `badBitmaskCheck` error:
```
src\engine\client\client.cpp:422:26: style: Operator '|' with one operand equal to zero is redundant. [badBitmaskCheck]
Packer.AddInt((0 << 1) | (pMsg->m_System ? 1 : 0)); // NETMSG_EX, NETMSGTYPE_EX
^
src\engine\shared\snapshot.cpp:40:45: style: Operator '|' with one operand equal to zero is redundant. [badBitmaskCheck]
int TypeItemIndex = GetItemIndex((0 << 16) | InternalType); // NETOBJTYPE_EX
^
src\engine\server\server.cpp:777:26: style: Operator '|' with one operand equal to zero is redundant. [badBitmaskCheck]
Packer.AddInt((0 << 1) | (pMsg->m_System ? 1 : 0)); // NETMSG_EX, NETMSGTYPE_EX
^
```
The previous hash function was heavily biased towards the hash buckets 64-79, making those buckets overflow faster, which results in snapshot CRC errors and lags.
Using the djb2 hash yields an almost even distribution over the entire range of values.
http://www.cse.yorku.ca/~oz/hash.html
Add `CSnapshot::IsValid` to check if a snapshot unpacked from a snapshot delta or demo is valid:
- ensure number of items and data size are not negative
- ensure that the actual size of the snapshot matches the size derived from its member variables
- ensure item offsets are within the valid range
- ensure item sizes are not negative
Add `CSnapshot::TotalSize` and `CSnapshot::OffsetSize` utility functions.
Minor improvements to related error messages.
Fixes buffer overflow:
```
==47744==ERROR: AddressSanitizer: global-buffer-overflow on address 0x558618e3767f at pc 0x558614b9bdfb bp 0x7ffe58a32cd0 sp 0x7ffe58a32cc0
READ of size 4 at 0x558618e3767f thread T0
0x558614b9bdfa in CSnapshotItem::Type() const src/engine/shared/snapshot.h:16
0x558615c3c911 in CSnapshot::GetItemType(int) const src/engine/shared/snapshot.cpp:29
0x558614aebaba in CClient::UnpackAndValidateSnapshot(CSnapshot*, CSnapshot*) src/engine/client/client.cpp:2264
0x558614af87cb in CClient::OnDemoPlayerSnapshot(void*, int) src/engine/client/client.cpp:2598
0x558615b9db1a in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:659
0x558615babd3f in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1007
0x558614afb08b in CClient::Update() src/engine/client/client.cpp:2686
0x558614b1d9eb in CClient::Run() src/engine/client/client.cpp:3296
0x558614b8e64f in main src/engine/client/client.cpp:4761
```
And fixes a buffer overflow that manifests itself as an internal ASan error:
```
=================================================================
==4755==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/asan/asan_descriptions.cc:79 "((0 && "Address is not in memory and not in shadow?")) != (0)" (0x0, 0x0)
0x7f0bf5f368be in AsanCheckFailed ../../../../src/libsanitizer/asan/asan_rtl.cc:72
0x7f0bf5f54eee in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cc:77
0x7f0bf5e4cb6f in GetShadowKind ../../../../src/libsanitizer/asan/asan_descriptions.cc:79
0x7f0bf5e4cb6f in __asan::GetShadowAddressInformation(unsigned long, __asan::ShadowAddressDescription*) ../../../../src/libsanitizer/asan/asan_descriptions.cc:95
0x7f0bf5e4cb6f in __asan::GetShadowAddressInformation(unsigned long, __asan::ShadowAddressDescription*) ../../../../src/libsanitizer/asan/asan_descriptions.cc:92
0x7f0bf5e4e386 in __asan::AddressDescription::AddressDescription(unsigned long, unsigned long, bool) ../../../../src/libsanitizer/asan/asan_descriptions.cc:440
0x7f0bf5e50e94 in __asan::ErrorGeneric::ErrorGeneric(unsigned int, unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long) ../../../../src/libsanitizer/asan/asan_errors.cc:380
0x7f0bf5f35f4d in __asan::ReportGenericError(unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) ../../../../src/libsanitizer/asan/asan_report.cc:460
0x7f0bf5e86f5e in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:762
0x558234873f1d in mem_zero src/base/system.cpp:213
0x55823481fc27 in CSnapshotBuilder::NewItem(int, int, int) src/engine/shared/snapshot.cpp:675
0x55823481be65 in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:380
0x558234776641 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:631
0x5582347861a9 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1007
0x5582336d4c7d in CClient::Update() src/engine/client/client.cpp:2695
0x5582336f75dd in CClient::Run() src/engine/client/client.cpp:3305
0x558233768241 in main src/engine/client/client.cpp:4770
```
src/engine/shared/snapshot.cpp:72:8: runtime error: signed integer overflow: -1297193910 + -1824658838 cannot be represented in type 'int'
#0 0x565165b0687d in CSnapshot::Crc() /media/ddnet/src/engine/shared/snapshot.cpp:72:8
#1 0x565165c4d20b in CClient::ProcessServerPacket(CNetChunk*) /media/ddnet/src/engine/client/client.cpp:1981:49
#2 0x565165c5d960 in CClient::PumpNetwork() /media/ddnet/src/engine/client/client.cpp:2589:6
#3 0x565165c67a71 in CClient::Update() /media/ddnet/src/engine/client/client.cpp:2856:2
#4 0x565165c72f4e in CClient::Run() /media/ddnet/src/engine/client/client.cpp:3237:4
#5 0x565165c94b7e in main /media/ddnet/src/engine/client/client.cpp:4341:11
#6 0x7fba5af2c151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
#7 0x5651659e1e0d in _start (/media/ddnet/DDNet+0x705e0d)
Purely automatic change. In case of conflict with this change, apply the
other change and rerun the formatting to restore it:
$ python scripts/fix_style.py
Replace `mem_free` by `free`, and `mem_alloc` by `malloc` or `calloc`
(the latter one being used to allocate a zeroed array of elements,
sometimes, this makes a call to `mem_zero` superfluous).
This results in having to remove `mem_stats` which previously recorded
the number of allocations and their size that the Teeworlds code did
directly.
Remove OOM handling in `src/game/client/components/binds.cpp`.
Remove needless copying in the CSV code in
`src/game/client/components/statboard.cpp`.
ChillerDragon
