[C++] Rare crash

[Verwijderd]

Hey fellow programmers,

Vandaag ben ik tegen een stuk code aangelopen, wat in eerste instantie crashte, uiteindelijk heb ik het verholpen door een reference te veranderen( zie code ). Nu is mijn vraag wat er is er mis met het gebruiken van een reference ( of pointer ) in de volgende code:? Ikzelf zie namelijk niet zo 123 waar het nu precies misgaat.

C++:

nodeList->push_back( parentNode );

    //push back parentnode
    nodeStack.push( 0 );

    bool leafReached = false;

    while( !nodeStack.empty() )
    {
        bool continueTraversal          = true;

        //fetch parent!
        int parentIndex                 = nodeStack.top();
        nodeStack.pop();


        /*
            Replace with the following to crash the program:
            alDepthNode& curNode            = (*nodeList)[ parentIndex ];

        */

        alDepthNode curNode         = (*nodeList)[ parentIndex ];

        //stop when max depth reached
        if( maxDepth > 0 )
        {
            if( curNode.m_depth == maxDepth )
                continueTraversal = false;
        }
        // stop when leafnode reached
        else if(curNode.m_node.m_bitSet & OCTREE_IS_LEAF )
        {
            continueTraversal = false;
        }
        if( continueTraversal )
        {
            int childCount              = 0;
            int sliceId             = curNode.m_node.getSliceId();
            int sliceOffset             = curNode.m_node.getOffsetInSlice();


            alRunTimeSlice *slicePtr        = m_treePtr->loadSlice( sliceId );

            for( int i = 0; i < 8; i++ )
            {
                if(  curNode.m_node.hasChild( i ) )
                {
                    alDepthNode childNode;

                    childNode.m_parentIndex         = parentIndex;
                    childNode.m_bounds          = alOctree::boundsForId( curNode.m_bounds, i );
                    childNode.m_bounds.print();

                    childNode.m_depth           = curNode.m_depth + 1;
                    childNode.m_node            = slicePtr->getNodeAt( sliceOffset + i );
                    //add to nodelist
                    nodeList->push_back( childNode );

                    int childIdx                = nodeList->size() - 1;
                    //store child index
                    curNode.m_childIndices[i]       = childIdx;

                    //push on stack
                    nodeStack.push( childIdx );

                }
            }

            //This is necessary because using references crashes??? the program
            (*nodeList)[parentIndex] = curNode;
        }
    }

[PhoenixT]

Die integer parentIndex is geen geheugenadres, maar gewoon een integer. Als je die dus dereferenced naar een nodeList object gaat dat fout. Zorg dus dat je een goede pointer hebt?

[ Voor 10% gewijzigd door PhoenixT op 15-01-2011 19:28 ]

[MLM]

een reference weghalen maakt dat je een kopie van je object maakt, en dat bewerkt (ie, je origineel is hetzelfde)
is dat wat je wilt bereiken?

ik ken de context van je programma niet, maar om dit te debuggen heb ik wel meer info nodig

Die integer parentIndex is geen geheugenadres, maar gewoon een integer. Als je die dus dereferenced naar een nodeList object gaat dat fout. Zorg dus dat je een goede pointer hebt?

Waar word een int gedereferenced dan? Ik zie nergens een evil-cast, alleen indexing (van een list... O(N), anyone?)

[ Voor 39% gewijzigd door MLM op 15-01-2011 19:33 ]

[MBV]

Ik neem aan dat nodeList een vector is? Als je een push_back doet gaat de vector groeien. Op dat moment worden alle objecten gekopieerd naar een nieuwe locatie, en wordt de oude locatie vrijgegeven. Mag jij raden wat er gebeurt met je reference

Ik weet niet wat hiervoor de gangbare oplossing is in C++. Je zou kunnen overwegen om niet op de referentie operaties uit te voeren, maar zo: nodeList->get(x).m_node *. Dan wordt altijd het goede geheugenadres gebruikt. Of je zou kunnen overwegen om de bewerking op nodelist (push_back etc) pas te doen nadat je de referentie weer buiten de scope is.

Disclaimer: ik heb geen stacktrace gezien, ik weet het type van nodeList niet eens , dus geen garantie

*) hoe roep je ook al weer operator[] op een pointer aan?

[ Voor 4% gewijzigd door MBV op 15-01-2011 19:43 ]

[Verwijderd]

Aaargh..., ja natuurlijk dat is het, ik heb het waarschijnlijk te snel gecode zonder over de gevolgen na te denken!! anyway thanks;)

MBV schreef op zaterdag 15 januari 2011 @ 19:39:
Ik neem aan dat nodeList een vector is? Als je een push_back doet gaat de vector groeien. Op dat moment worden alle objecten gekopieerd naar een nieuwe locatie, en wordt de oude locatie vrijgegeven. Mag jij raden wat er gebeurt met je reference

Ik weet niet wat hiervoor de gangbare oplossing is in C++. Je zou kunnen overwegen om niet op de referentie operaties uit te voeren, maar zo: nodeList->get(x).m_node. Dan wordt altijd het goede geheugenadres gebruikt. Of je zou kunnen overwegen om de bewerking op nodelist (push_back etc) pas te doen nadat je de referentie weer buiten de scope is.

Disclaimer: ik heb geen stacktrace gezien, ik weet het type van nodeList niet eens , dus geen garantie

[remco_k]

MLM schreef op zaterdag 15 januari 2011 @ 19:30:
ik ken de context van je programma niet, maar om dit te debuggen heb ik wel meer info nodig

Juistem. Om een beetje fatsoenlijk mee te kunnen denken is meer info nodig.
Zoals wat voor type nodeList is. En zo zijn er wel meer. De context en doel van deze code zijn ook een handige.
Maar vooral: definieer 'crash'. Als je dat tegen mij zegt, denk ik aan minstens 3 situaties:
Een access violation op regel x, een 'floep' (persoonlijke vakterm voor: de applicatie verdwijnt zonder blikken of blozen of foutmelding) of elke willekeurige andere foutmelding.

Met andere woorden: wat is het probleem en op welke regel komt het voor?

[MBV]

blijft er nog een vraag van mijn kant over: is er geen mooiere manier om operator[] op een pointer naar een vector aan te roepen? Mag je (*myVector + 42).x doen? * MBV zou waarschijnlijk myVector->at(42) aanroepen.

@Remco en anderen: jullie hebben helemaal gelijk, maar welke stl-klasses implementeren nog meer operator[] en push_back? En tja, crash duidt meestal op geheugenfouten.

[ Voor 27% gewijzigd door MBV op 15-01-2011 19:48 ]

[MLM]

mja, die nodeList is prolly een nodeVector dan (als het STL is), maar ik heb wel list implementaties gezien met indexing.

@MBV: een pointer indexen gaat uit van een array, waarvan de expressie ptr[N] een lvalue naar het N-de element in die array is

wat ik niet begrijp is dat je eerst (regel 23) een kopie pakt van een item in je list/vector, de kopie bewerkt en dan terugkopieert (regel 71), wat volgens mij EXACT hetzelfde is als een reference gebruiken, maar dan inefficient (disclaimer, uitgaande van een niet-exotisch type als item-type)

anyway, waar crashte het (welke regel)? met welke error/exception/code? wat waren de waardes van je locals toen het gebeurd? zit je niet stiekum je collecties te editten (insert/erase) in je loop?

[Verwijderd]

@MLM ik gebruik inderdaad stl( vectors, en stack ) , waarom het crashte merkte MBV al op, als de nodelist resized wordt is namelijk je reference( of pointer ), niet meer geldig en gebeuren er rare dingen. Dus het is in dit geval wel wenselijk, om een kopie te maken van de data, en later weer terug te kopieren;)

[Soultaker]

Als je problemen hebt met standard library containers en je gebruikt GCC, dan is het vaak nuttig om te hercompileren met -D_GLIBCXX_DEBUG als compileroptie. Dat zorgt ervoor dat er allerlei debugcode wordt geactiveerd die corruptie door verkeerd gebruik van container classes vaak kan detecteren. (Onder andere dereferencing van singular iterators, lezen voorbij het einde van de inhoud van een vector; dat soort dingen.)

Verder zou ik je aanraden om je code eens in een debugger te draaien zodat je kunt zien wáár en waarom het programma precies crasht. (Vaak is het dan het beste om eerst zonder optimalisaties te compileren, om te voorkomen dat inlining en andere optimalisaties delen van de code overslaan of verplaatsen, wat nogal verwarrend kan zijn.)

Het is mijns inziens niet de bedoeling om hier code neer te plempen met de melding "hij crasht!" terwijl het één minuut werk is om te achterhalen op welke regel dan precies, wat anderen al veel kan helpen om het probleem te analyseren.

[remco_k]

Verwijderd schreef op zondag 16 januari 2011 @ 11:57:
@MLM ik gebruik inderdaad stl( vectors, en stack ) , waarom het crashte merkte MBV al op, als de nodelist resized wordt is namelijk je reference( of pointer ), niet meer geldig en gebeuren er rare dingen. Dus het is in dit geval wel wenselijk, om een kopie te maken van de data, en later weer terug te kopieren;)

Niet om het één of ander, maar waarom beantwoord je andere vragen niet?
Zoals deze van mij (en ook anderen?)

remco_k schreef op zaterdag 15 januari 2011 @ 19:45:
[...]
Met andere woorden: wat is het probleem en op welke regel komt het voor?

[Verwijderd]

@remco_k, sorry als ik onduidelijk was, het was gisteren al laat, en heb 2 uur op deze bug gezeten op het einde zie je door de bomen het bos niet meer!! Uiteraard heb ik het ook gecompiled/gerund met een debug versie, sometimes crashte het programma na 10 iteraties sometimes na 1000. Maar het maakt natuurlijk sense dat pointers die je 'delete' nog een tijdje kan gebruiken, totdat het OS beslist om ze defitinief te overschrijven.

Dit is een stuk code die een octree optimalizeert voor het gebruik van een (CPU) ray-caster, basically het herschikt nodes die in op de harde schijf staan in een lineair formaat( in cpu mem ). Hope that helps;)

[MLM]

okay, dan is het mij dus duidelijk.
voor de rest:

regel 58 is het probleem, waar de vector (en dus niet lijst) word geresized (push_back), en je dus een undefined reference (op regel 19) gebruikte. de kopie van regel 23 (kopie maken) en regel 71 (kopie terugschrijven) lost jouw probleem op

iterator debugging had niet geholpen hier, die worden niet gebruikt

een fix voor de originele code is om je vector van te voren te resizen naar de grootte die je uiteindelijk gaat gebruiken (mits je die natuurlijk van te voren weet), dan zal push_back() nooit resizen, en bespaar je mogelijk overbodig geheugen. dit gaat ook tevens de beste performance opleveren omdat de items in de vector niet gekopieerd zullen worden bij elke resize. (een goede STL implementatie zal echt niet bij elke push_back gaan resizen, maar het kan best een aantal keer gebeuren).

indien niet mogelijk omdat je de uiteindelijk size van de vector niet weet, de huidige code kan nog wat performance winnen door ipv een kopie, een pointer naar het item in de list te gebruiken, en na elke push_back() die pointer refreshen (dus op regel 59 in dit geval). dat bespaart je twee assignments voor elke iteratie van de loop op regel 8, maar de daadwerkelijke winst is nogal afhankelijk van hoe "zwaar" het type alDepthNode is.

[Soultaker]

De voor de hand liggende oplossing om het probleem met references te omzeilen is om aan de huidige node te refereren aan de hand van z'n index, zoals je nu ook al doet voor child nodes.

(Feitelijk gebruik je dan overal (*nodeList)[parentIndex] waar je nu curNode gebruikt. Mooi is anders.)