TrustInSoft / tis-interpreter

An interpreter for finding subtle bugs in programs written in standard C
565 stars 28 forks source link

Unexpected error: long string literal in initializer #102

Open ch3root opened 8 years ago

ch3root commented 8 years ago

Source code:

#define a00 "a"
#define a01 a00 a00
#define a02 a01 a01
#define a03 a02 a02
#define a04 a03 a03
#define a05 a04 a04
#define a06 a05 a05
#define a07 a06 a06
#define a08 a07 a07
#define a09 a08 a08
#define a10 a09 a09
#define a11 a10 a10
#define a12 a11 a11
#define a13 a12 a12
#define a14 a13 a13
#define a15 a14 a14
#define a16 a15 a15
#define a17 a16 a16

int main()
{
  char s[] = a17;
}

tis-interpreter (d4961d3fa420ebd5b895e7c54636acae8ada344e) output:

[value] Analyzing a complete application starting at main
[value] Computing initial state
[value] Initial state computed
[kernel] Current source was: test.c:23
         The full backtrace is:

         Unexpected error (Stack overflow).
         Please report at https://github.com/TrustInSoft/tis-interpreter/issues
pascal-cuoq commented 8 years ago

On Linux, the stack size limit can be raised thus:

$ ulimit -S -s 10000000 
$ tis-interpreter.sh t.c
[value] Analyzing a complete application starting at main
[value] Computing initial state
[value] Initial state computed

There remains a performance issue that I am looking at.

pascal-cuoq commented 8 years ago

Note: if you look for them further you'll find other algorithms with what appear like sub-optimal behavior, but ~2^17 bytes is about the limit of what's reasonable for a local array. I can hard-code a limit or even let the user choose it, but there is no point in allowing a program that allocates an array of size PTRDIFF_MAX on the stack to be analyzed, as it won't execute correctly (with a quality implementation it will crash cleanly).

ch3root commented 8 years ago

Ok. I'm not that concerned by limits or sub-optimal behavior. At least, now. I guess everything could be moved from stack to heap but I don't know if it's worth it. Perhaps it's better to wait for a real-world example.

But the tis-interpreter output looks somewhat scary right now. First, it talks about "Unexpected error". Then it directs to "Please report". I did:-)

As for searching similar cases it seems easy. Almost as easy as crashing clang:-) E.g.:

#define a00 ;
#define a01 a00 a00
#define a02 a01 a01
#define a03 a02 a02
#define a04 a03 a03
#define a05 a04 a04
#define a06 a05 a05
#define a07 a06 a06
#define a08 a07 a07
#define a09 a08 a08
#define a10 a09 a09
#define a11 a10 a10
#define a12 a11 a11
#define a13 a12 a12
#define a14 a13 a13
#define a15 a14 a14
#define a16 a15 a15
#define a17 a16 a16
#define a18 a17 a17
#define a19 a18 a18

int main()
{
  a19;
}

or:

#define a00 + 1
#define a01 a00 a00
#define a02 a01 a01
#define a03 a02 a02
#define a04 a03 a03
#define a05 a04 a04
#define a06 a05 a05
#define a07 a06 a06
#define a08 a07 a07
#define a09 a08 a08
#define a10 a09 a09
#define a11 a10 a10
#define a12 a11 a11
#define a13 a12 a12
#define a14 a13 a13
#define a15 a14 a14
#define a16 a15 a15

int main()
{
  a16;
}
pascal-cuoq commented 8 years ago

I only meant that examples with large local arrays were not very interesting to us. Your two new examples are very welcome bug reports because they are not of that nature and can point to inefficiencies, as the first one did.

We could detect that a crash happened because of a stack overflow and not suggest to write a bug report, if that would help make it non-scary. We could suggest to raise the stack limit, or systematically raise it in tis-interpreter.sh.

@jakub-zwolakowski The first example seems to point to naïve use of List.append to grow a list by its end. Would it be possible to build the reverse of currentFunctionFDEC.sbody.bstmts during elaboration and reverse it at the end only? The second example is not as interesting: according to C syntax precedence, the expression is comb-shaped; binary functions cannot simply be made tail-recursive, and heap allocation is more expensive than stack allocation, so I'm not sure there is something to improve other than printing an error message telling the user to get a larger stack. The argument local_env of doExp appears to be the same for all recursive calls, so perhaps we can shorten the stack frame by one slot.