[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#780718: return in ?:-operator with function calls is incorrectly optimized



Package: g++
Version: 4:4.7.2-1
Severity: normal

Hi,

while testing apt with gcc-5 suite I noticed that while it compiles just
fine, the resulting binaries produce incorrect results failing our
integration tests (which aren't run at package buildtime, so this wasn't
detected in the gcc-5 mass-rebuild).

While trying to come up with a smaller testcase I noticed¹ that with
a slight variation even the g++-4.x series² emits the same strange
results… clang on the other hand produces the expected results, neither
of them generates any warnings about the code.


Attached is as far as I got in terms of a testcase. Still depends on
libapt as if I make the method directly available everything is fine…

The results I get are:
$ g++ -Wall -Wextra -O0 -lapt-pkg /tmp/test.cc -o /tmp/test && /tmp/test
amd64
i386
armel
$ g++ -Wall -Wextra -O2 -lapt-pkg /tmp/test.cc -o /tmp/test && /tmp/test
amd64,i386,armel

Obviously, I would expect the same result with/without optimization.

Doing the suggested modification in getDefaultVector() gets the g++-4.x
series to produce the expected results. g++-5 still emits the results
from above (this is a closer representation of the code in apt, which
is why this hasn't happend before in apt).



Note that getDefaultVector() is called in the false branch of the ?:
operator, but that should never happen as the expression evaluates to
true. This seems to nugde the return type optimization in the wrong
direction through…

Note also that the value of the vector isn't the value of either of the
branches of the ?: operator, but is equal to the first argument of the
method called in the true branch.

If the ?: is replaced with an if-else block everything is fine.
(which is what I am gonna do – way too complicated for an ?: …)

Note Note: I am really bad at guessing and I tend to dislike
bugreporters who do it myself, but I couldn't help it…
The subject being the biggest offender really…
So take everything I just noted with at least a bit of salt.


Best regards

David Kalnischkies

¹ with the help of Julien Cristau and Helmut Grohne
² 4.x series tested in a wheezy chroot, hence the 4:4.7.2-1 version tag
// g++ -Wall -Wextra -O0 -lapt-pkg /tmp/test.cc -o /tmp/test && /tmp/test
// g++ -Wall -Wextra -O2 -lapt-pkg /tmp/test.cc -o /tmp/test && /tmp/test
#include <string>
#include <map>
#include <vector>
#include <iostream>

#include <apt-pkg/strutl.h>
/* method code for reference
vector<string> VectorizeString(string const &haystack, char const &split)
{
   vector<string> exploded;
   if (haystack.empty() == true)
      return exploded;
   string::const_iterator start = haystack.begin();
   string::const_iterator end = start;
   do {
      for (; end != haystack.end() && *end != split; ++end);
      exploded.push_back(string(start, end));
      start = end + 1;
   } while (end != haystack.end() && (++end) != haystack.end());
   return exploded;
}
//*/

std::vector<std::string> getDefaultVector() {
	std::vector<std::string> r; // broken g++-4.x & g++-5
	//static std::vector<std::string> r; // just g++-5
	return r;
}

void parseOptions(std::map<std::string, std::string> const &Options, std::string const &field) {
	std::map<std::string, std::string>::const_iterator arch = Options.find(field);
	std::vector<std::string> Archs =
		(arch != Options.end()) ? VectorizeString(arch->second, ',') :
		getDefaultVector(); // <- never called, but method content effects result

	for (std::vector<std::string>::const_iterator a = Archs.begin(); a != Archs.end(); ++a)
		std::cout << *a << std::endl;
}

int main() {
	std::map<std::string, std::string> Options;
	Options["a"] = "amd64,i386,armel";
	parseOptions(Options, "a");

	return 0;
}

Attachment: signature.asc
Description: Digital signature


Reply to: