maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Michael Widenius <monty@xxxxxxxxxxxx> writes:

> Here is some comments, suggestions of what we still to do to make the
> buildbot part 'more clean:

Finally got to this after long wait ...

> We get a lot of warnings of the type:
>
> ctype-ucs2.c:206: warning: ‘s_wc’ may be used uninitialized in this function
>
> This are false warnings (The compiler is not smart enough to figure
> this out).  The variables are also protected with the LINT_INIT() macro.
>
> Suggestion (for Kristian):
>
> - If you compile with -Wuninitialized, also use the -DFORCE_INIT_OF_VARS

I added this to Buildbot, should happen for new builds.

To facilitate this, I added the feature to set extra flags and options when
using BUILD/compile-xxx scripts. For example:

    BUILD/compile-pentium64-valgrind-max --extra-flags="-Wno-uninitialized -O3"

New options are:

    --extra-cflags    # Added to CFLAGS
    --extra-cxxflags  # Added to CXXFLAGS
    --extra-flags     # Added to both CFLAGS and CXXFLAGS
    --extra-configs   # Added to ./configure line

The same can be achieved using environment variables:

    EXTRA_CFLAGS
    EXTRA_CXXFLAGS
    EXTRA_FLAGS
    EXTRA_CONFIGS

I use the environment variables in Buildbot instead of the options, as
Buildbot works on lots of branches, some of which do not support the new
options. Using environment variables avoid getting a build failure in these
branches or having to do extra configuration to do different kinds of build
depending on exactly what is in the branch.

> When compiling for valgrind, we should not use
> -Wuninitialized or -DFORCE_INIT_OF_VARS

This is already the case.

In Buildbot, I changed the Valgrind builds to compile with optimisations (as
we want to catch any problems due to compiler bugs or wrong use of C/C++ that
only shows in this case). But I kept the BUILD/compile-*-valgrind-* scripts
using no optimisation by default for easier debugging for developers.

Also, I did not change the default behaviour of BUILD/compile-pentium-max (and
similar scripts) to use -DFORCE_INIT_OF_VARS by default, as I was not sure it
is proper to have this option in build a build script that does a "release"
type build.

> I don't know if the following warning comes from us or from the
> include files:
>
> /usr/include/bits/string3.h:82: warning: call to ‘__warn_memset_zero_len’ declared with attribute warning: memset used with constant zero length parameter; this could be due to transposed parameters
>
> Need suggestions/help to solve this one.
> Either we need to find the code in MariaDB that generates the warning
> or disable warnings in buildbot from /usr/include

This comes from YaSSL. It has this code

class Source { ...
    explicit Source(word32 sz = 0) : buffer_(sz), current_(0) {}

Calling the constructor with no arguments

    Source cert;

eventually ends up in this code

    memset(buffer_, 0, sz_ * sizeof(T));

where sz_ is zero. So this translates into memset(buffer_, 0, 0), which glibc
has a warnings for when the last parameter is a _constant_ zero. So the
inlining is causing it here.

There are a couple ways to fix it, the simplest is probably just to add

    if (sz_ != 0)

just before the memset. I have pushed this into lp:maria.

 - Kristian.