Fix a use-after-free crash in ResetObjCLayout

[ahatanak]

operator[] can potentially cause reallocation and invalidate live iterators if it's called with a key that isn't present in the DenseMap. Call lookup() instead to prevent the function from inserting new entries into the DenseMap for ObjC classes that don't have any subclasses.

rdar://165448332

[llvmbot]

@llvm/pr-subscribers-clang

Author: Akira Hatanaka (ahatanak)

Changes

operator[] can potentially cause reallocation and invalidate live iterators if it's called with a key that isn't present in the DenseMap. Call lookup() instead to prevent the function from inserting new entries into the DenseMap for ObjC classes that don't have any subclasses.

rdar://165448332

---

Full diff: https://github.com/llvm/llvm-project/pull/170360.diff

1 Files Affected:

• (modified) clang/lib/AST/ASTContext.cpp (+1-1)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b359fc8350375..404ce3ffd77c7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12040,7 +12040,7 @@ bool ASTContext::mergeExtParameterInfo(
 void ASTContext::ResetObjCLayout(const ObjCInterfaceDecl *D) {
   if (auto It = ObjCLayouts.find(D); It != ObjCLayouts.end()) {
     It->second = nullptr;
-    for (auto *SubClass : ObjCSubClasses[D])
+    for (auto *SubClass : ObjCSubClasses.lookup(D))
       ResetObjCLayout(SubClass);
   }
 }

[ahatanak]

Without this fix, an ASan-enabled clang detects a use-after-free when compiling the following code:

@interface NSObject {
  int these, will, never, change, ever;
}
@end

@interface SuperClass2 : NSObject
@property int superClassProperty2;
@end

@interface C0 : SuperClass2
@end

@interface C1 : C0
@end

@interface C2 : C1
@end

@interface C3 : C2
@end

@interface C4 : C3
@end

@interface C5 : C4
@end

@interface C6 : C5
@end

@interface C7 : C6
@end

@interface C8 : C7
@end

@interface C9 : C8
@end

@interface C10 : C9
@end

@interface C11 : C10
@end

@interface C12 : C11
@end

@interface C13 : C12
@end

@interface C14 : C13
@end

@interface C15 : C14
@end

@interface C16 : C15
@end

@interface C17 : C16
@end

@interface C18 : C17
@end

@interface C19 : C18
@end

@interface C20 : C19
@end

@interface C21 : C20
@end

@interface C22 : C21
@end

@interface C23 : C22
@end

@interface C24 : C23
@end

@interface C25 : C24
@end

@interface C26 : C25
@end

@interface C27 : C26
@end

@interface C28 : C27
@end

@interface C29 : C28
@end

@interface C30 : C29
@end

@interface C31 : C30
@end

@interface C32 : C31
@end

@interface C33 : C32
@end

@interface C34 : C33
@end

@interface C35 : C34
@end

@interface C36 : C35
@end

@interface C37 : C36
@end

@interface C38 : C37
@end

@interface C39 : C38
@end

@interface C40 : C39
@end

@interface C41 : C40
@end

@interface C42 : C41
@end

@interface C43 : C42
@end

@interface C44 : C43
@end

@interface C45 : C44
@end

@interface IntermediateClass2 : SuperClass2
@property int IntermediateClass2Property;
@end

@interface IntermediateClass3 : SuperClass2
@property int IntermediateClass3Property;
@end

@interface IntermediateClass4 : SuperClass2
@end

@implementation IntermediateClass3
@end

@implementation SuperClass2
@end

The DenseMap grows from 64 buckets to 128 buckets when ResetObjCLayout(IntermediateClass3) is called recursively from ResetObjCLayout(SuperClass2), invalidating the iterators in the caller.

[ahatanak]

The bug was introduced in f5c5bc5.

[ojhunt]

t took me a little why to work out the path to this failure.

We really need a way to make these kinds of errors trigger deterministically (at least in debug builds)