Обсуждение: Clang 3.3 Analyzer Results
I've been tasked with a quick acceptance check of Postgres for an upcoming project. It's a quick check, so its limited to Clang's analyzer and sanitizers. The analyzer is reporting some findings, and some of the findings look legitimate. For example, it looks like there's a double `free` occurring in streamutil.c (around line 115). Here's a screen capture of it under scan-view: http://postimg.org/image/3ph4hkyav/. From the capture, it looks like `password` should be set to NULL after `free` because Clang found a path to get back to the top of the loop (which will free `password` again`). There's some others of interest, too. For example, Divide by Zero and Buffer Overflows. Here's the index.html from the scan-view report: http://postimg.org/image/tn2ovjout/. The scan-view tar ball is a 5.5 megabytes in size (its HTML based with a lot of mouse over markup to help understand flows), and I'm not sure the bug reporter will take it. Plus the developers may not want it added to the bug reporter. Would someone know the best way to get this to the right folks? Thanks in advance. (And sorry reporting to pgsql-general - the developer list states emails must go elsewhere first). Jeff
Hi, On 11 Listopad 2013, 7:33, Jeffrey Walton wrote: > I've been tasked with a quick acceptance check of Postgres for an > upcoming project. It's a quick check, so its limited to Clang's > analyzer and sanitizers. > > The analyzer is reporting some findings, and some of the findings look > legitimate. > > For example, it looks like there's a double `free` occurring in > streamutil.c (around line 115). Here's a screen capture of it under > scan-view: http://postimg.org/image/3ph4hkyav/. From the capture, it > looks like `password` should be set to NULL after `free` because Clang > found a path to get back to the top of the loop (which will free > `password` again`). Probably. From a quick glance at streamutil.c, it seems to have other issues too, not just the double free. For example it does a free on the password, but then uses the value for dbpassword (not sure if that code path actually is possible - maybe it always gets into the branch with password prompt). > There's some others of interest, too. For example, Divide by Zero and > Buffer Overflows. Here's the index.html from the scan-view report: > http://postimg.org/image/tn2ovjout/. > > The scan-view tar ball is a 5.5 megabytes in size (its HTML based with > a lot of mouse over markup to help understand flows), and I'm not sure > the bug reporter will take it. Plus the developers may not want it > added to the bug reporter. > > Would someone know the best way to get this to the right folks? > > Thanks in advance. (And sorry reporting to pgsql-general - the > developer list states emails must go elsewhere first). IMHO pgsql-hackers is the right audience for reports like this. The 'must ask somewhere else first' is meant for regular questions that are not that closely related to postgresql development, and are likely to be answered in the generic mailing lists. Please, upload the HTML report somewhere and post a link. If it's easier to the clang analysis, maybe post instructions on how to do that. regards Tomas
On Mon, Nov 11, 2013 at 2:00 AM, Tomas Vondra <tv@fuzzy.cz> wrote: > >> Would someone know the best way to get this to the right folks? >> >> Thanks in advance. (And sorry reporting to pgsql-general - the >> developer list states emails must go elsewhere first). > > IMHO pgsql-hackers is the right audience for reports like this. The 'must > ask somewhere else first' is meant for regular questions that are not that > closely related to postgresql development, and are likely to be answered > in the generic mailing lists. > > Please, upload the HTML report somewhere and post a link. If it's easier > to the clang analysis, maybe post instructions on how to do that. The instructions are kind of easy when they are given as a recipe. They get a little more involved when they steps have to be explained. The recipes are below and include building Clang 3.3 from sources. The Clang recipe lacks a couple of copies - for example, `asan_symbolize.py` needs to be copied into $PREFIX because `make install` does not do it. Keep the build directory around until you have everything you need. The Analyzer is invoked with scan-build. Its used when compiling the package because it performs static analysis. The Santizers are invoked with the runtime flags. They are used with the `check` program because they perform dynamic analysis. The more self test the better. Jeff ############## # Clang 3.3 # Run from a scratch directory, delete when finished wget http://llvm.org/releases/3.3/llvm-3.3.src.tar.gz wget http://llvm.org/releases/3.3/cfe-3.3.src.tar.gz wget http://llvm.org/releases/3.3/compiler-rt-3.3.src.tar.gz # wget http://llvm.org/releases/3.3/lldb-3.3.src.tar.gz tar xvf llvm-3.3.src.tar.gz cd llvm-3.3.src/tools tar xvf ../../cfe-3.3.src.tar.gz mv cfe-3.3.src clang # tar xvf ../../lldb-3.3.src.tar.gz # mv lldb-3.3.src/ lldb cd .. cd projects tar xvf ../../compiler-rt-3.3.src.tar.gz mv compiler-rt-3.3.src/ compiler-rt cd .. ./configure --enable-optimized --prefix=/usr/local make -j4 sudo make install # Install does not install scan-build and scan-view # Perform the copy, and/or put them on-path sudo mkdir /usr/local/bin/scan-build sudo cp -r tools/clang/tools/scan-build /usr/local/bin sudo mkdir /usr/local/bin/scan-view sudo cp -r tools/clang/tools/scan-view /usr/local/bin ############## # Scan-view make distclean export CC="/usr/local/bin/clang" export CXX="/usr/local/bin/clang++" /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang ./configure /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang make ############## # Sanitizers make distclean export DYLD_FALLBACK_LIBRARY_PATH=/usr/local/lib/clang/3.3/lib/darwin/ export CC=/usr/local/bin/clang export CXX=/usr/local/bin/clang++ export CFLAGS="-g3 -fsanitize=address -fsanitize=undefined" export CXXFLAGS="-g3 -fsanitize=address -fsanitize=undefined -fno-sanitize=vptr" ./configure make make check 2>&1 | asan_symbolize.py
[moving the discussion to pgsql-hackers] Jeffrey Walton <noloader@gmail.com> wrote: > The Analyzer is invoked with scan-build. Its used when compiling > the package because it performs static analysis. > > The Santizers are invoked with the runtime flags. They are used > with the `check` program because they perform dynamic analysis. > The more self test the better. Thanks for the recipes! > ############## > # Scan-view > > make distclean > > export CC="/usr/local/bin/clang" > export CXX="/usr/local/bin/clang++" > > /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang ./configure > > /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang make I'm currently capturing a text version of all the warnings from this. Will gzip and post when it finishes. It's generating a lot of warnings; I have no idea how many are PostgreSQL problems and how many are false positives; will just post the whole set FWIW. I am using the 3.4 development nightly snapshot with these commands: scan-build --use-analyzer=/usr/bin/clang ./configure --silent --prefix=$PWD/Debug --enable-debug --enable-cassert --enable-depend--with-libxml --with-libxslt --with-openssl --with-perl --with-python scan-build --use-analyzer=/usr/bin/clang make -s world > ############## > # Sanitizers > > make distclean > > export DYLD_FALLBACK_LIBRARY_PATH=/usr/local/lib/clang/3.3/lib/darwin/ > export CC=/usr/local/bin/clang > export CXX=/usr/local/bin/clang++ > export CFLAGS="-g3 -fsanitize=address -fsanitize=undefined" > export CXXFLAGS="-g3 -fsanitize=address -fsanitize=undefined -fno-sanitize=vptr" > > ./configure > > make > > make check 2>&1 | asan_symbolize.py I haven't tried this yet, but will have a go at it after I capture the other. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 11/11/13, 1:33 AM, Jeffrey Walton wrote: > The analyzer is reporting some findings, and some of the findings look > legitimate. We have been tracking clang scan-build results for some time, and fixed quite a few of them. Most of the remaining ones are false positives. Maybe there are still a few that could be fixed, but certainly scan-build is not suitable as an acceptance test of the PostgreSQL source at this point.