{"id":382114,"date":"2024-06-29T04:04:39","date_gmt":"2024-06-29T04:04:39","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=382114"},"modified":"-0001-11-30T00:00:00","modified_gmt":"-0001-11-29T21:00:00","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=382114","title":{"rendered":"Is PHP compilable?! PVS-Studio searches for errors in PeachPie<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
\n

PHP is widely known as an interpreted programming language used mainly for website development. However, few people know that PHP also has a compiler to .NET \u2013 PeachPie. But how well is it made? Will the static analyzer be able to find actual bugs in this compiler? Let’s find out!<\/p>\n

<\/figcaption><\/figure>\n

It’s been a while since we posted articles on the C# projects check using PVS-Studio… And we still have to make the 2021 Top list of bugs (by the way, 2020 Top 10 bugs, you can find here<\/a>)! Well, we need to mend our ways. I am excited to show you a review of the PeachPie<\/a> check results.<\/p>\n

<\/figcaption><\/figure>\n

To begin with, let me tell you a little about the project. PeachPie is a modern, open-source PHP language compiler and runtime for the .NET Framework and .NET. It is built on top of the Microsoft Roslyn compiler platform and is based on the first-generation Phalanger<\/a> project. In July 2017, the project became a member of the .NET Foundation<\/a>. The source code is available in the GitHub repository<\/a>.<\/p>\n

By the way, our C# analyzer also makes extensive use of the Roslyn<\/a> capabilities, so in a way, PeachPie and PVS-Studio have something in common :). We’ve worked with Roslyn before. Moreover, we wrote a whole article<\/a> about the basics of working with this platform.<\/p>\n

To check PeachPie, we had to install the analyzer, open the project in Visual Studio or Rider and run the analysis using the PVS-Studio plugin. For more details, see the documentation<\/a>.<\/p>\n

It was entertaining to check such a large and serious project. I hope you’ll also enjoy my review of the bugs found in PeachPie. Have fun reading!<\/p>\n

WriteLine problems<\/h3>\n

Well, let’s start with an easy one \ud83d\ude42 Sometimes bugs can appear in the most unexpected and at the same time the simplest places. For example, an error may even appear in an easy WriteLine<\/em> function call:<\/p>\n

public static bool mail(....) {   \/\/ to and subject cannot contain newlines, replace with spaces   to = (to != null) ? to.Replace(\"\\r\\n\", \" \").Replace('\\n', ' ') : \"\";   subject = (subject != null) ? subject.Replace(\"\\r\\n\", \" \").Replace('\\n', ' ')                               : \"\";    Debug.WriteLine(\"MAILER\",                   \"mail('{0}','{1}','{2}','{3}')\",                   to,                   subject,                   message,                    additional_headers);    var config = ctx.Configuration.Core;      .... } <\/code><\/pre>\n
The V3025<\/a> warning: Incorrect format. A different number of format items is expected while calling ‘WriteLine’ function. Arguments not used: 1st, 2nd, 3rd, 4th, 5th. Mail.cs 25<\/p>\n
You would think, what has gone wrong? Everything seems to be fine. Wait a minute, though! What argument should pass the format?<\/p>\n
Well, let’s take a look at the Debug.WriteLine<\/em> declaration:<\/p>\n
public static void WriteLine(string format, params object[] args); <\/code><\/pre>\n
The format string should be passed as the first argument, and the first argument in the code is «MAILER»<\/em>. Obviously, the developer mixed up the methods and passed the arguments incorrectly.<\/p>\n
Same cases in switch<\/h3>\n
This section is devoted to warnings associated with performing the same actions in different case branches:<\/p>\n
private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(....) {   var result = FlowAnalysisAnnotations.None;    foreach (var attr in attributes)   {     switch (attr.AttributeType.FullName)     {       case \"System.Diagnostics.CodeAnalysis.AllowNullAttribute\":         result |= FlowAnalysisAnnotations.AllowNull;         break;       case \"System.Diagnostics.CodeAnalysis.DisallowNullAttribute\":         result |= FlowAnalysisAnnotations.DisallowNull;         break;       case \"System.Diagnostics.CodeAnalysis.MaybeNullAttribute\":         result |= FlowAnalysisAnnotations.MaybeNull;         break;       case \"System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute\":         if (TryGetBoolArgument(attr, out bool maybeNullWhen))         {           result |= maybeNullWhen ? FlowAnalysisAnnotations.MaybeNullWhenTrue                                   : FlowAnalysisAnnotations.MaybeNullWhenFalse;         }         break;       case \"System.Diagnostics.CodeAnalysis.NotNullAttribute\":         result |= FlowAnalysisAnnotations.AllowNull;         break;     }   } } <\/code><\/pre>\n
This fragment contains if not an error, then at least a strange thing. How quickly can you find it?<\/p>\n
However, don’t waste your time, the analyzer found everything for us:<\/p>\n
private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(....) {   var result = FlowAnalysisAnnotations.None;    foreach (var attr in attributes)   {     switch (attr.AttributeType.FullName)     {       case \"System.Diagnostics.CodeAnalysis.AllowNullAttribute\":         result |= FlowAnalysisAnnotations.AllowNull;         break;       ....       case \"System.Diagnostics.CodeAnalysis.NotNullAttribute\":         result |= FlowAnalysisAnnotations.AllowNull;              \/\/ <=         break;     }   } } <\/code><\/pre>\n
The V3139<\/a> warning: Two or more case-branches perform the same actions. ReflectionUtils.Nullability.cs 170<\/p>\n
Isn’t it strange that two different cases are handled the same way? In fact, no, this happens quite often. However, there are 2 peculiarities.<\/p>\n
Firstly, it’s worth noting that there is more graceful way to treat two different cases in the same way. You can rewrite the above fragment as follows:<\/p>\n
switch (attr.AttributeType.FullName) {   case \"System.Diagnostics.CodeAnalysis.AllowNullAttribute\":   case \"System.Diagnostics.CodeAnalysis.NotNullAttribute\":     result |= FlowAnalysisAnnotations.AllowNull;     break;   .... } <\/code><\/pre>\n
However, developers often neglect this convenient method and prefer copy-paste. Therefore, the presence of two identical branches doesn’t seem so terrible. The fact that the FlowAnalysisAnnotations<\/em> enumeration has, among others, the FlowAnalysisAnnotations.NotNull <\/em>value is much more suspicious. This value seems to be used when the «System.Diagnostics.CodeAnalysis.NotNullAttribute»<\/em> value is processed:<\/p>\n
switch (attr.AttributeType.FullName) {   case \"System.Diagnostics.CodeAnalysis.AllowNullAttribute\":     result |= FlowAnalysisAnnotations.AllowNull;     break;   ....   case \"System.Diagnostics.CodeAnalysis.NotNullAttribute\":     result |= FlowAnalysisAnnotations.NotNull;              \/\/ <=     break; } <\/code><\/pre>\n
Immutable DateTime<\/h3>\n
Developers often make mistakes<\/a> because they don’t understand how the features of the «modifying» methods work. Here is the bug found in PeachPie:<\/p>\n
using System_DateTime = System.DateTime;  internal static System_DateTime MakeDateTime(....) { .... }  public static long mktime(....) {   var zone = PhpTimeZone.GetCurrentTimeZone(ctx);   var local = MakeDateTime(hour, minute, second, month, day, year);    switch (daylightSaving)   {     case -1:       if (zone.IsDaylightSavingTime(local))         local.AddHours(-1);                   \/\/ <=       break;     case 0:       break;     case 1:       local.AddHours(-1);                     \/\/ <=       break;     default:       PhpException.ArgumentValueNotSupported(\"daylightSaving\", daylightSaving);       break;   }   return DateTimeUtils.UtcToUnixTimeStamp(TimeZoneInfo.ConvertTime(local,                                                                     ....)); } <\/code><\/pre>\n
The PVS-Studio warnings:<\/p>\n