infinite loop involving prettyPrintFloat with -ffast-math

[peter]

edit: solved: the code is right, but -ffast-math ignores the checks that guard against infinities.

while shuffling parts between ships, I clicked the Info -> Ship Info button at a base. Vegastrike is now stuck. I was able to attach to the process with gdb, and it's spending all its time in a small loop in prettyPrintFloat().

Code: Select all

(gdb) bt
#0  0x0000000000a7a1a8 in prettyPrintFloat ()
#1  0x0000000000aa130c in showUnitStats ()
#2  0x0000000000aa74dd in BaseComputer::showShipStats ()
#3  0x00000000008797db in Window::processCommand ()
#4  0x000000000085d252 in EventManager::sendCommand ()
#5  0x0000000000865c35 in NewButton::processMouseUp ()
#6  0x000000000085d945 in EventManager::sendInputEvent ()
#7  0x000000000085dcad in EventManager::ProcessMouseClick ()
#8  0x0000000000b71ffa in winsys_process_events ()
#9  0x0000000000b59258 in GFXLoop ()
#10 0x0000000000b74ea1 in main ()

(gdb) disas
...
0x0000000000a7a1a8 <_Z16prettyPrintFloatPcfii+72>:      inc    %r8d
0x0000000000a7a1ab <_Z16prettyPrintFloatPcfii+75>:      mulss  %xmm1,%xmm0
0x0000000000a7a1af <_Z16prettyPrintFloatPcfii+79>:      comiss %xmm2,%xmm0
0x0000000000a7a1b2 <_Z16prettyPrintFloatPcfii+82>:      jae    0xa7a1a8 <_Z16prettyPrintFloatPcfii+72>
...

(gdb) info reg vec
xmm0           {v4_float = {0x0, 0x0, 0x0, 0x0}, v2_double = {0x0, 0x0}, 
  v16_int8 = {0x0, 0x0, 0x80, 0x7f, 0x0 <repeats 12 times>}, v8_int16 = {0x0, 
    0x7f80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, v4_int32 = {0x7f800000, 0x0, 0x0, 
    0x0}, v2_int64 = {0x7f800000, 0x0}, 
  uint128 = 0x0000000000000000000000007f800000}
xmm1           {v4_float = {0x0, 0x0, 0x0, 0x0}, v2_double = {0x0, 0x0}, 
  v16_int8 = {0xcd, 0xcc, 0xcc, 0x3d, 0x0 <repeats 12 times>}, v8_int16 = {
    0xcccd, 0x3dcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, v4_int32 = {0x3dcccccd, 
    0x0, 0x0, 0x0}, v2_int64 = {0x3dcccccd, 0x0}, 
  uint128 = 0x0000000000000000000000003dcccccd}
xmm2           {v4_float = {0x1, 0x0, 0x0, 0x0}, v2_double = {0x0, 0x0}, 
  v16_int8 = {0x0, 0x0, 0x80, 0x3f, 0x0 <repeats 12 times>}, v8_int16 = {0x0, 
    0x3f80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, v4_int32 = {0x3f800000, 0x0, 0x0, 
    0x0}, v2_int64 = {0x3f800000, 0x0}, 
  uint128 = 0x0000000000000000000000003f800000}

If I continue and break again, register r8 will be however much higher than last time, but the xmm regs will still all have the same values. I think these 4 insns are an infinite loop, because comiss sets the flags before the jae.

I'm going to sleep soon, so reply if you want me to try anything. Unfortunately, I didn't have -g on when I compiled, so I can't see values of variables, only CPU register contents.

If nobody wants me to do any specific debugging, I'll probably try to get it to continue outside the loop, because I didn't save some of the changes I made to my ships (adding some Ktek Beams from an Rlaan trantor planet I stumbled across ) before this bug struck. Then I'll be able to tell you exactly what was in the mostly-stripped-down pacifier.stock in question. It definitely had weapons but no reactor, capacitors, or sensors.

[peter]

ok, I successfully got out of the infloop with

Code: Select all

(gdb) set $r8 = 5

(gdb) jump *0x0000000000a7a1b4
Continuing at 0xa7a1b4.

The parts of the ship stats that look weird are:

Code: Select all

 Reactor nominal replenish time: ((,(((.(( seconds
 Reactor recharge slowdown caused by shield maintenance: nan %

So probably the replenish time called prettyPrintFloat with a weird argument. Probably +Infinity, since I have 150 GJ capacitance, and 0 MJ/s recharge rate. prettyPrintFloat should never get stuck even with bad args. Weird, though, since finite(3) says it should return non-zero for +/-Inf as well as NaN. Oh, maybe -ffast-math breaks those checks. I did compile with -ffast-math.

Yeah, on closer examination, I think -ffast-math is the problem. It sets -ffinite-math-only, among other things. I don't see any function calls in the disassembly of prettyPrintFloat, or anything that looks like a check for non-finite numbers. So -ffast-math -> assume finite() returns 0.

[ace123]

Don't know why we even have that function--it's only used in cmd/basecomputer.cpp and to me it looks like it could be easily replaced by a sprintf

[peter]

Don't know why we even have that function--it's only used in cmd/basecomputer.cpp and to me it looks like it could be easily replaced by a sprintf

AFAICT, prettyPrintFloat exists to put commas as a thousands separator. e.g. 10,000 MJ instead of 10000 MJ. IIRC, there's no printf format specifier for that. It should be used in more places, like for prices of things (e.g. in the trade window), since they often have more digits than you can perceive the cardinality of without counting.

Yeah, from printf(3):

For some numeric conversions a radix character ("decimal point") or thousands’ grouping character is
used. The actual character used depends on the LC_NUMERIC part of the locale. The POSIX locale
uses '.' as radix character, and does not have a grouping character. Thus,

printf("%'.2f", 1234567.89);

results in "1234567.89" in the POSIX locale, in "1234567,89" in the nl_NL locale, and in
"1.234.567,89" in the da_DK locale.


So getting thousands grouping is locale-dependent. Does the windows build use mingw? Can you set setlocale(LC_NUMERIC, "something") and have anything useful happen? If so, then besides windows VS only needs to be portable to modern Unix systems where locales do work properly. OTOH, when you don't want thousands grouping, there's no way to turn it off except by calling setlocale with a different LC_NUMERIC, probably. Yuck.

[ace123]

Ah, that makes sense. I am surprised that only the basecomputer uses this--It really should be used in more places.

Personally, I dislike locales as they are implemented in POSIX. To start, there is the fact that the environment variables are usually horribly wrong (or more often, nonexistent) even on modern distros. For example, in order to get filenames to correctly display in a terminal, I must do "LC_CTYPE=en_US.UTF-8 ls -l", or else some files show up as "?????" without even attempting to acknowledge utf-8.

For some never-explained reason, the default is something called a "C" or "POSIX" locale which seems to make stupid decisions (rather than picking a sane default like en-US) from what I can find, means that they sometimes use a system-dependent file, but echo $. Here's a page about locales: http://www.math.hkbu.edu.hk/parallel/pg ... a_9169.htm

And lastly, that still doesn't explain why we should be forced to rewrite something that printf should be able to do (I didn't realize it supported thousand separators). Perhaps there is a simple function to change something within the locale to force having a "," separator. I wasn't able to get your example to work. I installed "de" and "da" locale files, and neither one changed when I did:

LC_NUMERIC=da_DK.UTF-8 ./test
where test.c looks like:

Code: Select all

#include <stdio.h>
int main() {
        float myfloat=123456.0123;
        printf("%'.2f\n", myfloat);
        return 0;
}

How did you get that output with a comma?

[peter]

Ah, that makes sense. I am surprised that only the basecomputer uses this--It really should be used in more places.

Personally, I dislike locales as they are implemented in POSIX. To start, there is the fact that the environment variables are usually horribly wrong (or more often, nonexistent) even on modern distros. For example, in order to get filenames to correctly display in a terminal, I must do "LC_CTYPE=en_US.UTF-8 ls -l", or else some files show up as "?????" without even attempting to acknowledge utf-8.

For some never-explained reason, the default is something called a "C" or "POSIX" locale which seems to make stupid decisions (rather than picking a sane default like en-US) from what I can find, means that they sometimes use a system-dependent file, but echo $. Here's a page about locales: http://www.math.hkbu.edu.hk/parallel/pg ... a_9169.htm

The C locale is the behaviour described in the C standard, as described in K&R 2nd edition. isalpha() is true only for plain ASCII letters. etc. etc. See below for why this is the default. You need LANG=something.utf8 or .UTF-8 if you want locale-aware programs to use UTF-8. Programs tend to use the locale system for i18n, instead of being utf8 unless told otherwise. (e.g. fr_FR or fr_CA, or whatever.) If your xterm wasn't UTF-8 aware, you would want ls to print question marks instead of binary garbage that would mess up your screen. Even if that "binary garbage" is valid UTF-8, ls assumes you don't want it because you didn't tell it you wanted it.

But on the whole, I sympathize with your complaints. I use Ubuntu, where UTF-8 seems to work pretty much everywhere (although I have very few non-ascii filenames), and it does work, but logging in to other computers and having LANG set correctly has been a huge pain. It's almost as bad as fonts, another thing I want to just work and not care about because they don't look ugly so I don't have to fix them. (which fortunately is the case these days). Some unix tools aren't utf-8, though. e.g. tr(1) is not locale-aware, and is ASCII-only.

And lastly, that still doesn't explain why we should be forced to rewrite something that printf should be able to do (I didn't realize it supported thousand separators). Perhaps there is a simple function to change something within the locale to force having a "," separator. I wasn't able to get your example to work. I installed "de" and "da" locale files, and neither one changed when I did:

LC_NUMERIC=da_DK.UTF-8 ./test
where test.c looks like:

Code: Select all

#include <stdio.h>
int main() {
        float myfloat=123456.0123;
        printf("%'.2f\n", myfloat);
        return 0;
}

How did you get that output with a comma?

copy/paste from the printf(3) man page.

But the locale env vars aren't enough by themselves. You have to tell libc you want to have locale-dependent behaviour by calling setlocale, I think. setlocale(LC_NUMERIC, "en_CA") will affect sprintf and anything else that turns a number into a string. Probably it will let printf parse floats with , instead of .. setlocale(LC_CTYPE, "fr_CA") will make isalpha(3) be true for vowels with accents (in ISO Latin 1, ISO8859-1). It will make strcmp treat é, è, ê, and e as equal. Among other things. This is definitely not stuff you want to happen unexpectedly.

Much old code made assumptions that are only true in the C locale, so it has to be the default behaviour for non-locale-aware programs. e.g. you can't capitalize a lowercase é (LATIN SMALL LETTER E WITH ACUTE (U+00E9) in unicode) by subtracting 26 from a byte. In unicode, chararacter != byte, so there's no way to get unicode support for free, because a lot of code just depends on that.

Ubuntu does pretty well at setting a useful locale. It sets up a /etc/default/locale containing, for example, LANG="en_CA.UTF-8". It does pretty well at applying it to all means of logging in by using pam_env.so to read it, for login, ssh, gdm, gdm-autologin, and su.

[peter]

And lastly, that still doesn't explain why we should be forced to rewrite something that printf should be able to do (I didn't realize it supported thousand separators)

There are a lot of parts of the C library that could be a lot better. Some of it is just showing its age, e.g. it predates UTF-8 by a lot. I actually like printf a lot, though, and it would be nice if there was a format specifier, or a modifier letter for %f/%g/%d, to use a thousands separator. However, even the GNU version of printf can't do _everything_. So unless you want a printf with hooks for call-back functions to modify the behaviour, sometimes you're going to have to write your own versions of things that work slightly differently. This is why open source is good: you can just add your tweaks to something, instead of having to have an API that will let anyone do anything imaginable... (Donald Knuth has said he's in favour of code you can reuse by editting much more than the OO idea of trying to achieve code reusability by black-boxing things away and having APIs... And I'm pretty sure he was more eloquent and clear than that paraphrase. )

[peter]

I decided to try out this locales stuff

... How did you get that output with a comma?

copy/paste from the printf(3) man page.

I tried setlocale:

Code: Select all

#include <locale.h>
#include <stdio.h>

int main() {
	setlocale(LC_ALL, "");  // this sets locales from the env vars the normal way
        double myfloat=123456.0123;
        printf("%'.2f\n", myfloat);
        return 0;
}

peter@whale:~/src$ LANG=en_CA.utf8 ./localetest 
123,456.01
peter@whale:~/src$ LANG=en_US.utf8 ./localetest 
123,456.01
peter@whale:~/src$ LANG=en_DK.utf8 ./localetest 
123.456,01
peter@whale:~/src$ LANG=C ./localetest 
123456.01

So I guess even the normal en utf8 locales use a thousands separator. (If I say LANG= a locale not found in /usr/lib/locale/, then it behaves the same as LANG=C. except that .utf8 == .UTF-8.)

[ace123]

I didn't know about that setlocale(LC_ALL, ""); thing

So perhaps we could have a configuration variable called "locale" in vegastrike.config, and default it to the empty string "" (which does "POSIX" behavior). Then, it would theoretically be possible to get numbers in different languages (though not too useful since nothing is translated yet).

Also, there is the problem of Windows, where language and country names are fully spelled out. So "en_US.UTF-8" in windows terminology is "English_United States.65001" (The 65001 is 1337speak for utf-8) So we would need to have a reasonable default, and then inform users that the names are different in Win32 and POSIX.

[peter]

I didn't know about that setlocale(LC_ALL, ""); thing

No worries. I only know because I had to read way more than I ever wanted to know about locales to figure out why something wasn't working for a project I was working on...

So perhaps we could have a configuration variable called "locale" in vegastrike.config, and default it to the empty string "" (which does "POSIX" behavior).

Ye gods, don't add more complexity to the locale situation! If you're going to introduce locale support in VS, have it obey $LANG/$LC_whatever by just using setlocale(LC_ALL, "") like a normal program. Or maybe just LC_NUMERIC, LC_TIME, and LC_MESSAGES, if you don't want LC_CTYPE and LC_COLLATE to make strings sort case-insensitively and all that. LC_MESSAGES will only matter if someone ever writes some localized strings for a non-english locale, which as you say hasn't happened yet.

Then, it would theoretically be possible to get numbers in different languages (though not too useful since nothing is translated yet).

Did you mean, in languages that don't use arabic numerals (0-9)? That would be interesting, and might not be what people want, even if VS's fonts have glyphs for the characters. Of course, anyone can write a wrapper script vegastrike.sh, that sets some env vars before execcing vegastrike.

Also, there is the problem of Windows, where language and country names are fully spelled out. So "en_US.UTF-8" in windows terminology is "English_United States.65001" (The 65001 is 1337speak for utf-8) So we would need to have a reasonable default, and then inform users that the names are different in Win32 and POSIX.

Does mingw's runtime not do POSIX setlocale? If you want to put some hacks in to support i18n on windows, then maybe, but I wouldn't suggest using a VS config setting instead of the usual env vars on systems where they do work properly. Probably the thing to do is go look up how to do portable i18n/l10n, instead of trying to invent our own hacked-up way for VS.

If someone starts translating VS into another language, then it would really matter that l10n works on windows. Until then, it's just a cosmetic improvement (thousands separators in numbers for starters) on non-windows platforms if we just setlocale(), and it doesn't work on Windows. (err, assuming it doesn't actually break anything anywhere.)

[loki1950]

There is a German translation that IIRC is almost complete it was hosted as a separate project and the svn repo they used should still be there But the efforts lead is no longer active real life threw him a curve from his PM at the time.

Enjoy the Choice

[peter]

I was playing btanks yesterday, and noticed it had a languages option, and the default was "system default", but you could select a few other languages (maybe that it has translations for). That seemed like a good way to have a language config option: have the default be "system default" (i.e. what locale(1) reports, from $LANG, $LC_...)