2017-01-27 2 views
42

J'ai une classe de modèle AsIterator qui prend un type numérique comme dans cet exemple juste un int, et le convertit en un iterator (++ et -- augmenter et diminuer le nombre et operator* retourne juste une référence à celui-ci).itérateur inverse retourne les déchets lors de l'optimisation

Cela fonctionne bien à moins qu'il est enveloppé dans un std::reverse_iterator et compilé avec une optimisation (-O est suffisant). Lorsque j'optimise le binaire, le compilateur supprime l'appel de dereference au reverse_iterator et le remplace par une valeur bizarre. Il faut noter qu'il reste fait le bon nombre d'itérations. C'est juste la valeur obtenue par reverse iterator qui est garbage.

Consultez le code suivant:

#include <iterator> 
#include <cstdio> 

template<typename T> 
class AsIterator : public std::iterator<std::bidirectional_iterator_tag, T> { 
    T v; 
public: 
    AsIterator(const T & init) : v(init) {} 

    T &operator*() { return v; } 

    AsIterator &operator++() { ++v; return *this; } 
    AsIterator operator++(int) { AsIterator copy(*this); ++(*this); return copy; } 
    AsIterator &operator--() { --v; return *this; } 
    AsIterator operator--(int) { AsIterator copy(*this); --(*this); return copy; } 

    bool operator!=(const AsIterator &other) const {return v != other.v;} 
    bool operator==(const AsIterator &other) const {return v == other.v;} 
}; 

typedef std::reverse_iterator<AsIterator<int>> ReverseIt; 

int main() { 
    int a = 0, b = 0; 
    printf("Insert two integers: "); 
    scanf("%d %d", &a, &b); 
    if (b < a) std::swap(a, b); 

    AsIterator<int> real_begin(a); 
    AsIterator<int> real_end(b); 
    for (ReverseIt rev_it(real_end); rev_it != ReverseIt(real_begin); ++rev_it) { 
     printf("%d\n", *rev_it); 
    } 
    return 0; 
} 

Cela devrait boucle supposingly à partir du nombre inséré plus haut au plus bas et de les imprimer, comme dans cette course (compilé avec -O0):

Insert two integers: 1 4 
3 
2 
1 

Ce que je reçois avec -O est à la place:

Insert two integers: 1 4 
1 
0 
0 

Vous pouvez try it online here; les nombres peuvent varier mais ils sont toujours "faux" lors de l'optimisation du binaire.


Ce que j'ai essayé:

  • hardcoding les entiers d'entrée est suffisant pour produire le même résultat;
  • le problème persiste avec gcc 5.4.0 et clang 3.8.0, également lors de l'utilisation libC++;
  • faire tous les objets const (c'est-à-dire renvoyer const int &, et déclarer toutes les variables en tant que telles) ne le fixe pas; En utilisant le reverse_iterator de la même manière sur par exemple certains std::vector<int> fonctionne très bien;
  • Si j'utilise simplement AsIterator<int> pour une boucle normale avant ou arrière cela fonctionne bien.
  • dans mes tests, la 0 constante qui est imprimé est en fait hardcoded par le compilateur, les appels à printf tous ressembler à ceci lors de la compilation avec -S -O:
movl $.L.str.2, %edi # .L.str.2 is "%d\n" 
    xorl %eax, %eax 
    callq printf 

Compte tenu de la cohérence des clang et Le comportement de gcc ici Je suis assez sûr qu'ils le font bien et j'ai mal compris, mais je ne peux vraiment pas le voir.

+0

Fait intéressant, '-fsanitize = undefined' "fixe" il. http://coliru.stacked-crooked.com/a/ddcc7bbf26ab9da2 –

+2

Encore un point de données: g ++ 4.4 imprime '3 2 1' même avec' -O2' tandis que 4.7 affiche '0 0 0' pour moi. Cela crie de comportement indéfini mais je ne le vois pas encore. –

+1

@BaummitAugen Dans le contexte d'origine où ce MVCE est extrait, l'ajout d'une instruction print dans 'operator *' le corrigerait, mais pas dans cet exemple. –

Répondre

44

Regarder la mise en œuvre de libstdC++std::reverse_iterator révèle quelque chose d'intéressant:

/** 
    * @return A reference to the value at @c --current 
    * 
    * This requires that @c --current is dereferenceable. 
    * 
    * @warning This implementation requires that for an iterator of the 
    *   underlying iterator type, @c x, a reference obtained by 
    *   @c *x remains valid after @c x has been modified or 
    *   destroyed. This is a bug: http://gcc.gnu.org/PR51823 
    */ 
    _GLIBCXX17_CONSTEXPR reference 
    operator*() const 
    { 
    _Iterator __tmp = current; 
    return *--__tmp; 
    } 

La section @warning nous dit qu'une exigence du type iterator sous-jacente est que *x doit rester valable même après l'itérateur sous-jacente est modifiée /détruit.

En regardant le mentioned bug link révèle des informations plus intéressantes:

à un moment donné entre C++ 03 et C++ 11 la définition de reverse_iterator opérateur :: * a été modifié pour clarifier ce point, faisant libstdC++ 's mauvaise mise en œuvre. La norme dit maintenant:

[Note: Cette opération doit utiliser une variable de membre auxiliaire plutôt qu'une variable temporaire pour éviter de renvoyer une référence qui persiste au-delà de la durée de vie de son itérateur associé. (Voir 24.2.) Note -end]

commentaire par Jonathan Wakely (2012)

il ressemble un bug ... mais à la fin du sujet:

La définition de reverse_iterator a été ramenée à la version C++ 03, qui n'utilise pas de membre supplémentaire, de sorte que "steaking itérateurs" ne peut pas être utilisé avec reverse_iterator.

commentaire par Jonathan Wakely (2014)

Il semble donc que l'utilisation std::reverse_iterator avec "Stashing itérateurs" conduit effectivement à UB.


En regardant la DR 2204: "reverse_iterator should not require a second copy of the base iterator" précise encore la question:

Cette note 24.5.1.3.4 [reverse.iter.op.star]/2:

[ Remarque: Cette opération doit utiliser une variable de membre auxiliaire plutôt qu'une variable temporaire pour éviter de renvoyer une référence qui persiste au-delà de la durée de vie de son itérateur associé. (Voir 24.2.) Note -end]

[ma note: Je pense que la note ci-dessus fixerait votre problème UB]

est incorrecte parce que ces implémentations iterator sont exclues par 24.2. 5 [forward.iterators]/6, où il est dit:

Si a et b sont tous deux dereferenceable, puis a == b si et seulement si * a * et b sont liés au même objet.

+2

À droite, également pertinent à partir du [lien de bug] (https://gcc.gnu.org/bugzilla/show_bug.cgi ? id = 51823): * "Ce que vous avez écrit n'est pas un itérateur, c'est un générateur." * - "reverse_iterator" ne fonctionnerait pas correctement avec une référence non persistante renvoyée par l'itérateur interne. –

+2

Voir https://timsong-cpp.github.io/lwg-issues/2360 –