[clang][StaticAnalyzer] Crash on loop unrolling mode

[danix800]

godbolt

// test.c
// unroll-loops=true

void test_escaping_on_var_before_switch_case_no_crash(int c) {
  switch (c) {
    int i;
  case 0: {
    for (i = 0; i < 16; i++) {}
    break;
  }
  }
}

crash dump:

Reached root without finding the declaration of VD
UNREACHABLE executed at /home/xxxxx/Sources/llvm-project-main/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:248!

[llvmbot]

@llvm/issue-subscribers-clang-static-analyzer

Author: Ding Fei (danix800)

```c // test.c // unroll-loops=true

void test_escaping_on_var_before_switch_case_no_crash(int c) {
switch (c) {
int i;
case 0: {
for (i = 0; i < 16; i++) {}
break;
}
}
}

crash dump:
```c
Reached root without finding the declaration of VD
UNREACHABLE executed at /home/xxxxx/Sources/llvm-project-main/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:248!

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Ding Fei (danix800)

```c // test.c // unroll-loops=true

void test_escaping_on_var_before_switch_case_no_crash(int c) {
switch (c) {
int i;
case 0: {
for (i = 0; i < 16; i++) {}
break;
}
}
}

crash dump:
```c
Reached root without finding the declaration of VD
UNREACHABLE executed at /home/xxxxx/Sources/llvm-project-main/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:248!

[Da-Viper]

@danix800 please could you give some information on how to reproduce this as I am not sure how to test the issue

thanks

[steakhal]

@danix800 please could you give some information on how to reproduce this as I am not sure how to test the issue

thanks

Remember to use a build with assertions. I added a godbolt link to the summary, that will exercise this crash.

[Rajveer100]

@steakhal
I am currently working on this, regarding escaping loop counter for the above snippet, which of these does it fall into, considering I was able to reproduce the issue, I am looking for a fix:

// A loop counter is considered escaped if:
// case 1: It is a global variable.
// case 2: It is a reference parameter or a reference capture.
// case 3: It is assigned to a non-const reference variable or parameter.
// case 4: Has its address taken.

Also, what caused the Variable declaration failure (i.e, unassigned, hence leading to llvm_unreachable)?

[Rajveer100]

@steakhal
Could you give a brief outlook?

[steakhal]

@steakhal I am currently working on this, regarding escaping loop counter for the above snippet, which of these does it fall into, considering I was able to reproduce the issue, I am looking for a fix:

// A loop counter is considered escaped if:
// case 1: It is a global variable.
// case 2: It is a reference parameter or a reference capture.
// case 3: It is assigned to a non-const reference variable or parameter.
// case 4: Has its address taken.

This table makes sense to me.

Also, what caused the Variable declaration failure (i.e, unassigned, hence leading to llvm_unreachable)?

I'm not sure I understand this. To me, we have this issue about an unsupported feature flag, which is untested and crashing. I don't expect the majority of our users to set this flag, thus it falls out of scope to me, and I won't pursue fixing/investigating this.
However, I'm going to review the patches proposed for fixing stuff, even more so for fixing crashes (let it be under any feature flag). On the same token, I believe that every issue has two folds: 1) understanding why something happens, 2) fixing how it should happen instead. Both of these steps are for the patch author, and as a byproduct, once a PR is proposed, there should be a clear explanation/justification for the patch - including a test of course demonstrating the behavior the patch changes.
Consequently, how I look at my role here is that I make sure that the issue is reproducible and leave room for anyone interested to follow up and propose a patch if they want.

Of course, I'm here to discuss technical questions on a best-effort basis, so don't hesitate to ask if you have some.
I'm sorry if I missed some of the discussion, but I only do this in my limited free time.

[ranjanmangla1]

Hi, can anyone please assigned me this issue? would love to work on this & this would be my first or in llvm

[EugeneZelenko]

@ranjanmangla1: Just create pull request and mention it on this page.

[huang-me]

Hi, I am currently working on this. The crash seems to happen because we don't check the DeclStmt under any CompoundStmt when there's a SwitchStmt. Is it okay if I just iterate through all the CompoundStmts and check if the loop counter is declared within the SwitchStmt?