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

GCC 6 tiene una nueva función de optimizador : asume que thisno siempre es nulo y optimiza en base a eso.

La propagación del rango de valores ahora asume que este puntero de las funciones miembro de C ++ no es nulo. Esto elimina las comprobaciones comunes de puntero nulo, 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-checks. Se puede identificar un código incorrecto usando -fsanitize = undefined.

El documento de cambios claramente dice que esto es peligroso porque rompe una cantidad sorprendente de código de uso frecuente.

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

Supongo que la pregunta que debe responderse es por qué 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 que ocurre naturalmente.

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áctico 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 null era significativo y el compilador original de Cfront también aprovechó 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 para thishan cambiado a lo largo de 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 aprovechar la llamada a una función miembro donde thisbeing nullptres un comportamiento indefinido, lo que significa que esta condición es siempre false, y el compilador es libre de omitirla.

Eso significa que para hacer cualquier recorrido de este árbol, debe:

• Haga todas las comprobaciones 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 puede tener una raíz nula.

• No uses una función miembro

  Esto significa que está escribiendo el antiguo código de estilo C (tal vez como un método estático) y llamándolo con el objeto explícitamente como parámetro. p.ej. ha vuelto a escribir en Node::traverse_in_order(node);lugar de node->traverse_in_order();al 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 un 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 código incorrecto en this == nullptrlugar de usar una solución adecuada.

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

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

Es una práctica peligrosa, ya que el ajuste de punteros debido al recorrido de la jerarquía de clases puede convertir un 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 una clase base: no puede derivar de nada y no puede derivarse de. Estamos pasando rápidamente de lo práctico a lo feo .

En términos prácticos, el código no tiene por qué 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 (por ejemplo, root es nullptr), esta solución aún se basa en un comportamiento indefinido al llamar a traverse_in_order con un nullptr.

Si el árbol está vacío, también conocido como nulo Node* root, no se supone que debe llamar a ningún método no estático en él. Período. Está perfectamente bien tener un código de árbol similar a C que toma 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 C-with-objects de escribir dicho código sigue siendo mucho más agradable en el mundo de C ++, porque al menos puede ser seguro para los tipos. Básicamente, el nulo thises una micro-optimización, con un campo de uso tan estrecho, que rechazarlo está perfectamente bien en mi humilde opinión. Ninguna API pública debería depender de un valor nulo this.

El documento de cambios claramente dice que esto es peligroso porque rompe una cantidad sorprendente de código de uso frecuente.

El documento no lo llama peligroso. Tampoco afirma que rompa una cantidad sorprendente de código . Simplemente señala algunas bases de código populares que, según afirma, se basan 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 C ++ práctico?

Si el código práctico de C ++ se basa en un comportamiento indefinido, los cambios en ese comportamiento indefinido pueden romperlo. Esta es la razón por la que se debe evitar la UB, incluso cuando un programa que se basa en ella parece funcionar como se esperaba.

¿Existen patrones particulares en los que programadores descuidados o desinformados se basan en este comportamiento indefinido en particular?

No sé si es un anti- patrón generalizado, pero un programador desinformado podría pensar que pueden arreglar su programa para que no se bloquee haciendo:

if (this)
    member_variable = 42;

Cuando el error real es eliminar la referencia a un puntero nulo en otro lugar.

Estoy seguro de que si el programador está lo suficientemente desinformado, podrá crear patrones (anti) más avanzados que se basen en esta UB.

No puedo imaginarme a nadie escribiendo if (this == NULL)porque eso es muy antinatural.

Puedo.

Parte del código "práctico" (forma divertida de deletrear "buggy") que se rompió se veía 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ícita if (this == nullptr)o if (!p) return;cheques. Algunos casos eran simplemente funciones que no tenían acceso a ninguna variable miembro, por lo que 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, hay 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, por lo tanto, en la práctica, no es necesario acceder al puntero en absoluto. Entonces, con algunos compiladores parece funcionar bien: no hay una segfault para desreferenciar nulos, p->valid()es falso, por lo que el código llama do_something_else(p), que busca punteros nulos, por lo que no hace nada. No se observa ningún accidente o comportamiento inesperado.

Con GCC 6 todavía recibe la llamada a p->valid(), pero el compilador ahora infiere de esa expresión que pdebe ser no nula (de lo contrario p->valid(), sería un comportamiento indefinido) y toma nota de esa información. El optimizador usa 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 nulo, por lo que inserta 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 elimina la referencia a un puntero nulo, por lo que el código que anteriormente parecía funcionar deja de funcionar.

En este ejemplo, el error está en func, que debería haber verificado nulo primero (o las personas que llaman 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 un caso en el que el compilador diga "ah, el programador probó este puntero contra nulo, lo eliminaré solo por molestar". Lo que sucede es que varias optimizaciones corrientes, como la propagación del rango de valores y la alineación, se combinan para hacer que esas verificaciones sean redundantes, porque 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 de una función, y el puntero no se cambia antes de un punto posterior B en la misma función, entonces sabe que también es no nulo en B. Cuando ocurre la inserción los puntos A y B pueden ser en realidad fragmentos de código que originalmente estaban en funciones separadas, pero ahora se combinan en un solo fragmento de código, y el compilador puede aplicar su conocimiento de que el puntero no es nulo en más lugares. Esta es una optimización básica, pero muy importante, y si los compiladores no lo hicieran, el código diario sería considerablemente más lento y la gente se quejaría de las ramas innecesarias para volver a probar las mismas condiciones repetidamente.