PVS-Studio nei cloud:Azure DevOps

commenta

In altre parole, il progetto è piccolo, ma abbastanza sufficiente per dimostrare il lavoro di PVS-Studio insieme alla piattaforma cloud.

Iniziamo la configurazione

Per iniziare a lavorare in Azure DevOps seguiamo il link e premiamo su "Inizia gratis con GitHub".

Concedi all'applicazione Microsoft l'accesso ai dati dell'account GitHub.

Dovrai creare un account Microsoft per completare la registrazione.

Dopo la registrazione, crea un progetto:

Successivamente, dobbiamo passare a "Pipelines" - "Builds" e creare una nuova pipeline di build.

Quando ci viene chiesto dove si trova il nostro codice, risponderemo:GitHub.

Autorizza Azure Pipelines e scegli il repository con il progetto, per il quale configureremo l'esecuzione dell'analizzatore statico.

Nella finestra di selezione del modello, scegli "Progetto iniziale".

Possiamo eseguire l'analisi statica del codice del progetto in due modi:utilizzando agenti ospitati da Microsoft o self-hosted.

Innanzitutto, utilizzeremo agenti ospitati da Microsoft. Tali agenti sono normali macchine virtuali che si avviano quando eseguiamo la nostra pipeline. Vengono rimossi al termine dell'attività. L'utilizzo di tali agenti ci consente di non perdere tempo per il loro supporto e aggiornamento, ma impone alcune restrizioni, ad esempio l'impossibilità di installare software aggiuntivo utilizzato per costruire un progetto.

Sostituiamo la configurazione predefinita suggerita con quella seguente per l'utilizzo di agenti ospitati da Microsoft:

# Setting up run triggers
# Run only for changes in the master branch
trigger:
- master

# Since the installation of random software in virtual machines  
# is prohibited, we'll use a Docker container, 
# launched on a virtual machine with Windows Server 1803
pool:
  vmImage: 'win1803'
container: microsoft/dotnet-framework:4.7.2-sdk-windowsservercore-1803
           
steps:
# Download the analyzer distribution
- task: PowerShell@2
  inputs:
    targetType: 'inline'
    script: 'Invoke-WebRequest 
               -Uri https://files.pvs-studio.com/PVS-Studio_setup.exe 
               -OutFile PVS-Studio_setup.exe'
- task: CmdLine@2
  inputs:
    workingDirectory: $(System.DefaultWorkingDirectory)
    script: |
# Restore the project and download dependencies
      nuget restore .\ShareX.sln
# Create the directory, where files with analyzer reports will be saved
      md .\PVSTestResults
# Install the analyzer
      PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES 
/NORESTART /COMPONENTS=Core
#  Create the file with configuration and license information
         "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
          credentials 
    -u $(PVS_USERNAME) 
    -n $(PVS_KEY)

# Run the static analyzer and convert the report in html. 
    "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
         -t .\ShareX.sln  
         -o .\PVSTestResults\ShareX.plog
      "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" 
         -t html 
         -o .\PVSTestResults\ 
         .\PVSTestResults\ShareX.plog
    
# Save analyzer reports
- task: PublishBuildArtifacts@1
  inputs:
    pathToPublish: PVSTestResults
    artifactName: PVSTestResults

Nota: secondo la documentazione, il contenitore utilizzato deve essere memorizzato nella cache nell'immagine della macchina virtuale, ma al momento della stesura dell'articolo non funziona e il contenitore viene scaricato ad ogni avvio dell'attività, il che ha un impatto negativo sull'esecuzione tempistica.

Salviamo la pipeline e creiamo le variabili che verranno utilizzate per creare il file di licenza. Per fare ciò, apri la finestra di modifica della pipeline e fai clic su "Variabili" nell'angolo in alto a destra.

Quindi, aggiungi due variabili:PVS_USERNAME e PVS_KEY , contenente rispettivamente il nome utente e la chiave di licenza. Durante la creazione di PVS_KEY non dimenticare di selezionare "Mantieni segreto questo valore" per crittografare i valori della variabile con una chiave RSA a 2048 bit e per sopprimere l'output del valore della variabile nel registro delle prestazioni dell'attività.

Salva le variabili ed esegui la pipeline facendo clic su "Esegui".

La seconda opzione per eseguire l'analisi:utilizzare un agente self-hosted. Possiamo personalizzare e gestire noi stessi gli agenti self-hosted. Tali agenti offrono maggiori opportunità di installare il software, necessario per creare e testare il nostro prodotto software.

Prima di utilizzare tali agenti, è necessario configurarli in base alle istruzioni e installare e configurare l'analizzatore statico.

Per eseguire l'attività su un agente self-hosted, sostituiremo la configurazione suggerita con la seguente:

# Setting up triggers
# Run the analysis for master-branch
trigger:
- master

# The task is run on a self-hosted agent from the pool 'MyPool' 
pool: 'MyPool'

steps:
- task: CmdLine@2
  inputs:
    workingDirectory: $(System.DefaultWorkingDirectory)
    script: |
# Restore the project and download dependencies
      nuget restore .\ShareX.sln
# Create the directory where files with analyzer reports will be saved
      md .\PVSTestResults
# Run the static analyzer and convert the report in html. 
      "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
         -t .\ShareX.sln
         -o .\PVSTestResults\ShareX.plog
      "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe"
         -t html
         -o .\PVSTestResults\
         .\PVSTestResults\ShareX.plog
# Save analyzer reports
- task: PublishBuildArtifacts@1
  inputs:
    pathToPublish: PVSTestResults
    artifactName: PVSTestResults

Una volta completata l'attività, puoi scaricare l'archivio con i rapporti dell'analizzatore nella scheda "Riepilogo" oppure puoi utilizzare l'estensione Invia Mail che consente di configurare l'e-mail o prendere in considerazione un altro comodo strumento su Marketplace.

Risultati dell'analisi

Ora diamo un'occhiata ad alcuni bug trovati nel progetto testato, ShareX.

Controlli eccessivi

Per riscaldarci, iniziamo con semplici difetti nel codice, ovvero con controlli ridondanti:

private void PbThumbnail_MouseMove(object sender, MouseEventArgs e)
{
  ....
  IDataObject dataObject 
    = new DataObject(DataFormats.FileDrop,
                     new string[] { Task.Info.FilePath });

  if (dataObject != null)
  {
    Program.MainForm.AllowDrop = false;
    dragBoxFromMouseDown = Rectangle.Empty;
    pbThumbnail.DoDragDrop(dataObject, 
        DragDropEffects.Copy | DragDropEffects.Move);
    Program.MainForm.AllowDrop = true;
  }
  ....
}

Avviso di PVS-Studio: V3022 [CWE-571] L'espressione 'dataObject !=null' è sempre vera. TaskThumbnailPanel.cs 415

Prestiamo attenzione al controllo del dataObject variabile per null . Perché è qui? Oggetto dati non può essere nullo in questo caso, in quanto inizializzato da un riferimento su un oggetto creato. Di conseguenza, abbiamo un controllo eccessivo. Critico? No. Sembra succinto? No. È chiaramente meglio rimuovere questo controllo per non ingombrare il codice.

Diamo un'occhiata a un altro frammento di codice che possiamo commentare in modo simile:

private static Image GetDIBImage(MemoryStream ms)
{
  ....
  try
  {
    ....
    return new Bitmap(bmp);
    ....
  }
  finally
  {
    if (gcHandle != IntPtr.Zero)
    {
      GCHandle.FromIntPtr(gcHandle).Free();
    }
  }
  ....
}
private static Image GetImageAlternative()
{
  ....
  using (MemoryStream ms = dataObject.GetData(format) as MemoryStream)
  {
    if (ms != null)
    {
      try
      {
        Image img = GetDIBImage(ms);
        if (img != null)
        {
          return img;
        }
      }
      catch (Exception e)
      {
        DebugHelper.WriteException(e);
      }
    }
  }
  ....
}

Avviso di PVS-Studio: V3022 [CWE-571] L'espressione 'img !=null' è sempre vera. AppuntiHelpers.cs 289

In GetImageAlternative metodo, il img la variabile viene verificata che non sia nulla subito dopo una nuova istanza della Bitmap viene creata la classe. La differenza rispetto all'esempio precedente qui è che utilizziamo GetDIBImage invece del costruttore per inizializzare img variabile. L'autore del codice suggerisce che potrebbe verificarsi un'eccezione in questo metodo, ma dichiara solo i blocchi try e finalmente , omettendo cattura . Pertanto, se si verifica un'eccezione, il metodo chiamante GetImageAlternative non otterrà un riferimento a un oggetto della Bitmap type, ma dovrà gestire l'eccezione nel proprio catch bloccare. In questo caso, il img la variabile non verrà inizializzata e il thread di esecuzione non raggiungerà nemmeno img !=null controlla ma entrerà nel blocco catch. Di conseguenza, l'analizzatore ha indicato un controllo eccessivo.

Consideriamo il seguente esempio di avviso V3022:

private void btnCopyLink_Click(object sender, EventArgs e)
{
  ....
  if (lvClipboardFormats.SelectedItems.Count == 0)
  {
    url = lvClipboardFormats.Items[0].SubItems[1].Text;
  }
  else if (lvClipboardFormats.SelectedItems.Count > 0)
  {
    url = lvClipboardFormats.SelectedItems[0].SubItems[1].Text;
  }
  ....
}

Avviso di PVS-Studio: V3022 [CWE-571] L'espressione 'lvClipboardFormats.SelectedItems.Count> 0' è sempre vera. AfterUploadForm.cs 155

Diamo un'occhiata più da vicino alla seconda espressione condizionale. Lì controlliamo il valore del Count di sola lettura proprietà. Questa proprietà mostra il numero di elementi nell'istanza della raccolta SelectedItems . La condizione viene eseguita solo se il Count proprietà è maggiore di zero. Andrebbe tutto bene, ma nell'esterno se istruzione Conte è già verificato per 0. L'istanza di SelectedItems la raccolta non può avere un numero di elementi inferiore a zero, quindi Conte è uguale o maggiore di 0. Poiché abbiamo già eseguito il Conteggio controlla 0 nel primo se dichiarazione ed era falsa, non ha senso scrivere un altro Conte controlla se è maggiore di zero nel ramo else.

L'ultimo esempio di avviso V3022 sarà il seguente frammento di codice:

private void DrawCursorGraphics(Graphics g)
{
  ....
  int cursorOffsetX = 10, cursorOffsetY = 10, itemGap = 10, itemCount = 0;
  Size totalSize = Size.Empty;

  int magnifierPosition = 0;
  Bitmap magnifier = null;

  if (Options.ShowMagnifier)
  {
    if (itemCount > 0) totalSize.Height += itemGap;
    ....
  }
  ....
}

Avviso di PVS-Studio: V3022 L'espressione 'itemCount> 0' è sempre falsa. RegionCaptureForm.cs 1100

L'analizzatore ha notato che la condizione itemCount > 0 sarà sempre falso, come itemCount variabile viene dichiarata e allo stesso tempo assegnata zero sopra. Questa variabile non viene utilizzata da nessuna parte fino alla condizione stessa, quindi l'analizzatore aveva ragione sull'espressione condizionale, il cui valore è sempre falso.

Bene, ora diamo un'occhiata a qualcosa di veramente sapido.

Il modo migliore per comprendere un bug è visualizzare un bug

Ci sembra che in questo luogo sia stato trovato un errore piuttosto interessante:

public static void Pixelate(Bitmap bmp, int pixelSize)
{
  ....
  float r = 0, g = 0, b = 0, a = 0;
  float weightedCount = 0;

  for (int y2 = y; y2 < yLimit; y2++)
  {
    for (int x2 = x; x2 < xLimit; x2++)
    {
      ColorBgra color = unsafeBitmap.GetPixel(x2, y2);

      float pixelWeight = color.Alpha / 255;

      r += color.Red * pixelWeight;
      g += color.Green * pixelWeight;
      b += color.Blue * pixelWeight;
      a += color.Alpha * pixelWeight;

      weightedCount += pixelWeight;
    }
  }
  ....
  ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount),
    (byte)(g / weightedCount), (byte)(r / weightedCount),
    (byte)(a / pixelCount));
  ....
}

Non vorrei mostrare tutte le carte e rivelare cosa ha trovato il nostro analizzatore, quindi mettiamolo da parte per un po'.

Dal nome del metodo, è facile indovinare cosa sta facendo:gli dai un'immagine o un frammento di un'immagine e lo pixela. Il codice del metodo è piuttosto lungo, quindi non lo citeremo per intero, ma cercheremo solo di spiegare il suo algoritmo e spiegare che tipo di bug è riuscito a trovare PVS-Studio.

Questo metodo riceve due parametri:un oggetto della Bitmap tipo e il valore di int tipo che indica la dimensione della pixelizzazione. L'algoritmo di funzionamento è abbastanza semplice:

1) Dividere il frammento di immagine ricevuto in quadrati con il lato uguale alla dimensione della pixelizzazione. Ad esempio, se abbiamo la dimensione di pixel pari a 15, otterremo un quadrato, contenente 15x15=225 pixel.

2) Inoltre, attraversiamo ogni pixel in questo quadrato e accumuliamo i valori dei campi Rosso , Verde , Blu e Alfa nelle variabili intermedie, e prima moltiplicare il valore del colore corrispondente e del canale alfa per il pixelWeight variabile, ottenuta dividendo il Alpha valore di 255 (l'Alpha la variabile è del byte genere). Anche quando si attraversano i pixel si sommano i valori, scritti in pixelWeight nel conteggio ponderato variabile. Il frammento di codice che esegue le azioni precedenti è il seguente:

ColorBgra color = unsafeBitmap.GetPixel(x2, y2);

float pixelWeight = color.Alpha / 255;

r += color.Red * pixelWeight;
g += color.Green * pixelWeight;
b += color.Blue * pixelWeight;
a += color.Alpha * pixelWeight;

weightedCount += pixelWeight;

A proposito, nota che se il valore di Alpha la variabile è zero, pixelWeight non verrà aggiunto al conteggio ponderato variabile qualsiasi valore per questo pixel. Ne avremo bisogno in futuro.

3) Dopo aver attraversato tutti i pixel nel quadrato corrente, possiamo creare un colore "medio" comune per questo quadrato. Il codice che esegue questa operazione è il seguente:

ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount),
    (byte)(g / weightedCount), (byte)(r / weightedCount),
    (byte)(a / pixelCount));

4) Ora quando abbiamo ottenuto il colore finale e l'abbiamo scritto in averageColor variabile, possiamo nuovamente attraversare ogni pixel del quadrato e assegnargli un valore da averageColor .

5) Torna al punto 2 mentre abbiamo quadrati non gestiti.

Ancora una volta, il weightedCount variabile non è uguale al numero di tutti i pixel in un quadrato. Ad esempio, se un'immagine contiene un pixel completamente trasparente (valore zero nel canale alfa), il pixelWeight la variabile sarà zero per questo pixel (0 / 255 =0). Pertanto, questo pixel non influirà sulla formazione del conteggio ponderato variabile. È abbastanza logico:non ha senso prendere in considerazione i colori di un pixel completamente trasparente.

Quindi sembra tutto ragionevole:la pixelizzazione deve funzionare correttamente. E in effetti lo fa. Questo non è solo per le immagini png che includono pixel con valori nel canale alfa inferiori a 255 e diversi da zero. Nota l'immagine pixelata qui sotto:

Hai visto la pixelizzazione? Nemmeno noi. Ok, ora riveliamo questo piccolo intrigo e spieghiamo dove si nasconde esattamente il bug in questo metodo. L'errore si è insinuato nella riga di pixelWeight calcolo delle variabili:

float pixelWeight = color.Alpha / 255;

Il fatto è che quando si dichiara il pixelWeight variabile come flottante , l'autore del codice lo ha insinuato durante la divisione di Alpha campo per 255, otterrà numeri frazionari oltre a zero e uno. È qui che si nasconde il problema, come l'Alpha la variabile è del byte genere. Quando lo immergiamo di 255, otteniamo un valore intero. Solo dopo verrà eseguito il cast implicito nel float tipo, il che significa che la parte frazionaria viene persa.

È facile spiegare perché è impossibile pixelare le immagini png con una certa trasparenza. Poiché per questi pixel i valori del canale alfa sono compresi nell'intervallo 0 Alpha variabile divisa per 255 risulterà sempre in 0. Pertanto, i valori delle variabili pixelWeight , r , g , b , a , conteggio ponderato sarà anche sempre 0. Di conseguenza, il nostro averageColor sarà con valori zero in tutti i canali:rosso - 0, blu - 0, verde - 0, alfa - 0. Dipingendo un quadrato con questo colore, non cambiamo il colore originale dei pixel, come il averageColor è assolutamente trasparente. Per correggere questo errore, dobbiamo solo trasmettere in modo esplicito Alpha campo al virgola mobile genere. La versione fissa della riga di codice potrebbe essere simile a questa:

float pixelWeight = (float)color.Alpha / 255;

Bene, è giunto il momento di citare il messaggio di PVS-Studio per il codice errato:

Avviso di PVS-Studio: V3041 [CWE-682] L'espressione è stata convertita in modo implicito dal tipo 'int' al tipo 'float'. Considerare l'utilizzo di un cast di tipo esplicito per evitare la perdita di una parte frazionaria. Un esempio:doppia A =(doppia)(X) / Y;. ImageHelpers.cs 1119

Per confronto, citiamo lo screenshot di un'immagine veramente pixelata, ottenuta sulla versione corretta dell'applicazione:

Potenziale NullReferenceException

public static bool AddMetadata(Image img, int id, string text)
{
  ....
  pi.Value = bytesText;

  if (pi != null)
  {
    img.SetPropertyItem(pi);
    return true;
  }
  ....
}

Avviso di PVS-Studio: V3095 [CWE-476] L'oggetto 'pi' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:801, 803. ImageHelpers.cs 801

Questo frammento di codice mostra che l'autore si aspettava che il pi la variabile può essere null , ecco perché prima di chiamare il metodo SetPropertyItem , il controllo pi !=null ha luogo. È strano che prima di questo controllo alla proprietà venga assegnato un array di byte, perché if pi è nullo , un'eccezione di NullReferenceException verrà generato il tipo.

Una situazione simile è stata notata in un altro luogo:

private static void Task_TaskCompleted(WorkerTask task)
{
  ....
  task.KeepImage = false;

  if (task != null)
  {
    if (task.RequestSettingUpdate)
    {
      Program.MainForm.UpdateCheckStates();
    }
    ....
  }
  ....
}

Avviso di PVS-Studio: V3095 [CWE-476] L'oggetto 'attività' è stato utilizzato prima che fosse verificato rispetto a null. Righe di controllo:268, 270. TaskManager.cs 268

PVS-Studio ha riscontrato un altro errore simile. Il punto è lo stesso, quindi non c'è bisogno di citare il frammento di codice, sarà sufficiente il messaggio dell'analizzatore.

Avviso di PVS-Studio: V3095 [CWE-476] L'oggetto 'Config.PhotobucketAccountInfo' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:216, 219. UploadersConfigForm.cs 216

Lo stesso valore di ritorno

È stato trovato un frammento di codice sospetto in EvalWindows metodo di WindowsList class, che restituisce true in tutti i casi:

public class WindowsList
{
  public List<IntPtr> IgnoreWindows { get; set; }
  ....
  public WindowsList()
  {
    IgnoreWindows = new List<IntPtr>();
  }

  public WindowsList(IntPtr ignoreWindow) : this()
  {
    IgnoreWindows.Add(ignoreWindow);
  }
  ....
  private bool EvalWindows(IntPtr hWnd, IntPtr lParam)
  {
    if (IgnoreWindows.Any(window => hWnd == window))
    {
      return true;  // <=
    }

    windows.Add(new WindowInfo(hWnd));

    return true;  // <=
  }
}

Avviso di PVS-Studio: V3009 È strano che questo metodo restituisca sempre lo stesso valore di 'true'. WindowsList.cs 82

Sembra logico che se nell'elenco denominato IgnoraWindows c'è un puntatore con lo stesso nome di hWnd , il metodo deve restituire false .

Ignora Windows l'elenco può essere compilato quando si chiama il costruttore WindowsList(IntPtr ignora Finestra) o direttamente accedendo alla proprietà in quanto pubblica. Ad ogni modo, secondo Visual Studio, al momento nel codice questo elenco non è compilato. Questo è un altro posto strano di questo metodo.

Nota. Dopo aver parlato con uno degli sviluppatori di ShareX, abbiamo scoperto che il metodo EvalWindows che restituisce sempre il valore true è stato scritto intenzionalmente in questo modo.

Chiamata non sicura di gestori di eventi

protected void OnNewsLoaded()
{
  if (NewsLoaded != null)
  {
    NewsLoaded(this, EventArgs.Empty);
  }
}

Avviso di PVS-Studio: V3083 [CWE-367] Invocazione non sicura dell'evento 'NewsLoaded', NullReferenceException è possibile. Prendere in considerazione l'assegnazione di un evento a una variabile locale prima di richiamarla. NewsListControl.cs 111

Qui potrebbe verificarsi un caso molto brutto. Dopo aver controllato il NewsLoaded variabile per null, il metodo, che gestisce un evento, può essere annullato, ad esempio, in un altro thread. In questo caso, quando entriamo nel corpo dell'istruzione if, la variabile NewsLoaded sarà già nullo. Un NullReferenceException potrebbe verificarsi quando si tenta di chiamare abbonati dall'evento NewsLoaded , che è nullo. È molto più sicuro utilizzare un operatore condizionale nullo e riscrivere il codice sopra come segue:

protected void OnNewsLoaded()
{
  NewsLoaded?.Invoke(this, EventArgs.Empty);
}

L'analizzatore ha indicato 68 frammenti simili. Non li descriveremo tutti:hanno tutti uno schema di chiamata simile.

Restituisci null da ToString

Recentemente ho scoperto da un articolo interessante del mio collega che Microsoft sconsiglia di restituire null dal metodo sovrascritto ToString . PVS-Studio lo sa bene:

public override string ToString()
{
  lock (loggerLock)
  {
    if (sbMessages != null && sbMessages.Length > 0)
    {
      return sbMessages.ToString();
    }

    return null;
   }
 }

Avviso di PVS-Studio: V3108 Non è consigliabile restituire 'null' dal metodo 'ToSting()'. Logger.cs 167

Perché assegnato se non utilizzato?

public SeafileCheckAccInfoResponse GetAccountInfo()
{
  string url = URLHelpers.FixPrefix(APIURL);
  url = URLHelpers.CombineURL(APIURL, "account/info/?format=json");
....
}

Avviso di PVS-Studio: V3008 Alla variabile 'url' vengono assegnati valori due volte di seguito. Forse questo è un errore. Righe di controllo:197, 196. Seafile.cs 197

Come possiamo vedere dall'esempio, quando si dichiara l'url variabile, viene assegnato un valore, restituito dal metodo FixPrefix . Nella riga successiva, cancelliamo il valore ottenuto anche senza usarlo da nessuna parte. Otteniamo qualcosa di simile al codice morto:funziona, ma non influisce sul risultato. Molto probabilmente, questo errore è il risultato di un copia-incolla, poiché tali frammenti di codice si verificano in altri 9 metodi. Ad esempio, citeremo due metodi con una prima riga simile:

public bool CheckAuthToken()
{
  string url = URLHelpers.FixPrefix(APIURL);
  url = URLHelpers.CombineURL(APIURL, "auth/ping/?format=json");
  ....
}
....
public bool CheckAPIURL()
{
  string url = URLHelpers.FixPrefix(APIURL);
  url = URLHelpers.CombineURL(APIURL, "ping/?format=json");
  ....
}

Conclusioni

Come possiamo vedere, la complessità della configurazione dei controlli automatici dell'analizzatore non dipende da un sistema CI scelto. Ci sono voluti letteralmente 15 minuti e diversi clic del mouse per configurare il controllo del codice del nostro progetto con un analizzatore statico.

In conclusione, vi invitiamo a scaricare e provare l'analizzatore sui vostri progetti.