[PATCH] CFI: Implement bitset emission for the Microsoft ABI.

Peter Collingbourne peter at pcc.me.uk
Thu Jun 18 19:37:10 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/AST/MicrosoftMangle.cpp:2765
@@ +2764,3 @@
+  Linkage L = RD->getLinkageInternal();
+  if (L == InternalLinkage || L == UniqueExternalLinkage) {
+    // This part of the identifier needs to be unique across all translation
----------------
rnk wrote:
> What about NoLinkage, which I believe applies to types declared in functions? This check appears equivalent to !RD->isExternallyVisible(), which I think is probably what you want.
> 
> This is the test case I'm imagining:
> foo.h:
>   struct A { virtual int f(); };
>   void use_a(A&);
> a.cpp:
>   static void f() { struct B : A { virtual int f() { return 1; } } b; use_a(b); }
> b.cpp:
>   static void f() { struct B : A { virtual int f() { return 2; } } b; use_a(b); }
> 
> It seems like you'd need two distinct tags for B, right?
Good catch. `!RD->isExternallyVisible()` does seem like the correct test. The Itanium implementation had the same problem; I've fixed that as well and added a test for both.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1701-1705
@@ -1600,3 +1700,7 @@
 
   MicrosoftVTableContext::MethodVFTableLocation ML =
       CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
+  if (CGF.SanOpts.has(SanitizerKind::CFIVCall))
+    CGF.EmitVTablePtrCheck(getClassAtVTableLocation(getContext(), GD, ML),
+                           VTable, CodeGenFunction::CFITCK_VCall, Loc);
+
----------------
rnk wrote:
> Sigh. I wish we didn't have to do this. We should be able to provide the record decl that originally established the vftable slot in the method location, but it's not easy and requires a good understanding of the MS vftable code, which is pretty opaque. So let's do this for now.
> 
> I guess your algorithm also computes something subtly different. In this example, you'll get B when calling f:
>   struct A { virtual void f(); };
>   struct B : A { virtual void g(); };
> 
> Whereas I would think of it as being the vftable introduced by A.
Right, but I believe that we semantically analyse this as an implicit derived-to-base conversion followed by a call, so we do end up getting A in the end. (Ideally we probably do want to use the most-derived class to improve the precision of the check. See also the XFAILed test case `compiler-rt/test/cfi/sibling.cpp`.)

http://reviews.llvm.org/D10520

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list