Ya comprobamos Mozilla Firefox con el analizador PVS-Studio hace tres años. Era bastante inconveniente y problemático en ese momento. Verá, no hay un archivo de proyecto de Visual Studio para Firefox:la compilación se realiza con la ayuda de archivos MAKE. Es por eso que no puede simplemente tomar y verificar el proyecto. Tuvimos que integrar PVS-Studio en el sistema de compilación, lo que parecía una tarea difícil. Si no recuerdo mal, todo resultó en analizar con éxito solo una parte del proyecto. Pero todo es diferente ahora que tenemos PVS-Studio Standalone. Ahora podemos monitorear todos los lanzamientos de compiladores y verificar fácilmente el proyecto.
Mozilla Firefox
No creo que Firefox necesite presentación, pero el formato del artículo implica dar alguna descripción del proyecto bajo análisis. Bueno, soy demasiado perezoso, así que aquí tienes un extracto del artículo de Wikipedia:
Mozilla Firefox es un navegador web gratuito y de código abierto desarrollado para Windows, OS X y Linux, con una versión móvil para Android, por la Fundación Mozilla y su subsidiaria, Mozilla Corporation. Firefox utiliza el motor de diseño Gecko para representar páginas web, que implementa los estándares web actuales y anticipados.
Desde febrero de 2014, Firefox tiene entre un 12 % y un 22 % de uso mundial, lo que lo convierte en el tercer navegador web más popular.
Las características incluyen navegación con pestañas, revisión ortográfica, búsqueda incremental, marcadores en vivo, marcadores inteligentes, un administrador de descargas, navegación privada, navegación con reconocimiento de ubicación ("geolocalización") basada en un servicio de Google y un sistema de búsqueda integrado que usa Google de forma predeterminada en la mayoría de localizaciones. Las funciones se pueden agregar a través de extensiones, creadas por desarrolladores externos.
Ya intentamos analizar Firefox anteriormente e incluso lo logramos hasta cierto punto. Con base en los resultados del análisis, escribimos el artículo "Cómo cometer menos errores en la etapa de escritura de código. Parte N4". Lo difícil de verificar el proyecto en ese momento es que tuvimos que integrar la llamada de la versión de línea de comandos de PVS-Studio en los archivos MAKE. Hacer esto en un proyecto grande y desconocido suele ser problemático. Esa es la razón por la que nunca intentamos volver a analizar Firefox después de la primera comprobación. Todo cambió cuando se creó PVS-Studio Standalone.
PVS-Studio Independiente
PVS-Studio Standalone se puede utilizar en 2 modos:
- Visualización y manejo convenientes del archivo de informe (*.plog) con la información sobre los errores detectados en una computadora sin Visual Studio instalado.
- Supervisar los lanzamientos del compilador y recopilar toda la información necesaria para un análisis posterior. Es este modo el que nos interesa actualmente.
Ya no necesita integrar la versión de línea de comandos de PVS-Studio en archivos MAKE. Firefox ahora se puede verificar de una manera mucho más simple, y lo hemos usado. El algoritmo incluye los siguientes pasos:
- Iniciar PVS-Studio independiente;
- Ejecute el comando "Supervisión del compilador";
- Compilar el proyecto de Firefox;
- Detener el proceso de monitoreo ("Detener Monitoreo");
- Iniciar el análisis de los archivos;
- Examinar las advertencias generadas por el analizador.
Para obtener más detalles sobre cómo usar este modo, siga este enlace.
Resultados de análisis de Mozilla Firefox
El proyecto Firefox es de muy alta calidad. Además, encuentro alguna evidencia de que los desarrolladores utilizan herramientas de análisis de código estático en el proceso de desarrollo:Coverity y Klocwork; al menos, estas herramientas se mencionan en algunos archivos.
Teniendo todo eso en cuenta, sería un gran éxito encontrar algo digno en este proyecto. Entonces, averigüemos si hay algún mensaje de diagnóstico interesante de PVS-Studio para el proyecto Firefox.
Error tipográfico n.º 1
NS_IMETHODIMP
nsNativeThemeWin::WidgetStateChanged(....)
{
....
if (aWidgetType == NS_THEME_WINDOW_TITLEBAR ||
aWidgetType == NS_THEME_WINDOW_TITLEBAR_MAXIMIZED ||
aWidgetType == NS_THEME_WINDOW_FRAME_LEFT ||
aWidgetType == NS_THEME_WINDOW_FRAME_RIGHT ||
aWidgetType == NS_THEME_WINDOW_FRAME_BOTTOM ||
aWidgetType == NS_THEME_WINDOW_BUTTON_CLOSE ||
aWidgetType == NS_THEME_WINDOW_BUTTON_MINIMIZE || <<<===
aWidgetType == NS_THEME_WINDOW_BUTTON_MINIMIZE || <<<===
aWidgetType == NS_THEME_WINDOW_BUTTON_RESTORE) {
*aShouldRepaint = true;
return NS_OK;
....
}Mensaje de diagnóstico de PVS-Studio:V501 Hay subexpresiones idénticas 'aWidgetType ==237' a la izquierda y a la derecha de '||' operador. nsnativethemewin.cpp 2475
La variable 'aWidgetType' se compara dos veces con la constante NS_THEME_WINDOW_BUTTON_MINIMIZE. Esto es un error tipográfico:la variable debe compararse con la constante NS_THEME_WINDOW_BUTTON_MAXIMIZE por segunda vez.
Error tipográfico n.º 2
bool nsHTMLCSSUtils::IsCSSEditableProperty(....)
{
....
if (aAttribute && aAttribute->EqualsLiteral("align") &&
(nsEditProperty::ul == tagName <<<<====
|| nsEditProperty::ol == tagName
|| nsEditProperty::dl == tagName
|| nsEditProperty::li == tagName
|| nsEditProperty::dd == tagName
|| nsEditProperty::dt == tagName
|| nsEditProperty::address == tagName
|| nsEditProperty::pre == tagName
|| nsEditProperty::ul == tagName)) { <<<<====
return true;
}
....
}Mensaje de diagnóstico de PVS-Studio:V501 Hay subexpresiones idénticas 'nsEditProperty::ul ==tagName' a la izquierda y a la derecha de '||' operador. nshtmlcssutils.cpp 432
La variable 'tagName' se compara dos veces con nsEditProperty::ul. Tal vez una de las comprobaciones sea redundante o debería haberse comparado con otra cosa.
Error tipográfico n.º 3
void Reverb::process(....)
{
....
bool isCopySafe =
destinationChannelL &&
destinationChannelR &&
size_t(destinationBus->mDuration) >= framesToProcess &&
size_t(destinationBus->mDuration) >= framesToProcess;
....
}Mensaje de diagnóstico de PVS-Studio:V501 Hay subexpresiones idénticas 'size_t (destinationBus->mDuration)>=framesToProcess' a la izquierda ya la derecha del operador '&&'. reverb.cpp 192
La variable 'framesToProcess' se compara con 'size_t(destinationBus->mDuration)' dos veces.
Error tipográfico n.º 4
float
PannerNode::ComputeDopplerShift()
{
....
double scaledSpeedOfSound = listener->DopplerFactor() /
listener->DopplerFactor();
....
}Mensaje de diagnóstico de PVS-Studio:V501 Hay subexpresiones idénticas 'oyente->DopplerFactor()' a la izquierda ya la derecha del operador '/'. pannernode.cpp 529
Esa es una expresión muy sospechosa y debe ser examinada.
Error tipográfico n.º 5
bool DataChannelConnection::SendDeferredMessages()
{
....
if ((result = usrsctp_sendv(mSocket, data, ...., 0) < 0)) {
....
}
Mensaje de diagnóstico de PVS-Studio:V593 Considere revisar la expresión del tipo 'A =B
Un paréntesis está escrito en un lugar incorrecto. Simplifiquemos la expresión para aclarar el error:
Esta expresión se calcula de la siguiente manera. El resultado devuelto por la función se compara con 0; luego se escribe verdadero o falso en la variable 'resultado'. El error se trata de uno de los paréntesis de cierre escrito en un lugar equivocado. El programador en realidad quería que la expresión se viera de la siguiente manera:
En este caso, el resultado devuelto por la función se escribe primero en la variable 'resultado' y solo entonces se compara con 0.
Error tipográfico n.º 6
Mensaje de diagnóstico de PVS-Studio:V587 Una secuencia extraña de asignaciones de este tipo:A =B; B =A;. Verifique las líneas:358, 360. nsregion.cpp 360
Este código es sospechoso; debe haber algún error tipográfico en él.
Cheque n.° 1 incorrecto
Mensaje de diagnóstico de PVS-Studio:V616 La constante denominada 'eBorderStyle_none' con el valor 0 se utiliza en la operación bit a bit. nswindow.cpp 2278
La expresión "mBorderStyle &eBorderStyle_none" no tiene sentido. La ausencia de estilos (eBorderStyle_none) se codifica con el valor 0. Lo más probable es que el código de condición tenga el siguiente aspecto:
Cheque n.° 2 incorrecto
Mensaje de diagnóstico de PVS-Studio:V547 La expresión siempre es falsa. Probablemente el '||' El operador debe usarse aquí. nswindowsregkey.cpp 292
La variable 'tipo' no puede ser igual a dos valores diferentes a la vez. Simplifiquemos el código para ver más claramente lo que no le gusta al analizador en este ejemplo de código:
Esta condición siempre es falsa.
Lo más probable es que el código tenga el siguiente aspecto:
Cheque n.° 3 incorrecto
Mensaje de diagnóstico de PVS-Studio:V547 La expresión '-- guess>=minEntry' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. ion.cpp 1112
El ciclo solo terminará cuando se encuentre el elemento necesario. Si no existe tal elemento, la condición de finalización del ciclo nunca será verdadera y se producirá una saturación del conjunto.
La razón es que la variable 'suposición' no está firmada. Significa que la condición (--suposición>=0) siempre es verdadera.
Desatención No. 1
Mensaje de diagnóstico de PVS-Studio:V579 La función memset recibe el puntero y su tamaño como argumentos. Posiblemente sea un error. Inspeccione el tercer argumento. winutils.cpp 146
La expresión 'sizeof(utf8)' devuelve el tamaño del puntero, no el tamaño del búfer de memoria asignado. El código correcto debería verse así:
Desatención No. 2
Como de costumbre, hay algunos fragmentos de código que se encuentran donde los punteros se usan primero y solo luego se verifica que sean nulos. Citaré sólo una de estas muestras; Los autores de Firefox pueden usar nuestro analizador ellos mismos para encontrar todos los demás errores de este tipo.
Mensaje de diagnóstico de PVS-Studio:V595 El puntero de "cabeza" se utilizó antes de que se verificara contra nullptr. Verificar líneas:1915, 1920. nshttptransaction.cpp 1915
Primero se elimina la referencia al puntero 'head' en la expresión "head->Status()" y solo luego se comprueba si es nulo.
Desatención No. 3
Mensaje de diagnóstico de PVS-Studio:V611 La memoria se asignó con el operador 'nuevo' pero se liberó con la función 'libre'. Considere inspeccionar las lógicas de operación detrás de la variable 'instanceData'. nptest.cpp 1029
El operador 'nuevo' se usa para asignar memoria mientras que la función 'libre' se llama para liberarla. Da como resultado un comportamiento del programa indefinido. Sin embargo, no es tan crucial porque este fragmento de código está relacionado con las pruebas.
Falta de atención n.º 4
Otro fragmento de código encontrado en las pruebas. La variable 'dispositivo' podría quedar sin inicializar:
Mensaje de diagnóstico de PVS-Studio:V614 Se utilizó un "dispositivo" de puntero potencialmente no inicializado. nptest_windows.cpp 164
El propósito de este artículo no fue describir todos los errores que PVS-Studio puede detectar. Estoy seguro de que me he perdido algo; y algunos errores que no describí conscientemente. Por ejemplo, el analizador generó muchas advertencias V610 relacionadas con las operaciones de cambio que provocan un comportamiento indefinido. Pero todas estas advertencias se parecen, así que no las encuentro lo suficientemente interesantes como para mencionarlas aquí.
El artículo pretende mostrarle las capacidades del análisis estático y atraer la atención de los programadores hacia nuestra herramienta. Los desarrolladores de Firefox deberían llevar a cabo un análisis más exhaustivo de su proyecto, ya que les resultará mucho más fácil determinar si ciertos problemas son errores genuinos o no.
Una nota para los desarrolladores de Firefox. El proyecto es bastante grande, por lo que PVS-Studio genera una gran cantidad de falsos positivos. Sin embargo, la mayoría de ellos están relacionados con macros específicas. Puede reducir fácilmente la cantidad de falsos positivos varias veces agregando comentarios especiales en el código. Consulte la documentación para descubrir cómo suprimir las advertencias en determinadas macros (consulte la sección "Supresión de falsas alarmas"). Si está interesado en adquirir una licencia de PVS-Studio, también estamos dispuestos a participar en la eliminación de falsos positivos en su proyecto.
Hubo algunos fragmentos de código sospechosos en Firefox. La razón es que muchos errores ya se han detectado mediante otros métodos de prueba y analizadores estáticos. Los analizadores de código estático son más útiles cuando se usan regularmente, ya que le permiten detectar errores ya en la etapa de codificación. Para obtener más información sobre el tema, consulte el artículo "Leo Tolstoy y el análisis de código estático".
Le deseo buena suerte en la programación y el código sin errores.if ((result = foo() < 0))if ((result = foo()) < 0)void nsRegion::SimplifyOutwardByArea(uint32_t aThreshold)
{
....
topRects = destRect;
bottomRects = bottomRectsEnd;
destRect = topRects;
....
}enum nsBorderStyle {
eBorderStyle_none = 0,
....
};
....
NS_IMETHODIMP
nsWindow::SetNonClientMargins(nsIntMargin &margins)
{
if (!mIsTopWidgetWindow ||
mBorderStyle & eBorderStyle_none ||
mHideChrome)
return NS_ERROR_INVALID_ARG;
....
}if (!mIsTopWidgetWindow ||
mBorderStyle != eBorderStyle_none ||
mHideChrome)NS_IMETHODIMP nsWindowsRegKey::ReadStringValue(....)
{
....
DWORD type;
....
if (type != REG_SZ && type == REG_EXPAND_SZ &&
type == REG_MULTI_SZ)
return NS_ERROR_FAILURE;
....
}if (... && type == 2 && type == 7)if (type != REG_SZ && type != REG_EXPAND_SZ &&
type != REG_MULTI_SZ)const SafepointIndex *
IonScript::getSafepointIndex(uint32_t disp) const
{
....
size_t minEntry = 0;
....
size_t guess = ....;
....
while (--guess >= minEntry) {
guessDisp = table[guess].displacement();
JS_ASSERT(guessDisp >= disp);
if (guessDisp == disp)
return &table[guess];
}
....
}void WinUtils::LogW(const wchar_t *fmt, ...)
{
....
char* utf8 = new char[len+1];
memset(utf8, 0, sizeof(utf8));
....
}memset(utf8, 0, sizeof(*utf8) * (len+1));void
nsHttpTransaction::RestartVerifier::Set(
int64_t contentLength, nsHttpResponseHead *head)
{
if (mSetup)
return;
if (head->Status() != 200) <<<<====
return;
mContentLength = contentLength;
if (head) { <<<<====
....
}NPError NPP_New(....)
{
....
InstanceData* instanceData = new InstanceData;
....
NPError err = pluginInstanceInit(instanceData);
if (err != NPERR_NO_ERROR) {
NPN_ReleaseObject(scriptableObject);
free(instanceData);
return err;
}
....
}static ID3D10Device1* getD3D10Device()
{
ID3D10Device1 *device;
....
if (createDXGIFactory1)
{
....
hr = createD3DDevice(...., &device);
....
}
return device;
}Análisis más completo
Conclusión
Referencias