Мы успешно создали и продолжаем развивать анализатор PVS-Studio для языка C/C++. Со временем стало понятно, что многие из реализованных диагностик никак не связаны с конкретным языком программирования, и тогда мы решили попробовать применить свой опыт к другому языку программирования, к C#. В данной статье будет рассказано о проверке проекта Code Contracts от Microsoft с помощью нового C# анализатора.
Code Contracts предоставляют метод, с помощью которого выражаются предположения о коде программы в .NET приложениях. Контракты принимают форму предусловий, постусловий и инвариантов объекта, и выступают в роли проверенной документации ваших внешних и внутренних API. Контракты используются для того, чтобы улучшить процесс тестирования с помощью проверки во время выполнения программы, позволяя проводить статическую верификацию контрактов и генерацию документации.
Это проект среднего размера (около ~4000 исходных файлов), который активно развивается: содержит много недописанного и некорректно написанного кода. Именно на этом этапе необходимо внедрять статический анализ в процесс разработки.
Анализатор в скором времени перестанет быть экспериментальным. Мы планируем выпустить первую релизную версию PVS-Studio с поддержкой С# уже 22.12.2015. При этом анализатор меняет старший номер версии на 6.0.
Ценовая политика не претерпит никаких изменений. Раньше PVS-Studio позволял проверять программы на языках C, C++, C++/CLI, C++/CX. Теперь к этому списку просто добавляется C#.
Подготавливая статью о проверке открытого проекта, мы приводим в ней информацию далеко не о всех предупреждениях, которые выдаёт статический анализатор. Поэтому мы рекомендуем авторам проекта самостоятельно выполнить анализ проекта и изучить все выдаваемые анализатором сообщения.
V3025 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 3. Present: 2. VSServiceProvider.cs 515
void AskToReportError(Exception exn) {
....
var emailBody = new StringBuilder();
emailBody.AppendLine("Hi Code Contracts user,");
emailBody.AppendLine();
....
emailBody.AppendLine(
String.Format(".... {0} {1} Visual Studio {2} Bug Report",
typeof(VSServiceProvider).Assembly.GetName().Version,
#if DEBUG
"Debug"
#else
"Release"
#endif
));
....
}Функция String.Format() ожидает 3 аргумента, а передано всего 2 аргумента. В этом случае возникает FormatException.
V3014 It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'i'. SparseArray.cs 1956
override public string ToString()
{
StringBuilder str = new StringBuilder();
for (int i = 0; i < data.Length; i++)
{
if (data[i] != null)
{
for (int j = 0; j < lastElement[i]; i++) // <=
{
str.AppendFormat("({0},{1})", data[i][j].Index,
data[i][j].Value);
}
}
}
return str.ToString();
}Во вложенном цикле не изменяется переменная-счётчик 'j', т.к. вместо 'j++' выполняется изменение счётчика внешнего цикла 'i++'.
Ещё несколько подобных мест:
V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 203, 207. WeakestPreconditionProver.csToSMT2.cs 203
private BoxedExpression DeclareVariable(....)
{
var tmp = original.ToString().Replace(' ', '_');
this.Info.AddDeclaration(string.Format("....", tmp, type));
this.ResultValue = tmp;
if (type == FLOAT32) // <=
{
types[original] = FloatType.Float32;
}
else if (type == FLOAT64) // <=
{
types[original] = FloatType.Float64;
}
return original;
}Анализатор обнаружил два одинаковых условных выражения, из-за чего операторы во втором условии никогда не получат управления. Хотя, на первый взгляд, это не так, мы перейдём к определению констант FLOAT32 и FLOAT64, и увидим следующий код:
private const string FLOAT32 = "(_ FP 11 53)"; // To change!!!
private const string FLOAT64 = "(_ FP 11 53)";Константы действительно равны! Хотя здесь и есть комментарий, что необходимо произвести замену значения константы FLOAT32, это место легко пропустить в будущем. В развивающихся проектах важно помечать такие места как TODO и регулярно просматривать результаты статического анализа кода.
V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1200, 1210. OutputPrettyCS.cs 1200
public enum TypeConstraint
{
NONE,
CLASS, // <=
STRUCT, // <=
BASECLASS,
}
public void Output(OutputHelper oh)
{
Contract.Requires(oh != null);
oh.Output("where ", false);
mParent.OutputName(oh);
oh.Output(" : ", false);
//** base class
bool comma = false;
if (mTypeConstraint == TypeConstraint.CLASS) // <=???
{
oh.Output("class", false);
comma = true;
}
else if (mTypeConstraint == TypeConstraint.STRUCT)
{
oh.Output("struct", false);
comma = true;
}
else if (mTypeConstraint == TypeConstraint.CLASS) // <=???
{
oh.Output(mClassConstraint, false);
comma = true;
}
}В этом фрагменте кода одинаковые условия более очевидны. Скорее всего, в одном из условий переменную 'mTypeConstraint' хотели сравнить с константой TypeConstraint.BASECLASS, вместо TypeConstraint.CLASS.
V3022 Expression 'c > '\xFFFF'' is always false. Output.cs 685
private static string Encode(string s)
{
....
foreach( char c in s ) {
if (c == splitC || c == '\n' || c == '\\') {
specialCount++;
}
else if (c > '\x7F') {
if (c > '\xFFFF') specialCount += 9;
else specialCount += 5;
}
}
....
}Выражение "c > '\xFFFF'" никогда не будет истинным и оператор "specialCount += 9" никогда не выполнится. Переменная 'c' имеет тип Char, максимальное значение которого и есть "\xFFFF'. Не очень понятно, как должен работать этот код и как его надо исправить. Возможно мы имеем дело с опечаткой или фрагментом кода, позаимствованным из приложения на другом языке. Например, в языке Си/Си++ иногда используются символы разрядностью 32-бита. И там как раз "играют" со значением 0xFFFF. Пример подобного кода:
/* putUTF8 -- write a character to stdout in UTF8 encoding */
static void putUTF8(long c)
{
if (c <= 0x7F) { /* Leave ASCII encoded */
printf("%ld;", c);
} else if (c <= 0x07FF) { /* 110xxxxx 10xxxxxx */
putchar(0xC0 | (c >> 6));
putchar(0x80 | (c & 0x3F));
} else if (c <= 0xFFFF) { /* 1110xxxx + 2 */
putchar(0xE0 | (c >> 12));
putchar(0x80 | ((c >> 6) & 0x3F));
putchar(0x80 | (c & 0x3F));
} else if (c <= 0x1FFFFF) { /* 11110xxx + 3 */
putchar(0xF0 | (c >> 18));
putchar(0x80 | ((c >> 12) & 0x3F));
putchar(0x80 | ((c >> 6) & 0x3F));
putchar(0x80 | (c & 0x3F));
} else if (c <= 0x3FFFFFF) { /* 111110xx + 4 */
putchar(0xF8 | (c >> 24));
putchar(0x80 | ((c >> 18) & 0x3F));
putchar(0x80 | ((c >> 12) & 0x3F));
putchar(0x80 | ((c >> 6) & 0x3F));
putchar(0x80 | (c & 0x3F));
} else if (c <= 0x7FFFFFFF) { /* 1111110x + 5 */
putchar(0xFC | (c >> 30));
putchar(0x80 | ((c >> 24) & 0x3F));
putchar(0x80 | ((c >> 18) & 0x3F));
putchar(0x80 | ((c >> 12) & 0x3F));
putchar(0x80 | ((c >> 6) & 0x3F));
putchar(0x80 | (c & 0x3F));
} else { /* Not a valid character... */
printf("%ld;", c);
}
}V3008 The 'this.InsideMonitor' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 751, 749. AssertionCrawlerAnalysis.cs 751
private Data(Data state, Variable v)
{
this.IsReached = state.IsReached;
this.InsideMonitor = state.InsideMonitor; // <=
this.symbols = new List<Variable>(state.symbols) { v };
this.InsideMonitor = false; // <=???
}Очень подозрительно, что некая функция изменяет состояние объекта с помощью значений, переданных через параметры функции и в последним момент перезаписывает значение поля "this.InsideMonitor" константой 'false'. Ранее уже было выполнено присваивание "this.InsideMonitor = state.InsideMonitor".
V3009 It's odd that this method always returns one and the same value of 'true'. LinearEqualities.cs 5262
public bool TryGetFirstAvailableDimension(out int dim)
{
for (var i = 0; i < map.Length; i++)
{
if (!map[i])
{
dim = i;
return true;
}
}
map.Length++;
dim = map.Length;
return true;
}Анализатор обнаружил функцию, которая всегда возвращает одно значение "true". Можно предположить, что при выполнении условия "!map[i]" функция должна вернуть одно значение, а если условие ни разу не было истинным, то вернуть другое значение. Возможно, здесь ошибка.
V3025 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 1. Present: 2. Output.cs 68
public override void WriteLine(string value)
{
output.WriteLine(string.Format("{1}", DateTime.Now,
value.Replace("{", "{{").Replace("}","}}")));
//output.WriteLine(string.Format("[{0}] {1}",
//DateTime.Now., value));
}Раньше функция String.Format() принимала и печатала 2 значения: дату и некоторое значение. Потом этот код закомментировали и написали другой вариант, в котором аргумент с индексом 0 не используется, т.е. дата теперь не печатается.
Другие примеры вызова функций форматирования с неиспользованными аргументами:
V3004 The 'then' statement is equivalent to the 'else' statement. Metadata.cs 2602
private void SerializeFieldRvaTable(....)
{
....
switch (row.TargetSection){
case PESection.SData:
case PESection.TLS:
Fixup fixup = new Fixup();
fixup.fixupLocation = writer.BaseStream.Position;
fixup.addressOfNextInstruction = row.RVA;
if (row.TargetSection == PESection.SData){
sdataFixup.nextFixUp = fixup; // <=
sdataFixup = fixup; // <=
}else{
sdataFixup.nextFixUp = fixup; // <=
sdataFixup = fixup; // <=
}
writer.Write((int)0);
break;
....
}Анализатор обнаружил идентичные блоки кода в условном операторе. Это может быть лишним кодом или один блок кода забыли изменить после копирования. Copy-Paste не щадит и C# программистов.
Весь список подобных мест:
V3001 There are identical sub-expressions 'semanticType.Name == null' to the left and to the right of the '||' operator. ContractsProvider.cs 694
public bool TryGetTypeReference(....)
{
....
if (semanticType.Name == null || semanticType.Name == null)
goto ReturnFalse;
cciType = new Microsoft.Cci.MutableCodeModel.NamespaceTypeR....
{
ContainingUnitNamespace = cciNamespace,
GenericParameterCount = (ushort) (....),
InternFactory = Host.InternFactory,
IsValueType = semanticType.IsValueType,
IsEnum = semanticType.TypeKind == TypeKind.Enum,
Name = Host.NameTable.GetNameFor(semanticType.Name),
TypeCode=CSharpToCCIHelper.GetPrimitiveTypeCode(semanticType)
};
goto ReturnTrue;'
....
}Условие "semanticType.Name == null" проверяют 2 раза. Либо проверка избыточна и её можно упростить, либо забыли проверить другое поле у объекта.
Ещё одно предупреждение на эту тему:
V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'other', 'right'. CallerInvariant.cs 189
public override Predicate JoinWith(Predicate other)
{
var right = other as PredicateNullness;
if (other != null)
{
if (this.value == right.value)
{
return this;
}
}
return PredicateTop.Value;
}Анализатор обнаружил потенциальную ошибку, которая может привести к доступу по нулевой ссылке. Необходимо сравнивать с 'null' результат выполнения оператора 'as'.
Если возникнет ситуация, когда объект 'other' не будет нулевым, но его не удастся привести к типу 'PredicateNullness', то возникнет доступ по нулевой ссылке при обращении к "right.value".
Таких сравнений в проекте найдено много, вот весь список:
V3030 Recurring check. The 'this.lineOffsets == null' condition was already verified in line 612. Nodes.cs 613
public virtual void InsertOrDeleteLines(....)
{
....
if (this.lineOffsets == null)
if (this.lineOffsets == null) this.ComputeLineOffsets();
if (lineCount < 0)
this.DeleteLines(offset, -lineCount);
else
this.InsertLines(offset, lineCount);
....
}Идущие подряд две одинаковые проверки "this.lineOffsets == null". Такой код не имеет смысла. Возможно, хотели проверить что-то ещё.
V3002 The switch statement does not cover all values of the 'UnaryOperator' enum: Conv_dec. WeakestPreconditionProver.csToSMT2.cs 453
private string Combine(UnaryOperator unaryOperator, string arg)
{
Contract.Requires(arg != null);
var format = "({0} {1})";
string op = null;
switch (unaryOperator)
{
case UnaryOperator.Neg:
case UnaryOperator.Not:
case UnaryOperator.Not:
{
op = "not";
}
break;
case UnaryOperator.WritableBytes:
case UnaryOperator.Conv_i:
case UnaryOperator.Conv_i1:
case UnaryOperator.Conv_i2:
case UnaryOperator.Conv_i4:
case UnaryOperator.Conv_i8:
case UnaryOperator.Conv_r_un:
case UnaryOperator.Conv_r4:
case UnaryOperator.Conv_r8:
case UnaryOperator.Conv_u:
case UnaryOperator.Conv_u1:
case UnaryOperator.Conv_u2:
case UnaryOperator.Conv_u4:
case UnaryOperator.Conv_u8:
{
return null;
}
}
return string.Format(format, op, arg);
}Анализатор обнаружил оператор 'switch', в котором выбор варианта осуществляется по переменной enum-типа. При этом в операторе 'switch' пропущен 1 элемент "UnaryOperator. Conv_dec". Это очень подозрительно.
Ниже приведено определение перечисления "UnaryOperator":
public enum UnaryOperator
{
....
Conv_u8,
Conv_r_un,
Neg,
Not,
WritableBytes,
Conv_dec, // <=
}Возможная ошибка заключается в том, что данная функция реализована таким образом, чтобы для значения "UnaryOperator.Not" возвращать форматированную строку, а в остальных случаях значение 'null'. Но так как элемент "UnaryOperator. Conv_dec" пропущен, то для него значение переменной 'op' останется равно 'null' и оно попадёт в форматированную строку, которую вернёт функция.
Надеемся вам понравилась эта статья. Обещаем и дальше радовать наших читателей проверками интересных открытых проектов.
Как уже упоминалось выше, первый релиз намечен на 22.12.2015. Как правило в конце года принимаются решения о будущих закупках. Поэтому, всем заинтересовавшимся, предлагаем, не откладывая уже сейчас связаться с нами по вопросам приобретения PVS-Studio. И мы будем рады видеть вашу компанию среди наших довольных клиентов.
Спасибо за внимание. И безбажного вам кода!