Serialization: use the mangled class name for serializing vtables. #24705
Conversation
|
@swift-ci test |
| std::string mangledClassName = mangler.mangleNominalType(vt.getClass()); | ||
| size_t nameLength = mangledClassName.size(); | ||
| char *stringStorage = (char *)StringTable.Allocate(nameLength, 1); | ||
| std::memcpy(stringStorage, mangledClassName.data(), nameLength); |
There was a problem hiding this comment.
Can you use ASTContext::AllocateCopy() instead? Alternatively I think StringMap from llvm is designed to solve this problem
There was a problem hiding this comment.
This is probably more efficient than StringMap, since it's bump-pointer-allocated, but StringMap is simpler and that's probably better anyway. Definitely don't put it on the ASTContext because that doesn't get freed.
There was a problem hiding this comment.
StringMap does not really help here because I cannot (easily) change the type of VTableList.
There was a problem hiding this comment.
Ah, being a MapVector does make this trickier—you can't just stick a StringMap in there. Okay.
lib/Serialization/DeserializeSIL.h
Outdated
| @@ -141,7 +141,7 @@ namespace swift { | |||
| SILFunction *lookupSILFunction(StringRef Name, | |||
| bool declarationOnly = false); | |||
| bool hasSILFunction(StringRef Name, Optional<SILLinkage> Linkage = None); | |||
| SILVTable *lookupVTable(Identifier Name); | |||
| SILVTable *lookupVTable(const std::string &MangledClassName); | |||
|
Build failed |
|
Build failed |
| // RUN: %target-build-swift -I %t %s %t/classes.o -o %t/a.out | ||
| // RUN: %target-codesign %t/a.out | ||
| // RUN: %target-run %t/a.out | %FileCheck %s | ||
| // REQUIRES: executable_test |
There was a problem hiding this comment.
It should be possible to do this with an IRGen test, right? That'd be faster to run.
There was a problem hiding this comment.
I startet adding more executable tests recently because they cover more aspects of a problem. SIL/IR check tests and executable tests both have advantages and disadvantages. I feel that an executable tests is better for this case - and I think the execution time is not an issue here.
To distinguish between classes which have the same name (but are in different contexts). Fixes a miscompile if classes with the same name are used from a different module. SR-10634 rdar://problem/50538534
eeckstein
force-pushed
the
fix-vtable-serialization
branch
from
74775e4
to
c957c50
Compare
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
To distinguish between classes which have the same name (but are in different contexts).
Fixes a miscompile if classes with the same name are used from a different module.
SR-10634
rdar://problem/50538534