[ovs-dev] [PATCH v2] Makefile.am: Add clang static analysis support

Bodireddy, Bhanuprakash bhanuprakash.bodireddy at intel.com
Sat Jul 2 22:02:37 UTC 2016


>-----Original Message-----
>From: Ben Pfaff [mailto:blp at ovn.org]
>Sent: Saturday, July 2, 2016 9:31 PM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
>Cc: dev at openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v2] Makefile.am: Add clang static analysis
>support
>
>On Sat, Jul 02, 2016 at 08:14:02PM +0000, Bodireddy, Bhanuprakash wrote:
>> >-----Original Message-----
>> >From: Ben Pfaff [mailto:blp at ovn.org]
>> >Sent: Saturday, July 2, 2016 6:14 PM
>> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
>> >Cc: dev at openvswitch.org
>> >Subject: Re: [ovs-dev] [PATCH v2] Makefile.am: Add clang static
>> >analysis support
>> >
>> >On Tue, Jun 28, 2016 at 05:09:13PM +0100, Bhanuprakash Bodireddy wrote:
>> >> Clang Static Analyzer is a source code analysis tool to find bugs.
>> >> This patch adds make target to trigger static analysis using below
>commands.
>> >>
>> >> ./boot.sh
>> >> ./configure --with-dpdk(in case of DPDK datapath) make
>> >> clang-analyze scan-view --host=<ip address> --port <PORT>
>> >>   $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1>
>> >>   --allow-all-hosts
>> >>
>> >> Results can be viewed on browser: http://<ip address>:<PORT>/
>> >>
>> >> v1->v2:
>> >> * Change the output directory to tests/clang-analyzer-results
>> >> * Remove '-j' make option, This might potentially hang some system
>> >>   while spawning infinite jobs.
>> >>
>> >> Signed-off-by: Bhanuprakash Bodireddy
>> >> <bhanuprakash.bodireddy at intel.com>
>> >
>> >I'd tend to write this a little differently, maybe like this:
>> >
>> >clang-analyze: clean
>> >	@which clang scan-build >/dev/null 2>&1 || \
>> >	  (echo "Unable to find clang/scan-build, Install
>> >clang,clang-analyzer packages"; exit 1)
>> >	@$(MKDIR_P) "$(srcdir)/tests/clang-analyzer-results"
>> >	@scan-build -o $(srcdir)/tests/clang-analyzer-results --use-
>> >analyzer=/usr/bin/clang $(MAKE)
>> >.PHONY: clang-analyze
>>
>> This is fine for me.
>>
>> >
>> >But it doesn't work for me anyway.  When I run it from a build tree
>> >configured to use clang, I get the following:
>> >
>> >    make  all-recursive
>> >    make[2]: Entering directory '/home/blp/nicira/ovs/_clang'
>> >    Making all in datapath
>> >    make[3]: Entering directory '/home/blp/nicira/ovs/_clang/datapath'
>> >    make[4]: Entering directory '/home/blp/nicira/ovs/_clang/datapath'
>> >    make[4]: Leaving directory '/home/blp/nicira/ovs/_clang/datapath'
>> >    make[3]: Leaving directory '/home/blp/nicira/ovs/_clang/datapath'
>> >    make[3]: Entering directory '/home/blp/nicira/ovs/_clang'
>> >      CC       lib/aes128.lo
>> >    gcc: error: unrecognized command line option '-Wthread-safety'
>> >    gcc: error: unrecognized command line option '-Qunused-arguments'
>> >    gcc: error: unrecognized command line option '-fno-caret-diagnostics'
>> >    Makefile:4179: recipe for target 'lib/aes128.lo' failed
>> >    make[3]: *** [lib/aes128.lo] Error 1
>> >    make[3]: Leaving directory '/home/blp/nicira/ovs/_clang'
>> >    Makefile:4831: recipe for target 'all-recursive' failed
>> >    make[2]: *** [all-recursive] Error 1
>> >    make[2]: Leaving directory '/home/blp/nicira/ovs/_clang'
>> >    Makefile:2749: recipe for target 'all' failed
>> >    make[1]: *** [all] Error 2
>> >    make[1]: Leaving directory '/home/blp/nicira/ovs/_clang'
>> >    scan-build: Removing directory
>> >'/home/blp/nicira/ovs/tests/clang-analyzer-
>> >results/2016-07-02-101251-14653-1' because it contains no reports.
>> >    scan-build: No bugs found.
>> >    Makefile:5845: recipe for target 'clang-analyze' failed
>> >    make: *** [clang-analyze] Error 1
>> >
>> >Alternatively, if I run it from a build tree configured to use GCC, I
>> >get the
>> >following:
>> >
>> >    make  all-recursive
>> >    make[2]: Entering directory '/home/blp/nicira/ovs/_build'
>> >    Making all in datapath
>> >    make[3]: Entering directory '/home/blp/nicira/ovs/_build/datapath'
>> >    make[4]: Entering directory '/home/blp/nicira/ovs/_build/datapath'
>> >    make[4]: Leaving directory '/home/blp/nicira/ovs/_build/datapath'
>> >    make[3]: Leaving directory '/home/blp/nicira/ovs/_build/datapath'
>> >    make[3]: Entering directory '/home/blp/nicira/ovs/_build'
>> >      CC       lib/aes128.lo
>> >      CC       lib/backtrace.lo
>> >      CC       lib/bfd.lo
>> >    In file included from ../lib/bfd.h:24:0,
>> >                     from ../lib/bfd.c:16:
>> >    ../lib/packets.h: In function 'eth_addr_invert':
>> >    ../lib/packets.h:237:5: error: 'for' loop initial declarations
>> >are only allowed in
>> >C99 or C11 mode
>> >    ../lib/packets.h:237:5: note: use option -std=c99, -std=gnu99,
>> >-std=c11 or -
>> >std=gnu11 to compile your code
>>
>> I have tested this on F22 and didn't see this issue.  But when I tested it now
>on Ubuntu 14.04 LTS I could see the issue you reported here.
>> Adding CFLAGS="-std=gnu99" to make should fix this issue and I tested
>> it now on Ubuntu 14.04
>>
>> Can you try the below patch and see if you can generate clang analysis
>report properly this time?
>
>This change seems rather arbitrary.  Why doesn't the analysis phase use the C
>compiler flags that have already been found to be correct for compilation?

I tested the slightly tweaked patch updated below (added '--use-cc' flag) for both clang and gcc. 
I would let the user specify the CFLAGS at the configuration phase especially with gcc compiler on some distributions.
What do you think of this approach?

Clang:
./configure CC=clang --with-dpdk=$DPDK_BUILD
make clang-analyze

gcc:
./configure CC=gcc --with-dpdk=$DPDK_BUILD CFLAGS="-std=gnu99"
make clang-analyze

+clang-analyze: clean
+       @which clang scan-build >/dev/null 2>&1 || \
+               (echo "Unable to find clang/scan-build, Install clang,clang-analyzer packages"; exit 1)
+       @$(MKDIR_P) "$(srcdir)/tests/clang-analyzer-results"
+       @scan-build -o $(srcdir)/tests/clang-analyzer-results --use-cc=$(CC) --use-analyzer=/usr/bin/clang $(MAKE)
+.PHONY: clang-analyze
+
+

Regards,
Bhanuprakash.



More information about the dev mailing list