_CRT_SECURE_NO_DEPRECATE, _CRT_SECURE_NO_WARNINGS, _SCL_SECURE_NO_WARNINGS and
_CRT_NON_CONFORMING_SWPRINTFS all do one thing: hide the deprecation attributes
the CRT headers put on strcpy, sprintf, swprintf and friends. That warning is
C4996 for cl and -Wdeprecated-declarations for clang-cl, and cmake/Warnings.cmake
already suppresses both by name, per compiler, with a count next to it. Two
mechanisms for one warning, one of them invisible to anyone reading the warning
list.
_CRT_NON_CONFORMING_SWPRINTFS is the only one that could have done more, and it
does not: its macro redirect to the argument-count-free swprintf is guarded by
!defined __cplusplus (corecrt_wstdio.h), so in C++ the traditional overloads are
declared either way and only the deprecation text changes.
No codegen change. Recompiling sgp/video.cpp with and without the four defines
gives objects that differ in .debug$T alone, by the 128 bytes of the recorded
compiler command line; every other section is byte-identical. All four
applications build clean under clang-cl /W3 /WX.
The counts in cmake/Warnings.cmake for /wd4996 and -Wno-deprecated-declarations
were harvested with these defines in place, so both now understate the real
number. They are stale figures, not wrong suppressions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sgp/video.cpp guards _MT with #ifndef and then defines it 26 lines into the
file, long after every header that could read it, and both cl and clang-cl
already define _MT for the static runtime this project links. jascreens.cpp
defines _UNICODE after its last include.
The only effect on the binaries is 82 assert line numbers in video.cpp shifting
by four and one in jascreens.cpp shifting by one. No object file's section
sizes change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing was left in it but the include of profiler.h, and 132 translation units
were including it for that alone. Fifteen files were leaning on profiler.h to
drag in <set>, <vector> and <ostream> for them; those now include what they use.
This is the commit that moves line numbers. Removing an include line shifts
__LINE__ by one for everything below it, and __LINE__ is an immediate operand in
every Assert() and DebugMsg() call, so the four game executables differ from
their predecessors by roughly a thousand 32-bit constants each. Every one of
those is accounted for: each is a single immediate that moved by -1 where the
builddefines.h include went away, or +1 where a <set>/<vector> include was
added. Nothing else in .text, .rdata or .data moves, no object file's section
sizes change, and symbolize_crash and Ja2Export stay bit-identical.
The one non-immediate difference is that the 24 Editor translation units of the
non-editor apps stop emitting __Avx2WmemEnabledWeakValue, a 4-byte weak COMDAT
they only ever instantiated through profiler.h's <vector>. It is a UCRT weak
default that other translation units still provide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same reasoning as builddefines.h: BMP_RANDOM, CALLBACKTIMER, WINDOWED_MODE and
the three multiplayer switches are build configuration, and a header is the
wrong place to keep them. None of the six was ever toggled from source -- they
were unconditional #defines, or in WINDOWED_MODE's case keyed off _DEBUG.
WINDOWED_MODE now keys off the Debug configuration rather than _DEBUG. That is
the same thing in an ordinary Debug build, and fixes the asan Debug build,
which links the release CRT and so never saw _DEBUG at all.
All twelve executables stay bit-identical, this time with no differing bytes to
explain away at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These are compile-time build configuration, so they belong in CMakeLists.txt
where the build files can see and vary them, not in a header buried in the
source tree. An Opus 4.8 session lost a long hunt looking for one of them.
builddefines.h is left as an empty husk for now so that every translation unit
keeps its line numbering: __LINE__ is baked into each Assert() and DebugMsg()
call site, so dropping the include would move thousands of immediates and hide
any real code change in the noise. With the include left in place all twelve
executables (JA2, JA2MAPEDITOR, JA2UB, JA2UBMAPEDITOR, symbolize_crash and
Ja2Export, in Debug and RelWithDebInfo) come out bit-identical, once the two
timestamps and the CodeView GUID that lld-link rewrites on every link are
normalized away.
The commented-out block that also lived here is gone: JA2UB, JA2UBMAPS,
JA2EDITOR, JA2BETAVERSION, JA2TESTVERSION, DEBUG_ATTACKBUSY and JA113DEMO are
all names CMakeLists.txt already owns, and having them appear as dead #defines
in a header is what sends readers down the wrong path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EstimateActionPointCost adds the post-fence start-run flat at the
fence tile; real spend adds it on the next tile, with the diagonal
x1.4 - cursor read 1 AP low. Sum ActionPointCost direct instead, and
mark the fence landing non-running so the next tile re-charges
start-run itself: right tile, right multiplier.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The footstep-colour budget kept its own inline per-mode copy of the
movement-cost modifiers (walk/crawl/swat/run), the last duplicate of the
cost math left in PlotPath. Replace it with one EstimateActionPointCost
call per stance, so the reachability colours use the same per-tile cost
as the real spend, and drop the now-orphaned per-tile TerrainActionPoints
recompute and its dead locals (sTileCost, sMovementAPsCost,
sExtraCostStand).
Cosmetic-only: footprint colours now include the diagonal x1.4 the old
budget omitted, so they track the real reachable distance more closely.
The water->walk terrain override stays; it keeps the start-run charge
correct when a run path crosses water.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PlotPath carried its own inline copy of the per-tile movement-cost math,
which had drifted from ActionPointCost (the function the real per-step
movement and the AI path estimate both already use). That divergence is
what made the movement cursor mis-predict the real AP spend - e.g. the
start-run penalty getting the diagonal x1.4 in the real cost but not the
cursor.
Replace the inline switch (and its fence/start-run special-cases) with a
per-tile EstimateActionPointCost call, threading the simulated previous
tile mode so the one-time start-run charge lands exactly once. The cursor
estimate now equals what movement deducts, by construction.
Behavioural: player movement-cursor AP numbers (and path reachability
colouring that keys off the same total) now match the real spend. Real
per-step deduction and AI estimates are unchanged - they already used
ActionPointCost. The footstep-colour budget still has its own cost copy;
left for a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Forwards to the new ActionPointCost prev-mode overload so a path estimator
can pass its simulated prior tile mode. Existing 6-arg callers use the
soldier's live anim state and are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an ActionPointCost overload taking the previous tile's movement mode
explicitly, used to charge the one-time start-run penalty. The existing
4-arg overload forwards the soldier's live anim state, so real per-step
movement and every other caller are unchanged. This lets a path-cost
estimator - which does not move the soldier - supply its simulated prior
mode instead of reading a frozen live anim state, so the estimate and the
real deduction can share one cost function.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /libpath flags for the MSVC, UCRT, and Windows SDK x86 libraries
were only injected into CMAKE_EXE_LINKER_FLAGS_INIT, so linking a shared
library or module produced unresolved externals. Hoist the paths into a
local variable and feed it to the EXE, SHARED, and MODULE init flags.
Never bit because nothing in the tree builds a shared lib yet.
Select the clang-cl-asan CMake preset (RelWithDebInfo, clang-cl,
ADDRESS_SANITIZER=ON) to instrument first-party code with AddressSanitizer.
The wiring lives in cmake/AddressSanitizer.cmake; SANITIZERS.md tells how to
add the clang-cl tools, build, and read the report.
Details:
- Add the clang-cl-asan preset so the asan build is one selection in Visual
Studio, and a base for a CMakeUserPresets.json to inherit.
- Instrument first-party code only; the vendored libraries keep default flags.
- Use the release CRT and disable MSVC-STL container annotations, so
instrumented and un-instrumented TUs stay compatible.
- Pass /bigobj to the TUs asan inflates past the COFF section cap.
- Link the asan runtime for clang-cl (lld-link does not infer it).
- Stub Bink into the exe: retail binkw32.dll cannot load in an asan process
(its image base is the 32-bit shadow), so compile no-op exports instead.
- Route the asan report to gamedir/asan.report.<pid>, since every app is a
WIN32 GUI app with no console to receive the default stderr report.
- Opt functions with 32-bit inline __asm out of instrumentation with
cmake/asan-ignorelist.txt, one function at a time (asan reserves a register
the asm needs). The rest of each translation unit stays instrumented.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When no cardinal direction resolves, ubDirection stays 255 and the
tail feeds it to GetSectorMvtTimeForGroup, which reads
SectorInfo[...].ubTraversability[255] on a UINT8[5]. This runs in
release too, since that read is a plain call, not an assert. Bail out
instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gfEnteringMapScreenToEnterPreBattleInterface, gfAutomaticallyStartAutoResolve
and gfDelayAutoResolveStart are now file-static in PreBattle Interface.cpp
(the latter's definition moved here from Strategic Movement.cpp). External
access goes through accessors declared in the header; PBI.cpp touches the
statics directly.
Accessors exist only where an external caller needs one:
AutomaticallyStartAutoResolve() getter (read in Town Militia,
gamescreen, Player Command); SetAutomaticallyStartAutoResolve (set in
Creature Spreading, strategicmap); SetDelayAutoResolveStart (set in
Strategic Movement);
SetEnteringMapScreenToEnterPreBattleInterface (set in strategicmap).
gfEnteringMapScreen stays a raw cross-read into mapscreen state, not
PBI-owned. No behavior change: getter returns BOOLEAN.
Verify: grep the three flag names across the tree hits only PreBattle
Interface.cpp (three static defs, four accessor bodies, and in-file
Handle/reader access). No header externs, no external raw refs. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flag was initialized FALSE (mapscreen.cpp), read once in
HandlePreBattleInterfaceStates, and only ever set FALSE — never TRUE anywhere
in the tree. Its else-if branch was unreachable and its body did nothing but
re-clear the always-false flag.
Verify: grep 'gfTransitionMapscreenToAutoResolve' across the tree now hits
nothing. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gfPreBattleInterfaceActive and gfUsePersistantPBI are now file-static in
PreBattle Interface.cpp, reached only through Is/Set accessors declared in
the header. No behavior change: getters return BOOLEAN so ==TRUE/==FALSE
sites are untouched semantically.
Verify: grep 'gfPreBattleInterfaceActive\|gfUsePersistantPBI' across the tree
hits only the two static definitions, the four accessor bodies (all in
PreBattle Interface.cpp), and two commented-out Asserts in Strategic
Movement.cpp. Every other former site now calls an accessor. Build: JA2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* prepare gamedir for running the game
* Stop telling installers to overwrite the vanilla game
The release no longer contains anything that lands on a vanilla file:
its base data lives in Base, so unpacking it over the game directory
and copying the game's Data into the release now amount to the same
installation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Give the mod's base data its own VFS profile
Its files ship in Base now, so that a vanilla Data directory can be
copied in whole without a single collision. Mount Base right above the
vanilla dirs in every profile stack — the slot where the installer's
overwrites used to end up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Mount Base in the language overlays' VFS configs too
Assembling a release copies a <Language>_Version directory over
gamedir, its own copies of the configs included, so without the same
Base profile every non-English release would mount nothing from Base.
Their libraries move along with the rest of the base data: each
language slf, and the Russian data.slf that shadows the vanilla
Data.slf by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Ignore a vanilla Data directory dropped into gamedir
Running the game here needs one copied in, and it is an untouched copy
of somebody's retail install — nothing this repository should track.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Ignore only the player profile directory, not every Profiles
The unanchored pattern also swallowed Base/TableData/Profiles, the
mod's soldier profile XMLs, which are tracked and belong in a release.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* move gamedir/Data to gamedir/Base
* move gamedir-languages' Data to Base
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
sgp.cpp held two of these. One was a file-scope vfs::FileLogger* that was
never assigned, whose only other mention was a delete in shutdown that
could therefore never fire; it goes. The other is the adapter VFS actually
logs through, which was a bare new that nothing freed.
Make that one a function-local static rather than a scoped object. VFS
keeps the bare pointer and still logs from the shutdown that atexit runs
after WinMain has returned, so the adapter has to outlive the frame it is
declared in. Constructing it before InitializeStandardGamingPlatform
registers that handler is what puts its destructor after the handler
rather than before it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing Chromium-derived is left in either file - the stack tracer that
came from base/debug_util was the last of it - so the copyright header
credited Google for code it did not write and pointed at a LICENSE file
this repository does not have. Rename to what the files actually are.
While in there: say what the exception code's customer bit does and does
not guarantee, and note next to it how to stop a debugger breaking on
every assertion, which is what someone will be looking for when they grep
that number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ENABLE_STACK_TRACE has been 0 for as long as the file has been here, so
StackTrace's constructor captured nothing and every line it ever wrote to
stack_trace.log was a bare message with an empty frame list behind it. The
DbgHelp singleton underneath it resolved symbols for that empty list, and
the game linked dbghelp.lib to do it.
The crash reports cover what this was meant to cover, and the VFS errors
that were its only real content are already in vfs.log and game_log.log.
Drop the tracer, the log, and the dbghelp dependency.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_ExceptionMessage builds the full call stack of the exception it was given
and then calls _FailMessage with "",0,"", so every caught sgp:: or
vfs::Exception produces an identical, locationless report - the bucket
telemetry will see most of and can act on least. Fail with the innermost
frame instead, which is the frame that knows where it came from.
That feeds a runtime-built string to a _FailMessage that passed it to
sprintf as the format string, so copy it bounded instead. AssertMsg call
sites already build messages out of game state, and a %s in one of those
would read arguments that were never passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assertions ship in every configuration - Ja2/builddefines.h defines
FORCE_ASSERTS_ON unconditionally - so a player on a release build hits the
runtime error screen with a line and a file, and that is all anyone ever
gets. The crash handler that would have written a report never runs,
because an assertion faults nothing.
Raise a software exception from _FailMessage so it does. The code has the
customer bit set, the handler recognizes it, and the report carries the
assertion's line, file and message alongside the usual registers, module
table and frame chain, which symbolizes back to the assertion site. The
exception is swallowed again immediately: first-chance is all the handler
needs, and letting it travel further would kill a game that means to show
its error screen.
The dumper needed two adjustments to cope with an exception nobody faulted:
its one-report-per-address rule keys on the assertion's own file and line,
since every raise shares RaiseException's address, and the re-entry latch
is lifted in the __except, which is the only code that runs if writing a
report faults.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EvalLua had no callers anywhere in the tree — it converted a wide string
to UTF-8, ran it as a chunk and printf'd the error, which was the hook for
a developer console that is not wired up. It was the only user of stdio,
MemMan and windows.h in this file.
InitializeLua and ShutdownLua are live, called from InitOverhead and
ShutdownOverhead. The calls in InitializeGame and ShutdownGame were
commented out when they moved there; remove them, and the commented
ACCESSOR_TABLE block in InitializeLua that refers to a macro no longer
defined anywhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tree carried two prebuilt Lua static libraries and a copy of the Lua
public headers. lua51.lib is 5.1.2 and matched the headers; lua51.vc9.lib
is 5.1.4 and was dead weight — it came second in Ja2_Libraries, so the
linker resolved every Lua symbol out of lua51.lib and never pulled an
object from it. Just as well, since it asks for /DEFAULTLIB:MSVCRT while
we build /MT.
ext/lua-5.1.5 is the upstream tarball unmodified, built as lua51 the way
the other vendored libraries are, and its src directory replaces lua/ as
the home of lua.h, luaconf.h, lauxlib.h and lualib.h. Those four headers
were stock 5.1.2 retabbed, so 5.1.5 is bugfix-only against what the game
compiled against; the bytecode format is unchanged across 5.1.x and every
script under gamedir is plain source anyway. lua/lua.hpp had no includers
and returns to etc/ where upstream keeps it.
/SAFESEH:NO goes with it. Its two stated reasons were lua51.lib and the
smackw32 import library, and both are now gone: every remaining prebuilt
static library is SAFESEH-clean (libexpatMT.lib 5 of 5 members with
@feat.00 = 0x1, RakNetLibStatic.lib 79 of 79), and lld-link emits a
3155-entry SEHandlerTable without it.
The /MT comment blamed the wrong library. lua51.lib carried no linker
directives at all; RakNetLibStatic.lib is what pins us to the static
runtime, with /DEFAULTLIB:LIBCMT and /DEFAULTLIB:libcpmt.
Verified by building all four applications in Debug and Release, and by
linking the tarball's own lua.c against our lua51.lib with the build's
clang-cl flags and running it under Wine: 5.1 stdlib, GC, coroutines and
the x86 __asm fld/fistp lua_number2int fast path all behave. The game
itself could not be launched here — this checkout's gamedir has loose
Data directories but no SLF archives, so VFS aborts on Data\Ambient.slf
long before any script runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The assemble job only ran on Windows for the case-insensitive filesystem, which
the overlays no longer depend on. Checking out the ~93000 game data files is
most of what the job does, and ubuntu runners are quicker and cheaper at it.
Naming the executable has to be exact now: the artifact is named after the
matrix entry, which is lowercase, while the executable inside it is named after
the CMake target, which is the uppercased application. Only NTFS was making
those agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing stops the inconsistency the previous commit cleaned up from coming
straight back. The game reads its data through FileMan, which hands every path
to VFS, and VFS compares paths case-insensitively (vfs::Path::Less ->
vfs::String::less). A file added under Data/Mercedt/ therefore works perfectly
well beside Data/MercEdt/ right up until a release is assembled on a
case-sensitive filesystem and both of them survive into the archive, at which
point the game serves whichever the directory yields first.
Directories count as much as files: a translated-only file below Data/Mercedt/
collides with Data/MercEdt/ even when no file does, so each overlay is merged
onto gamedir the way a release does and every path prefix is compared.
Runs on pull requests only, and in its own workflow rather than in build.yml,
which release.yml also calls: a check has no business running again while a
release is assembled. It reads the git index rather than the files, so a
blobless sparse checkout is enough and no game data is fetched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The overlays were authored on Windows, where a filesystem that folds case hid
the fact that thousands of translated files spell their path differently from
the gamedir file they are meant to replace: Data-1.13/Mercedt/170.EDT over
Data-1.13/MercEdt/170.EDT, 149_ATTN.wav over 149_ATTN.WAV. Copying an overlay
onto gamedir on a case-sensitive filesystem leaves both files instead of
replacing one, and VFS, which folds case itself, then serves whichever the
directory happened to yield first.
Every overlay path is now spelled component for component the way gamedir
spells it. gamedir was already self-consistent, so nothing there moves and no
new convention is invented. The transformation is mechanical and no file gains
or loses content:
git diff --shortstat HEAD~1 HEAD
5374 files changed, 0 insertions(+), 0 deletions(-)
git diff --name-status -M HEAD~1 HEAD | cut -f1 | sort -u
R100
git ls-tree -r --name-only HEAD gamedir-languages | wc -l
28884, the same as before
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Smacker header declares a max audio chunk size, and libsmacker sized its
output buffer from it — then wrote each frame's audio trusting the frame's
own unpacked size, unchecked. The fan-localized intro videos (Chinese among
others) declare max_buffer=2304 but carry ~97KB audio frames: every decode
was a heap overflow, crashing the intro. The original SMACKW32.DLL played
these files, so treat the per-frame size as truth and grow the buffer,
bounded by a 16MB sanity cap; a chunk beyond that fails the frame as corrupt.
Covers both the raw-PCM and DPCM paths.
Verified with an ASan/UBSan harness over all 19 vanilla and Chinese intro
SMKs: previously all 8 Chinese files faulted, now all decode both passes
clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A "build local" report has no released PDB behind it — the telemetry sink
cannot symbolize it. Skip these when draining reports at startup: not sent,
not reaped by the 30-day cleanup, left on disk for the developer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
These triggered source's release workflow on gamedir pushes. In the
monorepo a push to master triggers it directly, and GitHub ignores
workflow files outside the repository root anyway.
gamedir and gamedir-languages now live in this repository, so the build
no longer clones them separately or pins their commits in versions.env:
one commit SHA identifies everything. The compile job sparse-checks-out
the source only, and the assemble job the game data only, both with
blob:none so neither downloads the other half. Dist names drop the
G....L.... gamedir suffixes since GAME_VERSION now covers the data too.
Full history of majcosta/gamedir-languages imported via git filter-repo
--to-subdirectory-filter. Blobs unreachable from its HEAD whose content
is binary (11847, NUL-byte sniff as git does it) were stripped; text
file history is kept in full. Working tree content is identical to
gamedir-languages HEAD; verify with:
git diff --stat gl/master HEAD -- gamedir-languages/
Full history of majcosta/gamedir imported via git filter-repo
--to-subdirectory-filter. Blobs unreachable from its HEAD whose content
is binary (37314, NUL-byte sniff as git does it) were stripped; text
file history is kept in full. Working tree content is identical to
gamedir HEAD; verify with:
git diff --stat gd/master HEAD -- gamedir/
Fix case of WinFont.h include in WinFont.cpp
Fix case of "Strategic Status.h" include in Queen Command.cpp
Fix case of "mapscreen.h" include in Strategic Merc Handler.cpp
Fix case of "Handle Items.h" include across Tactical/TileEngine/Utils
Mechanical: normalized every #include of handle items.h (any case) to
match the actual filename "Handle Items.h". Verify with:
grep -rn "handle items.h" -i --include=*.cpp --include=*.h . | grep -v "\"Handle Items.h\""
Fix case of "World Items.h" include across Tactical/Editor
Mechanical: normalized every #include of world items.h (any case) to
match the actual filename "World Items.h". Verify with:
grep -rn "world items.h" -i --include=*.cpp --include=*.h . | grep -v "\"World Items.h\""
Fix case of "Arms Dealer Init.h" include in Overhead.cpp
Fix case of "Meanwhile.h" include in TeamTurns.cpp
Fix case of "timer.h" include in Utils All.h and Event Pump.cpp
Fix case of "Store Inventory.h" include in XML_Items.cpp
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Frame pointers on: dbghelp's stack walker gets usable frames in crash
reports. /Oy- rather than -fno-omit-frame-pointer -- clang-cl rejects
the GNU spelling:("unknown argument ignored"), and cl.exe takes /Oy-
too. x86-only option; a 64-bit target would ignore it and rely on
unwind data instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OBJECTTYPE::exists() and SOLDIERTYPE::exists()/DeleteSoldier() are
called on null pointers by design and guard with `this == NULL`. That
is undefined behaviour, so clang infers `this` is non-null: it deletes
the guard inside the callee *and* deletes null checks that follow a
call in the caller, which is an access violation at /O2 in code MSVC
has always compiled the naive way. The caller-side inference happens
in every translation unit that calls one of these, so this has to be
global rather than per-file; there is no per-function attribute or
pragma for it. Drop it once nothing relies on a null `this`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The client would send up to 256 KB and the sink answered a settling 400 above
64 KB, so the two disagreed about what a valid report is, and the client
deletes what it is told is invalid. Unreachable in practice — the module table
and backtrace are both bounded, which puts the ceiling near 10 KB — but the
two constants have to agree for the disagreement not to matter.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The endpoint is public and unauthenticated, so the whole uploaded file is
attacker-chosen, not only the part the client copied from Ja2.ini. The build
field sat inside backticks a backtick closes, and the access-violation text
went in raw, so either could carry markdown or a link into the channel.
One clean(): printable ASCII minus what Discord reads as markup or a URL,
length-capped. It replaces the handle's own stripping and absorbs the .trim()
the field getters did, which also drops the CR that "(.+)$" captures off a
CRLF report.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
