I haven't been keeping up with the code, but after Marvin's comments
about excess warnings I did try building "compiler/c" with GCC 5.3,
Clang 3.8, and ICC 16.0.1.
Particularly with Clang, the noise from the warnings was overwhelming.
But after a little more inspection, it turned out that most of these
were an interaction between Clang and ccache
(https://ccache.samba.org/).
If you happen to be using ccache (generally highly recommended) you
should check out the instructions here:
http://petereisentraut.blogspot.com/search/label/ccache
In case the compiler versions I'm using are different, I'll point out
a couple warnings besides the "unused variables" and "inline" that
might be worth paying attention to:
Clang: ../src/CFCPython.c:141:9: warning: string literal of length
8477 exceeds maximum length 4095 that ISO C99 compilers are required
to support [-Woverlength-strings] "static PyObject*\n"
GCC: ../src/CFCPython.c: In function ‘S_gen_callbacks’:
../src/CFCPython.c:352:9: warning: string length ‘8477’ is greater
than the length ‘4095’ ISO C99 compilers are required to support
[-Woverlength-strings]
ICC also reports a number of "Inlining inhibited by limit max-size" and
"Inlining inhibited by limit max-total-size" warnings. I don't think
there's much that can or should be done about these.
ICC does offer some nice optimization diagnostics. Assuming most
people aren't using it, I've attached a sample here for
"-qopt-report=4 -qopt-report-phase cg,ipo,loop,par,vec" which might be
worth glancing at.
As Nick hints, there are a _lot_ of warnings that appear for all the
compilers if you add -Wconversion to CFLAGS. Note that these warnings
are in addition to -Wextra. For library code like this, it's probably
worth adding this flag, and fixing the warnings. While most are false
alarms, there are likely are some real bugs hiding in there.
I also tested with "-fsanitize=address". This reported a couple
dozen potential leaks, although I don't know if they are real or just
testing artifacts. I've attached the report in case someone with
more knowledge wants to check.
I tjhen tested to see if adding "-fsanitize=undefined" (GCC and
Clang), and "-fsanitize=memory" (Clang only) reported anything, but
these came up clean. I only tested for 'compiler/c', though, might
be worth trying the other directories.
The various -fsanitize options are really useful, and surprisingly
underutilized. it probably would be good practice to start testing
with them. Adding them to CFLAGS is a bit difficult in this project,
but using "CC='clang -fsanitize=address' configure" seems to work.
Hope this is helpful, and sorry I'm sending just suggestions rather
than actual patches.
--nate
On Sat, Mar 12, 2016 at 6:28 AM, Nick Wellnhofer <[email protected]> wrote:
> On 12/03/2016 00:11, Marvin Humphrey wrote:
>>
>> There are more compiler
>> warnings than I'd prefer, but I don't think those should block the release
>> at
>> this point.
>
>
> On my Linux/i386 setup, the Clownfish build only complains about a couple of
> unused variables in the CFCPython code and an unused function in the
> generated callbacks. I don't get any warnings for Lucy.
>
> But I haven't tested on OS X and on 64-bit for a while. So I just built
> everything on OS X and there are a couple of warnings that I didn't see
> before, mainly because of redefining "inline" for MSVC in the cmark code.
> That's my fault and I'll fix that in the 0.5 branch.
>
> I'm also aware that there are many unchecked 64-to-32-bit conversions in the
> Lucy code after we switched to size_t indices in Vector and Hash. Some
> compilers complain loudly about that, but it's not trivial to fix. I think
> it would be best to use size_t indices in the Lucy codebase as well.
>
>> LICENSE, NOTICE, CONTRIBUTING.md, INSTALL.md, and README.md all looked
>> good in
>> a spot check -- except for one thing. The copyright notice in NOTICE
>> still
>> reads 2014 and should have been updated to 2016. This isn't the end of
>> the
>> world though, and I'm still willing to vote +1 despite it.
>
>
> That's a stupid oversight on my part. It's not a big deal to prepare another
> release candidate, but I'm on vacation the next week. If anyone wants to
> build and upload an RC2 in the next days, I'd be grateful. But I'm also OK
> with releasing RC1 as is.
>
> Nick
=================================================================
==12139==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 2107 byte(s) in 8 object(s) allocated from:
#0 0x2b9af595a44a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9444a)
#1 0x418781 in CFCUtil_wrapped_malloc ../src/CFCUtil.c:258
Direct leak of 432 byte(s) in 3 object(s) allocated from:
#0 0x2b9af595a5b1 in __interceptor_calloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x945b1)
#1 0x418a1a in CFCUtil_wrapped_calloc ../src/CFCUtil.c:274
Direct leak of 144 byte(s) in 2 object(s) allocated from:
#0 0x2b9af595a44a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9444a)
#1 0x41f9f9 in yyalloc ../src/CFCLexHeader.c:2006
#2 0x41f9f9 in yy_scan_buffer ../src/CFCLexHeader.c:1750
Direct leak of 72 byte(s) in 3 object(s) allocated from:
#0 0x2b9af595a76a in realloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9476a)
#1 0x418a91 in CFCUtil_wrapped_realloc ../src/CFCUtil.c:291
Indirect leak of 1968 byte(s) in 37 object(s) allocated from:
#0 0x2b9af595a5b1 in __interceptor_calloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x945b1)
#1 0x418a1a in CFCUtil_wrapped_calloc ../src/CFCUtil.c:274
Indirect leak of 1897 byte(s) in 2 object(s) allocated from:
#0 0x2b9af595a44a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9444a)
#1 0x41fbd5 in yyalloc ../src/CFCLexHeader.c:2006
#2 0x41fbd5 in yy_scan_bytes ../src/CFCLexHeader.c:1799
Indirect leak of 1266 byte(s) in 89 object(s) allocated from:
#0 0x2b9af595a44a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9444a)
#1 0x418781 in CFCUtil_wrapped_malloc ../src/CFCUtil.c:258
Indirect leak of 208 byte(s) in 11 object(s) allocated from:
#0 0x2b9af595a76a in realloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9476a)
#1 0x418a91 in CFCUtil_wrapped_realloc ../src/CFCUtil.c:291
SUMMARY: AddressSanitizer: 8094 byte(s) leaked in 155 allocation(s).
