View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006280 | The Dark Mod | Coding | public | 30.03.2023 18:41 | 07.04.2023 19:08 |
Reporter | stgatilov | Assigned To | stgatilov | ||
Priority | low | Severity | tweak | Reproducibility | N/A |
Status | resolved | Resolution | fixed | ||
Product Version | TDM 2.11 | ||||
Target Version | TDM 2.12 | Fixed in Version | TDM 2.12 | ||
Summary | 0006280: Default constructor and initialization | ||||
Description | The long-time weird thing is that idVec3 is the only type which zeros itself in default constructor. It is inconsistent, and it happened only because someone added zeroing in TDM team. Maybe time to revert back and try to fix all uninitialized issues =( Also, many custom types have empty default constructor. With C++11, it's better to mark then as "= default". | ||||
Tags | No tags attached. | ||||
As of now, I have not removed initialization from idVec3 on trunk, but I frequently remove it locally. Note that marking constructors as "= default" allows to find more unused variables: r10335 Removed trivial default constructors for all vectors and matrices except idVec3. r10336 Fixed unused/uninitialized variable warnings. r10338 Added some more initialization. r10339 Deleted ton of unused variables. r10340 Replaced many empty constructors with "= default" constructors. Also replaced some empty destructors with "= default". r10341 Deleted more unused variables. Some initialization fixes: r10342 Assert in idAASLocal::GetAreaBounds for file not loaded, and return zero box. r10343 Added more members initialization to game classes. The plan for future is to try ASAN and static analyzer (maybe even make a new configuration for them) to find more usages of uninitialized idVec3. |
|
Static analysis is total trash. Two options are available in MSVC: Microsoft Code Analysis and clang-tidy. I enabled "static analyzer" ruleset in clang-tidy, and it simple hanged. I waited for 1.5 hours, after which it did not finish, but printed many messages like "cannot process abcdef.cpp file". I stopped it after that: it is horribly slow and won't give proper results anyway. The Microsoft Code Analysis is somewhat better. I tried it with "Native code Minimum" ruleset. The first two runs ate all memory and sent my Windows into memory-related BSOD. I even checked that I really have swap file allocated =) Maybe that's some stability issue with my hardware... I guess I should check. Anyway, on third time I decided to pause it when memory usage came close to RAM size, then continue. After 3-5 chunks, I got the results =) There was insane number of messages, and 99.99% of them are just trash. One message relevant to initialization is "this member is not initialized in constructors". Damn, is this trivial kind of message the best that the modern static analysis can provide?! The second message is "possible uninitialized memory read". I looked at some such cases, and all of them were false positives like this: idMat matrix; matrix = ...; I suppose analyzer saw that operator= was called which accepts "matrix" by reference, and it assumes that the value is used inside (while it is not). So static analysis is trash in our case, not worth rewriting all the code to please these stupid garbage rules. |
|
ASAN is much better though! However, it does not catch uninitialized memory usages. In clang + LLVM, MSAN is used for this purpose, and it is only available on Linux =( MSVC only has "Address Sanitizer", which does not catch uninitialized memory usage according to docs. It catches read-after-poisoning, but happens only when we explicitly poison memory after deallocation (in custom allocators). Anyway, I added Sanitize configuration, based on Debug Fast: r10352 Added "Sanitize" configuration with Address Sanitizer turned on. It works pretty slowly: I have only about 10 FPS on Inn Business. Applying it to Release build does not make any difference in performance... ASAN detected a two real out-of-bounds reads and one false positive: r10350 Don't round up number of bytes when calling Alloc[Vertex|Index]. r10353 Fixing issues found by ASAN. The false positive was related to using const reference to prolong the lifetime of temporary... I managed to complete Inn Business (first night) with ASAN. |
|
Anyway, I removed initialization: r10355 Removed initialization from idVec3. r10356 Deleted all hacky idRaw optimizations. Let's hope it won't cause too much harm... |
|
Date Modified | Username | Field | Change |
---|---|---|---|
30.03.2023 18:41 | stgatilov | New Issue | |
30.03.2023 18:41 | stgatilov | Status | new => assigned |
30.03.2023 18:41 | stgatilov | Assigned To | => stgatilov |
02.04.2023 09:51 | stgatilov | Note Added: 0015987 | |
07.04.2023 18:58 | stgatilov | Note Added: 0015992 | |
07.04.2023 19:07 | stgatilov | Note Added: 0015993 | |
07.04.2023 19:08 | stgatilov | Note Edited: 0015993 | |
07.04.2023 19:08 | stgatilov | Note Added: 0015994 | |
07.04.2023 19:08 | stgatilov | Status | assigned => resolved |
07.04.2023 19:08 | stgatilov | Resolution | open => fixed |
07.04.2023 19:08 | stgatilov | Fixed in Version | => TDM 2.12 |