Semgrep: Semgrep fails to parse C function calls at the top level

[clintgibler]

Describe the bug
Semgrep appears to fail to parse C code with function calls at the top level scope.

~~~c
// This fails to parse
SOME_FUNC(1);

// This is OK
int main() {
SOME_FUNC(1);
}
~~~

To Reproduce
https://semgrep.dev/s/clintgibler:c-top-level-fxn-call

Expected behavior
I would expect this to parse.

Screenshots
If applicable, add screenshots to help explain your problem.

What is the priority of the bug to you?
P2

[clintgibler]

A user reported this bug based on something they were trying to do.

I know we're not actively supporting C, curious if @aryx thinks this is trivial to fix.

[jynik]

Hi there - just wanted join in to elaborate a bit further and provide some more real-world context.

This pattern can be found in upstream U-Boot code. SOME_FUNC() corresponds to a function-like macro which actually expands to a structure declaration; essentially, it's registering some command handlers using a "linker list" (a linker-generated array).

So -- looks wild and crazy, but I can attest to the fact that it is indeed real. :)

Example:
https://gitlab.denx.de/u-boot/u-boot/-/blob/v2020.10/cmd/axi.c#L352

U_BOOT_CMD() macro definition can be found here:

• https://gitlab.denx.de/u-boot/u-boot/-/blob/master/include/command.h#L368
• https://gitlab.denx.de/u-boot/u-boot/-/blob/master/include/command.h#L325
• https://gitlab.denx.de/u-boot/u-boot/-/blob/v2020.10/include/linker_lists.h#L33

Encountered this while looking to write a rule whose first step is to identify all U_BOOT_CMD(...) instances.

Happy to kick some ideas around with regard to working with the semgrep API and passing in appropriate preprocessed code via a call-out to cpp.

[aryx]

Complex issue. See my paper about it 10 years ago: https://link.springer.com/content/pdf/10.1007/978-3-642-00722-4_9.pdf

pfff and semgrep-core provides a way to pass a configuration file to partially evaluate some cpp macros that leads to
parsing errors, but this is not currently surfaced in the semgrep CLI.

Of course you can also call on your own the preprocessor and pass the resulting file to semgrep, but that means you need to know how to call correctly the preprocessor (which -I, which -D), in which case you lose some of the advantages of semgrep (the ability to run without building the project).

[aryx]

BTW @clintgibler toplevel calls in C are not accepted by the C grammar (here the macro indeed expands to a full function definition, not a toplevel function call).

[aryx]

Another problem with calling the preprocessor is that the resulting code is not what the user sees, so the user may write patterns that involves macros that would not be found in the resulting code.

[jynik]

@aryx - Agreed and everything makes sense; all of the challenges are things I've been encountering when attempting to develop some analysis tooling, sans semgrep. Will look into the literature you mentioned to better appreciate the challenges.

pfff and semgrep-core provides a way to pass a configuration file to partially evaluate some cpp macros that leads to
parsing errors, but this is not currently surfaced in the semgrep CLI.

I'm interested in building some tooling atop of the semgrep API, so this seems like a perfectly reasonable approach for me. At some point, the CLI just becomes quick-access to the API, and as user I feel that it is not unreasonable to be told to just write some quick scripts atop of the API. :)

Been looking at relevant grammar and mulling over what it would take to write a pure-Python and best-effort C preprocessor (re-invent a wheel?) that can fail gracefully (warn, rather than fail) and also provide traceability back to original source. Feels like this is going to be a "must-have" if I'm ever going to build some source analysis tooling... using semgrep or anything else.

[aryx]

Well there's code inside pfff/lang_cpp/parsing/pp_token.ml that implements a partial C preprocessor, that is you can
pass to semgrep-core a config file with some macros, and semgrep-core will expands those macros during parsing and
mark them specially in the AST so it knows where it comes from. It's just that it's hard to automatically generate a minimal config file that will enable 100% parsing of the code. Right now there's some tooling around helping detect the most problematic macros, but it's still lots of manual work to setup the config file for each project (see the pdf I linked to which discuss this config file).

[jynik]

Thank you, @aryx. Over the coming months I'll dive in deeper and explore a bit.

Just some unsolicited thoughtsfrom the security practitioner user side of things (specifically, one focused on lower-level firmware, drivers, OS) -- I'd say that there's definitely an acceptable threshold of "good enough" in terms of parsing that folks like me would want. In many situations, I'd be happy collecting some warnings and having the parser push onward, either skipping portions of code that are "uninteresting" or just giving me some feedback of how I could further iteratively tune things to get the results I need.

Would appreciate if you'd keep this ticket open, although renaming the title if needed -- that was a miscommunication on my part when chatting with @clintgibler out-of-band.

[aryx]

We're now also using the tree-sittter based C parser which does better error recovery than the one in pfff/, so the example you posted now actually parse with this new parser.

[aryx]

I'll close this issue since the example now works, but feel free to open a new issue with more discussions on parsing C and cpp.