29

I've had a really bizarre problem that I've reduced to the following test case:

#include <iostream> #include <map> #include <string> struct Test { std::map<std::string, void (Test::*)()> m; Test() { this->m["test1"] = &Test::test1; this->m["test2"] = &Test::test2; } void test1() { } void test2() { } void dispatch(std::string s) { if (this->m.at(s) == &Test::test1) { std::cout << "test1 will be called..." << std::endl; } else if (this->m.at(s) == &Test::test2) { std::cout << "test2 will be called..." << std::endl; } (this->*this->m.at(s))(); } }; int main() { Test t; t.dispatch("test1"); t.dispatch("test2"); }

It outputs

test1 will be called...
test1 will be called...

when optimizations are enabled, which is really bizarre. What's going on?

Improve this question
22
  • 1
    Using function pointers as unique identifying "handles" in this way seems like a bit of a non-starter, for precisely this reason. Commented Jan 5, 2013 at 21:01
  • 7
    @Mehrdad: Because the question is about C++, and the testcase is written in C++, and C++ is a different language to C with different rules. Commented Jan 5, 2013 at 21:05
  • 4
    @LightnessRacesinOrbit Please stay constructive. "I don't care about anybody who wants to google this issue" and "change it now but IDK what to change it" sounds selfish. Commented Jan 5, 2013 at 21:25
  • 2
    @Mehrdad: I chose that phrase not without precedent. See Andromeda wherein Trance says "I don't care about the bones", implying not that she has an expressly negative personal and emotional attachment to them but instead that her experience and wisdom has placed no great importance on them, in contrast with the claims of others nearby that the bones had significant value. Similarly here I meant only that, in my view, tagging questions correctly is more important than maximising search engine hits. Otherwise let's tag the question php, java and mysql too to get everyone involved! =) Commented Jan 5, 2013 at 21:41
  • 2
    @JamesMcNellis: Done! Feel free to ping me if you notice them post a follow-up that I don't notice within a day or so. Commented Jan 6, 2013 at 7:46

3 Answers 3

Reset to default
27

This is a by-product of what Visual C++ refers to as Identical COMDAT Folding (ICF). It merges identical functions into a single instance. You can disable it by adding the following switch to the linker commandline: /OPT:NOICF (from the Visual Studio UI it is found under Properties->Linker->Optimization->Enable COMDAT Folding)

You can find details at the MSDN article here: /OPT (Optimizations)

The switch is a linker-stage switch, which means you won't be able to enable it just for a specific module or a specific region of code (such as __pragma( optimize() ) which is available for compiler-stage optimization).

In general, however, it is considered poor practice to rely on either function pointers or literal string pointers (const char*) for testing uniqueness. String folding is widely implemented by almost all C/C++ compilers. Function folding is only available on Visual C++ at this time, though increased widespread use of template<> meta-programming has increased requests for this feature to be added to gcc and clang toolchains.

Edit: Starting with binutils 2.19, the included gold linker supposedly also supports ICF, though I have been unable to verify it on my local Ubuntu 12.10 install.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

In general, however, it is considered poor practice to rely on either function pointers ... I agree with this and I don't see why anyone would need to rely on this. See a more general discussion here
19

It turns out Visual C++'s linker can merge functions with identical definitions into one.
Whether that's legal or not according to C++, I have no idea; it affects observable behavior, so it looks like a bug to me. Someone else with more information may want to chime in on that though.

Improve this answer

14 Comments

Or, more accurately, "implementers taking liberties".
I don't see anything in 5.3.1 to prohibit this. & should give you a pointer to the member function, but it says nothing about any requirement that this must be unique.
@Lightness Races in Orbit: This joke is too predictable to be truly funny :)
@AndreyT: I wasn't joking!
@Lightness Races in Orbit: I cannot find the connect issue, but this is the thread I posted at the same time on MS forums: social.msdn.microsoft.com/Forums/en/vclanguage/thread/…
|
7

C++11 5.3.1 describes what & does; in this instance, it gives you a pointer to the member function in question, and the passage makes no requirement that this pointer must be unique.

However, 5.10/1 says about ==:

Two pointers of the same type compare equal if and only if they are both null, both point to the same function, or both represent the same address.

The question then becomes... are test1 and test2 "the same function"?

Though the optimizer has collapsed them into a single definition, arguably the two names identify two functions and, as such, this would seem to be an implementation bug.

(Note, though, that the VS team don't care and consider it "valid enough" to warrant the benefits of the optimisation. That, or they don't realise that it's invalid.)

I'd stick to using the strings as "handles" for your function pointers.

Improve this answer

10 Comments

+1, but I would suggest you use int as "handles" as they are faster to compare.
@Caesar: Not if you factor in the time to create and maintain a look-up for those integers.
@Caesar: Why not enum class? they're fast as well as they've names!
They both represent the same address. Therefore Visual C++'s optimization is still standards compliant.
@jstine: Perhaps you can clarify precisely what it is about the standard that I have misinterpreted. As for VC++, it's not baseless -- in the thread I linked to, Passant and his cohorts explicitly claim that the functionality is not a bug, even though it is. These conclusions of mine do rely on the notion that I haven't made a mistake; I'm certainly open to the possibility that I have, but perhaps you could identify it for me rather than simply screaming "you're wrong", downvoting and running away?
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.