mid-optimizer warning

[llvmbot]

Bugzilla Link	14722
Version	unspecified
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@lattner,@zygoloid

Extended Description

Please reimplement this mid-optimizer warning (currently in instcombine) in clang instead. Since static analysis can't find all these cases, please also implement it under -fsanitize=undefined. Testcase:

--

typedef void FTC_Node; typedef void FTC_GQuery; typedef void FTC_GNode; typedef void FTC_Cache; typedef void* FT_Pointer;

static unsigned char FTC_GNode_Compare(FTC_GNode gnode, FTC_GQuery gquery) { return 0; }

typedef unsigned char (*FTC_Node_CompareFunc)( FTC_Node node, FT_Pointer key, FTC_Cache cache );

void test() { FTC_Node *_node = 0; FTC_Cache _cache = 0; FTC_GQuery query = 0; FTC_Node_CompareFunc _nodcomp = (FTC_Node_CompareFunc)(FTC_GNode_Compare); _nodcomp( _node, &query, _cache ); }

--

I preserved the names from freetype2 in case anybody wants to go talk to them about how they're calling a function with an entirely wrong signature.

Previously this caused the following behaviour:

nlewycky@ducttape:~$ llvm/Debug+Asserts/bin/clang -O2 -Wall -Werror warn.c -c WARNING: While resolving call to function 'FTC_GNode_Compare' arguments were dropped! nlewycky@ducttape:~$ llvm/Debug+Asserts/bin/clang -Wall -Werror warn.c -c

As of r171041 it does this: nlewycky@ducttape:~$ llvm/Debug+Asserts/bin/clang -O2 -Wall -Werror warn.c -c error: while resolving call to function 'FTC_GNode_Compare' arguments were dropped 1 error generated. nlewycky@ducttape:~$ llvm/Debug+Asserts/bin/clang -Wall -Werror warn.c -c

which is now build-breaking. I'd also like some advice from the language folks, whether we may optimize to 'unreachable' here. I suspect there's some reason we don't already (K&R?).

[llvmbot]

Has this been fixed? I see that the warning itself has been removed.

[lattner]

Ok, fair point!

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

We don't want warnings to change when optimization level changes iff the warning can have false positives. If the warning is false positive free (as this one should be?) then it is suboptimal, but better than never detecting the problem.

This error (not warning!) is not false-positive-free. It's completely bogus on code like this, when building with -DNDEBUG:

include

inline void call(int arg, void fn) { if (arg == 0) ((void()(int, int))fn)(1, 2); else ((void(*)(int))fn)(arg); }

void fn(int k) {}

void g(int k) { assert(k != 0); call(k, (void*)fn); }

[lattner]

While that's true in general (e.g. for uninitialized variables etc), that's because of false positives. We don't want warnings to change when optimization level changes iff the warning can have false positives. If the warning is false positive free (as this one should be?) then it is suboptimal, but better than never detecting the problem.

[llvmbot]

...but we really want to avoid producing different warnings depending on the level of optimization.

[lattner]

One of the (few) advantages of instcombine producing this warning was that it could detect ODR violations at link time with LTO.

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

We should also revert whatever changed this from a warning to an error. It's not OK to reject this code; we should be compiling it to an 'unreachable'.

Catching this at runtime would indeed be nice. An augmented CFI-style technique could be effective (augmenting the function pointer lookup table with type information). Alternatively, -fsanitize=undefined could maintain a side-table with type information.