Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frontend issue on conditional operator in C #522

Closed
skcho opened this issue Dec 5, 2016 · 5 comments
Closed

Frontend issue on conditional operator in C #522

skcho opened this issue Dec 5, 2016 · 5 comments

Comments

@skcho
Copy link
Contributor

skcho commented Dec 5, 2016

int foo ()
{
  int x = ({1 ? 1 : 1;});
  return 0;
}

Current front-end parses the above example into an incorrect control flow graph, where return node is unreachable from the entry node. This issue was initially observed when compiling strspn() with gcc -O2 and clang -O2 as follows.

#include <string.h>

int foo (char *s)
{
  size_t n_slash = strspn (s, "/");
  return 0;
}
@akotulski
Copy link
Contributor

Thanks for reporting - we did take some shortcuts translating statement expressions and if you pair it with conditional operators it may end up here.

On the other hand, how did you make source of strspnavailable for infer? Usually we use models for C library

@skcho
Copy link
Contributor Author

skcho commented Dec 6, 2016

Thank you for the reply. We just downloaded gnu gzip 1.8 and run

./configure
infer -a checkers -- make

The default setting of the Makefile uses the -O2 option, and then strspn is inlined.

@akotulski
Copy link
Contributor

I looked more into the issue and found the problem: clang AST is looking differently when compiled with -O2 and we don't want that. Infer has a model for strspn which we should use.
I'm going to fix issue with -O2 and I'll leave "conditional inside statement expression" problem

facebook-github-bot pushed a commit that referenced this issue Dec 13, 2016
Summary:
This is to prevent clang from changing AST to make it more performant and less readable.
Reported in #522

+ unrelated `refmt` fix

Reviewed By: jvillard

Differential Revision: D4319731

fbshipit-source-id: 176dfcf
@skcho
Copy link
Contributor Author

skcho commented Dec 14, 2016

Thanks. It works now in our case.

@akotulski
Copy link
Contributor

I just tested the repro case and it works now, probably because of e366b0d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants