Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Denis V. Razumovsky
Please remove multiple warnings from compiler about optimisation,
variable conversion, signed overflow and many more potential errors.

1. Optimisation solutions from GCC:
If you would like to add extra warning attributes to compile SQLite with
gcc (and many others modern compilers) you will see the warnings about
pure, const and other optimisation suggestion. All those optimizations
can add more performance to final product due to various assembly
optimisations.

Static build of the last SQLite amalgamation version 3.20.0 with gcc -03
optimisation which uses the following parameters:
 -Wsuggest-attribute=const -Wsuggest-attribute=pure
-Wsuggest-attribute=noreturn -Wsuggest-attribute=format
-Wmissing-format-attribute

has  87 recommendations as for “pure” functions.

2. Error warnings:

With -Wall -Wextra -Wpedantic -Wshadow options we can see extra warnings
about potential errors as follows:

217 warnings “is not defined” like:

SQLite3.c:74:5: warning: "SQLITE_4_BYTE_ALIGNED_MALLOC" is not defined
[-Wundef]
 #if SQLITE_4_BYTE_ALIGNED_MALLOC
     ^

20 warnings “cast discards __attribute__((noreturn))” like:

SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
qualifier from pointer target type [-Wcast-qual]
   memcpy((void*)&aHdr[1], (const void*)&pWal->hdr, sizeof(WalIndexHdr));

768 warnings “may change the sign of the result” like:

SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
change the sign of the result [-Wsign-conversion]

75 warnings “signed overflow” like:
SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
reducing constant in comparison [-Wstrict-overflow]

As a result I would like to recommend to build SQLite with all extra
warning options. In total, I have counted unbelievable 2628 warnings of
potential errors without any efforts from my side.

3. How to reproduce:

a) amalgamation SQLite version 3.20.0

b) gcc version:
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
5.4.0-6ubuntu1~16.04.4'
--with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs
--enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++
--prefix=/usr --program-suffix=-5 --enable-shared
--enable-linker-build-id --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-libmpx --enable-plugin
--with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
--enable-gtk-cairo
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre
--enable-java-home
--with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64
--with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64
--with-arch-directory=amd64
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
--enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4)

с) Lubuntu 16.04

d) Files:
$ ls
main.c    Makefile sqlite3.c  sqlite3.h

$ cat Makefile
# Compiler flags
#
CC ?= cc

CFLAGS += -pipe -std=c11
CFLAGS += -fbuiltin
SQLITEFLAGS += -DSQLITE_THREADSAFE=0
SQLITEFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION
CFLAGS += $(SQLITEFLAGS)
EXE = sqlite_test
STATIC = -static
STRIP = -s

WFLAGS += -Wall -Wextra -Wpedantic -Wshadow
WFLAGS += -Wconversion -Wsign-conversion -Winit-self -Wunreachable-code
-Wformat-y2k
WFLAGS += -Wformat-nonliteral -Wformat-security -Wmissing-include-dirs
WFLAGS += -Wswitch-default -Wtrigraphs -Wstrict-overflow=5
WFLAGS += -Wfloat-equal -Wundef -Wshadow
WFLAGS += -Wbad-function-cast -Wcast-qual -Wcast-align
WFLAGS += -Wwrite-strings
WFLAGS += -Winline

ifneq ($(CC), clang)
WFLAGS += -Wlogical-op
CFLAGS += -finline-functions
CFLAGS += -flto
# Perform a number of minor optimizations that are relatively expensive.
CFLAGS += -fexpensive-optimizations
# Attempt to remove redundant extension instructions. This is especially
helpful for the x86-64 architecture, which implicitly zero-extends in
64-bit registers after writing to their lower 32-bit half.
CFLAGS += -free
endif

#
# Project files
#
SRCS = $(wildcard *.c)
HDRS = $(wildcard *.h)
# Exclude a file
OBJS = $(SRCS:.c=.o)

#
# Release build settings
#
RELDIR = release
RELEXE = $(RELDIR)/$(EXE)
RELOBJS = $(addprefix $(RELDIR)/, $(OBJS))
RELCFLAGS = -O3 -funroll-loops -DNDEBUG
RELCFLAGS += -march=native
# GCC only options
ifneq ($(CC), clang)
RELWFLAGS += -Wsuggest-attribute=const -Wsuggest-attribute=pure
-Wsuggest-attribute=noreturn -Wsuggest-attribute=format
-Wmissing-format-attribute
endif

.PHONY: all clean release

# Default build
all: release

#
# Release rules
#
release: $(RELEXE)

$(RELEXE): $(RELOBJS)
    $(CC) $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) $(STATIC)
$(STRIP) -o $(RELEXE) $^ $(LDFLAGS) $(RELLDFLAGS) $(TCMALLOC)
    echo "$(RELEXE) linked."

$(RELDIR)/%.o: %.c $(HDRS)
    mkdir -p $(RELDIR)
    $(CC) -c $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) -o $@ $<
    echo $<" compiled."


clean:
    rm -rf $(RELEXE) $(RELOBJS)
    test -d $(RELDIR) && rm -d $(RELDIR) || true


$ cat ./main.c
#include "sqlite3.h"
#include <stdio.h>

int main(void){

    printf("SQLite version: %s\n",sqlite3_libversion());

    return 0;

}

e) Run:
$ ./release/sqlite_test
SQLite version: 3.21.0


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Jens Alfke-2


> On Sep 29, 2017, at 1:07 AM, Denis V. Razumovsky <[hidden email]> wrote:
>
> Please remove multiple warnings from compiler about optimisation,
> variable conversion, signed overflow and many more potential errors.

This comes up a lot. SQLite is incredibly thoroughly tested* and performance-tuned, but the developers have chosen not to use compiler warnings to identify problems. It’s not the decision I’d have made personally, but I can’t argue with the results.

—Jens

* http://www.sqlite.org/testing.html
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Denis V. Razumovsky
I would like to draw attention to the document: "The Power of 10: Rules
for Developing Safety-Critical Code" from  NASA/JPL Laboratory.
https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code

 
They tells, for example:
 
Rule 10: All code must be compiled, from the first day of development,
with all compiler warnings enabled at the most
pedantic setting available. All code must compile without warnings. All
code must also be checked daily with at least one,
but preferably more than one, strong static source code analyzer and
should pass all analyses with zero warnings.
 
You can see entire document here:
https://web.eecs.umich.edu/~imarkov/10rules.pdf


On 29.09.2017 19:28, Jens Alfke wrote:

>
>> On Sep 29, 2017, at 1:07 AM, Denis V. Razumovsky <[hidden email]> wrote:
>>
>> Please remove multiple warnings from compiler about optimisation,
>> variable conversion, signed overflow and many more potential errors.
> This comes up a lot. SQLite is incredibly thoroughly tested* and performance-tuned, but the developers have chosen not to use compiler warnings to identify problems. It’s not the decision I’d have made personally, but I can’t argue with the results.
>
> —Jens
>
> * http://www.sqlite.org/testing.html
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Jens Alfke-2


> On Sep 29, 2017, at 10:14 AM, Denis V. Razumovsky <[hidden email]> wrote:
>
> I would like to draw attention to the document: "The Power of 10: Rules
> for Developing Safety-Critical Code" from  NASA/JPL Laboratory.
> https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code <https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code>

I’m not going to go into the philosophies of writing reliable code. I just want to point out that it’s not a good idea to walk into a community and immediately tell everyone that they’re doing things the wrong way and should start doing them the way you recommend. It’s very likely to be considered rude; people are going to assume that since you’re new there you don’t know their subject well yet; and since they don’t know you, they have no reason to assume you're particularly knowledgeable at what you’re talking about.

—Jens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Richard Hipp-3
On 9/29/17, Jens Alfke <[hidden email]> wrote:
>
> it’s not a good idea to walk into a community and
> immediately tell everyone that they’re doing things the wrong way

It's worse than that.  The very first sentence we heard from Mr.
Razumovsky was an imperative: "Remove warnings!".  And though he did
at least say "Please", starting out with a command is not the most
endearing way to enter a community.

We have yet to learn who Mr. Razumovsky is, or why he feels it is so
urgent that we spend weeks of time churning the SQLite code (and
likely introducing bugs) to silence a bunch of harmless warnings.

The "Power Of 10" webpage appears to be a distillation of the MISRA C
guidelines.  The "maximum warnings enabled" rule is number 10.  It is
worth pointing out that SQLite fails the other 9 rules too, some of
them spectacularly.  For example, sqlite3.c contains 818 goto
statements. And the function that implements the byte-code engine is
over 121 printed pages long.

I studied MISRA C in detail a decade or so ago and I was not
impressed.  MISRA seems focused on improving quality by imposing
stylistic guidelines.  All the MISRA guidelines seems to be created
with an eye toward being able to verify them at compile-time.  MISRA
is concerned with how long your functions are, and how many assert()
and goto statements you use, whereas I think it is more important to
focus on getting the correct answer.  DO-178B puts more emphasis on
run-time verification, which is why I prefer using it over MISRA.

This statement is still true:  More bugs have been introduced into
SQLite trying to silence compiler warnings than compiler warnings have
found.

--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Scott Robison-2
In reply to this post by Denis V. Razumovsky
On Fri, Sep 29, 2017 at 11:14 AM, Denis V. Razumovsky <[hidden email]> wrote:
> I would like to draw attention to the document: "The Power of 10: Rules
> for Developing Safety-Critical Code" from  NASA/JPL Laboratory.
> https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code

The problem is that there is no one best practice for resolving all
such warnings in a way that makes all compilers happy. It is possible
to fix all the warnings for one platform, then move on to the next
platform and fix all its warnings, and return to the original platform
and discover that new warnings have been introduced.

Additionally, SQLite has never been responsible for the destruction of
a space craft, so maybe SQLite has something right that NASA has
wrong. :)
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Simon Slavin-3
In reply to this post by Denis V. Razumovsky


On 29 Sep 2017, at 6:14pm, Denis V. Razumovsky <[hidden email]> wrote:

> Rule 10: All code must be compiled, from the first day of development,
> with all compiler warnings enabled at the most
> pedantic setting available. All code must compile without warnings.

NASA's code is developed to run on one platform and to be compiled by one toolchain.  SQLite has to be compilable by every C compiler anyone might feasibly use, and has to run on everything from supercomputers to set top boxes to mass spectrometers to those handheld things delivery companies use to schedule deliveries and take your signature.  And they all use different compilers and CPUs, and the compilers are all designed for different versions of C, and the compilers all generate different warnings depending on which compilaton options you chose.

Here’s what happens when you try to get rid of warnings: for every compiler warning you remove in GCC you get one new one in LLVM.  If you do manage to get rid of them both you find you have a new one in CLANG, which one of the sposors happens to use and is therefore a big problem.  Working just as hard as you know how, you finally manage to make all three of those compilers happy, only to find that SQLite now won’t compile at all in the toolchain used by the Navigation Data Standard, which develops the maps used in most SatNav devices.

Alternatively, the developer team could do the work it’s currently doing, which gets rid of all compiler /errors/, and have code which works identically on all the literally billions of devices SQLite is installed on.

Simon.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

J Decker
In reply to this post by Denis V. Razumovsky
On Fri, Sep 29, 2017 at 1:07 AM, Denis V. Razumovsky <[hidden email]> wrote:

> Please remove multiple warnings from compiler about optimisation,
> variable conversion, signed overflow and many more potential errors.
>
> 1. Optimisation solutions from GCC:
> If you would like to add extra warning attributes to compile SQLite with
> gcc (and many others modern compilers) you will see the warnings about
> pure, const and other optimisation suggestion. All those optimizations
> can add more performance to final product due to various assembly
> optimisations.
>
> Static build of the last SQLite amalgamation version 3.20.0 with gcc -03
> optimisation which uses the following parameters:
>  -Wsuggest-attribute=const -Wsuggest-attribute=pure
> -Wsuggest-attribute=noreturn -Wsuggest-attribute=format
> -Wmissing-format-attribute
>
> has  87 recommendations as for “pure” functions.
>

https://stackoverflow.com/questions/11153796/benefits-of-pure-function
Interesting.


>
> 2. Error warnings:
>
> With -Wall -Wextra -Wpedantic -Wshadow options we can see extra warnings
> about potential errors as follows:
>
> 217 warnings “is not defined” like:
>
> SQLite3.c:74:5: warning: "SQLITE_4_BYTE_ALIGNED_MALLOC" is not defined
> [-Wundef]
>  #if SQLITE_4_BYTE_ALIGNED_MALLOC
>      ^
>

that's known to evaluate as 0.  It's part of c standard.  it's just silly
to make a warning about it.


>
> 20 warnings “cast discards __attribute__((noreturn))” like:
>
> SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
> qualifier from pointer target type [-Wcast-qual]
>    memcpy((void*)&aHdr[1], (const void*)&pWal->hdr, sizeof(WalIndexHdr));
>
> 768 warnings “may change the sign of the result” like:
>
> SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
> change the sign of the result [-Wsign-conversion]
>
> 75 warnings “signed overflow” like:
> SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
> reducing constant in comparison [-Wstrict-overflow]
>
> As a result I would like to recommend to build SQLite with all extra
> warning options. In total, I have counted unbelievable 2628 warnings of
> potential errors without any efforts from my side.
>
>
As for the above... those can be catastrophic; and I'm surprised there are
so many sign conversion warnings generated...


> 3. How to reproduce:
>
> a) amalgamation SQLite version 3.20.0
>
> b) gcc version:
> $ gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper
> Target: x86_64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
> 5.4.0-6ubuntu1~16.04.4'
> --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs
> --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++
> --prefix=/usr --program-suffix=-5 --enable-shared
> --enable-linker-build-id --libexecdir=/usr/lib
> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
> --enable-nls --with-sysroot=/ --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
> --disable-vtable-verify --enable-libmpx --enable-plugin
> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
> --enable-gtk-cairo
> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre
> --enable-java-home
> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64
> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64
> --with-arch-directory=amd64
> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
> --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
> --enable-checking=release --build=x86_64-linux-gnu
> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> Thread model: posix
> gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4)
>
> с) Lubuntu 16.04
>
> d) Files:
> $ ls
> main.c    Makefile sqlite3.c  sqlite3.h
>
> $ cat Makefile
> # Compiler flags
> #
> CC ?= cc
>
> CFLAGS += -pipe -std=c11
> CFLAGS += -fbuiltin
> SQLITEFLAGS += -DSQLITE_THREADSAFE=0
> SQLITEFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION
> CFLAGS += $(SQLITEFLAGS)
> EXE = sqlite_test
> STATIC = -static
> STRIP = -s
>
> WFLAGS += -Wall -Wextra -Wpedantic -Wshadow
> WFLAGS += -Wconversion -Wsign-conversion -Winit-self -Wunreachable-code
> -Wformat-y2k
> WFLAGS += -Wformat-nonliteral -Wformat-security -Wmissing-include-dirs
> WFLAGS += -Wswitch-default -Wtrigraphs -Wstrict-overflow=5
> WFLAGS += -Wfloat-equal -Wundef -Wshadow
> WFLAGS += -Wbad-function-cast -Wcast-qual -Wcast-align
> WFLAGS += -Wwrite-strings
> WFLAGS += -Winline
>
> ifneq ($(CC), clang)
> WFLAGS += -Wlogical-op
> CFLAGS += -finline-functions
> CFLAGS += -flto
> # Perform a number of minor optimizations that are relatively expensive.
> CFLAGS += -fexpensive-optimizations
> # Attempt to remove redundant extension instructions. This is especially
> helpful for the x86-64 architecture, which implicitly zero-extends in
> 64-bit registers after writing to their lower 32-bit half.
> CFLAGS += -free
> endif
>
> #
> # Project files
> #
> SRCS = $(wildcard *.c)
> HDRS = $(wildcard *.h)
> # Exclude a file
> OBJS = $(SRCS:.c=.o)
>
> #
> # Release build settings
> #
> RELDIR = release
> RELEXE = $(RELDIR)/$(EXE)
> RELOBJS = $(addprefix $(RELDIR)/, $(OBJS))
> RELCFLAGS = -O3 -funroll-loops -DNDEBUG
> RELCFLAGS += -march=native
> # GCC only options
> ifneq ($(CC), clang)
> RELWFLAGS += -Wsuggest-attribute=const -Wsuggest-attribute=pure
> -Wsuggest-attribute=noreturn -Wsuggest-attribute=format
> -Wmissing-format-attribute
> endif
>
> .PHONY: all clean release
>
> # Default build
> all: release
>
> #
> # Release rules
> #
> release: $(RELEXE)
>
> $(RELEXE): $(RELOBJS)
>     $(CC) $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) $(STATIC)
> $(STRIP) -o $(RELEXE) $^ $(LDFLAGS) $(RELLDFLAGS) $(TCMALLOC)
>     echo "$(RELEXE) linked."
>
> $(RELDIR)/%.o: %.c $(HDRS)
>     mkdir -p $(RELDIR)
>     $(CC) -c $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) -o $@ $<
>     echo $<" compiled."
>
>
> clean:
>     rm -rf $(RELEXE) $(RELOBJS)
>     test -d $(RELDIR) && rm -d $(RELDIR) || true
>
>
> $ cat ./main.c
> #include "sqlite3.h"
> #include <stdio.h>
>
> int main(void){
>
>     printf("SQLite version: %s\n",sqlite3_libversion());
>
>     return 0;
>
> }
>
> e) Run:
> $ ./release/sqlite_test
> SQLite version: 3.21.0
>
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

J Decker
On Fri, Sep 29, 2017 at 11:58 AM, J Decker <[hidden email]> wrote:

>
>
> On Fri, Sep 29, 2017 at 1:07 AM, Denis V. Razumovsky <[hidden email]> wrote:
>
>>
>> SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
>> qualifier from pointer target type [-Wcast-qual]
>>    memcpy((void*)&aHdr[1], (const void*)&pWal->hdr, sizeof(WalIndexHdr));
>>
>> 768 warnings “may change the sign of the result” like:
>>
>> SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
>> change the sign of the result [-Wsign-conversion]
>>
>> 75 warnings “signed overflow” like:
>> SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
>> reducing constant in comparison [-Wstrict-overflow]
>>
>> As a result I would like to recommend to build SQLite with all extra
>> warning options. In total, I have counted unbelievable 2628 warnings of
>> potential errors without any efforts from my side.
>>
>>
> As for the above... those can be catastrophic; and I'm surprised there are
> so many sign conversion warnings generated...
>


I see; I wondered how it complied so cleanly; I don't enable a lot of no
warning flags on my code... so there's these for MSVC :)

#pragma warning(disable : 4054)
#pragma warning(disable : 4055)
#pragma warning(disable : 4100)
#pragma warning(disable : 4127)
#pragma warning(disable : 4130)
#pragma warning(disable : 4152)
#pragma warning(disable : 4189)
#pragma warning(disable : 4206)
#pragma warning(disable : 4210)
#pragma warning(disable : 4232)
#pragma warning(disable : 4244)
#pragma warning(disable : 4305)
#pragma warning(disable : 4306)
#pragma warning(disable : 4702)
#pragma warning(disable : 4706)

I guess GCC by default is just quieter.


>
>
>> 3. How to reproduce:
>>
>> a) amalgamation SQLite version 3.20.0
>>
>> b) gcc version:
>> $ gcc -v
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper
>> Target: x86_64-linux-gnu
>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>> 5.4.0-6ubuntu1~16.04.4'
>> --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs
>> --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++
>> --prefix=/usr --program-suffix=-5 --enable-shared
>> --enable-linker-build-id --libexecdir=/usr/lib
>> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
>> --disable-vtable-verify --enable-libmpx --enable-plugin
>> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
>> --enable-gtk-cairo
>> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre
>> --enable-java-home
>> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64
>> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64
>> --with-arch-directory=amd64
>> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
>> --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
>> --enable-checking=release --build=x86_64-linux-gnu
>> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
>> Thread model: posix
>> gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4)
>>
>> с) Lubuntu 16.04
>>
>> d) Files:
>> $ ls
>> main.c    Makefile sqlite3.c  sqlite3.h
>>
>> $ cat Makefile
>> # Compiler flags
>> #
>> CC ?= cc
>>
>> CFLAGS += -pipe -std=c11
>> CFLAGS += -fbuiltin
>> SQLITEFLAGS += -DSQLITE_THREADSAFE=0
>> SQLITEFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION
>> CFLAGS += $(SQLITEFLAGS)
>> EXE = sqlite_test
>> STATIC = -static
>> STRIP = -s
>>
>> WFLAGS += -Wall -Wextra -Wpedantic -Wshadow
>> WFLAGS += -Wconversion -Wsign-conversion -Winit-self -Wunreachable-code
>> -Wformat-y2k
>> WFLAGS += -Wformat-nonliteral -Wformat-security -Wmissing-include-dirs
>> WFLAGS += -Wswitch-default -Wtrigraphs -Wstrict-overflow=5
>> WFLAGS += -Wfloat-equal -Wundef -Wshadow
>> WFLAGS += -Wbad-function-cast -Wcast-qual -Wcast-align
>> WFLAGS += -Wwrite-strings
>> WFLAGS += -Winline
>>
>> ifneq ($(CC), clang)
>> WFLAGS += -Wlogical-op
>> CFLAGS += -finline-functions
>> CFLAGS += -flto
>> # Perform a number of minor optimizations that are relatively expensive.
>> CFLAGS += -fexpensive-optimizations
>> # Attempt to remove redundant extension instructions. This is especially
>> helpful for the x86-64 architecture, which implicitly zero-extends in
>> 64-bit registers after writing to their lower 32-bit half.
>> CFLAGS += -free
>> endif
>>
>> #
>> # Project files
>> #
>> SRCS = $(wildcard *.c)
>> HDRS = $(wildcard *.h)
>> # Exclude a file
>> OBJS = $(SRCS:.c=.o)
>>
>> #
>> # Release build settings
>> #
>> RELDIR = release
>> RELEXE = $(RELDIR)/$(EXE)
>> RELOBJS = $(addprefix $(RELDIR)/, $(OBJS))
>> RELCFLAGS = -O3 -funroll-loops -DNDEBUG
>> RELCFLAGS += -march=native
>> # GCC only options
>> ifneq ($(CC), clang)
>> RELWFLAGS += -Wsuggest-attribute=const -Wsuggest-attribute=pure
>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=format
>> -Wmissing-format-attribute
>> endif
>>
>> .PHONY: all clean release
>>
>> # Default build
>> all: release
>>
>> #
>> # Release rules
>> #
>> release: $(RELEXE)
>>
>> $(RELEXE): $(RELOBJS)
>>     $(CC) $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) $(STATIC)
>> $(STRIP) -o $(RELEXE) $^ $(LDFLAGS) $(RELLDFLAGS) $(TCMALLOC)
>>     echo "$(RELEXE) linked."
>>
>> $(RELDIR)/%.o: %.c $(HDRS)
>>     mkdir -p $(RELDIR)
>>     $(CC) -c $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) -o $@ $<
>>     echo $<" compiled."
>>
>>
>> clean:
>>     rm -rf $(RELEXE) $(RELOBJS)
>>     test -d $(RELDIR) && rm -d $(RELDIR) || true
>>
>>
>> $ cat ./main.c
>> #include "sqlite3.h"
>> #include <stdio.h>
>>
>> int main(void){
>>
>>     printf("SQLite version: %s\n",sqlite3_libversion());
>>
>>     return 0;
>>
>> }
>>
>> e) Run:
>> $ ./release/sqlite_test
>> SQLite version: 3.21.0
>>
>>
>> _______________________________________________
>> sqlite-users mailing list
>> [hidden email]
>> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>>
>
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Bob Friesenhahn
In reply to this post by Scott Robison-2
On Fri, 29 Sep 2017, Scott Robison wrote:
>
> The problem is that there is no one best practice for resolving all
> such warnings in a way that makes all compilers happy. It is possible
> to fix all the warnings for one platform, then move on to the next
> platform and fix all its warnings, and return to the original platform
> and discover that new warnings have been introduced.

My own experience has been that it is possible to write valid C code
which does not produce warnings at high warning levels on just about
any standard C compiler.  It is not necessarily a case of "whack a
mole". The most annoying exception is the Microsoft Visual C Compiler,
which produces deprecation warnings for standard functions.

One does need to be very careful when fixing compiler warnings so as
to not introduce new bugs.  The most dangerous warnings to work on are
those involving signed vs unsigned types.

Bob
--
Bob Friesenhahn
[hidden email], http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,    http://www.GraphicsMagick.org/
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Scott Robison-2
On Fri, Sep 29, 2017 at 1:20 PM, Bob Friesenhahn
<[hidden email]> wrote:

> On Fri, 29 Sep 2017, Scott Robison wrote:
>>
>>
>> The problem is that there is no one best practice for resolving all
>> such warnings in a way that makes all compilers happy. It is possible
>> to fix all the warnings for one platform, then move on to the next
>> platform and fix all its warnings, and return to the original platform
>> and discover that new warnings have been introduced.
>
>
> My own experience has been that it is possible to write valid C code which
> does not produce warnings at high warning levels on just about any standard
> C compiler.  It is not necessarily a case of "whack a mole". The most
> annoying exception is the Microsoft Visual C Compiler, which produces
> deprecation warnings for standard functions.
>
> One does need to be very careful when fixing compiler warnings so as to not
> introduce new bugs.  The most dangerous warnings to work on are those
> involving signed vs unsigned types.

Except for the fact that the OP called for maximum pedantic warnings.
In that case, you can't reliably fix all the warnings, because
different compilers have different ideas of what maximum means.

In this very thread there is a warning from GCC about

#if SQLITE_4_BYTE_ALIGNED_MALLOC

not being defined. But the standard requires that undefined symbols
being replaced with 0 during preprocessing. How is that warning
useful? It is by definition standard compliant and well defined.

The problem is not just with MSVC.

It is not that warning free code is impossible to create, it just
depends on the details, which sometimes make it very difficult.

--
Scott Robison
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Denis V. Razumovsky
In reply to this post by Richard Hipp-3
I am absolutely sure that sqlite is one of the best and the most tested
software product in the nature and having such a point of view  I was
rather surprised at number of warnings coming from the compiler. I have
no plans to  thrust my opinion on the community, just to draw attention
to the details I have noticed in my practical activity. Naturally,
exposing my personal attitude, I pretend to nothing but receiving
professional evaluation of my remarks. I will be thankful for the
understanding of persons of like mind.

As for the “endearing way to enter a community”.  I did not think of
finding an endearing way to enter a community – I thought of practical
meaning of my words.

On 29.09.2017 21:51, Richard Hipp wrote:

> On 9/29/17, Jens Alfke <[hidden email]> wrote:
>> it’s not a good idea to walk into a community and
>> immediately tell everyone that they’re doing things the wrong way
> It's worse than that.  The very first sentence we heard from Mr.
> Razumovsky was an imperative: "Remove warnings!".  And though he did
> at least say "Please", starting out with a command is not the most
> endearing way to enter a community.
>
> We have yet to learn who Mr. Razumovsky is, or why he feels it is so
> urgent that we spend weeks of time churning the SQLite code (and
> likely introducing bugs) to silence a bunch of harmless warnings.
>
> The "Power Of 10" webpage appears to be a distillation of the MISRA C
> guidelines.  The "maximum warnings enabled" rule is number 10.  It is
> worth pointing out that SQLite fails the other 9 rules too, some of
> them spectacularly.  For example, sqlite3.c contains 818 goto
> statements. And the function that implements the byte-code engine is
> over 121 printed pages long.
>
> I studied MISRA C in detail a decade or so ago and I was not
> impressed.  MISRA seems focused on improving quality by imposing
> stylistic guidelines.  All the MISRA guidelines seems to be created
> with an eye toward being able to verify them at compile-time.  MISRA
> is concerned with how long your functions are, and how many assert()
> and goto statements you use, whereas I think it is more important to
> focus on getting the correct answer.  DO-178B puts more emphasis on
> run-time verification, which is why I prefer using it over MISRA.
>
> This statement is still true:  More bugs have been introduced into
> SQLite trying to silence compiler warnings than compiler warnings have
> found.
>


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Denis V. Razumovsky
In reply to this post by J Decker
Many thanks to Mr. J Decker. I'm glad if I've made anybody think about
that warnings. My mission is complete if it's true.

On 29.09.2017 21:58, J Decker wrote:

> On Fri, Sep 29, 2017 at 1:07 AM, Denis V. Razumovsky <[hidden email]> wrote:
>
>> Please remove multiple warnings from compiler about optimisation,
>> variable conversion, signed overflow and many more potential errors.
>>
>> 1. Optimisation solutions from GCC:
>> If you would like to add extra warning attributes to compile SQLite with
>> gcc (and many others modern compilers) you will see the warnings about
>> pure, const and other optimisation suggestion. All those optimizations
>> can add more performance to final product due to various assembly
>> optimisations.
>>
>> Static build of the last SQLite amalgamation version 3.20.0 with gcc -03
>> optimisation which uses the following parameters:
>>  -Wsuggest-attribute=const -Wsuggest-attribute=pure
>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=format
>> -Wmissing-format-attribute
>>
>> has  87 recommendations as for “pure” functions.
>>
> https://stackoverflow.com/questions/11153796/benefits-of-pure-function
> Interesting.
>
>
>> 2. Error warnings:
>>
>> With -Wall -Wextra -Wpedantic -Wshadow options we can see extra warnings
>> about potential errors as follows:
>>
>> 217 warnings “is not defined” like:
>>
>> SQLite3.c:74:5: warning: "SQLITE_4_BYTE_ALIGNED_MALLOC" is not defined
>> [-Wundef]
>>  #if SQLITE_4_BYTE_ALIGNED_MALLOC
>>      ^
>>
> that's known to evaluate as 0.  It's part of c standard.  it's just silly
> to make a warning about it.
>
>
>> 20 warnings “cast discards __attribute__((noreturn))” like:
>>
>> SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
>> qualifier from pointer target type [-Wcast-qual]
>>    memcpy((void*)&aHdr[1], (const void*)&pWal->hdr, sizeof(WalIndexHdr));
>>
>> 768 warnings “may change the sign of the result” like:
>>
>> SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
>> change the sign of the result [-Wsign-conversion]
>>
>> 75 warnings “signed overflow” like:
>> SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
>> reducing constant in comparison [-Wstrict-overflow]
>>
>> As a result I would like to recommend to build SQLite with all extra
>> warning options. In total, I have counted unbelievable 2628 warnings of
>> potential errors without any efforts from my side.
>>
>>
> As for the above... those can be catastrophic; and I'm surprised there are
> so many sign conversion warnings generated...
>
>
>> 3. How to reproduce:
>>
>> a) amalgamation SQLite version 3.20.0
>>
>> b) gcc version:
>> $ gcc -v
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper
>> Target: x86_64-linux-gnu
>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>> 5.4.0-6ubuntu1~16.04.4'
>> --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs
>> --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++
>> --prefix=/usr --program-suffix=-5 --enable-shared
>> --enable-linker-build-id --libexecdir=/usr/lib
>> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
>> --disable-vtable-verify --enable-libmpx --enable-plugin
>> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
>> --enable-gtk-cairo
>> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre
>> --enable-java-home
>> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64
>> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64
>> --with-arch-directory=amd64
>> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
>> --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
>> --enable-checking=release --build=x86_64-linux-gnu
>> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
>> Thread model: posix
>> gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4)
>>
>> с) Lubuntu 16.04
>>
>> d) Files:
>> $ ls
>> main.c    Makefile sqlite3.c  sqlite3.h
>>
>> $ cat Makefile
>> # Compiler flags
>> #
>> CC ?= cc
>>
>> CFLAGS += -pipe -std=c11
>> CFLAGS += -fbuiltin
>> SQLITEFLAGS += -DSQLITE_THREADSAFE=0
>> SQLITEFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION
>> CFLAGS += $(SQLITEFLAGS)
>> EXE = sqlite_test
>> STATIC = -static
>> STRIP = -s
>>
>> WFLAGS += -Wall -Wextra -Wpedantic -Wshadow
>> WFLAGS += -Wconversion -Wsign-conversion -Winit-self -Wunreachable-code
>> -Wformat-y2k
>> WFLAGS += -Wformat-nonliteral -Wformat-security -Wmissing-include-dirs
>> WFLAGS += -Wswitch-default -Wtrigraphs -Wstrict-overflow=5
>> WFLAGS += -Wfloat-equal -Wundef -Wshadow
>> WFLAGS += -Wbad-function-cast -Wcast-qual -Wcast-align
>> WFLAGS += -Wwrite-strings
>> WFLAGS += -Winline
>>
>> ifneq ($(CC), clang)
>> WFLAGS += -Wlogical-op
>> CFLAGS += -finline-functions
>> CFLAGS += -flto
>> # Perform a number of minor optimizations that are relatively expensive.
>> CFLAGS += -fexpensive-optimizations
>> # Attempt to remove redundant extension instructions. This is especially
>> helpful for the x86-64 architecture, which implicitly zero-extends in
>> 64-bit registers after writing to their lower 32-bit half.
>> CFLAGS += -free
>> endif
>>
>> #
>> # Project files
>> #
>> SRCS = $(wildcard *.c)
>> HDRS = $(wildcard *.h)
>> # Exclude a file
>> OBJS = $(SRCS:.c=.o)
>>
>> #
>> # Release build settings
>> #
>> RELDIR = release
>> RELEXE = $(RELDIR)/$(EXE)
>> RELOBJS = $(addprefix $(RELDIR)/, $(OBJS))
>> RELCFLAGS = -O3 -funroll-loops -DNDEBUG
>> RELCFLAGS += -march=native
>> # GCC only options
>> ifneq ($(CC), clang)
>> RELWFLAGS += -Wsuggest-attribute=const -Wsuggest-attribute=pure
>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=format
>> -Wmissing-format-attribute
>> endif
>>
>> .PHONY: all clean release
>>
>> # Default build
>> all: release
>>
>> #
>> # Release rules
>> #
>> release: $(RELEXE)
>>
>> $(RELEXE): $(RELOBJS)
>>     $(CC) $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) $(STATIC)
>> $(STRIP) -o $(RELEXE) $^ $(LDFLAGS) $(RELLDFLAGS) $(TCMALLOC)
>>     echo "$(RELEXE) linked."
>>
>> $(RELDIR)/%.o: %.c $(HDRS)
>>     mkdir -p $(RELDIR)
>>     $(CC) -c $(CFLAGS) $(WFLAGS) $(RELWFLAGS) $(RELCFLAGS) -o $@ $<
>>     echo $<" compiled."
>>
>>
>> clean:
>>     rm -rf $(RELEXE) $(RELOBJS)
>>     test -d $(RELDIR) && rm -d $(RELDIR) || true
>>
>>
>> $ cat ./main.c
>> #include "sqlite3.h"
>> #include <stdio.h>
>>
>> int main(void){
>>
>>     printf("SQLite version: %s\n",sqlite3_libversion());
>>
>>     return 0;
>>
>> }
>>
>> e) Run:
>> $ ./release/sqlite_test
>> SQLite version: 3.21.0
>>
>>
>> _______________________________________________
>> sqlite-users mailing list
>> [hidden email]
>> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users


_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Denis V. Razumovsky
In reply to this post by Scott Robison-2

On 29.09.2017 22:33, Scott Robison wrote:

> On Fri, Sep 29, 2017 at 1:20 PM, Bob Friesenhahn
> <[hidden email]> wrote:
>> > On Fri, 29 Sep 2017, Scott Robison wrote:
>>> >>
>>> >>
>>> >> The problem is that there is no one best practice for resolving all
>>> >> such warnings in a way that makes all compilers happy. It is possible
>>> >> to fix all the warnings for one platform, then move on to the next
>>> >> platform and fix all its warnings, and return to the original platform
>>> >> and discover that new warnings have been introduced.
>> >
>> >
>> > My own experience has been that it is possible to write valid C code which
>> > does not produce warnings at high warning levels on just about any standard
>> > C compiler.  It is not necessarily a case of "whack a mole". The most
>> > annoying exception is the Microsoft Visual C Compiler, which produces
>> > deprecation warnings for standard functions.
>> >
>> > One does need to be very careful when fixing compiler warnings so as to not
>> > introduce new bugs.  The most dangerous warnings to work on are those
>> > involving signed vs unsigned types.
> Except for the fact that the OP called for maximum pedantic warnings.
> In that case, you can't reliably fix all the warnings, because
> different compilers have different ideas of what maximum means.
>
> In this very thread there is a warning from GCC about
>
> #if SQLITE_4_BYTE_ALIGNED_MALLOC

What can be wrong for _any_ of the compilers if you will define
SQLITE_4_BYTE_ALIGNED_MALLOC as 0 in sqlite3.h? It's so simple. I think
it should only get better for all platforms and compilers )



_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Simon Slavin-3


On 29 Sep 2017, at 9:06pm, Denis V. Razumovsky <[hidden email]> wrote:

> What can be wrong for _any_ of the compilers if you will define
> SQLITE_4_BYTE_ALIGNED_MALLOC as 0 in sqlite3.h? It's so simple. I think
> it should only get better for all platforms and compilers )

If SQLITE_4_BYTE_ALIGNED_MALLOC always means 0 under all circumstances, why did someone bother to make it a named option ?  Why doesn’t the code just assume zero ?

Under Windows malloc() returns a 4-byte aligned pointer, i.e. that the option has to be set to 1.  The same occurs in many embedded processors, which have far less memory than you’re used to on a multi-purpose desktop computer.  It’s not simple.  And it doesn’t work on all platforms.

You are still proceeding as if your own development platform and target CPU is the only one SQLite has to work for.

Simon.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Igor Korot
Simon,

On Fri, Sep 29, 2017 at 4:38 PM, Simon Slavin <[hidden email]> wrote:

>
>
> On 29 Sep 2017, at 9:06pm, Denis V. Razumovsky <[hidden email]> wrote:
>
>> What can be wrong for _any_ of the compilers if you will define
>> SQLITE_4_BYTE_ALIGNED_MALLOC as 0 in sqlite3.h? It's so simple. I think
>> it should only get better for all platforms and compilers )
>
> If SQLITE_4_BYTE_ALIGNED_MALLOC always means 0 under all circumstances, why did someone bother to make it a named option ?  Why doesn’t the code just assume zero ?
>
> Under Windows malloc() returns a 4-byte aligned pointer, i.e. that the option has to be set to 1.  The same occurs in many embedded processors, which have far less memory than you’re used to on a multi-purpose desktop computer.  It’s not simple.  And it doesn’t work on all platforms.
>
> You are still proceeding as if your own development platform and target CPU is the only one SQLite has to work for.

But then why not give it some default value ("0" maybe") and default
it to "1" only if needed during configure?

Thank you.

>
> Simon.
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Bob Friesenhahn
In reply to this post by Denis V. Razumovsky
On Fri, 29 Sep 2017, Denis V. Razumovsky wrote:
>>
>> In this very thread there is a warning from GCC about
>>
>> #if SQLITE_4_BYTE_ALIGNED_MALLOC
>
> What can be wrong for _any_ of the compilers if you will define
> SQLITE_4_BYTE_ALIGNED_MALLOC as 0 in sqlite3.h? It's so simple. I think
> it should only get better for all platforms and compilers )

This causes problems given that there are varying configuration
methods, but this annoying logic should get rid of the warning:

#if defined(SQLITE_4_BYTE_ALIGNED_MALLOC) && SQLITE_4_BYTE_ALIGNED_MALLOC

Bob
--
Bob Friesenhahn
[hidden email], http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,    http://www.GraphicsMagick.org/
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

Richard Damon
In reply to this post by J Decker
On 9/29/17 2:58 PM, J Decker wrote:

>> 20 warnings “cast discards __attribute__((noreturn))” like:
>>
>> SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
>> qualifier from pointer target type [-Wcast-qual]
>>     memcpy((void*)&aHdr[1], (const void*)&pWal->hdr, sizeof(WalIndexHdr));
>>
>> 768 warnings “may change the sign of the result” like:
>>
>> SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
>> change the sign of the result [-Wsign-conversion]
>>
>> 75 warnings “signed overflow” like:
>> SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
>> reducing constant in comparison [-Wstrict-overflow]
>>
>> As a result I would like to recommend to build SQLite with all extra
>> warning options. In total, I have counted unbelievable 2628 warnings of
>> potential errors without any efforts from my side.
>>
>>
> As for the above... those can be catastrophic; and I'm surprised there are
> so many sign conversion warnings generated...
>
>
Note, the warning doesn't say that an overflow has/will occur, just that
it assumed it won't and optimized based on that. If the program took
care to avoid overflow, all is good. If the program made the (invalid)
assumption that signed arithmetic wrapped on overflow like unsigned,
then the program has a problem.

My guess is that the code is good enough not to have the overflow issue.

That sort of warning is aimed at 'sloppy programmers' who think that
just because it seems to work, it must be right and to the standard.

--
Richard Damon

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

J Decker
On Fri, Sep 29, 2017 at 5:12 PM, Richard Damon <[hidden email]>
wrote:

> On 9/29/17 2:58 PM, J Decker wrote:
>
>> 20 warnings “cast discards __attribute__((noreturn))” like:
>>>
>>> SQLite3.c:55734:10: warning: cast discards ‘__attribute__((noreturn))’
>>> qualifier from pointer target type [-Wcast-qual]
>>>     memcpy((void*)&aHdr[1], (const void*)&pWal->hdr,
>>> sizeof(WalIndexHdr));
>>>
>>> 768 warnings “may change the sign of the result” like:
>>>
>>> SQLite3.c:19330:11: warning: conversion to ‘unsigned int’ from ‘int’ may
>>> change the sign of the result [-Wsign-conversion]
>>>
>>> 75 warnings “signed overflow” like:
>>> SQLite3.c:61116:3: warning: assuming signed overflow does not occur when
>>> reducing constant in comparison [-Wstrict-overflow]
>>>
>>> As a result I would like to recommend to build SQLite with all extra
>>> warning options. In total, I have counted unbelievable 2628 warnings of
>>> potential errors without any efforts from my side.
>>>
>>>
>>> As for the above... those can be catastrophic; and I'm surprised there
>> are
>> so many sign conversion warnings generated...
>>
>>
>> Note, the warning doesn't say that an overflow has/will occur, just that
> it assumed it won't and optimized based on that. If the program took care
> to avoid overflow, all is good. If the program made the (invalid)
> assumption that signed arithmetic wrapped on overflow like unsigned, then
> the program has a problem.
>
> My guess is that the code is good enough not to have the overflow issue.
>
> That sort of warning is aimed at 'sloppy programmers' who think that just
> because it seems to work, it must be right and to the standard.

I've been trying to remember the case... it worked for a lot of years(a
decade+);  think it was something like uint16_t w = 100; int32_t o = -1;
and the integer was compared as unsigned...  and o was greater than w.
either that or it was a uin16_t x = 0x8000 that was converted to signed and
was sign extended.... but I think it was the former.

Sort of, I found that if the comparison can be done wrong MS will pick the
way to compile a signed/unsigned comparison so it fails most often.
It really could have been defined in the standard such that if unsigned, if
(highest bit set in unsigned) set always greater, if highest bit is set in
signed, set always lesser... otherwise compare as normal.

I had started to try to standardize to signed or  unsigned values; but that
started to cause other problems.  I eventually just implemented a bunch of
comparison macros that just do the right thing....

(SUS = signed-unsigned; USS = unsigned-signed)

#  define SUS_GT(a,at,b,bt)   (((a)<0)?0:(((bt)a)>(b)))
#  define USS_GT(a,at,b,bt)   (((b)<0)?1:((a)>((at)b)))

#  define SUS_LT(a,at,b,bt)   (((a)<0)?1:(((bt)a)<(b)))
#  define USS_LT(a,at,b,bt)   (((b)<0)?0:((a)<((at)b)))

#  define SUS_GTE(a,at,b,bt)  (((a)<0)?0:(((bt)a)>=(b)))
#  define USS_GTE(a,at,b,bt)  (((b)<0)?1:((a)>=((at)b)))

#  define SUS_LTE(a,at,b,bt)  (((a)<0)?1:(((bt)a)<=(b)))
#  define USS_LTE(a,at,b,bt)  (((b)<0)?0:((a)<=((at)b)))


>
> --
> Richard Damon
>
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Please remove multiple warnings from compiler about optimisation, variable conversion, signed overflow and many more potential errors.

James K. Lowden
In reply to this post by Igor Korot
On Fri, 29 Sep 2017 16:55:05 -0400
Igor Korot <[hidden email]> wrote:

> But then why not give it some default value ("0" maybe") and default
> it to "1" only if needed during configure?

Because complexity.  It takes effort --- unnecessary effort -- to set
up that default.  That effort could introduce an error, whereas no
effort *cannot* introduce an error.  Less is more.  

The assumption on the part of the guideline authors seems to be that if
something is undefined, it might have been overlooked, and the best way
to make sure it's not overlooked is by ensuring there's always an
explicit definition.  That's a debatable proposition.  The mere fact
something is defined in no wise ensures it is defined correctly.  

In this case, the tools themselves provide the definition.  For those
that do, the code compiles one way.  For those that do not, another
way.  It's entirely automatic.  How could supplying those definitions
manually be an improvement?  

--jkl

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
12