¿Por qué el optimizador GCC 6 mejorado rompe el código práctico de C ++?

148

GCC 6 tiene una nueva función optimizadora : se supone que thisno siempre es nula y se optimiza en función de eso.

La propagación del rango de valores ahora supone que este puntero de las funciones miembro de C ++ no es nulo. Esto elimina las comprobaciones de puntero nulo comunes, pero también rompe algunas bases de código no conformes (como Qt-5, Chromium, KDevelop) . Como solución temporal, se puede utilizar -fno-delete-null-pointer-check. El código incorrecto se puede identificar usando -fsanitize = undefined.

El documento de cambio claramente lo llama peligroso porque rompe una cantidad sorprendente de código de uso frecuente.

¿Por qué esta nueva suposición rompería el código práctico de C ++? ¿Existen patrones particulares en los que los programadores descuidados o desinformados confían en este comportamiento indefinido en particular? No puedo imaginar a nadie escribiendo if (this == NULL)porque eso es muy poco natural.

boot4life
fuente
21
@Ben Espero que lo digas en el buen sentido. El código con UB debe reescribirse para no invocar a UB. Es tan simple como eso. Diablos, a menudo hay preguntas frecuentes que te dicen cómo lograrlo. Por lo tanto, no es un problema real en mi humilde opinión. Todo bien.
Vuelva a instalar Mónica
49
Me sorprende ver a personas que defienden la eliminación de referencias de punteros nulos en el código. Simplemente asombroso.
SergeyA
19
@Ben, explotar comportamientos indefinidos ha sido la táctica de optimización muy efectiva durante mucho tiempo. Me encanta, porque me encantan las optimizaciones que hacen que mi código se ejecute más rápido.
SergeyA
17
Estoy de acuerdo con SergeyA. Todo el brouhaha comenzó porque las personas parecen detenerse en el hecho de que thisse pasa como un parámetro implícito, por lo que luego comienzan a usarlo como si fuera un parámetro explícito. No es. Cuando desreferencia un nulo esto, está invocando UB como si desreferenciara cualquier otro puntero nulo. Eso es todo lo que hay que hacer. Si desea pasar nullptrs, use un parámetro explícito, DUH . No será más lento, no será más complicado, y el código que tiene esa API es profundo en las partes internas de todos modos, por lo que tiene un alcance muy limitado. Fin de la historia, creo.
Vuelva a instalar Mónica
41
Felicitaciones a GCC por romper el ciclo de código incorrecto -> compilador ineficiente para admitir código incorrecto -> código más incorrecto -> compilación más ineficiente -> ...
MM

Respuestas:

87

Supongo que la pregunta que debe responderse es por qué las personas bien intencionadas escribirían los cheques en primer lugar.

El caso más común es probablemente si tiene una clase que es parte de una llamada recursiva natural.

Si tuvieras:

struct Node
{
    Node* left;
    Node* right;
};

en C, podrías escribir:

void traverse_in_order(Node* n) {
    if(!n) return;
    traverse_in_order(n->left);
    process(n);
    traverse_in_order(n->right);
}

En C ++, es bueno hacer de esto una función miembro:

void Node::traverse_in_order() {
    // <--- What check should be put here?
    left->traverse_in_order();
    process();
    right->traverse_in_order();
}

En los primeros días de C ++ (antes de la estandarización), se enfatizó que las funciones miembro eran azúcar sintáctica para una función donde el thisparámetro está implícito. El código fue escrito en C ++, convertido a C equivalente y compilado. Incluso hubo ejemplos explícitos de que comparar thiscon nulo era significativo y el compilador original de Cfront también se aprovechó de esto. Entonces, viniendo de un fondo C, la opción obvia para la verificación es:

if(this == nullptr) return;      

Nota: Bjarne Stroustrup incluso menciona que las reglas thishan cambiado con los años aquí

Y esto funcionó en muchos compiladores durante muchos años. Cuando ocurrió la estandarización, esto cambió. Y, más recientemente, los compiladores comenzaron a tomar ventaja de llamar a una función miembro donde thisel ser nullptres un comportamiento indefinido, lo que significa que esta condición es siempre false, y el compilador es libre de omitirlo.

Eso significa que para atravesar este árbol, debe:

  • Haga todas las verificaciones antes de llamar traverse_in_order

    void Node::traverse_in_order() {
        if(left) left->traverse_in_order();
        process();
        if(right) right->traverse_in_order();
    }

    Esto significa también verificar en CADA sitio de llamadas si podría tener una raíz nula.

  • No use una función miembro

    Esto significa que está escribiendo el antiguo código de estilo C (quizás como un método estático) y lo está llamando explícitamente al objeto como parámetro. p.ej. ha vuelto a escribir en Node::traverse_in_order(node);lugar de node->traverse_in_order();en el sitio de la llamada.

  • Creo que la forma más fácil / ordenada de arreglar este ejemplo en particular de una manera que cumpla con los estándares es usar un nodo centinela en lugar de a nullptr.

    // static class, or global variable
    Node sentinel;
    
    void Node::traverse_in_order() {
        if(this == &sentinel) return;
        ...
    }

Ninguna de las dos primeras opciones parece tan atractiva, y aunque el código podría salirse con la suya, escribieron un código incorrecto en this == nullptrlugar de usar una solución adecuada.

Supongo que así es como evolucionaron algunas de estas bases de código para tener this == nullptrcontroles en ellas.

jtlim
fuente
66
¿Cómo puede 1 == 0ser un comportamiento indefinido? Es simplemente false.
Johannes Schaub - litb
11
La verificación en sí no es un comportamiento indefinido. Siempre es falso y, por lo tanto, el compilador lo elimina.
SergeyA
15
Hmm ... el this == nullptridioma es un comportamiento indefinido porque has llamado una función miembro en un objeto nullptr antes de eso, que no está definido. Y el compilador es libre de omitir el cheque
jtlim
66
@Joshua, el primer estándar se publicó en 1998. Lo que sucedió antes fue lo que toda implementación quería. Edad Oscura.
SergeyA
26
Je, wow, no puedo creer que alguien haya escrito código que se basara en llamar a funciones de instancia ... sin una instancia . Hubiera usado instintivamente el extracto marcado "Hacer todas las comprobaciones antes de llamar a traverse_in_order", sin siquiera pensar en thisser anulable. Supongo que tal vez este es el beneficio de aprender C ++ en una era en la que existe SO para afianzar los peligros de UB en mi cerebro y disuadirme de hacer hacks extraños como este.
underscore_d
65

Lo hace porque el código "práctico" estaba roto e involucraba un comportamiento indefinido para empezar. No hay ninguna razón para usar un valor nulo this, aparte de una microoptimización, generalmente muy prematura.

Es una práctica peligrosa, ya que el ajuste de los punteros debido al recorrido de la jerarquía de clases puede convertir un valor nulo thisen uno no nulo. Entonces, como mínimo, la clase cuyos métodos se supone que funcionan con un valor nulo thisdebe ser una clase final sin clase base: no puede derivarse de nada, y no puede derivarse de ella. Nos estamos yendo rápidamente de lo práctico a lo feo .

En términos prácticos, el código no tiene que ser feo:

struct Node
{
  Node* left;
  Node* right;
  void process();
  void traverse_in_order() {
    traverse_in_order_impl(this);
  }
private:
  static void traverse_in_order_impl(Node * n)
    if (!n) return;
    traverse_in_order_impl(n->left);
    n->process();
    traverse_in_order_impl(n->right);
  }
};

Si tenía un árbol vacío (p. Ej., Root es nullptr), esta solución aún se basa en un comportamiento indefinido llamando a traverse_in_order con un nullptr.

Si el árbol está vacío, es decir, un valor nulo Node* root, no se debe llamar a ningún método no estático en él. Período. Está perfectamente bien tener un código de árbol tipo C que tome un puntero de instancia mediante un parámetro explícito.

El argumento aquí parece reducirse a la necesidad de escribir métodos no estáticos en objetos que podrían llamarse desde un puntero de instancia nula. No hay tal necesidad. La forma en que C-con-objetos escribe dicho código es aún más agradable en el mundo de C ++, porque al menos puede ser de tipo seguro. Básicamente, el valor nulo thises una microoptimización, con un campo de uso tan estrecho, que rechazarlo está perfectamente bien. Ninguna API pública debería depender de un valor nulo this.

Reinstalar a Mónica
fuente
18
@Ben, quien escribió este código estaba equivocado en primer lugar. Es curioso que nombres proyectos tan terriblemente rotos como MFC, Qt y Chromium. Buen viaje con ellos.
SergeyA
19
@Ben, los estilos de codificación terribles en Google me son bien conocidos. El código de Google (al menos disponible públicamente) a menudo está mal escrito, a pesar de que varias personas creen que el código de Google es el ejemplo brillante. Puede ser que esto les haga volver a visitar sus estilos de codificación (y pautas mientras están en él).
SergeyA
18
@Ben Nadie está reemplazando retroactivamente Chromium en estos dispositivos con Chromium compilado con gcc 6. Antes de compilar Chromium con gcc 6 y otros compiladores modernos, será necesario repararlo. Tampoco es una tarea enorme; los thiscontroles son recogidos por varios analizadores de código estático, por lo que no es como si alguien tiene a ellos cazar manualmente todo abajo. El parche sería probablemente un par de cientos de líneas de cambios triviales.
Vuelva a instalar Mónica
8
@Ben En términos prácticos, una thisdesreferencia nula es un bloqueo instantáneo. Estos problemas se descubrirán muy rápidamente, incluso si a nadie le importa ejecutar un analizador estático sobre el código. C / C ++ sigue el mantra "paga solo por las funciones que usas". Si desea verificaciones, debe ser explícito sobre ellas y eso significa no hacerlo this, cuando es demasiado tarde, ya que el compilador supone thisque no es nulo. De lo contrario, tendría que verificar this, y para el 99.9999% del código, tales comprobaciones son una pérdida de tiempo.
Vuelva a instalar Mónica
10
mi consejo para cualquiera que piense que el estándar está roto: use un idioma diferente. No hay escasez de lenguajes similares a C ++ que no tienen la posibilidad de un comportamiento indefinido.
MM
35

El documento de cambio claramente lo llama peligroso porque rompe una cantidad sorprendente de código de uso frecuente.

El documento no lo llama peligroso. Tampoco afirma que rompe una cantidad sorprendente de código . Simplemente señala algunas bases de código populares que, según afirma, se sabe que confían en este comportamiento indefinido y se romperían debido al cambio a menos que se use la opción de solución alternativa.

¿Por qué esta nueva suposición rompería el código práctico de C ++?

Si el código práctico de c ++ se basa en un comportamiento indefinido, los cambios en ese comportamiento indefinido pueden romperlo. Es por eso que se debe evitar UB, incluso cuando un programa que se basa en él parece funcionar según lo previsto.

¿Existen patrones particulares en los que los programadores descuidados o desinformados confían en este comportamiento indefinido en particular?

No sé si es anti- patrón extendido , pero un programador desinformado podría pensar que puede arreglar su programa de fallar haciendo:

if (this)
    member_variable = 42;

Cuando el error real hace referencia a un puntero nulo en otro lugar.

Estoy seguro de que si el programador no está lo suficientemente informado, podrán crear patrones (anti) más avanzados que dependan de este UB.

No puedo imaginar a nadie escribiendo if (this == NULL)porque eso es muy poco natural.

Puedo.

eerorika
fuente
11
"Si el código práctico de c ++ se basa en un comportamiento indefinido, los cambios en ese comportamiento indefinido pueden romperlo. Es por eso que se debe evitar UB" this * 1000
underscore_d
if(this == null) PrintSomeHelpfulDebugInformationAboutHowWeGotHere(); Como un buen registro fácil de leer de una secuencia de eventos que un depurador no puede contarte fácilmente. Diviértete depurando esto ahora sin pasar horas colocando cheques en todas partes cuando hay un nulo aleatorio repentino en un gran conjunto de datos, en un código que no has escrito ... Y la regla UB sobre esto se hizo más tarde, después de que se creó C ++. Solía ​​ser válido.
Stephane Hockenhull
@StephaneHockenhull Para eso -fsanitize=nullestá.
eerorika
@ user2079303 Problemas: ¿Eso ralentizará el código de producción hasta el punto en que no pueda dejar el cheque mientras se ejecuta, lo que le costará mucho dinero a la empresa? ¿Va a aumentar el tamaño y no encajar en flash? ¿Funciona en todas las plataformas de destino, incluido Atmel? ¿Puede -fsanitize=nullregistrar los errores en la tarjeta SD / MMC en los pines # 5,6,10,11 usando SPI? Esa no es una solución universal. Algunos han argumentado que ir en contra de los principios orientados a objetos para acceder a un objeto nulo, sin embargo, algunos lenguajes OOP tienen un objeto nulo que se puede operar, por lo que no es una regla universal de OOP. 1/2
Stephane Hockenhull
1
... una expresión regular que coincide con dichos archivos? Decir que, por ejemplo, si se accede a un valor l dos veces, un compilador puede consolidar los accesos a menos que el código entre ellos haga alguna de las cosas específicas, sería mucho más fácil que tratar de definir las situaciones precisas en las que se permite el acceso al almacenamiento del código.
supercat
25

Algunos de los códigos "prácticos" (forma divertida de deletrear "errores") que se rompieron se veían así:

void foo(X* p) {
  p->bar()->baz();
}

y se olvidó de tener en cuenta el hecho de que a p->bar()veces devuelve un puntero nulo, lo que significa que desreferenciarlo para llamar baz()no está definido.

No todo el código que se rompió contenía explícitos if (this == nullptr)o if (!p) return;verificaciones. Algunos casos eran simplemente funciones que no tenían acceso a ninguna variable miembro y, por lo tanto, parecían funcionar bien. Por ejemplo:

struct DummyImpl {
  bool valid() const { return false; }
  int m_data;
};
struct RealImpl {
  bool valid() const { return m_valid; }
  bool m_valid;
  int m_data;
};

template<typename T>
void do_something_else(T* p) {
  if (p) {
    use(p->m_data);
  }
}

template<typename T>
void func(T* p) {
  if (p->valid())
    do_something(p);
  else 
    do_something_else(p);
}

En este código, cuando llama func<DummyImpl*>(DummyImpl*)con un puntero nulo, existe una desreferencia "conceptual" del puntero a llamar p->DummyImpl::valid(), pero de hecho esa función miembro simplemente regresa falsesin acceder *this. Eso return falsepuede estar en línea y, en la práctica, no es necesario acceder al puntero. Entonces, con algunos compiladores parece funcionar bien: no hay segfault para desreferenciar nulo, p->valid()es falso, por lo que el código llama do_something_else(p), que comprueba los punteros nulos, y no hace nada. No se observan accidentes ni comportamientos inesperados.

Con GCC 6 todavía recibe la llamada p->valid(), pero el compilador ahora deduce de esa expresión que no pdebe ser nula (de lo contrario p->valid()sería un comportamiento indefinido) y toma nota de esa información. El optimizador utiliza esa información inferida, de modo que si la llamada a do_something_else(p)se inserta, la if (p)verificación ahora se considera redundante, porque el compilador recuerda que no es nula, y así alinea el código para:

template<typename T>
void func(T* p) {
  if (p->valid())
    do_something(p);
  else {
    // inlined body of do_something_else(p) with value propagation
    // optimization performed to remove null check.
    use(p->m_data);
  }
}

Esto ahora realmente hace referencia a un puntero nulo, por lo que el código que antes parecía funcionar deja de funcionar.

En este ejemplo, el error está en func, que debería haber verificado primero nulo (o los llamadores nunca deberían haberlo llamado con nulo):

template<typename T>
void func(T* p) {
  if (p && p->valid())
    do_something(p);
  else 
    do_something_else(p);
}

Un punto importante para recordar es que la mayoría de las optimizaciones como esta no son el caso del compilador que dice "ah, el programador probó este puntero contra nulo, lo eliminaré solo para ser molesto". Lo que sucede es que varias optimizaciones de rutina como la línea y la propagación del rango de valores se combinan para hacer que esas verificaciones sean redundantes, ya que vienen después de una verificación anterior, o una desreferencia. Si el compilador sabe que un puntero no es nulo en el punto A en una función, y el puntero no se cambia antes de un punto B posterior en la misma función, entonces sabe que también es no nulo en B. Cuando ocurre la alineación los puntos A y B en realidad podrían ser piezas de código que originalmente estaban en funciones separadas, pero ahora se combinan en una sola pieza de código, y el compilador puede aplicar su conocimiento de que el puntero no es nulo en más lugares.

Jonathan Wakely
fuente
¿Es posible instrumentar GCC 6 para emitir advertencias en tiempo de compilación cuando encuentra tales usos de this?
jotik
3
@jotik, ^^^ lo que dijo TC. Sería posible, pero obtendría esa advertencia PARA TODO EL CÓDIGO, TODO EL TIEMPO . La propagación del rango de valores es una de las optimizaciones más comunes, que afecta a casi todo el código, en todas partes. Los optimizadores solo ven el código, que se puede simplificar. No ven "un código escrito por un idiota que quiere ser advertido si su tonto UB se optimiza". No es fácil para el compilador distinguir entre "verificación redundante de que el programador quiere ser optimizado" y "verificación redundante que el programador cree que ayudará, pero es redundante".
Jonathan Wakely
1
Si desea instrumentar su código para dar errores de tiempo de ejecución para varios tipos de UB, incluidos los usos no válidos de this, simplemente use-fsanitize=undefined
Jonathan Wakely
-25

El estándar C ++ se rompe de manera importante. Desafortunadamente, en lugar de proteger a los usuarios de estos problemas, los desarrolladores de GCC han optado por usar un comportamiento indefinido como una excusa para implementar optimizaciones marginales, incluso cuando se les ha explicado claramente lo dañino que es.

Aquí una persona mucho más inteligente que la que explico con gran detalle. (Está hablando de C pero la situación es la misma allí).

¿Por qué es dañino?

Simplemente recompilar el código seguro que funcionaba anteriormente con una versión más nueva del compilador puede introducir vulnerabilidades de seguridad . Si bien el nuevo comportamiento se puede deshabilitar con una marca, los archivos MAKE existentes no tienen esa marca establecida, obviamente. Y como no se produce ninguna advertencia, no es obvio para el desarrollador que el comportamiento previamente razonable haya cambiado.

En este ejemplo, el desarrollador ha incluido una comprobación de desbordamiento de enteros, que utilizará assert, que terminará el programa si se proporciona una longitud no válida. El equipo de GCC eliminó la verificación sobre la base de que el desbordamiento de enteros no está definido, por lo tanto, la verificación se puede eliminar. Esto resultó en instancias reales de esta base de código que se volvió vulnerable después de que se solucionó el problema.

Lee todo el asunto. Es suficiente para hacerte llorar.

OK, pero ¿qué hay de este?

Hace mucho tiempo, había un idioma bastante común que decía algo así:

 OPAQUEHANDLE ObjectType::GetHandle(){
    if(this==NULL)return DEFAULTHANDLE;
    return mHandle;

 }

 void DoThing(ObjectType* pObj){
     osfunction(pObj->GetHandle(), "BLAH");
 }

Entonces, el idioma es: si pObjno es nulo, usa el identificador que contiene, de lo contrario, utiliza un identificador predeterminado. Esto está encapsulado en la GetHandlefunción.

El truco es que llamar a una función no virtual en realidad no hace ningún uso del thispuntero, por lo que no hay violación de acceso.

Todavía no lo entiendo

Existe mucho código que está escrito así. Si alguien simplemente lo vuelve a compilar, sin cambiar una línea, cada llamada a DoThing(NULL)es un error de bloqueo, si tiene suerte.

Si no tiene suerte, las llamadas a fallos se convierten en vulnerabilidades de ejecución remota.

Esto puede ocurrir incluso automáticamente. Tienes un sistema de construcción automatizado, ¿verdad? Actualizarlo al último compilador es inofensivo, ¿verdad? Pero ahora no lo es, no si su compilador es GCC.

OK entonces diles!

Se les ha dicho. Lo están haciendo con pleno conocimiento de las consecuencias.

¿pero por qué?

¿Quién puede decir? Quizás:

  • Valoran la pureza ideal del lenguaje C ++ sobre el código real
  • Creen que las personas deberían ser castigadas por no seguir el estándar
  • No entienden la realidad del mundo.
  • Ellos están ... introduciendo errores a propósito. Quizás para un gobierno extranjero. ¿Dónde vives? Todos los gobiernos son ajenos a la mayor parte del mundo, y la mayoría son hostiles a algunos del mundo.

O tal vez algo más. ¿Quién puede decir?

Ben
fuente
32
No estoy de acuerdo con todas y cada una de las líneas de la respuesta. Se hicieron los mismos comentarios para las estrictas optimizaciones de alias, y es de esperar que ahora se descarten. La solución es educar a los desarrolladores, no evitar optimizaciones basadas en malos hábitos de desarrollo.
SergeyA
30
Fui a leer todo lo que dijiste, y de hecho lloré, pero principalmente por la estupidez de Felix, que no creo que fuera lo que estabas tratando de transmitir ...
Mike Vine
33
Votado en contra de la diatriba inútil. "Están ... introduciendo errores a propósito. Quizás para un gobierno extranjero". De Verdad? Esto no es / r / conspiración.
isanae
31
Los programadores decentes que repiten una y otra vez el mantra no invocan un comportamiento indefinido , sin embargo, estos nonks han seguido adelante y lo han hecho de todos modos. Y mira lo que pasó. No tengo simpatía alguna. Esto es culpa de los desarrolladores, así de simple. Necesitan asumir la responsabilidad. ¿Recuerda eso? ¿Responsabilidad personal? Las personas que confían en tu mantra "¡pero qué pasa en la práctica !" es precisamente cómo surgió esta situación en primer lugar. Evitar tonterías como esta es precisamente la razón por la cual existen estándares en primer lugar. Codifique según los estándares y no tendrá ningún problema. Período.
ligereza corre en órbita el
18
"Simplemente recompilar un código seguro que funcionaba anteriormente con una versión más nueva del compilador puede introducir vulnerabilidades de seguridad", eso siempre sucede . A menos que desee exigir que una versión de un compilador sea el único compilador que se permitirá por el resto de la eternidad. ¿Recuerdas cuando el kernel de Linux solo podía compilarse con exactamente gcc 2.7.2.1? El proyecto gcc incluso se bifurcó porque la gente estaba harta de bullcrap. Tomó mucho tiempo superar eso.
MM