Unity3D è uno dei motori di gioco più promettenti e in rapido sviluppo fino ad oggi. Di tanto in tanto, gli sviluppatori caricano nuove librerie e componenti nel repository ufficiale, molti dei quali non erano disponibili come progetti open source fino a poco tempo. Sfortunatamente, il team di sviluppatori di Unity3D ha permesso al pubblico di analizzare solo alcuni dei componenti, delle librerie e delle demo impiegati dal progetto, mantenendo chiusa la maggior parte del suo codice. In questo articolo cercheremo di trovare bug e refusi in quei componenti con l'aiuto dell'analizzatore statico PVS-Studio.
Introduzione
Abbiamo deciso di controllare tutti i componenti, le librerie e le demo in C#, il cui codice sorgente è disponibile nel repository ufficiale del team di sviluppatori Unity3D:
- Sistema UI - sistema per lo sviluppo della GUI.
- Rete - sistema per implementare la modalità multiplayer.
- MemoryProfiler - sistema per la profilazione delle risorse in uso.
- XcodeAPI - componente per l'interazione con l'IDE Xcode.
- PlayableGraphVisualizer - sistema per la visualizzazione dell'esecuzione del progetto.
- UnityTestTools - Utilità di test Unity3D (nessun test unitario incluso).
- AssetBundleDemo - progetto con i file sorgente e le demo di AssetBundleServer per il sistema AssetBundle.
- AudioDemos - progetti demo per il sistema audio.
- NativeAudioPlugins - plug-in audio (siamo interessati solo alle demo di questi plug-in).
- GraphicsDemos - progetti demo per il sistema grafico.
Vorrei che potessimo dare un'occhiata ai file sorgente del kernel stesso del motore, ma, sfortunatamente, nessuno, tranne gli sviluppatori stessi, ha accesso ad esso attualmente. Quindi, quello che abbiamo oggi sul nostro tavolo operatorio è solo una piccola parte dei file sorgente del motore. Siamo più interessati al sistema dell'interfaccia utente progettato per implementare una GUI più flessibile rispetto a quella più vecchia e goffa e alla libreria di rete, che ci serviva a mani e piedi prima del rilascio di UNet.
Siamo anche molto interessati a MemoryProfiler, che è uno strumento potente e flessibile per la profilazione delle risorse e del carico.
Errori e frammenti sospetti trovati
Tutti gli avvisi emessi dall'analizzatore sono raggruppati in 3 livelli:
- Alto - quasi certamente un errore.
- Medio - possibile errore o errore di battitura.
- Basso:errore o errore di battitura improbabile.
Discuteremo solo i livelli alti e medi in questo articolo.
La tabella seguente presenta l'elenco dei progetti che abbiamo verificato e le statistiche di analisi di tutti i progetti. Le colonne "Nome progetto" e "Numero di LOC" sono autoesplicative, ma la colonna "Avvisi emessi" necessita di alcune spiegazioni. Contiene informazioni su tutti gli avvisi emessi dall'analizzatore. Gli avvisi positivi sono avvisi che puntano direttamente o indirettamente a errori reali o errori di battitura nel codice. I falsi avvisi, o falsi positivi, sono quelli che interpretano il codice corretto come difettoso. Come ho già detto, tutti gli avvisi sono raggruppati in 3 livelli. Discuteremo solo gli avvisi di livello alto e medio, poiché il livello basso riguarda principalmente messaggi informativi o errori improbabili.
Per i 10 progetti controllati, l'analizzatore ha emesso 16 avvisi di alto livello, il 75% dei quali indicava correttamente difetti reali nel codice, e 18 avvisi di livello medio, il 39% dei quali indicava correttamente difetti reali nel codice. Il codice è sicuramente di alta qualità, in quanto il rapporto medio tra errori riscontrati e numero di LOC è di un errore ogni 2000 righe di codice, il che è un buon risultato.
Ora che abbiamo finito con le statistiche, vediamo quali errori e refusi siamo riusciti a trovare.
Espressione regolare errata
V3057 Modello di espressione regolare non valido nel costruttore. Esamina il primo argomento. AssetBundleDemo ExecuteInternalMono.cs 48
private static readonly Regex UnsafeCharsWindows =
new Regex("[^A-Za-z0-9\\_\\-\\.\\:\\,\\/\\@\\\\]"); // <=Quando si tenta di creare un'istanza di Regex classe che utilizza questo modello, una System.ArgumentException verrà generata un'eccezione con il seguente messaggio:
parsing \"[^A-Za-z0-9\\_\\-\\.\\:\\,\\/\\@\\]\" -
Unrecognized escape sequence '\\_'.Questo messaggio indica che il modello utilizzato non è corretto e che Regex la classe non può essere istanziata usandola. Il programmatore deve aver commesso un errore durante la progettazione del modello.
Possibile accesso a un oggetto utilizzando un riferimento null
V3080 Possibile dereferenziazione nulla. Prendi in considerazione l'ispezione di 't.staticFieldBytes'. MemoryProfiller CrawledDataUnpacker.cs 20
.... = packedSnapshot.typeDescriptions.Where(t =>
t.staticFieldBytes != null & t.staticFieldBytes.Length > 0 // <=
)....Si accede a un oggetto dopo un controllo nullo. Tuttavia, vi si accede indipendentemente dal risultato del controllo, che potrebbe causare la generazione di NullReferenceException . Il programmatore deve aver inteso utilizzare il condizionale AND operatore (&& ) ma ha commesso un errore di battitura e ha scritto il logico AND operatore (& ) invece.
Accesso a un oggetto prima di un controllo nullo
V3095 L'oggetto 'uv2.gameObject' è stato utilizzato prima di essere verificato rispetto a null. Linee di controllo:1719, 1731. UnityEngine.Networking NetworkServer.cs 1719
if (uv2.gameObject.hideFlags == HideFlags.NotEditable ||
uv2.gameObject.hideFlags == HideFlags.HideAndDontSave)
continue;
....
if (uv2.gameObject == null)
continue;Viene prima effettuato l'accesso a un oggetto e solo dopo viene testato per null . Se il riferimento all'oggetto risulta nullo, siamo quasi sicuri di ottenere NullReferenceException prima di arrivare all'assegno.
Oltre a quell'errore, l'analizzatore ne ha trovati altri 2 simili:
- V3095 L'oggetto 'm_HorizontalScrollbarRect' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:214, 220. UnityEngine.UI ScrollRect.cs 214
- V3095 L'oggetto 'm_VerticalScrollbarRect' è stato utilizzato prima di essere verificato rispetto a null. Righe di controllo:215, 221. UnityEngine.UI ScrollRect.cs 215
Due istruzioni "se" con la stessa condizione e il "ritorno" incondizionato istruzione nel blocco 'then'
È una questione piuttosto interessante, che è un'illustrazione perfetta di quanto sia potente il copia-incolla; un classico esempio di errore di battitura.
V3021 Esistono due istruzioni 'if' con espressioni condizionali identiche. La prima istruzione 'if' contiene il metodo return. Ciò significa che la seconda affermazione 'se' non ha senso UnityEngine.UI StencilMaterial.cs 64
if (!baseMat.HasProperty("_StencilReadMask"))
{
Debug.LogWarning(".... _StencilReadMask property", baseMat);
return baseMat;
}
if (!baseMat.HasProperty("_StencilReadMask")) // <=
{
Debug.LogWarning(".... _StencilWriteMask property", baseMat);
return baseMat;
}Il programmatore deve aver copiato e incollato un frammento di codice ma ha dimenticato di modificare la condizione.
Sulla base di questo errore di battitura, direi che il secondo controllo doveva assomigliare a questo:
if (!baseMat.HasProperty("_StencilWriteMask"))
Creazione di un'istanza di una classe di eccezione senza utilizzare ulteriormente l'istanza
V3006 L'oggetto è stato creato ma non viene utilizzato. Potrebbe mancare la parola chiave 'throw':throw new ApplicationException(FOO). AssetBundleDemo AssetBundleManager.cs 446
if (bundleBaseDownloadingURL.ToLower().StartsWith("odr://"))
{
#if ENABLE_IOS_ON_DEMAND_RESOURCES
Log(LogType.Info, "Requesting bundle " + ....);
m_InProgressOperations.Add(
new AssetBundleDownloadFromODROperation(assetBundleName)
);
#else
new ApplicationException("Can't load bundle " + ....); // <=
#endif
}Classe ApplicationException viene creato ma non utilizzato in alcun modo. Il programmatore deve aver voluto che fosse generata un'eccezione ma ha dimenticato di aggiungere il lancio parola chiave quando si forma l'eccezione.
Argomenti non utilizzati in un metodo di formattazione di stringhe
Come tutti sappiamo, il numero di {N} gli elementi di formato utilizzati per la formattazione delle stringhe devono corrispondere al numero di argomenti passati al metodo.
V3025 Formato errato. È previsto un numero diverso di elementi di formato durante la chiamata alla funzione 'WriteLine'. Argomenti non utilizzati:port. AssetBundleDemo AssetBundleServer.cs 59
Console.WriteLine("Starting up asset bundle server.", port); // <=
Console.WriteLine("Port: {0}", port);
Console.WriteLine("Directory: {0}", basePath);A giudicare dalla logica di questo codice, sembra che il programmatore abbia dimenticato di rimuovere l'argomento nella prima riga. Questo errore di battitura non è critico dal punto di vista tecnico e non causerà alcun errore, ma non ha comunque alcun significato.
Un ciclo che può diventare infinito in determinate condizioni
V3032 L'attesa di questa espressione non è affidabile, poiché il compilatore potrebbe ottimizzare alcune variabili. Utilizzare variabili volatili o primitive di sincronizzazione per evitare ciò. AssetBundleDemo AssetBundleServer.cs 16
Process masterProcess = Process.GetProcessById((int)processID);
while (masterProcess == null || !masterProcess.HasExited) // <=
{
Thread.Sleep(1000);
}Il programmatore deve aver inteso che il ciclo iterasse fino al completamento di un processo esterno, ma non ha tenuto conto del fatto che masterProcess la variabile potrebbe inizialmente avere il valore null se il processo non è stato trovato, ciò causerebbe un ciclo infinito. Per far funzionare correttamente questo algoritmo, devi accedere al processo usando il suo identificatore ad ogni iterazione:
while (true) {
Process masterProcess = Process.GetProcessById((int)processID);
if (masterProcess == null || masterProcess.HasExited) // <=
break;
Thread.Sleep(1000);
}
Inizializzazione evento non sicura
L'analizzatore ha rilevato una chiamata potenzialmente non sicura a un gestore eventi, che potrebbe causare la generazione di NullReferenceException .
V3083 Invocazione non sicura dell'evento 'unload', NullReferenceException è possibile. Prendere in considerazione l'assegnazione di un evento a una variabile locale prima di richiamarla. AssetBundleDemo AssetBundleManager.cs 47
internal void OnUnload()
{
m_AssetBundle.Unload(false);
if (unload != null)
unload(); // <=
}In questo codice, scarica il campo è testato per null e quindi viene chiamato questo evento. Il controllo nullo ti consente di evitare di generare un'eccezione nel caso in cui l'evento non abbia iscritti nel momento in cui viene chiamato.
Immagina, tuttavia, che l'evento abbia un abbonato. Nel punto tra il controllo nullo e la chiamata al gestore dell'evento, l'abbonato può annullare l'iscrizione all'evento, ad esempio, in un altro thread. Per proteggere il tuo codice in questa situazione, puoi risolverlo nel modo seguente:
internal void OnUnload()
{
m_AssetBundle.Unload(false);
unload?.Invoke(); // <=
}Questa soluzione ti aiuterà ad assicurarti che il test dell'evento sia null e la chiamata al suo gestore verrà eseguita come un'unica istruzione, rendendo sicura la chiamata dell'evento.
Parte di un'espressione logica sempre vera o falsa
V3063 Una parte dell'espressione condizionale è sempre falsa:connId <0. UnityEngine.Networking ConnectionArray.cs 59
public NetworkConnection Get(int connId)
{
if (connId < 0)
{
return m_LocalConnections[Mathf.Abs(connId) - 1];
}
if (connId < 0 || connId > m_Connections.Count) // <=
{
...
return null;
}
return m_Connections[connId];
}Il connId < 0 l'espressione restituirà sempre false la seconda volta viene controllato in get funzione, poiché la funzione termina sempre dopo il primo controllo. Pertanto, valutare questa espressione per la seconda volta non ha senso.
L'analizzatore ha rilevato un altro errore simile.
public bool isServer
{
get
{
if (!m_IsServer)
{
return false;
}
return NetworkServer.active && m_IsServer; // <=
}
}Sicuramente sai che questa proprietà può essere facilmente semplificata come segue:
public bool isServer
{
get
{
return m_IsServer && NetworkServer.active;
}
}Oltre a questi due esempi, ci sono altri 6 problemi di questo tipo:
- L'espressione V3022 'm_Peers ==null' è sempre falsa. UnityEngine.Networking NetworkMigrationManager.cs 710
- L'espressione V3022 'uv2.gameObject ==null' è sempre falsa. UnityEngine.Networking NetworkServer.cs 1731
- L'espressione V3022 'newEnterTarget !=null' è sempre vera. UnityEngine.UI BaseInputModule.cs 147
- L'espressione V3022 'pointerEvent.pointerDrag !=null' è sempre falsa. UnityEngine.UI TouchInputModule.cs 227
- V3063 Una parte dell'espressione condizionale è sempre vera:currentTest !=null. UnityTestTools TestRunner.cs 237
- V3063 Una parte dell'espressione condizionale è sempre falsa:connId <0. UnityEngine.Networking ConnectionArray.cs 86
Conclusione
Proprio come qualsiasi altro progetto, questo contiene una serie di errori e refusi. Come probabilmente avrai notato, PVS-Studio è particolarmente bravo a rilevare errori di battitura.
Puoi anche provare il nostro analizzatore statico con il tuo progetto o quello di qualcun altro in C/C++/C#.
Grazie a tutti per la lettura! Possa il tuo codice rimanere senza bug!