This is GCC Bugzilla
This is GCC Bugzilla Version 2.20+
View Bug Activity | Format For Printing | Clone This Bug
Ubuntu's builds of GCC enable non-standard options (specifically -Wformat-security and -Wformat). In an attempt to maintain parity with the upstream testsuite output, I've gone through the testsuite and located many architecture-dependent format-string/type discontinuities (e.g. %d should be %ld for a pointer on x86_64). This patch seeks to address all the mismatches through a standardized use of %p with (void*), and in several cases, a forced (int) cast. Are these changes interesting? I am hoping they are not needlessly invasive, but I am obviously open to feedback.
Created an attachment (id=17530) [edit] testsuite updates for format strings and casts
Let's look at the testcases: g++.dg/ext/align1.C: a run testcase, the printf can be removed in this case. g++.old-deja/g++.law/operators28.C: An assemble only testcase so the undefined behavior is not invoked at runtime so it does not matter. gcc.dg/matrix/matrix-2.c: Weird as it has two dg-do, I don't know which one should be used here, but those changes look correct g++.dg/opt/alias2.C: that looks obviously correct. g++.old-deja/g++.abi/vbase1.C: Likewise. g++.old-deja/g++.brendan/template8.C: Again it only assembles the testcase so the undefined behavior does not matter. g++.old-deja/g++.eh/ptr1.C: that looks obviously correct. g++.old-deja/g++.jason/access23.C: Just gets assembled so the undefined behavior does not matter. g++.old-deja/g++.law/cvt8.C: Likewise. g++.old-deja/g++.mike/net35.C: that looks obviously correct. g++.old-deja/g++.mike/offset1.C: Likewise. g++.old-deja/g++.mike/p12306.C: That code is obviously dead code, it is if(0) out so maybe it should be removed ... g++.old-deja/g++.mike/p3579.C: : that looks obviously correct. g++.old-deja/g++.mike/p3708a.C: Likewise. g++.old-deja/g++.mike/p3708b.C: Likewise. g++.old-deja/g++.mike/p3708.C: Likewise. g++.old-deja/g++.mike/p646.C: Just gets assembled so the undefined behavior does not matter. g++.old-deja/g++.mike/p710.C: Likewise. g++.old-deja/g++.mike/p789a.C: that looks obviously correct. g++.old-deja/g++.mike/pmf2.C: Likewise. g++.old-deja/g++.mike/temp.C: Likewise. g++.old-deja/g++.other/temporary1.C: Likewise. g++.old-deja/g++.other/virtual8.C: Likewise. g++.old-deja/g++.pt/memtemp23.C: This testcase just links so the undefined behavior does not matter. g++.old-deja/g++.pt/memtemp24.C: Likewise. g++.old-deja/g++.pt/memtemp25.C: Likewise. g++.old-deja/g++.pt/memtemp26.C: Likewise. g++.old-deja/g++.pt/t39.C: Looks wrong, it should use %p for even ptr[2], the printf itself does not matter. g++.old-deja/g++.robertl/eb17.: that looks obviously correct. gcc.dg/pch/inline-4.c: This does not matter as only the assembly is compared so calling printf with a non string constant is ok. In summary some of the changes are correct, others technically don't matter and really are really changing for the better as the testcase changes itself and changes what the testcase is testing and Ubuntu is broken for enabling those warnings by default even when testing GCC.
g++.old-deja/g++.pt/t39.C: It looks like ptr[0],[1],[2] are either int or const char (i.e. ptr is int* or const char*). %p doesn't make much sense in those cases, so I opted for a cast. I'm not sure I followed your last sentence; I realize that many people find the Ubuntu default compiler flags inappropriate, but I'm hoping that can be set aside in the interests of helping to improve our ability to compare testsuite output. As you say, many of the changes are correct, and the rest have no negative impact.
(In reply to comment #3) > g++.old-deja/g++.pt/t39.C: It looks like ptr[0],[1],[2] are either int or const > char (i.e. ptr is int* or const char*). %p doesn't make much sense in those > cases, so I opted for a cast. Woops I needed to look at the testcase closer. Yes casting to int here is correct. > I'm not sure I followed your last sentence; I realize that many people find the > Ubuntu default compiler flags inappropriate, but I'm hoping that can be set > aside in the interests of helping to improve our ability to compare testsuite > output. As you say, many of the changes are correct, and the rest have no > negative impact. What I am saying is that those testcase does not need to change as the undefined behavior is not invoked so the warning does not matter, which case Ubuntu's default compiler flags are very inappropriate. Remember undefined behavior at runtime cannot cause a diagnostic by default according to the C/C++ standard and this is only undefined behavior at runtime.
Subject: Re: New: overhaul printf formats and type casts in testsuite The testsuite in FSF GCC tests correct behavior of FSF GCC and code compiled with it. If correct behavior for Ubuntu GCC and code compiled with that is different, Ubuntu needs to maintain its own testsuite patches. If a test will actually fail (compilation or execution) *with FSF GCC* on some platform at present, then a fix (to gcc-patches in the usual way following the documentation in contribute.html) would be appropriate.
I'm trying to minimize the Ubuntu patch by getting changes accepted for the FSF GCC testsuite. I'm hoping to demonstrate that many of the changes are valid and represent stricter C coding, though none change the behavior of the test itself. Are there any parts of the above patchset that would be acceptable? Anything I can trim or change? (Also, please see bug 39536, which should also be trivial and useful to add.)
Gentoo's >=gcc-4.3.3 is build with -Wformat and -Wformat-security enable to.
In general patches need to be sent to gcc-patches@gcc.gnu.org together with a ChangeLog entry following existing practice and a note how the patch was tested. Copyright assignment to the FSF is required for non-trivial patches, see also http://gcc.gnu.org/contribute.html.