Index: include/clang/AST/VTableBuilder.h
===================================================================
--- include/clang/AST/VTableBuilder.h	(revision 180598)
+++ include/clang/AST/VTableBuilder.h	(working copy)
@@ -354,6 +354,8 @@
 
   /// getNumVirtualFunctionPointers - Return the number of virtual function
   /// pointers in the vtable for a given record decl.
+  // TODO: I've noticed this is used only in VTableBuilder.cpp
+  // Should we do anything about it?
   uint64_t getNumVirtualFunctionPointers(const CXXRecordDecl *RD);
 
   /// getMethodVTableIndex - Return the index (relative to the vtable address
Index: lib/AST/VTableBuilder.cpp
===================================================================
--- lib/AST/VTableBuilder.cpp	(revision 180598)
+++ lib/AST/VTableBuilder.cpp	(working copy)
@@ -34,7 +34,8 @@
   const CXXRecordDecl *DerivedClass;
   
   /// VirtualBase - If the path from the derived class to the base class
-  /// involves a virtual base class, this holds its declaration.
+  /// involves virtual base classes, this holds the declaration of the last
+  /// virtual base in this path (which is closest to the base class).
   const CXXRecordDecl *VirtualBase;
 
   /// NonVirtualOffset - The offset from the derived class to the base class.
@@ -219,16 +220,15 @@
   const CXXRecordDecl *VirtualBase = 0;
   
   // First, look for the virtual base class.
-  for (unsigned I = 0, E = Path.size(); I != E; ++I) {
-    const CXXBasePathElement &Element = Path[I];
-    
+  for (int I = Path.size(), E = 0; I != E; --I) {
+    const CXXBasePathElement &Element = Path[I - 1];
+
     if (Element.Base->isVirtual()) {
-      // FIXME: Can we break when we find the first virtual base?
-      // (If we can't, can't we just iterate over the path in reverse order?)
-      NonVirtualStart = I + 1;
+      NonVirtualStart = I;
       QualType VBaseType = Element.Base->getType();
-      VirtualBase = 
+      VirtualBase =
         cast<CXXRecordDecl>(VBaseType->getAs<RecordType>()->getDecl());
+      break;
     }
   }
   
@@ -1237,7 +1237,16 @@
       return Offset;
     }      
   }
-  
+
+  /*llvm_unreachable("Couldn't find the given base subobject "
+                   "in the derived subobject");*/
+
+  // TODO: Hm, I'd expect this to be unreachable.
+  // However, we do get here on A vs E at test/CodeGenCXX/vtable-layout.cpp:686
+  // Why isn't this case covered by the previous for loop?
+  // If this is intentional, we should put a comment here explaining when this
+  // happens and why this is a correct result.
+
   return BaseOffset();
 }
   
@@ -1410,7 +1419,7 @@
 }
 
 /// FindNearestOverriddenMethod - Given a method, returns the overridden method
-/// from the nearest base. Returns null if no method was found.
+/// from the nearest primary base. Returns null if no method was found.
 static const CXXMethodDecl * 
 FindNearestOverriddenMethod(const CXXMethodDecl *MD,
                             VTableBuilder::PrimaryBasesSetVectorTy &Bases) {
@@ -2420,9 +2429,7 @@
   // Add the known thunks.
   Thunks.insert(Builder.thunks_begin(), Builder.thunks_end());
 
-  // If we don't have the vbase information for this class, insert it.
-  // getVirtualBaseOffsetOffset will compute it separately without computing
-  // the rest of the vtable related information.
+  // Now it's time for vbase information unless there are no virtual bases.
   if (!RD->getNumVBases())
     return;
   
@@ -2430,6 +2437,8 @@
     RD->vbases_begin()->getType()->getAs<RecordType>();
   const CXXRecordDecl *VBase = cast<CXXRecordDecl>(VBaseRT->getDecl());
   
+  // getVirtualBaseOffsetOffset might have computed the vbase information
+  // already without computing the rest of the vtable related information.
   if (VirtualBaseClassOffsetOffsets.count(std::make_pair(RD, VBase)))
     return;