creachadair
commented
5 years ago This looks like a bug in the Java client, rather than the C++ driver (though there could also be a bug in the latter that triggers this).
Closed jjhenkel closed 5 years ago
creachadair
commented
5 years ago This looks like a bug in the Java client, rather than the C++ driver (though there could also be a bug in the latter that triggers this).
jjhenkel
commented
5 years ago Probably the latter: just checked and I can replicate using the python client library.
bzz
commented
5 years ago Indeed, this looks suspicious, thank you for detailed report @jjhenkel !
The simplest thing I can think of, in order to check if this has to do with Cpp driver is to start the only the driver and try parsing with bblfsh-cli binary.
docker run --rm -it -p 9432:9432 bblfsh/cpp-driver:v1.2.0 --log-level=debug
wget -O setproctitle.c https://github.com/bblfsh/cpp-driver/files/2916974/setproctitle.c.txt
bblfsh-cli setproctitle.cAnd as you mention, this does produce very similar logs (in details)
The file seems to be syntactically sound, according to Clang gcc setproctitle.c -fsyntax-only
I'm going to take a closer look and attempt a fix, but realistically if no one steps up before that - will only have time early next week.
Will re-assign back as soon as it's WIP to avoid the confusion.
bzz
commented
5 years ago Seems to be caused by an issue in the native driver while serializing a MacroReference of IASTPreprocessorIfndefStatement node, that according to CDT docs
represent a preprocessor #ifndef statement
According to .getMacroReference() docs it
Returns the macro reference, or null if the macro does not exist.
but the driver does not check for null, which seems to be a root cause of NPE.
If @juanjux does not have other suggestions, I'm going to localize minimal code snippet that triggers this, add it to the driver fixtures for reproducibility and then ~add a null check to~ wrap that case in new String(...).
bzz
commented
5 years ago Interestingly enough, distilling minimum reproducible examples is not that simple!
Right now there are 2 places in the setproctitle.c that hit this: L216 \w #ifndef SPT_MAXTITLE and L70 \w #ifndef SPT_MIN - both get null in the MacroReference, but if any of those ifdefs are just moved to some existing fixture, like preprocessor_ifdef.cpp - it all gets parsed nice and clean, (MacroReference gets set to SPT_MAXTITLE) and it's not reproducible any more :/
And just adding the whole setproctitle.c file would complicate bblfsh org lives, as it's part of the BSD-3-Clause project and all drivers are GPLv3 and so we would need to keep the NOTICE as part of the source code distribution artifacts for drivers that we release which seems like unnecessary legal risk of getting it wrong.
@creachadair as you know most probably know the language much better, would you be able to tell if I'm missing something here and how come that the same
#ifndef SPT_MAXTITLE
#define SPT_MAXTITLE 255
#endifin two different files, when parsed, produces different AST?
jjhenkel
commented
5 years ago I might be able to delta-debug that example. It'll take a bit to setup a script but once I have that I can send you a reduce file.
bzz
commented
5 years ago @jjhenkel nice idea! Thank you for suggestion, indeed it's a smallest example file so far.
There is a simple way to do that locally:
./fixturesgo run vendor/gopkg.in/bblfsh/sdk.v2/cmd/bblfsh-sdk/main.go testthat would build the native driver inside the container and run all test over the ./fixtures/*.
I also just realized that to speed every iteration out, one can also skip building the container every time and run the tests manually with
docker run --rm -u 501:20 -e BBLFSH_TEST_LOCAL=true -v "${PWD}/fixtures":/opt/fixtures --workdir /opt/driver/bin --entrypoint ./fixtures.test sha256:<latest image sha>This seems to be a minimal example (now I used a delta-debugging script and a simple grep-based tester so this may be another, unrelated, null-reference error; but my hope is it is the same one):
#if (defined __linux || defined __APPLE__)
#ifndef SPT_MINAlso, in case it helps in solving issues like this in the future, here was my setup:
engine v0.11.0 (downloaded the .tar for linux from the GitHub releases page).C file using engine and check for the error we were seeing:
#!/bin/bashDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
${DIR}/engine_linux_amd64/srcd \ parse uast \ --mode=native \ --lang=cpp \ "${1}" 2>&1 \ | grep -q 'Error serializing the AST to JSON: com.fasterxml.jackson.databind.JsonMappingException: (was java.lang.NullPointerException)'
exit $?
3. I applied `https://github.com/renatahodovan/picire` which has an implementation of the Delta Debugging algorithm from `A. Zeller: "Yesterday, my program worked", ESEC/FSE 1999`.
4. I ran the above tool using the following flags (where `setproctitle.c` was our starting input):
```bash
picire --input=setproctitle.c --test=wrapper.sh ddmin algorithm. That file was the wonderfully small 2-line example:
#if (defined __linux || defined __APPLE__)
#ifndef SPT_MINAll of that without having to guess and check which lines are causing the issue myself! (That's the real advantage of ddmin.) And picire seems to be a solid implementation.
jjhenkel
commented
5 years ago Whoops---my tester script doesn't ensure the minimized file is syntactically valid. Here's an example with that constraint applied:
#if E
#if
#ifdef
#endif
#endif
#endifAnd now here is the updated tester script and command line invocation for future reference:
tester.sh:
#!/bin/bash
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
${DIR}/engine_linux_amd64/srcd \
parse uast \
--mode=native \
--lang=cpp \
"${1}" 2>&1 \
| grep -q 'Error serializing the AST to JSON: com.fasterxml.jackson.databind.JsonMappingException: (was java.lang.NullPointerException)'
RC1=$?
gcc -fsyntax-only "${1}" &> /dev/null
RC2=$?
if [ "${RC1}" -eq "0" ]; then
if [ "${RC2}" -eq "0" ]; then
# Interseting! File failed to parse in the correct way
# but GCC says it is still snytactically valid
exit 0
fi
fi
# Not interesting (either parse worked/has different error/or file is
# no longer snytactically valid)
exit 1Invocation:
# --atom=both will reduced on lines and then chars
picire --atom=both --input=setproctitle.c --test=wrapper.shVery nice! 👏
Was able to manually boil down to a very similar
#if (defined __linux || defined __APPLE__)
#ifndef SPT_MIN
#define SPT_MIN(a, b) (((a) < (b))? (a) : (b))
#endif
#endif /* __linux || __APPLE__ */Ok, let me submit a fix
When trying to parse
./src/setproctitle.cfrom Redis I get the following error:Command used:
This also happens trying to parse
./src/debug.cand./src/config.h.Here are the exact files that caused errors during parsing:
setproctitle.c config.h debug.c