Blender: New error with min/max function not in scope

[Richard Shaw]

I was performing a rebuild of Blender after fixing a FTBFS error with
openCOLLADA and ran into an interesting error:

[ 78%] Building CXX object
source/blender/imbuf/intern/dds/CMakeFiles/bf_imbuf_dds.dir/DirectDrawSurface.cpp.o
cd
/builddir/build/BUILD/blender-2.76/cmake-make/source/blender/imbuf/intern/dds
&& /usr/lib64/ccache/c++   -DHAVE_STDBOOL_H -DNDEBUG -DWITH_DDS
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE
-D__LITTLE_ENDIAN__ -D__MMX__ -D__SSE2__ -D__SSE__
-I/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds
-I/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern
-I/builddir/build/BUILD/blender-2.76/source/blender/imbuf
-I/builddir/build/BUILD/blender-2.76/source/blender/blenkernel
-I/builddir/build/BUILD/blender-2.76/source/blender/blenlib
-I/builddir/build/BUILD/blender-2.76/source/blender/makesdna
-I/builddir/build/BUILD/blender-2.76/intern/guardedalloc
-I/builddir/build/BUILD/blender-2.76/intern/utfconv  -Wredundant-decls
-Wall -Wno-invalid-offsetof -Wno-sign-compare -Wlogical-op -Winit-self
-Wmissing-include-dirs -Wno-div-by-zero -Wtype-limits -Werror=return-type
-Werror=declaration-after-statement -Werror=implicit-function-declaration
-Wno-char-subscripts -Wno-unknown-pragmas -Wpointer-arith
-Wunused-parameter -Wwrite-strings -Wundef -Wformat-signedness
-Wuninitialized -Wundef -Wmissing-declarations -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -fPIC
-funsigned-char -fno-strict-aliasing  -fuse-ld=gold -fopenmp  -msse2  -msse
-pipe -fPIC -funsigned-char -fno-strict-aliasing -O2 -DNDEBUG   -o
CMakeFiles/bf_imbuf_dds.dir/DirectDrawSurface.cpp.o -c
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:
In member function 'void DirectDrawSurface::mipmap(Image*, uint, uint)':
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:1105:20:
error: 'max' was not declared in this scope
   w = max(1U, w / 2);
                    ^
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:
In member function 'void DirectDrawSurface::readBlockImage(Image*)':
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:1226:39:
error: 'min' was not declared in this scope
    for (uint y = 0; y < min(4U, h-4*by); y++)
                                       ^
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:
In function 'Color32 buildNormal(uint8, uint8)':
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:1243:56:
error: 'max' was not declared in this scope
  uint8 z = clamp(int(255.0f * (nz + 1) / 2.0f), 0, 255);
                                                        ^
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:1243:64:
error: 'min' was not declared in this scope
  uint8 z = clamp(int(255.0f * (nz + 1) / 2.0f), 0, 255);
                                                                ^
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:
In member function 'uint DirectDrawSurface::mipmapSize(uint) const':
/builddir/build/BUILD/blender-2.76/source/blender/imbuf/intern/dds/DirectDrawSurface.cpp:1382:20:
error: 'max' was not declared in this scope
   w = max(1U, w / 2);
                    ^
source/blender/imbuf/intern/dds/CMakeFiles/bf_imbuf_dds.dir/build.make:110:
recipe for target
'source/blender/imbuf/intern/dds/CMakeFiles/bf_imbuf_dds.dir/DirectDrawSurface.cpp.o'
failed
--- end ---

This seems like a pretty basic error in something that has worked fine for
a very long time. Is there anything in the GCC 6 update that would cause
this?

Thanks,
Richard

[Jonathan Wakely]

On 16/02/16 10:13 -0600, Richard Shaw wrote:
> This seems like a pretty basic error in something that has worked fine
> for
> a very long time. Is there anything in the GCC 6 update that would cause
> this? 
This no longer compiles with GCC 6:

#define max(a, b) (a > b ? a : b)
#include <stdlib.h>
int i = max(0,1);

The reason is that 'max' is used throughout the standard library, and
it's undefined behaviour to define a macro that clashes with any name
defined in the standard library. Previously <stdlib.h> was not
provided by GCC's C++ std::lib, so didn't #undef min and max. Now GCC
provides its own C++-conforming <stdlib.h> and so it does #undef min
and #undef max.

If that's the cause, the code might have been working fine but was
always relying on undefined behaviour.

I suggest #include <algorithm> and using std::max;

[Jonathan Wakely]

On 16/02/16 16:31 +0000, Jonathan Wakely wrote:
> On 16/02/16 10:13 -0600, Richard Shaw wrote:
> >This seems like a pretty basic error in something that has worked fine for
> >a very long time. Is there anything in the GCC 6 update that would cause
> >this?
>
> This no longer compiles with GCC 6:
>
> #define max(a, b) (a > b ? a : b)
> #include <stdlib.h>
> int i = max(0,1);
>
> The reason is that 'max' is used throughout the standard library, and
> it's undefined behaviour to define a macro that clashes with any name
> defined in the standard library. Previously <stdlib.h> was not
> provided by GCC's C++ std::lib, so didn't #undef min and max. Now GCC
> provides its own C++-conforming <stdlib.h> and so it does #undef min
> and #undef max.
>
> If that's the cause, the code might have been working fine but was
> always relying on undefined behaviour. 

As I guessed, blender does this:

#ifndef __COMMON_H__
#define __COMMON_H__

#ifndef min
#define min(a,b) ((a) <= (b) ? (a) : (b))
#endif
#ifndef max
#define max(a,b) ((a) >= (b) ? (a) : (b))
#endif
#ifndef clamp
#define clamp(x,a,b) min(max((x), (a)), (b))
#endif

That's undefined behaviour for two separate reasons, yay.

The include guard uses a reserved name containing two underscores,
undefined.

It defines macros "min" and "max", undefined.

It assumes that those macros will remain defined after including
standard library headers, which is not a valid assumption because
standard library headers can assume nobody defines macros with names
that clash with the standard library.

> I suggest #include <algorithm> and using std::max; 
This will work. Alternatively, #include <stdlib.h> before defining
those macros, so that the library's #undef happens first, and then the
undefined behaviour comes later.

[Richard Shaw]

On Tue, Feb 16, 2016 at 10:45 AM, Jonathan Wakely <jwakely(a)fedoraproject.org
>  wrote: 
>  On 16/02/16 16:31 +0000, Jonathan Wakely wrote:
>
> > On 16/02/16 10:13 -0600, Richard Shaw wrote:
> >
> >> This seems like a pretty basic error in something that has worked fine
> >> for
> >> a very long time. Is there anything in the GCC 6 update that would cause
> >> this?
> >>
> >
> > This no longer compiles with GCC 6:
> >
> > #define max(a, b) (a > b ? a : b)
> > #include <stdlib.h>
> > int i = max(0,1);
> >
> > The reason is that 'max' is used throughout the standard library, and
> > it's undefined behaviour to define a macro that clashes with any name
> > defined in the standard library. Previously <stdlib.h> was not
> > provided by GCC's C++ std::lib, so didn't #undef min and max. Now GCC
> > provides its own C++-conforming <stdlib.h> and so it does #undef min
> > and #undef max.
> >
> > If that's the cause, the code might have been working fine but was
> > always relying on undefined behaviour.
> >
>
>
>  As I guessed, blender does this:
>
>  #ifndef __COMMON_H__
>  #define __COMMON_H__
>
>  #ifndef min
>  #define min(a,b) ((a) <= (b) ? (a) : (b))
>  #endif
>  #ifndef max
>  #define max(a,b) ((a) >= (b) ? (a) : (b))
>  #endif
>  #ifndef clamp
>  #define clamp(x,a,b) min(max((x), (a)), (b))
>  #endif
>
>  That's undefined behaviour for two separate reasons, yay.
>
>  The include guard uses a reserved name containing two underscores,
>  undefined.
>
>  It defines macros "min" and "max", undefined.
>
>  It assumes that those macros will remain defined after including
>  standard library headers, which is not a valid assumption because
>  standard library headers can assume nobody defines macros with names
>  that clash with the standard library.
>
>  I suggest #include <algorithm> and using std::max;
> >
>
>  This will work. Alternatively, #include <stdlib.h> before defining
>  those macros, so that the library's #undef happens first, and then the
>  undefined behaviour comes later. 

That got it. Thanks for the tip. I have submitted the patch upstream.

Thanks,
Richard