vector going out of bounds depending on a swap statement

Question

For this piece of code-

vector<int> v{ 417, 929, 845, 462, 675, 175, 73, 867, 14, 201, 777, 407, 80, 882, 785, 563, 209, 261, 776, 362, 730, 74, 649, 465, 353, 801, 503, 154, 998, 286, 520, 692, 68, 805, 835, 210, 819, 341, 564, 215, 984, 643, 381, 793, 726, 213, 866, 706, 97, 538, 308, 797, 883, 59, 328, 743, 694, 607, 729, 821, 32, 672, 130, 13, 76, 724, 384, 444, 884, 192, 917, 75, 551, 96, 418, 840, 235, 433, 290, 954, 549, 950, 21, 711, 781, 132, 296, 44, 439, 164, 401, 505, 923, 136, 317, 548, 787, 224, 23, 185, 6, 350, 822, 457, 489, 133, 31, 830, 386, 671, 999, 255, 222, 944, 952, 637, 523, 494, 916, 95, 734, 908, 90, 541, 470, 941, 876, 264, 880, 761, 535, 738, 128, 772, 39, 553, 656, 603, 868, 292, 117, 966, 259, 619, 836, 818, 493, 592, 380, 500, 599, 839, 268, 67, 591, 126, 773, 635, 800, 842, 536, 668, 896, 260, 664, 506, 280, 435, 618, 398, 533, 647, 373, 713, 745, 478, 129, 844, 640, 886, 972, 62, 636, 79, 600, 263, 52, 719, 665, 376, 351, 623, 276, 66, 316, 813, 663, 831, 160, 237, 567, 928, 543, 508, 638, 487, 234, 997, 307, 480, 620, 890, 216, 147, 271, 989, 872, 994, 488, 291, 331, 8, 769, 481, 924, 166, 89, 824, -4, 590, 416, 17, 814, 728, 18, 673, 662, 410, 727, 667, 631, 660, 625, 683, 33, 436, 930, 91, 141, 948, 138, 113, 253, 56, 432, 744, 302, 211, 262, 968, 945, 396, 240, 594, 684, 958, 343, 879, 155, 395, 288, 550, 482, 557, 826, 598, 795, 914, 892, 690, 964, 981, 150, 179, 515, 205, 265, 823, 799, 190, 236, 24, 498, 229, 420, 753, 936, 191, 366, 935, 434, 311, 920, 167, 817, 220, 219, 741, -2, 674, 330, 909, 162, 443, 412, 974, 294, 864, 971, 760, 225, 681, 689, 608, 931, 427, 687, 466, 894, 303, 390, 242, 339, 252, 20, 218, 499, 232, 184, 490, 4, 957, 597, 477, 354, 677, 691, 25, 580, 897, 542, 186, 359, 346, 409, 655, 979, 853, 411, 344, 358, 559, 765, 383, 484, 181, 82, 514, 582, 593, 77, 228, 921, 348, 453, 274, 449, 106, 657, 783, 782, 811, 333, 305, 784, 581, 746, 858, 249, 479, 652, 270, 429, 614, 903, 102, 378, 575, 119, 196, 12, 990, 356, 277, 169, 70, 518, 282, 676, 137, 622, 616, 357, 913, 161, 3, 589, 327 }; 
int n{v.size()};
for(int i{};i<n;i++){
    if(A.at(i)<=n && A.at(i)>0){
        swap(A.at(A.at(i)-1),A.at(i));
        if(A.at(i)!=A.at(A.at(i)-1)){
            i--;
        }
    }
}

I'm getting a out of bound exception.
terminate called after throwing an instance of 'std::out_of_range'
what(): vector::_M_range_check: __n (which is 588) >= this->size() (which is 418)

I don't see with the included conditions if(A.at(i)<=n && A.at(i)>0), how could I possibly go out of bounds.
Also, if I put the swap statement inside the body of the next if (above i--), I do not get the exception, and the loop runs fine.

And when you used your debugger to run your program, what did you see? This is precisely what a debugger is for. If you don't know how to use a debugger this is a good opportunity to learn how to use it to run your program one line at a time, ***monitor all variables and their values as they change***, and analyse your program's logical execution flow. Knowing how to use a debugger is a required skill for every C++ developer, no exceptions. With your debugger's help you should be able to quickly find all bugs in this and all future programs you write, without having to ask anyone for help. — Sam Varshavchik, Jun 16 '20 at 03:17
@SamVarshavchik I've recently learnt how to debug, but I'm worried about the large size of the vector used here. I thought it would take too much of time to debug than finding the error myself or reaching out for help. — anonymous38653, Jun 16 '20 at 03:25
This is not large, by any means. `find * -name '*.[CH]' -print | xargs cat | wc -l` tells me I have approximately 260,000 lines of code in my current project, and I routinely fire up my debugger to sift my way through all of this. The best way to learn how to debug is to learn it by yourself. You won't learn absolutely crucial debugging skills just by asking others. You have to burn a lot of time dealing with it yourself. That's the only way. — Sam Varshavchik, Jun 16 '20 at 03:28
`A.at(A.at(i)-1)` means `swap()` i using the value at position `i` as its index. That's probably not what you want. — , Jun 16 '20 at 03:40

score 2 · Accepted Answer · answered Jun 16 '20 at 03:44

In the first loop iteration:

i is 0
A.at(i) is 417
A.at(A.at(i)-1) is A.at(416) which is 589
Then you swap A.at(0) with A.at(416), so now A.at(0) is 589 and A.at(416) is 417.
Then you execute if(A.at(i)!=A.at(A.at(i)-1)). But since A.at(i)-1 is 588, so this does if(589 != A.at(588)) which accesses out of bounds.

Perhaps you meant to do the if test first, i.e.:

if(A.at(i)<=n && A.at(i)>0){
   if(A.at(i)!=A.at(A.at(i)-1)){
        swap(A.at(A.at(i)-1),A.at(i));
        i--;
    }
}

IMO it would be easier to read to use some aliases, e.g.:

auto& current = A.at(i);

if ( current > 0 && current <= n )
{
    auto& other = A.at(current - 1);
    if ( current != other )
    {
        swap(current, other);
        --i;
    }
}

vector going out of bounds depending on a swap statement

1 Answers1