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

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.

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.

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.

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.

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.

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í).

• https://groups.google.com/forum/m/#!msg/boring-crypto/48qa1kWignU/o8GGp2K1DAAJ

¿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.

• https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30475

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?