Repository navigation
Uninitialized variable replace usage in charmapencode_output (Objects/unicodeobject.c) discovered by scan-build #146429
Description
Activity
Static analysis with scan-build points to a potential use of an uninitialized variable in Objects/unicodeobject.c
What is scan-build? Is it a static analyzer?
If charmapencode_lookup returns a valid PyLong object but fails to update the reference to replace (or purely from a static analysis perspective), this leads to reading uninitialized stack memory.
I don't understand the "fails to update the reference to replace" part.
If charmapencode_lookup() returns a Python int (if
PyLong_Check()is true), replace is initialized and so it's safe to use it.I don't see any code path where replace is used whereas it's not initialized. It looks like a false-alarm of a static analyzer to me.
@vstinner
scan-buildis the standard command-line utility for the Clang Static Analyzer (LLVM project). It essentially runsclang --analyze.Even if
charmapencode_lookupguarantees writing toreplace(making this a false positive for now), the compiler cannot verify this invariant. This leaves the code fragile:- It triggers warnings in standard static analysis tools.
- Future changes to
charmapencode_lookupcould silently break this implicit contract, introducing UB.
Zero-cost initialization (
unsigned char replace = 0;) fixes the warning and future-proofs the code.Zero-cost initialization (unsigned char replace = 0;)
I'm not sure that it has a zero-cost. We don't have to fix all false-alarms triggered by the various static analyzers.
cc @serhiy-storchaka who modified
charmapencode_lookup()recently.I suggest closing this issue as "won't fix".
Reacted by Serhiy StorchakaI agree with @vstinner. This is a problem of the static analyzing tool.
The code is not simple, and I do not see a way to rewrite it in significantly simpler way. An attempt to rewrite is the event that can introduce a bug. Adding initialization to silence the static analyzer complain can hide future bugs.
Reacted by Shamil and Victor Stinner
Bug report
Bug description:
Static analysis with
scan-buildpoints to a potential use of an uninitialized variable inObjects/unicodeobject.c, specifically within thecharmapencode_outputfunction.The variable
unsigned char replaceis declared on the stack without initialization. It is passed by reference tocharmapencode_lookup(&replace). Later, inside theif (PyLong_Check(rep))block,replaceis cast and assigned to the output buffer:If
charmapencode_lookupreturns a validPyLongobject but fails to update the reference toreplace(or purely from a static analysis perspective), this leads to reading uninitialized stack memory.Suggested Fix:
Either initialize
replaceupon declaration or retrieve the value directly from therepobject, which is guaranteed to be aPyLongin that scope:CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux