Segmentation fault during array access

Question

I want to draw 1 integer from [1,10] a large number of times, then check how many times each integer appears. I wrote this code, it is compiling but showing segmentation fault. Can you, please, point out where the problem is?

#include <iostream>
#include <random>
#include <array>

int main(){
        std::random_device rd;
        std::mt19937 dre(rd());
        std::uniform_int_distribution <int> di(1,10);
        std::array<int,10> count{0};
        for(int i=0;i<10000;++i)
        {
                int rand=di(dre);
                count[rand]++;
        }
        for (int foo: count){
                count[foo]/=10000;
                std::cout << foo << " " << count[foo] << std::endl;
        }
}

This is a problem that can be easily solved, or at least drastically narrowed down by debugging. Please take the time to learn how to use a debugger. It's immensely helpful and will you save you many hours of just staring at your code. I cannot stress enough how important is to able to debug your own code. Learn it sooner rather than later! — bolov
– bolov, Commented Oct 26, 2017 at 9:56
If I use the -std=gnu++11 compiler option no Segfault occurs - which compiler are you using? — Thrawn
– Thrawn, Commented Oct 26, 2017 at 9:58
@Thrawn as shown in the answers the problem is the out of bounds access. This is Undefined Behavior, which means anything can happen. It can segfault, it can not segfault. — bolov
– bolov, Commented Oct 26, 2017 at 10:01
If, after people have solved the problem that your question was about, you find a new problem, then post a new question. — underscore_d
– underscore_d, Commented Oct 26, 2017 at 10:27

Steve · Accepted Answer · 2017-10-26 10:14:31Z

4

If you define an array consisting of 10 elements, like you do here:

std::array<int,10> count{0};

Then the array will have indices 0-9. So count will range from count[0] to count[9].

However, here:

count[rand]++;

when rand is 10, you're trying to access count [10], which doesn't exist.

To answer the followup question in your edit, you're looping round and creating 10000 random numbers, here:

 for(int i=0;i<10000;++i)
 {
     int rand=di(dre);

And as you're picking between 10 different numbers, you'd expect the count of each one to be approximately 1000, with a uniform distribution.

However, when you come to print the results, you divide each count by 10000:

count[foo]/=10000;

So this means it's highly likely that each count is now approx 0.1. As you're storing it in an int, this gets rounded down to zero.

edited Oct 26, 2017 at 10:14

answered Oct 26, 2017 at 9:53

Steve

1,72813 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

anotherone Over a year ago

@Steve Thanks. The problem was I was doing an integer division inside the for loop. That is why I was getting 0. :P

gonutz · Accepted Answer · 2017-10-26 09:53:40Z

2

You call count[rand] where count has 10 items, meaning indices in the range of 0..9 but rand is in the range of 1..10 so every once in a while it will call count[10] which causes your seg fault.

Make the distribution use [0..9] instead: std::uniform_int_distribution <int> di(0,9);

answered Oct 26, 2017 at 9:53

gonutz

5,7123 gold badges28 silver badges43 bronze badges

Comments

Quentin · Accepted Answer · 2017-10-26 09:53:45Z

2

With for (int foo: count), foo is equal to each element in count in turn. You need to use foo on its own in the loop instead of count[foo], or use an explicit for loop if you need the index.

Additionally, std::uniform_int_distribution is bounds-inclusive, so you need to initialize it with 0, 9 instead of 1, 10 to index into your ten-elements count.

answered Oct 26, 2017 at 9:53

Quentin

63.5k7 gold badges139 silver badges199 bronze badges

1 Comment

anotherone Over a year ago

I have changed the distribution. But I could not understand your comment about the for loop. I changed it a bit, can you kindly see my comment and explain your correction a bit?

Ron · Accepted Answer · 2017-10-26 10:14:14Z

2

Your uniform distribution should be defined as:

std::uniform_int_distribution <int> di(0, 9);

because your array elements are indexed from 0 to 9. As-is your rand variable will eventually become greater than 9 at which point you are reading out of bounds thus causing undefined behavior. Even if rand stays within boundaries your range based for loop will exhibit UB because foo there is the value of the actual array element yet used as an index. Should be passed by reference instead:

for (int& foo : count) {
    foo /= 10000;
    std::cout << foo << '\n';
}

Also if you are using C++11 then you will need double braces for the std::array initializer here:

std::array<int, 10> count{ { 0 } };

edited Oct 26, 2017 at 10:14

answered Oct 26, 2017 at 9:53

Ron

15.7k4 gold badges39 silver badges51 bronze badges

2 Comments

anotherone Over a year ago

Thank you for your answer. I was missing the the reference and also doing an integer division inside the for loop that is why I was getting 0 for all. Everything is fixed now.

anotherone Over a year ago

For the compiler I was using g++-7 so double braces should not be a problem I think.

Collectives™ on Stack Overflow

Segmentation fault during array access

4 Answers 4

1 Comment

Comments

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related