+cfe-dev, as this concerns Clang.
I am also in favour of a source annotation approach in general, but I have a few nitpicks on syntax.
- Can we use a name with "cfi" in it somewhere, so that it is clearer what the purpose of the attribute is?
- I would prefer to remove the quotes and just have the arguments be declaration references. To resolve circular references you can always forward declare the classes, i.e.
On Mon, Jun 19, 2017 at 8:16 AM, Enes Göktaş via llvm-dev <[hidden email]> wrote:
cfe-dev mailing list
I'm extremely concerned by this change.
A sanitizer has found a bug in your code. This is a bug that optimizations might take advantage of, resulting in your code misbehaving in all sorts of ways. Examples:
* we can and should use different TBAA keys for different vtable pointer slots. If we do so, we might in some cases reorder a vptr load (of a JSComponentMath vptr, for instance) before a ProxyClass vptr store.
* we might generate calls equivalent to dynamic_cast for some speculative devirtualization purposes, and those may not function properly because the type_info for ProxyClass does not indicate that it has a (say) JSComponentMath base class.
* ubsan's vptr sanitizer will (as in, today, this is not speculative) diagnose all virtual calls you make to a ProxyClass object that has been cast to JSComponentMath*
* ubsan's function sanitizer will diagnose three of the four virtual calls, because the function type does not match between caller and callee.
* LLVM will replace your entire 'main' function with an 'unreachable' instruction due to the calling convention mismatch between the function call and the devirtualized callee. This happens today if you just add -O2 to the clang command line in your reproducer script.
Making the sanitizer ignore the bug does not solve the problem, it just hides it for a while.
It seems to me that there are two plausible paths forward:
1) Change Firefox so that it does not do this any more, or at least, not in a way that a compiler can see through. I would guess the goal here is to share the member function definitions across a number of class hierarchies; do you also care about avoiding having a vtable and a type_info object per class you want to proxy?
2) Get all of your supported implementations to supply a language extension that guarantees the behaviour you want. But you'll need to enumerate what behavior you actually want defined here, including things like the calling convention mismatch and the bogus type info pointer in your vtables.
Given the other problems here, just allowing the CFI checker to pretend this problem doesn't exist doesn't seem like a valuable use of our time.
On 19 June 2017 at 10:06, Peter Collingbourne via llvm-dev <[hidden email]> wrote:
cfe-dev mailing list
|Free forum by Nabble||Edit this page|