PVS-Studio in 2021
Error Examples
󰐮 russian version
Resources released twice
Miranda NG
static INT_PTR ServiceCreateMergedFlagIcon(....)
{
HRGN hrgn;
....
if (hrgn!=NULL) {
SelectClipRgn(hdc,hrgn);
DeleteObject(hrgn);
....
DeleteObject(hrgn);
}
....
}
3
V586 The 'DeleteObject' function is called twice for deallocation of the same resource.
Unreachable code
Bouncy Castle
public void testSignSHA256CompleteEvenHeight2() {
....
int height = 10;
....
for (int i = 0; i < (1 << height); i++) {
byte[] signature = xmss.sign(new byte[1024]);
switch (i) {
case 0x005b:
assertEquals(signatures[0], Hex.toHexString(signature));
break;
case 0x0822:
assertEquals(signatures[1], Hex.toHexString(signature));
break;
....
}
}
}
V6019 Unreachable code detected. It is possible that an error is present.
5
Incorrect shift operations
V8 JavaScript Engine
U_CFUNC int32_t U_CALLCONV
ucol_calcSortKey(....)
{
....
if((caseBits & 0xC0) == 0) {
*(cases-1) |= 1 << (--caseShift);
} else {
*(cases-1) |= 0 << (--caseShift);
....
}
V684 A value of the variable '* (cases - 1)' is not modified. Consider inspecting the expression. It is possible that '1'
should be present instead of '0'. 7
Incorrect type handling
Qemu
static inline uint32_t extract32(uint32_t value, int start, int length);
....
static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
ARMMMUIdx mmu_idx)
{
....
bool epd, hpd;
....
hpd &= extract32(tcr, 6, 1);
}
V1046 Unsafe usage of the 'bool' and 'unsigned int' types together in the operation '&='.
9
Azure SDK for .NET
public static class Tag
{
....
[Flags]
public enum BlocksUsing
{
MonitorEnter,
MonitorWait,
ManualResetEvent,
AutoResetEvent,
....
OtherInternalPrimitive,
OtherFrameworkPrimitive,
OtherInterop,
Other,
NonBlocking,
}
....
}
V3121 An enumeration 'BlocksUsing' was declared with 'Flags' aribute, but does not set any
initializers to override default values. 10
Method / class works
not as intended
ClickHouse
int mainEntryClickhousePerformanceTest(int argc, char ** argv) {
std::vector<std::string> input_files;
....
for (const auto filename : input_files) {
FS::path file(filename);
if (!FS::exists(file))
throw DB::Exception(....);
if (FS::is_directory(file)) {
input_files.erase(
std::remove(input_files.begin(), input_files.end(), filename),
input_files.end() );
getFilesFromDir(file, input_files, recursive);
}
....
}
....
}
V789 Iterators for the 'input_files' container, used in the range-based for loop, become invalid upon
the call of the 'erase' function. 12
Accord.Net
public class DenavitHartenbergNodeCollection :
Collection<DenavitHartenbergNode>
{ .... }
[Serializable]
public class DenavitHartenbergNode
{
....
public DenavitHartenbergNodeCollection Children
{
get;
private set;
}
....
}
V3097 Possible exception: the 'DenavitHartenbergNode' type marked by [Serializable] contains non-serializable
members not marked by [NonSerialized]. 13
GitExtensions
public override bool Equals(object obj)
{
return GetHashCode() == obj.GetHashCode();
}
V3115 Passing 'null' to 'Equals(object obj)' method should not result in 'NullReferenceException'.
14
Typos and copy-pasted code
LibreOice
inline bool equalFont( Style const & style1, Style const & style2 ) {
....
return ( f1.Name == f2.Name &&
f1.Height == f2.Height &&
f1.Width == f2.Width &&
f1.StyleName == f2.StyleName &&
f1.Family == f2.Family &&
f1.CharSet == f2.CharSet &&
f1.Pitch == f2.CharSet &&
f1.CharacterWidth == f2.CharacterWidth &&
f1.Weight == f2.Weight &&
.... &&
bool(f1.Kerning) == bool(f2.Kerning) &&
bool(f1.WordLineMode) == bool(f2.WordLineMode) &&
f1.Type == f2.Type &&
style1._fontRelief == style2._fontRelief &&
style1._fontEmphasisMark == style2._fontEmphasisMark
);
}
V1013 Suspicious subexpression f1.Pitch == f2.CharSet in a sequence of similar comparisons.
16
TON
int compute_compare(const VarDescr& x, const VarDescr& y, int mode) {
switch (mode) {
case 1: // >
return x.always_greater(y) ? 1 : (x.always_leq(y) ? 2 : 3);
case 2: // =
return x.always_equal(y) ? 1 : (x.always_neq(y) ? 2 : 3);
case 3: // >=
return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3);
....
case 5: // <>
return x.always_neq(y) ? 1 : (x.always_equal(y) ? 2 : 3);
case 6: // >=
return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3);
case 7: // <=>
return .... ;
default:
return 7;
}
}
V1037 Two or more case-branches perform the same actions.
17
Azure PowerShell
public class HelpMessages
{
public const string SubscriptionId = "Subscription Id of the subscription
associated with the management";
public const string GroupId = "Management Group Id";
public const string Recurse = "Recursively list the children of the
management group";
public const string ParentId = "Parent Id of the management group";
public const string GroupName = "Management Group Id";
public const string DisplayName = "Display Name of the management group";
public const string Expand = "Expand the output to list the children of the
management group";
public const string Force = "Force the action and skip confirmations";
public const string InputObject = "Input Object from the Get call";
public const string ParentObject = "Parent Object";
}
V3091 It is possible that a typo is present inside the string literal: "Management Group Id"
.
The 'Id' word is suspicious. 18
RunUO
private bool m_IsRewardItem;
[CommandProperty( AccessLevel.GameMaster )]
public bool IsRewardItem
{
get{ return m_IsRewardItem; }
set{ m_IsRewardItem = value; InvalidateProperties(); }
}
private bool m_East;
[CommandProperty( AccessLevel.GameMaster )]
public bool East
{
get{ return m_East; }
set{ m_IsRewardItem = value; InvalidateProperties(); }
}
V3140 Property accessors use dierent backing fields.
19
Ghidra
final static Map<Character, String> DELIMITER_NAME_MAP = new HashMap<>(20);
// Any non-alphanumeric char can be used as a delimiter.
static {
DELIMITER_NAME_MAP.put(' ', "Space");
DELIMITER_NAME_MAP.put('~', "Tilde");
DELIMITER_NAME_MAP.put('`', "Back quote");
DELIMITER_NAME_MAP.put('@', "Exclamation point");
DELIMITER_NAME_MAP.put('@', "At sign");
DELIMITER_NAME_MAP.put('#', "Pound sign");
DELIMITER_NAME_MAP.put('$', "Dollar sign");
DELIMITER_NAME_MAP.put('%', "Percent sign");
....
}
V6033 An item with the same key '@' has already been added.
20
Security issues
Tor
int
crypto_pk_private_sign_digest(....)
{
char digest[DIGEST_LEN];
....
memset(digest, 0, sizeof(digest));
return r;
}
V597 The compiler could delete the 'memset' function call, which is used to flush 'digest' buer. The
RtlSecureZeroMemory() function should be used to erase the private data. 22
FreeRDP
BOOL certificate_data_replace(rdpCertificateStore* certificate_store,
rdpCertificateData* certificate_data)
{
HANDLE fp;
....
fp = CreateFileA(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0,
NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
....
if (size < 1) {
CloseHandle(fp);
return FALSE;
}
....
if (!data) {
fclose(fp);
return FALSE;
}
....
}
V1005 The resource was acquired using 'CreateFileA' function but was released using incompatible
'fclose' function. 23
.NET Core Libraries (CoreFX)
internal void SetSequence()
{
if (TypeDesc.IsRoot)
return;
StructMapping start = this;
// find first mapping that does not have the sequence set
while (!start.BaseMapping.IsSequence &&
start.BaseMapping != null &&
!start.BaseMapping.TypeDesc.IsRoot)
start = start.BaseMapping;
....
}
V3027 The variable 'start.BaseMapping' was utilized in the logical expression before it was
verified against null in the same logical expression. 24
Confusion with
operation precedence
Spvolren
void ppmWrite(char *filename, PPMFile *ppmFile)
{
....
FILE *fp;
if (! (fp = fopen(filename, "wb")) == -1) {
perror("opening image file failed");
exit(1);
}
....
}
V562 It’s odd to compare a bool type value with a value of -1: !(fp = fopen (filename, "wb")) == - 1.
26
Media Portal 2
return config.EpisodesLoaded || !checkEpisodesLoaded &&
config.BannersLoaded || !checkBannersLoaded &&
config.ActorsLoaded || !checkActorsLoaded;
V3130 Priority of the '&&' operator is higher than that of the '||' operator. Possible missing
parentheses. 27
How do we find
all this?
29
Data-flow analysis is used to evaluate limitations that are imposed on
variable values when processing various language constructs
Method annotations provide more information about the used methods
than one can obtain by analyzing only their signatures
Symbolic execution evaluates variables' values that can lead to errors,
checks of values' range
Type inference provides the analyzer with full information about all
variables and statements in the code
Paern-based analysis searches for fragments in the source code that
are similar to the known code paerns with an error
Interested?
Find out more on our website
🔗 More examples
🔗 All diagnostics list
🔗 More about the product
Feature overview

PVS-Studio in 2021 - Error Examples

  • 1.
    PVS-Studio in 2021 ErrorExamples 󰐮 russian version
  • 2.
  • 3.
    Miranda NG static INT_PTRServiceCreateMergedFlagIcon(....) { HRGN hrgn; .... if (hrgn!=NULL) { SelectClipRgn(hdc,hrgn); DeleteObject(hrgn); .... DeleteObject(hrgn); } .... } 3 V586 The 'DeleteObject' function is called twice for deallocation of the same resource.
  • 4.
  • 5.
    Bouncy Castle public voidtestSignSHA256CompleteEvenHeight2() { .... int height = 10; .... for (int i = 0; i < (1 << height); i++) { byte[] signature = xmss.sign(new byte[1024]); switch (i) { case 0x005b: assertEquals(signatures[0], Hex.toHexString(signature)); break; case 0x0822: assertEquals(signatures[1], Hex.toHexString(signature)); break; .... } } } V6019 Unreachable code detected. It is possible that an error is present. 5
  • 6.
  • 7.
    V8 JavaScript Engine U_CFUNCint32_t U_CALLCONV ucol_calcSortKey(....) { .... if((caseBits & 0xC0) == 0) { *(cases-1) |= 1 << (--caseShift); } else { *(cases-1) |= 0 << (--caseShift); .... } V684 A value of the variable '* (cases - 1)' is not modified. Consider inspecting the expression. It is possible that '1' should be present instead of '0'. 7
  • 8.
  • 9.
    Qemu static inline uint32_textract32(uint32_t value, int start, int length); .... static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va, ARMMMUIdx mmu_idx) { .... bool epd, hpd; .... hpd &= extract32(tcr, 6, 1); } V1046 Unsafe usage of the 'bool' and 'unsigned int' types together in the operation '&='. 9
  • 10.
    Azure SDK for.NET public static class Tag { .... [Flags] public enum BlocksUsing { MonitorEnter, MonitorWait, ManualResetEvent, AutoResetEvent, .... OtherInternalPrimitive, OtherFrameworkPrimitive, OtherInterop, Other, NonBlocking, } .... } V3121 An enumeration 'BlocksUsing' was declared with 'Flags' aribute, but does not set any initializers to override default values. 10
  • 11.
    Method / classworks not as intended
  • 12.
    ClickHouse int mainEntryClickhousePerformanceTest(int argc,char ** argv) { std::vector<std::string> input_files; .... for (const auto filename : input_files) { FS::path file(filename); if (!FS::exists(file)) throw DB::Exception(....); if (FS::is_directory(file)) { input_files.erase( std::remove(input_files.begin(), input_files.end(), filename), input_files.end() ); getFilesFromDir(file, input_files, recursive); } .... } .... } V789 Iterators for the 'input_files' container, used in the range-based for loop, become invalid upon the call of the 'erase' function. 12
  • 13.
    Accord.Net public class DenavitHartenbergNodeCollection: Collection<DenavitHartenbergNode> { .... } [Serializable] public class DenavitHartenbergNode { .... public DenavitHartenbergNodeCollection Children { get; private set; } .... } V3097 Possible exception: the 'DenavitHartenbergNode' type marked by [Serializable] contains non-serializable members not marked by [NonSerialized]. 13
  • 14.
    GitExtensions public override boolEquals(object obj) { return GetHashCode() == obj.GetHashCode(); } V3115 Passing 'null' to 'Equals(object obj)' method should not result in 'NullReferenceException'. 14
  • 15.
  • 16.
    LibreOice inline bool equalFont(Style const & style1, Style const & style2 ) { .... return ( f1.Name == f2.Name && f1.Height == f2.Height && f1.Width == f2.Width && f1.StyleName == f2.StyleName && f1.Family == f2.Family && f1.CharSet == f2.CharSet && f1.Pitch == f2.CharSet && f1.CharacterWidth == f2.CharacterWidth && f1.Weight == f2.Weight && .... && bool(f1.Kerning) == bool(f2.Kerning) && bool(f1.WordLineMode) == bool(f2.WordLineMode) && f1.Type == f2.Type && style1._fontRelief == style2._fontRelief && style1._fontEmphasisMark == style2._fontEmphasisMark ); } V1013 Suspicious subexpression f1.Pitch == f2.CharSet in a sequence of similar comparisons. 16
  • 17.
    TON int compute_compare(const VarDescr&x, const VarDescr& y, int mode) { switch (mode) { case 1: // > return x.always_greater(y) ? 1 : (x.always_leq(y) ? 2 : 3); case 2: // = return x.always_equal(y) ? 1 : (x.always_neq(y) ? 2 : 3); case 3: // >= return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3); .... case 5: // <> return x.always_neq(y) ? 1 : (x.always_equal(y) ? 2 : 3); case 6: // >= return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3); case 7: // <=> return .... ; default: return 7; } } V1037 Two or more case-branches perform the same actions. 17
  • 18.
    Azure PowerShell public classHelpMessages { public const string SubscriptionId = "Subscription Id of the subscription associated with the management"; public const string GroupId = "Management Group Id"; public const string Recurse = "Recursively list the children of the management group"; public const string ParentId = "Parent Id of the management group"; public const string GroupName = "Management Group Id"; public const string DisplayName = "Display Name of the management group"; public const string Expand = "Expand the output to list the children of the management group"; public const string Force = "Force the action and skip confirmations"; public const string InputObject = "Input Object from the Get call"; public const string ParentObject = "Parent Object"; } V3091 It is possible that a typo is present inside the string literal: "Management Group Id" . The 'Id' word is suspicious. 18
  • 19.
    RunUO private bool m_IsRewardItem; [CommandProperty(AccessLevel.GameMaster )] public bool IsRewardItem { get{ return m_IsRewardItem; } set{ m_IsRewardItem = value; InvalidateProperties(); } } private bool m_East; [CommandProperty( AccessLevel.GameMaster )] public bool East { get{ return m_East; } set{ m_IsRewardItem = value; InvalidateProperties(); } } V3140 Property accessors use dierent backing fields. 19
  • 20.
    Ghidra final static Map<Character,String> DELIMITER_NAME_MAP = new HashMap<>(20); // Any non-alphanumeric char can be used as a delimiter. static { DELIMITER_NAME_MAP.put(' ', "Space"); DELIMITER_NAME_MAP.put('~', "Tilde"); DELIMITER_NAME_MAP.put('`', "Back quote"); DELIMITER_NAME_MAP.put('@', "Exclamation point"); DELIMITER_NAME_MAP.put('@', "At sign"); DELIMITER_NAME_MAP.put('#', "Pound sign"); DELIMITER_NAME_MAP.put('$', "Dollar sign"); DELIMITER_NAME_MAP.put('%', "Percent sign"); .... } V6033 An item with the same key '@' has already been added. 20
  • 21.
  • 22.
    Tor int crypto_pk_private_sign_digest(....) { char digest[DIGEST_LEN]; .... memset(digest, 0,sizeof(digest)); return r; } V597 The compiler could delete the 'memset' function call, which is used to flush 'digest' buer. The RtlSecureZeroMemory() function should be used to erase the private data. 22
  • 23.
    FreeRDP BOOL certificate_data_replace(rdpCertificateStore* certificate_store, rdpCertificateData*certificate_data) { HANDLE fp; .... fp = CreateFileA(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); .... if (size < 1) { CloseHandle(fp); return FALSE; } .... if (!data) { fclose(fp); return FALSE; } .... } V1005 The resource was acquired using 'CreateFileA' function but was released using incompatible 'fclose' function. 23
  • 24.
    .NET Core Libraries(CoreFX) internal void SetSequence() { if (TypeDesc.IsRoot) return; StructMapping start = this; // find first mapping that does not have the sequence set while (!start.BaseMapping.IsSequence && start.BaseMapping != null && !start.BaseMapping.TypeDesc.IsRoot) start = start.BaseMapping; .... } V3027 The variable 'start.BaseMapping' was utilized in the logical expression before it was verified against null in the same logical expression. 24
  • 25.
  • 26.
    Spvolren void ppmWrite(char *filename,PPMFile *ppmFile) { .... FILE *fp; if (! (fp = fopen(filename, "wb")) == -1) { perror("opening image file failed"); exit(1); } .... } V562 It’s odd to compare a bool type value with a value of -1: !(fp = fopen (filename, "wb")) == - 1. 26
  • 27.
    Media Portal 2 returnconfig.EpisodesLoaded || !checkEpisodesLoaded && config.BannersLoaded || !checkBannersLoaded && config.ActorsLoaded || !checkActorsLoaded; V3130 Priority of the '&&' operator is higher than that of the '||' operator. Possible missing parentheses. 27
  • 28.
    How do wefind all this?
  • 29.
    29 Data-flow analysis isused to evaluate limitations that are imposed on variable values when processing various language constructs Method annotations provide more information about the used methods than one can obtain by analyzing only their signatures Symbolic execution evaluates variables' values that can lead to errors, checks of values' range Type inference provides the analyzer with full information about all variables and statements in the code Paern-based analysis searches for fragments in the source code that are similar to the known code paerns with an error
  • 30.
    Interested? Find out moreon our website 🔗 More examples 🔗 All diagnostics list 🔗 More about the product Feature overview