Showing posts with label bug. Show all posts
Showing posts with label bug. Show all posts

Tuesday, May 24, 2016

Is TSX busted on Skylake, too? No, it's just buggy software

The story about Intel recalling Transactional Synchronization Extensions
from Haswell and Broadwell lines of their CPUs by means of a microcode update has hit the web in the past. But it looks like this is not the end of the story.

The company I work for has a development server in Hetzner, and it uses this type of CPU:


processor : 0
vendor_id : GenuineIntel
cpu family : 6
model  : 94
model name : Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
stepping : 3
microcode : 0x39
cpu MHz  : 3825.265
cache size : 8192 KB
physical id : 0
siblings : 8
core id  : 0
cpu cores : 4
apicid  : 0
initial apicid : 0
fpu  : yes
fpu_exception : yes
cpuid level : 22
wp  : yes
flags  : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb 
rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est 
tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch intel_pt 
tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep 
bmi2 erms invpcid rtm mpx rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 
dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp
bugs  :
bogomips : 6816.61
clflush size : 64
cache_alignment : 64
address sizes : 39 bits physical, 48 bits virtual
power management:


I.e. it is a Skylake. The server is running Ubuntu 16.04, and the CPU has HLE and RTM families of instructions.

One of my recent tasks was to prepare, on this server, an LXC container based on Ubuntu 16.04 with a lightweight desktop accessible over VNC, for "remote classroom" purposes. We already have such containers on other servers, but they were based on Ubuntu 14.04. Such containers work well on this server, too, but it's time to upgrade. In these old containers, we use a regular Xorg server with a "dummy" video driver, and export the screen using x11vnc.

So, I decided to clone the old container and update Ubuntu there. Result: x11vnc, or sometimes Xorg, now crashes (SIGSEGV) when one attempts to change the desktop resolution. The backtrace points into the __lll_unlock_elision() function which is a part of glibc implementation of mutexes for CPUs with Hardware Lock Elision instructions.

This crash doesn't happen when I run the same container on a server with an older CPU (which doesn't have TSX in the first place), or if I try to reproduce the bug at home (where I have a Haswell, with TSX disabled by the new microcode).

So, all apparently points to a bug related to these extensions. Or does it?

The __lll_unlock_elision() function has this helpful comment in it:

  /* When the lock was free we're in a transaction.
     When you crash here you unlocked a free lock.  */

And indeed, there is some discussion of another crash in __lll_unlock_elision(), related to NVidia driver (which is not used here). In that discussion, it was highlighted that an unlock of already-unlocked mutex would be silently ignored if a mutex implementation not optimized for TSX is used, but a CPU with TSX would expose such latent bug. Locking balance bugs are easily verified using valgrind. And indeed:

DISPLAY=:1 valgrind --tool=helgrind x11vnc
...
==4209== ---Thread-Announcement------------------------------------------
==4209== 
==4209== Thread #1 is the program's root thread
==4209== 
==4209== ----------------------------------------------------------------
==4209== 
==4209== Thread #1 unlocked a not-locked lock at 0x9CDA00
==4209==    at 0x4C326B4: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==4209==    by 0x4556B2: ??? (in /usr/bin/x11vnc)
==4209==    by 0x45A35E: ??? (in /usr/bin/x11vnc)
==4209==    by 0x466646: ??? (in /usr/bin/x11vnc)
==4209==    by 0x410E30: ??? (in /usr/bin/x11vnc)
==4209==    by 0x717D82F: (below main) (libc-start.c:291)
==4209==  Lock at 0x9CDA00 was first observed
==4209==    at 0x4C360BA: pthread_mutex_init (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==4209==    by 0x40FECC: ??? (in /usr/bin/x11vnc)
==4209==    by 0x717D82F: (below main) (libc-start.c:291)
==4209==  Address 0x9cda00 is in the BSS segment of /usr/bin/x11vnc
==4209== 
==4209== 

It is a software bug, not a CPU bug. But still - until such bugs are eliminated from the distribution, I'd rather not use it on a server with a CPU with TSX.

Sunday, May 8, 2016

Root filesystem snapshots and kernel upgrades

On my laptop (which is running Arch), I decided to have periodic snapshots of the filesystem, in order to revert bad upgrades (especially those involving a large and unknown set of interdependent packages) easily. My toolset for this task is LVM2 and Snapper. Yes, I know that LVM2 is kind-of discouraged, and Snapper also supports btrfs, but most of the points below apply to btrfs, too.

Snapper, when used with LVM2, requires not just LVM2, but thinly-provisioned LVM2 volumes. Fortunately, Arch can have root filesystem on such volumes, so this is not a problem.

So, I have /boot on /dev/sda1, LVM on LUKS on /dev/sda2, root on a thinly-provisioned logical volume, and /home on another thinly-provisioned volume. And also swap on a non-thinly-provisioned volume. A separate /boot partition is needed because boot loaders generally don't understand thinly-provisioned LVM volumes, especially on encrypted disks. A separate volume for /home is needed because I don't want my new files in /home to be lost if I revert the system to its old snapshot. The same need to make a separate volume applies to other directories that contain data that should be preserved, but there are no such directories on my laptop. They can appear if I install e.g PostgreSQL.

And now there is a problem. Rollback to a snapshot works, but only if there were no kernel updates between the time when the snapshot was taken and when an attempt to revert was made. The root cause is that the kernel image is in /boot, and loadable modules for it are in /usr/lib/modules. The modules are reverted, but the boot loader still loads a new kernel, which now has no corresponding modules.

There are two solutions: either revert the kernel and its initramfs, too, when reverting the root file system, or make sure that modules are not reverted. I have not investigated how to make the first option possible, even though it would be a perfect solution. However, I have tried to make sure that modules are not reverted, and I am not satisfied with the result.

The idea was to move modules to /boot/modules, and make this location available somehow as /usr/lib/modules. Here "somehow" can mean either a symlink, or a bind mount. A symlink doesn't work, because the kernel upgrade in Arch will restore it back to a directory. A bind mount doesn't work, either. The issue is that, by putting modules on non-root filesystem, one creates a circular dependency between local filesystem mounting and udev (this would apply to a symlink, too).

Indeed, systemd-udevd, on startup, maps the /usr/lib/modules/`uname -r`/modules.alias.bin file into memory. So, now it has a (real) dependency on /usr/lib/modules being mounted. However, mounting local filesystems from /etc/fstab sometimes depends on systemd-udevd, because of device nodes. So, bind-mounting /usr/lib/modules merely from /etc/fstab, using built-in systemd tools, cannot work.

But it can work from a wrapper that starts before the real init:

#!/bin/sh
mount -n /boot              # /dev/sda1 is in devtmpfs and doesn't need udev
mount -n /usr/lib/modules   # there is still a line in fstab about that
exec /sbin/init "$@" 

But that's ugly. In the end, I removed the wrapper, installed an old known-working "linux" package, made a copy of the kernel, its initramfs and modules, upgraded the kernel again, and put the saved files back, so that they are now not controlled by the package manager. So now I have a known good kernel down in the boot menu, and knowledge that its modules will always be present in my root filesystem if I don't revert further than up to today's state.

And now one final remark. Remember that I said: "The same need to make a separate volume applies to other directories that contain data that should be preserved"? There is a temptation to apply this to the whole /var directory, but that would be wrong. If a system is being reverted to its old snapshot, a package database (which is in /var/lib/pacman) should be reverted, too. But /var/lib/pacman is under /var.

The conclusion is that Linux plumbers should think a bit about this "revert the whole system" use case, and maybe move some directories.

Sunday, October 18, 2015

Still using icims.com for recruiting? Think again!

If your company has open vacancies and uses some system for pre-screening candidates (e.g. by giving them questions), I have a "small" task for you. Go to your system, answer the questions as if you were a candidate, validate the answers as you would expect from a candidate (e.g. actually perform the actions that the answer describes), and then save the results. Look at the whole process. Make a conclusion for yourself whether your system is usable for the stated purpose. Communicate it to your management, if needed.

If you are using icims.com for hiring technical candidates, the answer is most probably "not suitable at all".

The most annoying bug that icims.com has is that it does not allow the candidate to enter certain characters in certain positions. The exact error message is:
Q3 2 Contains invalid characters. You cannot use the characters: ' " \ / or ` in an enclosing instance of <>, <<, >> or ><.
 This triggers at least on the following types of input:
  • XML or HTML
  • Command redirections, e.g.: echo "foo bar" >> baz.txt
  • Sequences of menu items to click, e.g.: "File > New > Folder", if a bad character happens to be before that
So, you cannot ask questions about HTML, shell scripting, or even general questions about using GUI-based applications.

This error message probably means that they are concerned about XSS attacks. However, filtering out invalid characters is a very sloppy way of protection against such attacks. And it imposes completely unreasonable restrictions on the user input.

In fact, any kind of input (including XML, bash scripts or text about clicking the menu) should be suitable, and can be made to display safely and properly in any browser, just by escaping the special characters when generating the HTML page. Many template engines exist that do this escaping for you automatically. Today, there is simply no reason not to use them.

If a candidate sees such error, he/she becomes demotivated. It is a stupid barrier before getting the correct answer to you. It also indicates that you don't care about your customers (by choosing business partners that allow such sloppy practices). Worse, some of your candidates (who see icims.com for the first time) can think that it is your product, or your internal system, and that you (not icims.com) have web developers with insufficient skills. I.e. that your company is not good enough to work in, because you don't weed out underqualified workers.

You don't want to lose candidates. So you don't want to use icims.com. Really.

Tuesday, December 18, 2012

Stupid MySQL Help and Parser

Just stumbled upon this:

mysql> help CAST
Name: 'CAST'
Description:
Syntax:
CAST(expr AS type)

The CAST() function takes a value of one type and produce a value of
another type, similar to CONVERT(). See the description of CONVERT()
for more information.

URL: http://dev.mysql.com/doc/refman/5.1/en/cast-functions.html

mysql> select CAST ('2012-01-01 12:00:00' AS DATETIME);
ERROR 1305 (42000): FUNCTION CAST does not exist
mysql> select CAST('2012-01-01 12:00:00' AS DATETIME);
+-----------------------------------------+
| CAST('2012-01-01 12:00:00' AS DATETIME) |
+-----------------------------------------+
| 2012-01-01 12:00:00                     |
+-----------------------------------------+
1 row in set (0,00 sec)

I.e. the help claims that CAST is a function, but if you call it with a stray space after the name, it does not exist.

Tuesday, September 27, 2011

Autoconf and defaults

Autoconf is based on a set of macros. these macros are put by developers into the configure.ac file, and the "autoconf" program generates a proper configure script from that. The configure script commonly accepts options such as --enable-foo and --with-foo. It is important that they behave in a sensible way.

The --with-foo argument is processed with the AC_ARG_WITH macro. It is documented as follows:

AC_ARG_WITH (package, help-string, [action-if-given], [action-if-not-given])
If the user gave configure the option --with-package or --without-package, run shell commands action-if-given. If neither option was given, run shell commands action-if-not-given. The name package indicates another software package that this program should work with. It should consist only of alphanumeric characters, dashes, plus signs, and dots.
The option's argument is available to the shell commands action-if-given in the shell variable withval, which is actually just the value of the shell variable named with_package, with any non-alphanumeric characters in package changed into ‘_’. You may use that variable instead, if you wish.
 Let's see what's wrong with this piece of configure.ac, found in lxdm-0.4.1:

AC_ARG_WITH(pam,AC_HELP_STRING([--without-pam],[build without pam]),
[],[AC_CHECK_LIB([pam], [pam_open_session])])

This does nothing if the --without-pam option is not given, and checks for the PAM library if the option is given. Looks right? No!

This also does nothing if the --with-pam option is given, thus, resulting in a build without PAM support!

Note that the documentation for AC_ARG_WITH even provides examples how to use the macro properly. Use them as a reference, and have the following checklist:
  • Use the $withval or $with_package variable.
  • Test it for values such as "yes", "no", "auto" and explicit path.
  • Check the presence of the package unless it is explicitly disabled or the support is experimental.
  • Fail if the external package is requested but not available.
Yes, this becomes verbose, but it is necessary for correctness.

The first point also applies to the AC_ARG_ENABLE macro:

AC_ARG_ENABLE(debug,         AS_HELP_STRING([--enable-debug],
                                  [Enable debugging (default: disabled)]),
                                [DEBUGGING=$enableval], [DEBUGGING=no])

Saturday, March 12, 2011

The case of a non-raised exception

As you probably know, python uses exceptions for error handling. It is considered a good style to avoid adding error-handling code in the form of conditional statements. Instead, one should rely on the fact that an appropriate exception is raised once an error condition is detected, and caught when it can be dealt with.

So, let'a assume that you are given a task of downloading a file given the URL and the file name on disk, using Python. You may want to write the following code and hope that you don't have to add any error-handling because (as you think) all errors that can happen are either network errors or file write errors, and those two types of errors already raise exceptions for you.

#!/usr/bin/python

import urllib2
import sys
import socket

def download(url, fname):
    net = urllib2.urlopen(url)
    f = open(fname, "wb")
    
    while True:
        data = net.read(4096)
        if not data:
            break
        f.write(data)
    
    net.close()
    f.close()

if __name__ == "__main__":
    if len(sys.argv) != 3:
        print "Usage: download.py URL filename"
    
    url = sys.argv[1]
    fname = sys.argv[2]
    
    socket.setdefaulttimeout(30)
    
    download(url, fname)

Indeed, this code downloads existing files via HTTP just fine. Also, it provides sensible tracebacks for non-existing hosts, 404 errors, full-disk situations, and socket timeouts. So, it looks like the result of calling the download() fnction is either a successfully downloaded file, or an exception that the other part of the application will likely be able to deal with.

But actually, it only looks like this. Consider a situation when the HTTP server closes the connection gracefully at the TCP level, but prematurely. You can test this by starting your own Apache web server, putting a large file there, and calling "apache2ctl restart" while the client is downloading the file. Result: an incompletely downloaded file, and no exceptions.

I don't know if it should be considered a bug in urllib2 or in the example download() function above. In fact, urllib2 could have noticed the mismatch of the total number of bytes before the EOF and the value in the Content-Length HTTP header.

Here is a version of the download() function that detects incomplete downloads based on the Content-Length header:

def download(url, fname):
    net = urllib2.urlopen(url)
    contentlen = net.info().get("Content-Length", "")
    f = open(fname, "wb")
    datalen = 0
    
    while True:
        data = net.read(4096)
        if not data:
            break
        f.write(data)
 datalen += len(data)
    
    net.close()
    f.close()

    try:
        contentlen = int(contentlen)
    except ValueError:
        contentlen = None

    if contentlen is not None and contentlen != datalen:
        raise urllib2.URLError("Incomplete download")

Saturday, October 23, 2010

DNS servers in resolv.conf

Yesterday I found a problem with my Linode VPS. When running "aptitude update", it complained about not being able to resolve the name for security.debian.org (a server from where Debian distributes security updates).

Here is the /etc/resolv.conf file from that VPS, generated by dhclient:

domain members.linode.com
search members.linode.com
nameserver 109.74.194.20
nameserver 109.74.192.20
nameserver 109.74.193.20

It looks like the 109.74.194.20 server is overloaded and returns a SERVFAIL ("server failure - The name server was unable to process this query due to a problem with the name server") answer from time to time. Unfortunately, Debian has a bug in its version of libc that prevents the resolver from trying the other (working) servers if the first one returns an answer indicating a temporary failure.

The bug does not exist in glibc-2.12.1 on my Gentoo box at home. However, if the server does not respond at all, the Gentoo resolver spends 15 seconds before trying the next one, which is also not nice.

To work around the bug, you can run your own DNS server on 127.0.0.1 that forwards all queries to the official servers, and put 127.0.0.1 as the only nameserver in /etc/resolv.conf.

Monday, May 10, 2010

Converting audio samples

Different audio applications process audio in different sample formats. Some use 16-bit samples internally, others use 32-bit or floating point samples. In either case, there is a need to convert the sample format for the purposes of export and playback. Sounds like a simple task? No, a rich field for disagreements and bugs, even without taking dithering into account.

E.g., when converting from floating-point samples to 16 bits, there is no universal agreement on the scale. Some (e.g., JACK) think that 1.0 should convert to 32767 and -1.0 to -32767. Within that model, there are two opinions how to get -32768 (the most negative integer representable in 16 bits): "-1.00003" or "you can't". The other viewpoint, implemented, e.g., in ALSA-lib, is that -1.0 should map to -32768, 0.99997 should give 32767 and 1.0 should clip. There is also a third possibility, namely, to use different scale factors for negative and positive sample values, so that 0.0 maps to 0, 1.0 maps to 32767 and -1.0 maps to -32768.

And now some code. If one adopts the 32767.0 scale factor (as in JACK) and declares the -32768 sample value as being unreachable, one can use the following C function for conversion:


int16_t to_int16(float f)
{
if (f > 1.0) f = 1.0;
if (f < -1.0) f = -1.0;
return (int16_t)(f * 0x7fff);
}


As expected, to_int16(1.0) == 32767 and to_int16(-1.0) == -32767. However, the following quite similar function that is supposed to convert floating-point samples to 32-bit signed ones doesn't actually work properly:


int32_t to_int32(float f)
{
if (f > 1.0) f = 1.0;
if (f < -1.0) f = -1.0;
return (int32_t)(f * 0x7fffffff);
}


Indeed, to_int32(1.0) returns the minimum, not the maximum possible 32-bit integer. That happens because, unlike 0x7fff, one cannot represent 0x7fffffff exactly as a floating-point number. So the nearest representable one is substituted, and that's 0x80000000. So the overflow happens, and the function returns a wrong result.

One fix would be to cast 0x7fffffff to a double-precision floating point number. Another (but not 100% equivalent) option is to change the scale factor so that it is close enough, but doesn't cause this overflow, i.e. to 0x7fffff00.

Sunday, March 8, 2009

Don't use old dtoa.c

A lot of projects use the dtoa.c file that bears the following copyright notice:

 * The author of this software is David M. Gay.
*
* Copyright (c) 1991, 2000, 2001 by Lucent Technologies.


If your software uses it, please take steps to update this file or stop using it. The reason is very simple: your program will compile just fine, but will work incorrectly when compiled with gcc-4.4.0 (not released yet). Here is a short testcase (save as test.c):

#include <stdio.h>

double strtod(const char *s00, char **se);

int main(int argc, char* argv[])
{
double result = strtod(argv[1], 0);
printf("%f\n", result);
return 0;
}


Compile as follows: gcc-4.4 -O2 -Dstrtod=my_strtod -DIEEE_8087=1 -DHonor_FLT_ROUNDS=1 -DTrust_FLT_ROUNDS -DOmit_Private_Memory=1 -DNO_INFNAN_CHECK=1 -DNO_HEX_FP=1 dtoa.c test.c

The -Dstrtod=my_strtod flag is needed in order to make sure that the version of strtod() from dtoa.c, not from your system C library, is used. -O2 is the default optimization level used by many software projects. The rest of the flags are explained in the dtoa.c file.

The testcase converts its argument to a double-precision floating-point number, and then prints the result. So, when given a number, this testcase should print it back. let's try:

$ ./a.out 1.1
11.000000


The result is clearly incorrect. The reason (as one can see by appending -Wall to the compiler command line) is that strict aliasing rules are violated in dtoa.c. Indeed, if one adds the -fno-strict-aliasing flag to the compiler command line, the resulting program will behave correctly:

$ ./a.out 1.1
1.100000


Here is the formulation of the aliasing rules from ISO/IEC 9899:TC2, section 6.5:


An object shall have its stored value accessed only by an lvalue expression that has one of the following types:


  • a type compatible with the effective type of the object,

  • a qualified version of a type compatible with the effective type of the object,

  • a type that is the signed or unsigned type corresponding to the effective type of the object,

  • a type that is the signed or unsigned type corresponding to a qualified version of the effective type of the object,

  • an aggregate or union type that includes one of the aforementioned types among its members (including, recursively, a member of a subaggregate or contained union), or

  • a character type.



dtoa.c does this:

typedef union { double d; ULong L[2]; } U;

#define word0(x) ((U*)&x)->L[1]
#define word1(x) ((U*)&x)->L[0]
#define dval(x) ((U*)&x)->d


So, it stores doubles, but reads this memory using lvalue expressions of the "ULong" type (and the other way round), contrary to the standard.

GCC allows such access only if the memory is accessed through a union type. The TC3 revision of the C99 standard also allows type punning through the union in the footnote in the section 6.5.2.3:

If the member used to access the contents of a union object is not the same as the member last used to store a value in the object, the appropriate part of the object representation of the value is reinterpreted as an object representation in the new type as described in 6.2.6 (a process sometimes called "type punning".


In dtoa.c, the union is "invented" each time the code wants to reinterpret a double value as two ULongs. So, it doesn't make sense to speak about the member last used to store a value in the union, and the note above doesn't apply. I.e., instead of this code,

double a;
word0(a) = L;
word1(a) = 0;
return dval(a);


this should be used:

U a;
a.L[0] = L;
a.L[1] = 0;
return a.d;


Note that there are no pointer casts in the corrected code.

The topic of strict aliasing rules, with working and non-working examples, is explained in the GCC manual. An updated version of dtoa.c and the needed header are available in the sources of GCC itself.

Saturday, December 6, 2008

Fixes

Some time ago, I mentioned in this blog some bugs that annoyed me. Some of them got fixes:
  • Speaker-test fails with pcsp: fixed in ALSA Git.
  • Kernel panic due to iwlwifi: fixed in linux-2.6.27.6
  • Internal compiler error in GCC-4.3 on proprietary code: fixed in GCC SVN, the fix will be a part of GCC-4.4.

Sunday, November 23, 2008

Linux audio sucks

because of bugs in drivers, libraries, and applications. Here are some of them. They are quite real, because they interfere with my not-so-unrealistic requirements (play music through headphones connected to the onboard HD audio chip, make some noise with the PC speaker when someone calls me via SIP, and copy sound from my TV tuner to the HD audio chip when I watch TV).
  • In a driver. Take, for example, saa7134-alsa (a kernel module that allows recording audio from SAA713X-based TV tuners). It advertises support for the following sample rates: 32 kHz and 48 kHz. What it fails to mention is that it doesn't support recording at 48 kHz except for the case of "original SAA7134 with the MIXER_ADDR_LINE2 capture source". In all other cases (e.g., SAA7133 with the MIXER_ADDR_TVTUNER capture source), it gives out 32 kHz samples, but mislabels them as 48 kHz. Thus, applications such as MPlayer have to be configured to use only the 32 kHz sample rate. Not a big problem if such configuration is possible (i.e.: if not using PulseAudio together with HAL), but still a bug.
  • In the ALSA library. Today I discovered that the PC speaker at home no longer plays ring tones in SIP clients. It did work before, but I can no longer find a working revision. While testing this sound device with speaker-test, I found this floating point exception inside the ALSA library.
  • In applications. The most frequent bug is that an application doesn't have a way to use an arbitrary ALSA device, and instead has a drop-down box with pre-defined choices. Such applications often cannot work with Bluetooth headsets (that I don't have), FireWire audio cards (that I don't have either), and through non-default PulseAudio devices (and this has bit me when I tried to use Linphone with PulseAudio - I couldn't configure it to use the PC speaker through PulseAudio). What's even worse is that HAL endorses this faulty "drop-down box" enumeration scheme.

Saturday, September 27, 2008

Kernel panic

kernel panic

When I moved to another room in the office, my laptop started getting this kernel panic. So far, this means that I can't use the wireless network. I can work around this by either blacklisting the iwl4965 module or by turning the wireless card off with the hardware rfkill switch.

Maybe related: this laptop gets "ACPI: EC: non-query interrupt received, switching to interrupt mode" during boot.

I will report this panic to both the debian bug tracker and the linux-wireless list, let's see how they resolve it.

Saturday, September 20, 2008

Wrong macro

Here is how you shouldn't test for old compilers:

#if _MSC_VER < 1400
/* some workaround */
#endif


Reason: the workaround will also be applied to non-Microsoft compilers, e.g., gcc. Here is a more correct version:

#if defined(_MSC_VER) && _MSC_VER < 1400
/* some workaround */
#endif

Wednesday, September 17, 2008

gcc-4.3 troubles

After obtaining access to the CVS server at Yandex, I tried to build some internal utilities for daily use (e.g., for examining database contents) on my work laptop. The first attempt failed, because Debian Lenny comes with gcc-4.3.1, and the official build machines (that I was in fact supposed to use instead of the laptop) still use gcc-4.2.x.

First, some of the "known good" snapshots of external projects that are used by the to-be-built utilities have gcc-4.3 related issues. Fortunately, Debian's bug and package database was very helpful, and I was able to extract the needed patches.

Second, gcc-4.3 added some new warnings, and the build system is set up to use -Werror by default. While some of the new warnings are helpful, others are not. Try, e.g., compiling this simple function with g++ -Wconversion -c test.cpp:

typedef unsigned char uint8_t;
void grow(uint8_t & u)
{
u *= 7;
}

Result:

test.cpp: In function 'void grow(uint8_t&)':
test.cpp:4: warning: conversion to 'unsigned char' from 'int' may alter its value


I.e., almost every in-place mathematical operation with shorts and chars results in warnings! With -Werror, this means errors in many files. Obviously, this isn't worth fixing, so I removed -Wconversion from the compiler flags in my checkout. Similar bugs are already recorded in GCC bugzilla.

Third, even after fixing fixable warnings and removing -Wconversion, I hit an internal compiler error. Since I found no similar reports (the error manifested itself on proprietary code, after all), I reduced the testcase and submitted it to GCC bugzilla.

After that, I gave up. I posted my changes to the internal code review list (just so that they don't get lost), switched the compiler to gcc-4.2 (it is available in Debian side-by-side with 4.3), and compiled what I needed without further problems.

BTW, Debian still uses gcc-4.1.2 for compiling their kernels. It would be interesting to know why.