[Linux] MB_LEN_MAX incorrect

• Key: STDCXX-436
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Critical
• Assignee: Travis Vitek
• Reporter: Mark Brown
• Component/s: 18. Language Support
• Affects Version/s: 4.1.3
• Fix Version/s: 4.2.0

Description

On my Linux system MB_LEN_MAX is normally defined to 16 but when I use the macro in a program compiled with stdcxx the macro evaluates to 1. The test case goes like this:

$ cat test.cpp && make CPPOPTS="-DGETCONF_MB_LEN_MAX=`getconf MB_LEN_MAX`" test && ./test
#include <assert.h>
#include <limits.h>

int main ()
{
assert (MB_LEN_MAX == GETCONF_MB_LEN_MAX);
}
gcc -c -I/home/mbrown/stdcxx/include/ansi -D_RWSTDDEBUG -I/home/mbrown/stdcxx/include -I/home/mbrown/stdcxx-gcc-4.1.1-11s/include -I/home/mbrown/stdcxx/examples/include -DGETCONF_MB_LEN_MAX=16 -pedantic -nostdinc++ -g -W -Wall -Wcast-qual -Winline -Wshadow -Wwrite-strings -Wno-long-long -Wcast-align test.cpp
gcc u.o -o u -L/home/mbrown/stdcxx-gcc-4.1.1-11s/lib -lstd11s -lsupc++ -lm
test: test.cpp:6: int main(): Assertion `1 == 16' failed.
Aborted

Comments

[Martin Sebor]

Confirmed on Red Hat EL 4.4:

$ cat /etc/redhat-release && icc --version && make CPPOPTS="-DGETCONF_MB_LEN_MAX=`getconf MB_LEN_MAX`" t && ./t
Red Hat Enterprise Linux AS release 4 (Nahant Update 4)
icc (ICC) 10.0 20070613
Copyright (C) 1985-2007 Intel Corporation. All rights reserved.

icc -c -I/amd/devco/sebor/stdcxx/include/ansi -D_RWSTDDEBUG -D_REENTRANT -I/amd/devco/sebor/stdcxx/include -I/build/sebor/stdcxx-icc-10.0.025-15D/include -I/amd/devco/sebor/stdcxx/examples/include -DGETCONF_MB_LEN_MAX=16 -cxxlib-nostd -g -w1 t.cpp
icc t.o -o t -cxxlib-nostd -lpthread -L/build/sebor/stdcxx-icc-10.0.025-15D/lib -Wl,-R/build/sebor/stdcxx-icc-10.0.025-15D/lib -lstd15D -lcxaguard -lsupc++ -lm
t: t.cpp:6: int main(): Assertion `1 == 16' failed.
Aborted

[Martin Sebor]

Also confirmed with 4.1.3 so it's not a regression. The problem doesn't exist on HP-UX or Solaris.

[Martin Sebor]

This seems like a pretty serious problem that I think should be fixed for 4.2. Bumping up Priority and scheduled accordingly.

[Martin Sebor]

Travis, can you please look into this when you have a moment?

[Travis Vitek]

2007-09-07 Travis Vitek <[e-mail]>

STDCXX-436
limits.h (MB_LEN_MAX): redefine MB_LEN_MAX if the current
value is different from the configured value.
climits (MB_LEN_MAX): ditto.

[Travis Vitek]

I have one comment on this. Here is a quick summary of the problem.

1. t.cpp includes <limits.h> which is actually our include/ansi/limits.h
2. include/ansi/limits.h includes the gcc modified limits.h
3. gcc modified limits.h attempts to use #include_next to include /usr/include/limits.h, but this actually gets include/ansi/limits.h again, which is a no-op because of the header guards.

The following is evidence of the include issue...

[vitek@robot] 623 % cat t.cpp
#include <limits.h>
int main() { return 0; }

[vitek@robot] 623 % g++ -H t.cpp
. /amd/packages/mdx/redhat/em64t/compilers/gcc/3.4.6-3/bin/../lib/gcc/x86_64-redhat-linux/3.4.6/include/limits.h
.. /amd/packages/mdx/redhat/em64t/compilers/gcc/3.4.6-3/bin/../lib/gcc/x86_64-redhat-linux/3.4.6/include/syslimits.h
... /amd/packages/mdx/redhat/em64t/compilers/gcc/3.4.6-3/bin/../lib/gcc/x86_64-redhat-linux/3.4.6/include/limits.h
.... /usr/include/limits.h
..... /usr/include/features.h
...... /usr/include/sys/cdefs.h
...... /usr/include/gnu/stubs.h
..... /usr/include/bits/posix1_lim.h
...... /usr/include/bits/local_lim.h
....... /usr/include/linux/limits.h
..... /usr/include/bits/posix2_lim.h
..... /usr/include/bits/xopen_lim.h
...... /usr/include/bits/stdio_lim.h

[vitek@robot] 624 % gmake CPPOPTS="-H" t
gcc -c -I/amd/devco/vitek/stdcxx/trunk/include/ansi -D_RWSTDDEBUG -pthread -I/amd/devco/vitek/stdcxx/trunk/include -I/nfs/devbuild/vitek/stdcxx/gcc-346/2007-09-07/include -I/amd/devco/vitek/stdcxx/trunk/tests/include -H -pedantic -nostdinc++ -g -W -Wall -Wcast-qual -Winline -Wshadow -Wwrite-strings -Wno-long-long -Wcast-align t.cpp
. /amd/devco/vitek/stdcxx/trunk/include/ansi/limits.h
.. /amd/devco/vitek/stdcxx/trunk/include/rw/_defs.h
... /amd/devco/vitek/stdcxx/trunk/include/rw/_config.h
.... /nfs/devbuild/vitek/stdcxx/gcc-346/2007-09-07/include/config.h
.... /amd/devco/vitek/stdcxx/trunk/include/rw/_config-gcc.h
.. /amd/packages/mdx/redhat/em64t/compilers/gcc/3.4.6-3/bin/../lib/gcc/x86_64-redhat-linux/3.4.6/include/../include/limits.h
... /amd/packages/mdx/redhat/em64t/compilers/gcc/3.4.6-3/bin/../lib/gcc/x86_64-redhat-linux/3.4.6/include/../include/syslimits.h
.... /amd/devco/vitek/stdcxx/trunk/include/ansi/limits.h

So the problem that I see with the workaround for this issue is that anything included from /usr/include/limits.h is not seen when building using our headers. This would cause problems for a user program that depends on macros, functions, or types declared to be included from /usr/include/limits.h such as this one,

#include <limits.h>
int main ()
{
char host [HOST_NAME_MAX];
return 0;
}

[Martin Sebor]

The fact that gcc's #include_next brings in a file that's already been included seems like a bug, don't you think?

About the patch, it looks good. Perhaps the only change we might want to consider is simplifying the preprocessor conditional to #if (MB_LEN_MAX != _RWSTD_MB_LEN_MAX). We should #define MB_LEN_MAX even (and especially) when it's not defined.

[Travis Vitek]

Yes, I agree that it sounds like a bug when gcc include_next includes a file that has already been included. It looks like that include_next mechanism is a big kludge that has infected the system headers because there is a bunch of stink in /usr/include/limits.h related to it.

Here is an updated patch. I've tested it on linux/gcc and it still works. I guess if I were being picky I'd move this block inside the block that includes _RWSTD_ANSI_C_LIMITS_H, but I'm happy with it the way that it is if you are.

[Martin Sebor]

Resolved by the committed patch: http://svn.apache.org/viewvc?rev=573904&view=rev.

Travis, can you put together a regression test suitable for our test suite?

[Travis Vitek]

The attached patch includes a regression test for this issue. Unfortunately this test fails on windows due to what appears to be a configuration test error.

The LIMITS.cpp configuration test has a block that attempts to detect MB_LEN_MAX, and chooses an appropriate default if it is not found. On some platforms, it appears that limits.h is included indirectly via stdio.h. Because the test does not include limits.h explicitly MB_LEN_MAX is not always defined, and the 'appropriate' default value of 8 is incorrect on windows [at least on my 32-bit XP configuration].

[Travis Vitek]

Additional patch that appears to address the above mentioned issue.

[Martin Sebor]

Travis, about the regression test, I was hoping for one that doesn't depend on our own implementation details. I.e., one that fails even when both MB_LEN_MAX and _RWSTD_MB_LEN_MAX are equally wrong. On UNIX, we can do it by invoking the 'getconf MB_LEN_MAX' command within the test and reading its output, like so:

FILE *f = popen("getconf MB_LEN_MAX");
int mb_len_max = -1;
fscanf (f, "%d", &mb_len_max);
assert (MB_LEN_MAX == mb_len_max);

I'm not sure if this is 100% portable even on UNIX or at all doable on Windows but that shouldn't stop us from implementing it on platforms where the getconf command works reliably.

[Martin Sebor]

Also, it seems I missed the LIMITS.cpp.patch you added on 9/7. Now that I've looked at it I'm pretty sure it contains a typo in the preprocessor directive: #dlif?

[Martin Sebor]

Reopened until a corrected Windows patch is committed.

[Travis Vitek]

Fix typographical error in patches conditional code

[Travis Vitek]

Updated regression test compiles a simple program that simulates getconf and then uses that program to get the values to compare against. The alternative was to just invoke getconv as suggested, but that didn't work on windows, and getconf on some platforms doesn't provide values for all constants [LONG_MIN and LONG_MAX on linux is one example].

[Martin Sebor]

Travis, please attach a ChangeLog for the new LIMITS.cpp patch.

I'll hold off on committing the regression test until we settle the question here:
http://www.nabble.com/-jira--Created%3A-%28STDCXX-436%29--Linux--MB_LEN_MAX-incorrect-tf4386078.html

[Travis Vitek]

Doh.

2007-09-13 Travis Vitek <[e-mail]>

STDCXX-436

• LIMITS.cpp: include limits.h to get definition of MB_MAX_LEN
  so that we don't have to guess the correct value. If we must
  guess a default on _WIN32, use a known good value.

[Martin Sebor]

Thanks. I massaged your Change Log a bit (corrected the typo in MB_LEN_MAX and added the [_WIN32] tag to indicate that the patch applies to _WIN32 only – see
http://www.gnu.org/prep/standards/html_node/Conditional-Changes.html#Conditional-Changes

Corrected patch committed thus: http://svn.apache.org/viewvc?rev=575978&view=rev

[Travis Vitek]

updated regression test.