[PR42748, armel] excess errors in libstdc++ testsuite when built with glibc-2.11

see http://gcc.gnu.org/ml/gcc-patches/2010-01/msg00670.html and followups. seen on the gcc-4_4-branch and the trunk, glibc version is eglibc-2.11.1

when running the libstdc++v3 testsuite on arm-linux-gnueabi, a lot of test cases fail with excess errors, e.g.

  FAIL: 17_intro/using_namespace_std_tr1_neg.cc (test for excess errors)

Excess errors:
In file included from /home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/bits/basic_string.h:2562,
                 from /home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/string:53,
                 from /home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/bitset:49,
                 from /home/doko/gcc/4.4/gcc-4.4-4.4.2/src/libstdc++-v3/testsuite/17_intro/using_namespace_std_tr1_neg.cc:23:
/home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/ext/string_conversions.h: In instantiation of '_String __gnu_cxx::__to_xstring(int (*)(_CharT*, size_t, const _CharT*, __va_list), size_t, const _CharT*, ...) [with _String = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, _CharT = char]':
/home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/bits/basic_string.h:2610: instantiated from here
/home/doko/gcc/4.4/gcc-4.4-4.4.2/build/arm-linux-gnueabi/libstdc++-v3/include/ext/string_conversions.h:90: note: the mangling of 'va_list' has changed in GCC 4.4

http://launchpadlibrarian.net/37789038/buildlog_ubuntu-lucid-armel.gcc-snapshot_20100109-0ubuntu2_FULLYBUILT.txt.gz

Mark, can you have a look to this issue, thanks!

Paolo --

I think that the warning is accurate; the mangling of va_list has indeed changed on ARM in GCC 4.4 in order to conform to the ARM ABI specifications. There is an option to turn off warnings about PSABI issues; -Wno-psabi. I think that option (if not some stronger option) should be used.

Oh, wait. Maybe we should not warn about this in a system header. That's probably the right choice and a trivial change to arm_mangle_type.

Do you agree?

-- Mark

Indeed. Matthias filed the PR and then I only changed the Summary to better clarify that the point is simply not warning, *ever*, in system headers. Nice that you agree about that analysis of mine and that a fix seems easy ;)

OK, I will fix this one.

Subject: Bug 42748

Author: mmitchel
Date: Mon Jan 25 03:14:25 2010
New Revision: 156202

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156202
Log:
 PR c++/42748
 * config/arm/arm.c (arm_mangle_type): Do not warn about changes to
 mangling of va_list in system headers.

 PR c++/42748
 * g++.dg/abi/arm_va_list2.C: New test.
 * g++.dg/abi/arm_va_list2.h: Companion header file.

Added:
    trunk/gcc/testsuite/g++.dg/abi/arm_va_list2.C
    trunk/gcc/testsuite/g++.dg/abi/arm_va_list2.h
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog

Fixed.

please consider a backport for the 4.4 branch

If you say 'consider' and are talking to a GWP and release manager, it seems unpolite to re-open at once.

Subject: Re: warnings about 'mangling of 'va_list' has changed
 in GCC 4.4' not suppressed in sytem headers

paolo dot carlini at oracle dot com wrote:

> If you say 'consider' and are talking to a GWP and release manager, it seems
> unpolite to re-open at once.

I certainly took no offense.

I do think the patch would apply easily to GCC 4.4, and I think it's
appropriate to apply it there. However, I have so little bandwidth for
GCC development these days that I will not be able to quickly do the
appropriate patch/test cycle.

Matthias, would you like to do that? If so, the patch is certainly
pre-approved. If not, I'll put it in my queue, but please be patient.

Thanks,

I didn't intend to be inpolite.

Currently running a test build; will commit if the build succeeds without regressions.

Actually, the preferred spelling is impolite, thus neither unpolite, nor inpolite ;) Anyway, IMVHO the patch is really safe, thus, great that Matthias can do the backport.

In general, I just *hate* these quick reopenings after somebody knowledgeable has closed an issue for a reason.

checked the patch on the 4.4 branch; the error message for the test case in the original report is gone, but there are some warnings about the mangling in the libstdc++ files as well. not reopening yet, as I didn't check a build on the trunk yet.

/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/18_support/exception
_ptr/rethrow_exception.cc:114: note: the mangling of 'va_list' has changed in GC
C 4.4
output is:
/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/18_support/exception
_ptr/rethrow_exception.cc:114: note: the mangling of 'va_list' has changed in GC
C 4.4

FAIL: 18_support/exception_ptr/rethrow_exception.cc (test for excess errors)

/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/19_diagnostics/error
_category/cons/default.cc:33: note: the mangling of 'va_list' has changed in GCC
 4.4
output is:
/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/19_diagnostics/error
_category/cons/default.cc:33: note: the mangling of 'va_list' has changed in GCC
 4.4

FAIL: 19_diagnostics/error_category/cons/default.cc (test for excess errors)
/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/19_diagnostics/error
_category/operators/equal.cc:34: note: the mangling of 'va_list' has changed in
GCC 4.4

FAIL: 19_diagnostics/error_code/cons/1.cc (test for excess errors)
Excess errors:
/home/doko/gcc/4.4/gcc-4.4-4.4.3/src/libstdc++-v3/testsuite/19_diagnostics/error_code/cons/1.cc:45: note: the mangling of 'va_list' has changed in GCC 4.4

Why is this a note and not simply a warning? Anyway, in_system_header should slowly go away, either use diagnostic_report_warnings_p (input_location) or in_system_header_at (input_location). None of them is necessary if you just used warning(OPT_Wpsabi,).

I can see that exception_ptr.h doesn't have the system header pragma, I think on purpose, some time ago we briefly discussed that and decided to not add it to the libsupc++ user-visible headers, if possible. <system_error> however *does* have it, thus, if there are problems with it in mainline, it's because inform do not behave exactly like normal warnings, I think Joseph warned us about that...

In general, I would say Manuel has very good points about this issue.

Subject: Re: warnings about 'mangling of 'va_list' has changed
 in GCC 4.4' not suppressed in sytem headers

manu at gcc dot gnu dot org wrote:

> Why is this a note and not simply a warning?

Because, as noted earlier, it's not reflective of any likely problem in
the user's code. I think a warning is appropriate if the compiler
detects something which might indicate a bug in the application, but
this is just the compiler telling you about that something might go
wrong if you linked with code form a different version of G++. Which is
unlikely, and might go wrong for other reasons too.

Paolo --

I don't understand why all libstdc++ headers should not have #pragma GCC system_header in them. Would you please explain that?

I think there's a semantic hair that we could split about whether they are "system" headers or "compiler" headers, but in either case, we never want to warn about them. If we made this note into a warning, we would still warn because we would still see that this is not a system header.

Thanks,

-- Mark

(In reply to comment #16)
> I don't understand why all libstdc++ headers should not have #pragma GCC
> system_header in them. Would you please explain that?

Hi Mark. For sure all the *library proper* headers should have (and afaik, have already) the pragma. If I understand correctly, the doubt is about the libsupc++ headers: at some point in the past we discussed that briefly and we came to the conclusion that if we could avoid decorating those too it would be cleaner (Gaby agreed, I'm 100% sure). But indeed, nothing set in stone, at least for 4.5.x, if we want to decorate the few user visibile headers in libsupc++, I do not object.

Paolo --

I think libsupc++ is just as much a "system library" as libstdc++. It doesn't have an ISO-standard API, of course, but that's not the point; it's just as much a part of the system/implementation as libstdc++. To the extent a program is going to use a header that we ship from libsupc++, that program certainly doesn't want to get random warnings about those headers.

However, Julian Brown warns me that he's been playing with this and that it doesn't entirely solve the problem. It may be that my earlier patch to arm.c is insufficient in some cases. So, I'd like the libstdc++ maintainers to make the change to the libsupc++ headers -- unless someone can articulate a good reason not to do so of course -- but there may still be more work to do. We will continue to investigate the possible non-workingness of the patch.

Thanks,

-- Mark

Ok, it's just an handful of files, after all. If Julian wants to add the pragmas, the change is pre-approved, otherwise, just let me know when you have the other issues under control, and I'll do it.

(In reply to comment #15)
> Subject: Re: warnings about 'mangling of 'va_list' has changed
> in GCC 4.4' not suppressed in sytem headers
>
> manu at gcc dot gnu dot org wrote:
>
> > Why is this a note and not simply a warning?
>
> Because, as noted earlier, it's not reflective of any likely problem in
> the user's code. I think a warning is appropriate if the compiler
> detects something which might indicate a bug in the application, but
> this is just the compiler telling you about that something might go
> wrong if you linked with code form a different version of G++. Which is
> unlikely, and might go wrong for other reasons too.

So it is the compiler telling you that something might go wrong and you can silence it with -Wno-psabi, which is a warning option. Moreover, as it stands now, if someone has -Werror=psabi, the compilation will not stop and they might get bitten by this. Moreover2, -w will not silence the note. I still completely fail to see why it is a note and not a warning.

In any case, using diagnostic_report_warnings_p (location) should fix it.

Subject: Re: warnings about 'mangling of 'va_list' has changed
 in GCC 4.4' not suppressed in sytem headers

manu at gcc dot gnu dot org wrote:

> In any case, using diagnostic_report_warnings_p (location) should fix it.

AFAICT, this is not the case; at the point of mangling, input_location
does not necessarily reflect the location at which the function was
declared. Julian Brown and I are looking into this.

Thanks,

Subject: Bug 42748

Author: mmitchel
Date: Sun Feb 28 17:07:54 2010
New Revision: 157124

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157124
Log:
2010-02-27 Mark Mitchell <email address hidden>

 PR c++/42748
 * cp-tree.h (push_tinst_level): Declare.
 (pop_tinst_level): Likewise.
 * pt.c (push_tinst_level): Give it external linkage.
 (pop_tinst_level): Likewise.
 * mangle.c (mangle_decl_string): Set the source location to that
 of the decl while mangling.

2010-02-27 Mark Mitchell <email address hidden>

 PR c++/42748
 * g++.dg/abi/mangle11.C: Adjust mangling warning locations.
 * g++.dg/abi/mangle12.C: Likewise.
 * g++.dg/abi/mangle20-2.C: Likewise.
 * g++.dg/abi/mangle17.C: Likewise.
 * g++.dg/template/cond2.C: Likewise.
 * g++.dg/template/pr35240.C: Likewise.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/mangle.c
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/abi/mangle11.C
    trunk/gcc/testsuite/g++.dg/abi/mangle12.C
    trunk/gcc/testsuite/g++.dg/abi/mangle17.C
    trunk/gcc/testsuite/g++.dg/abi/mangle20-2.C
    trunk/gcc/testsuite/g++.dg/template/cond2.C
    trunk/gcc/testsuite/g++.dg/template/pr35240.C

*** Bug 40459 has been marked as a duplicate of this bug. ***