-3

First time, I coded like this (I was making a trie) and when I called add("911"); I got a segmentation fault at there when *str == '1'

struct Node {
    int check, check2;
    Node* chd[10];
    Node() : check(0), check2(0), chd{0, } {}
    ~Node() {
        for (int i = 0; i < 10; i++)
            if (chd[i])
                delete chd[i];
    }
    int add(char* str) {
        if (!str[0])
            return 0;
        if (chd[*str-'0'] == 0)          // here I got SIGSEGV
            chd[*str-'0'] = new Node();
        ...
        return chd[*str-'0']->add(++str);
    }
}

then I fixed that part like this

        int i = *str-'0';
        if (chd[i] == 0)
            chd[i] = new Node();

there was no error.

What's the difference? What made the first code make error?

(I was solving ACM-ICPC > Regionals > Europe > Northwestern European Regional Contest > Nordic Collegiate Programming Contest > NCPC 2007 A)

nngm
  • 129
  • 1
  • 1
  • 8
  • please show a [mvce], weird changes in behaviour in one place are often caused by undefined behaviour elsewhere in the code. – Alan Birtles Feb 27 '19 at 14:14
  • Possible duplicate of [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Raedwald Feb 27 '19 at 14:14
  • 1
    Please [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). And note that the cause of the problem might not be where the crash actually happens. And besides a [mcve], can you please include the call-stack when the crash happens, as well as the values of involved variables (most importantly what is `*str`)? – Some programmer dude Feb 27 '19 at 14:17

2 Answers2

2

return chd[*str-'0']->add(++str);

contains undefined behavoir as you are modifying and using str unsequenced.

Your 'fix' has the classic behavoir of moving something around enough that the undefined behavoir is hidden. Remeber that Undefined Behavoir is undefined. It doens't have to crash. It doesn't even have to make sense. And most scarily it can do what you expect... for the time being at least.

A simple change is:

return chd[*str-'0']->add(str+1);

Mike Vine
  • 9,468
  • 25
  • 44
0

This line has undefined behaviour

return chd[*str-'0']->add(++str);

because the modification and the use of str are unsequenced.

The quick fix is (don't say ++ when you mean + 1):

return chd[*str-'0']->add(str + 1);

but you can also avoid the problem by computing the relevant index just once (it sounds like this is how you fixed the problem, and it's the better solution):

int add(char* str) {
    if (!str[0])
        return 0;
    char c = str[0] - '0';
    if (chd[c] == 0)
        chd[c] = new Node();
    ...
    return chd[c]->add(str + 1); // And ++str would also work...
molbdnilo
  • 64,751
  • 3
  • 43
  • 82