>-----Original Message-----
>From: Ben Pfaff [mailto:b...@ovn.org]
>Sent: Sunday, July 3, 2016 1:12 AM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com>
>Cc: dev@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v2] Makefile.am: Add clang static analysis
>support
>
>On Sat, Jul 02, 2016 at 10:02:37PM +0000, Bodireddy, Bhanuprakash wrote:
>>
>> >-----Original Message-----
>> >From: Ben Pfaff [mailto:b...@ovn.org]
>> >Sent: Saturday, July 2, 2016 9:31 PM
>> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com>
>> >Cc: dev@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:b...@ovn.org]
>> >> >Sent: Saturday, July 2, 2016 6:14 PM
>> >> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com>
>> >> >Cc: dev@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.bodire...@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
>
>The point I'm making is that the configure process already chooses a correct
>set of CFLAGS and already allows the user to override them,
Completely Agree Ben, Using GCC 4.8.4 , the configure process in the default
case
(i) ./configure --with-dpdk=/root/dpdk-16.04/x86_64-native-linuxapp-gcc, sets
'CFLAGS= -g -O2' and 'CC' as below.
$ cat Makefile | grep "^CFLAGS ="
CFLAGS = -g -O2
$ cat Makefile | grep "^CC ="
CC = $(if $(C),env REAL_CC="gcc -std=gnu99" CHECK="$(SPARSE) -I
$(top_srcdir)/include/sparse $(SPARSEFLAGS) $(SPARSE_EXTRA_INCLUDES) " cgcc
$(CGCCFLAGS),gcc -std=gnu99)
clang-analyze target in this case fails as below.
$make clang-analyze
(Omitted clean ouput here)
scan-build: unrecognized option '-std=gnu99'
make: *** [clang-analyze] Error 1
To get around this error, when CFLAGS is overridden from cmdline as
(ii) ./configure CC=gcc --with-dpdk=$DPDK_BUILD CFLAGS="-std=gnu99", sets
'CFLAGS = -std=gnu99' and 'CC' as below.
$ cat Makefile | grep "^CFLAGS ="
CFLAGS = -std=gnu99
$ cat Makefile | grep "^CC ="
CC = $(if $(C),env REAL_CC="gcc" CHECK="$(SPARSE) -I
$(top_srcdir)/include/sparse $(SPARSEFLAGS) $(SPARSE_EXTRA_INCLUDES) " cgcc
$(CGCCFLAGS),gcc)
Clang-analyze target works perfectly in this case.
> so why doesn't
>the clang-analyzer invocation use them?
Clang-analyzer picks the CFLAGS automatically when invoked, output snippet is
pasted from case (ii).
'/bin/bash ./libtool --tag=CC --mode=compile
/usr/share/clang/scan-build/ccc-analyzer -DHAVE_CONFIG_H -I. -I ./include -I
./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra
-Wno-sign-compare -Wpointer-arith -Wformat-security -Wswitch-enum
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wmissing-prototypes
-Wmissing-field-initializers -fno-strict-aliasing -mssse3
-I/root/dpdk-16.04/x86_64-native-linuxapp-gcc/include -D_FILE_OFFSET_BITS=64
-std=gnu99 -MT lib/dpif.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif.lo
lib/dpif.c'
I have also attached the configuration logs for case (i) and (ii). This was
tested using GCC version 4.8.4 on Ubuntu 14.04
In case (i), config log shows: checking for gcc option to accept ISO C99...
-std=gnu99
In case (ii), config log shows : checking for gcc option to accept ISO C99...
none needed
This isn't observed when using GCC version 5.3.1 on F22. Am I missing
something?
Regards,
Bhanu Prakash.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
