Any way to generate warnings for function-pointer comparisons?
Asked Answered
T

1

30

It took me forever to track down that there was a bug in my code being triggered by /OPT:ICF:

Because /OPT:ICF can cause the same address to be assigned to different functions or read-only data members (const variables compiled by using /Gy), it can break a program that depends on unique addresses for functions or read-only data members.

(I had been storing and comparing function pointers for equality, which breaks when the linker throws away identical functions.)

Now I need to find every place where I might have done such a thing.

The test case is of course trivial:

//MSVC: /Gy /link /OPT:ICF
int test1(void) { return 0; }
int test2(void) { return 0; }
int main(void) { return test1 == test2; }

I've tried -Wall, -Wextra, -Weverything, -pedantic, etc. but none of them generate warnings.

Is there any compiler option or tool (whether part of Visual C++, GCC, Clang, or other) that can analyze my code and tell me where I'm comparing function pointers with each other, like in the code above?

Tollgate answered 29/1, 2018 at 5:8 Comment(30)
I love this question. Since it's up to the compiler to determine whether your comparison is true, you would like a diagnostic, yes?Fruition
@DrewDormann: Yes. I'm frankly flabbergasted that there doesn't already seem to be one as far as I can see.Tollgate
This appears to just be a tool recommendation question. It might help to just ask if there is a compiler option you can use.Cobblestone
@Tas: Edited...Tollgate
This is an invalid transformation the linker is making.Mundane
@R..: Interesting; citation? (though it wouldn't affect my question)Tollgate
You need software that will generate an abstract syntax tree for your code, and then you can find the comparisons on function pointers in the tree.Britteny
Note MSVC is performing a non-conforming optimization see Do distinct functions have distinct addresses? for details. Also see Is Visual Studio 2013 optimizing correctly in the presence of /OPT:ICF?Splendent
If you are ready to put in some effort, you can use LibTooling with clang to test for such cases. But ofcourse since your code is for MSVC, all the intrinsics and compiler extensions might not be supported.Potted
I know that absolutely does not answer the question, but C/C++ and coding languages in general are made so the functions are discriminated by names, and so functions pointers pointing to the same place should be the same name by conception. If you need to check equality between function pointers to discriminate functions it means there is a conception issue somewhere. Or maybe I'm wrong and then I need some explanations.Chrysolite
@DrewDormann well what MSVC is doing is not a valid folding, so MSVC should just not do this.Splendent
@BenjaminBarrois: If you'd like to ask "why would anyone ever think about comparing function pointers" then post it as a separate question and feel free to ping me and I'll be happy to post an answer. It's not relevant here though; I'm trying to remove them from my code, not add them.Tollgate
@Mehrdad That's an idea ;-)Chrysolite
@Mehrdad: wouldn't be deactivating the fusion of duplicated code in the linker a tweak (though not elegant and probably leading to execution inefficiency)?Chrysolite
@BenjaminBarrois: I don't understand your question. Are you asking if that would give me working code? Yes, but then that obviously means users of my code (including myself) can no longer use /OPT:REF.Tollgate
@Mehrdad: Yes they can if they also use /OPT:NOICF.Chrysolite
@BenjaminBarrois: Sigh, I (obviously) mistyped. I meant my users/myself can't use /OPT:ICF anymore.Tollgate
The OP deserves a big round of applause to spot this bug. Just amazing.Ot
@DeanSeo: Haha, thanks. It took me hours, and that was with my knowledge of what /OPT:ICF does. I just didn't realize it might be the culprit because I didn't think I was comparing function pointers anywhere, so it took me forever to realize I should try toggling it (and after that it was "merely" the pain of isolating the code). I can't imagine what another poor soul who doesn't know about the flag's behavior will have to go through to figure it out...Tollgate
@DeanSeo: P.S. I just noticed this is deja-vu...Tollgate
@Mehrdad should this question be closed as a duplicate then ? :pPotted
@AjayBrahmakshatriya: No, not a duplicate. This specific question (generate warnings) addresses countermeasures, whereas the linked question talks about causes.Humanism
@Humanism well since the MSVC optimization is non-conforming then why would any compiler issue a warning for this? If anything this should be a bug against MSVC or maybe if they don't want to fix it they could add the warning the OP wants as a work-around.Splendent
@ShafikYaghmour: MSVC++ has a perfectly conforming /OPT:NOICF which is the default; you can't blame Microsoft for adding a fast-but-dangerous option. Same with fast floating point math - if you don't understand the trade-off, stick with the safe default.Humanism
@Humanism sure, fair point but I guess my main point is it a reasonable expectation that any compiler besides maybe MSVC would produce a warning for this case.Splendent
@Shafik Why would a compiler produce warnings for valid c++ code, just because it can cause troubles with some non-standard linker options? Does g++ produce warnings for this use case?Cestode
@Voo: Because it is the entity in the best position to do so?Tollgate
@Mehrdad That would mean producing warnings for perfectly valid c++ code, just because some other tool violates the standard. Certainly we could do so, but this really seems like something a separate tool would be preferable. Shouldn't be too hard with LLVM actually, I'm afraid MSVC is far behind the curve on these kind of things though.Cestode
I guess you can hack your copy of gcc to do so, if you really wish to go in this direction.Luther
@ShafikYaghmour Well, one can (or could? I don't know whether they changed the relevant part of the standard) argue that folding those two functions is conforming.Immensurable
H
9

Is there any compiler option or tool (whether part of Visual C++, GCC, Clang, or other) that can analyze my code and tell me where I'm comparing function pointers with each other, like in the code above?

I'm not sure if there exists such a compiler option.

However, there is such a tool. clang-tidy. You can write your own checks for clang-tidy, it's actually remarkably easy if you follow this blog. Specifically, the AST comes with a bunch of matchers already, which should handle the use-case you want.

Something like this seems to work:

binaryOperator(
    anyOf(hasOperatorName("=="), hasOperatorName("!=")),
    hasLHS(ignoringImpCasts(declRefExpr(hasType(functionType())))),
    hasRHS(ignoringImpCasts(declRefExpr(hasType(functionType())))))

Which flags the example in the OP:

fp.cxx:3:25: note: "root" binds here
int main(void) { return test1 == test2; }
                        ^~~~~~~~~~~~~~

That works specifically for the OP case, but you actually have to be more explicit to match all the other likely cases:

const auto AnyFunc = ignoringImpCasts(declRefExpr(hasType(anyOf(
    functionType(),
    pointsTo(functionType()),
    references(functionType())))));

Finder->AddMatcher(binaryOperator(
    anyOf(hasOperatorName("=="), hasOperatorName("!=")),
    hasLHS(AnyFunc),
    hasRHS(AnyFunc)).bind("op"), this);

Or something close to that effect.

Hahnert answered 30/1, 2018 at 17:6 Comment(1)
Holy cow, I didn't realize just how easy it was to do this. I'll give this a try when I get a chance; thanks!!Tollgate

© 2022 - 2024 — McMap. All rights reserved.