Skip to content

Commit c11cd5b

Browse files
authored
Fix memory leaks in addCapability and addConditionalCapability (#3403)
Compiling with `-fsanitize=address` uncovered 2 memory leaks related to the use of raw pointers in `addCapability` and `addConditionalCapability`. Instead of manually handling memory, use `std::unique_ptr` to track the ownership of the pointed object and free the memory for us.
1 parent 57c254d commit c11cd5b

File tree

3 files changed

+18
-19
lines changed

3 files changed

+18
-19
lines changed

lib/SPIRV/libSPIRV/SPIRVFnVar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ bool specializeFnVariants(SPIRVModule *BM, std::string &ErrMsg) {
169169
for (const auto &CondCap : BM->getConditionalCapabilities()) {
170170
const SPIRVId Condition = CondCap.first.first;
171171
const Capability Cap = CondCap.first.second;
172-
const SPIRVConditionalCapabilityINTEL *Entry = CondCap.second;
172+
const SPIRVConditionalCapabilityINTEL *Entry = CondCap.second.get();
173173
bool ShouldKeep = false;
174174
if (!evaluateConstant(BM, Entry->getCondition(), ShouldKeep, ErrMsg)) {
175175
return false;

lib/SPIRV/libSPIRV/SPIRVModule.cpp

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -665,12 +665,6 @@ SPIRVModuleImpl::~SPIRVModuleImpl() {
665665
for (auto I : IdEntryMap)
666666
delete I.second;
667667

668-
for (auto C : CapMap)
669-
delete C.second;
670-
671-
for (auto C : ConditionalCapMap)
672-
delete C.second;
673-
674668
for (auto *M : ModuleProcessedVec)
675669
delete M;
676670
}
@@ -789,10 +783,12 @@ void SPIRVModuleImpl::addCapability(SPIRVCapabilityKind Cap) {
789783
addCapabilities(SPIRV::getCapability(Cap));
790784
SPIRVDBG(spvdbgs() << "addCapability: " << SPIRVCapabilityNameMap::map(Cap)
791785
<< '\n');
792-
if (hasCapability(Cap))
786+
787+
auto [It, Inserted] = CapMap.try_emplace(Cap);
788+
if (!Inserted)
793789
return;
794790

795-
auto *CapObj = new SPIRVCapability(this, Cap);
791+
auto CapObj = std::make_unique<SPIRVCapability>(this, Cap);
796792
if (AutoAddExtensions) {
797793
// While we are reading existing SPIR-V we need to read it as-is and don't
798794
// add required extensions for each entry automatically
@@ -801,15 +797,16 @@ void SPIRVModuleImpl::addCapability(SPIRVCapabilityKind Cap) {
801797
addExtension(Ext.value());
802798
}
803799

804-
CapMap.insert(std::make_pair(Cap, CapObj));
800+
It->second = std::move(CapObj);
805801
}
806802

807803
void SPIRVModuleImpl::addCapabilityInternal(SPIRVCapabilityKind Cap) {
808804
if (AutoAddCapability) {
809-
if (hasCapability(Cap))
805+
auto [It, Inserted] = CapMap.try_emplace(Cap);
806+
if (!Inserted)
810807
return;
811808

812-
CapMap.insert(std::make_pair(Cap, new SPIRVCapability(this, Cap)));
809+
It->second = std::make_unique<SPIRVCapability>(this, Cap);
813810
}
814811
}
815812

@@ -818,18 +815,19 @@ void SPIRVModuleImpl::addConditionalCapability(SPIRVId Condition,
818815
SPIRVDBG(spvdbgs() << "addConditionalCapability: "
819816
<< SPIRVCapabilityNameMap::map(Cap)
820817
<< ", condition: " << Condition << '\n');
821-
if (ConditionalCapMap.find(std::make_pair(Condition, Cap)) !=
822-
ConditionalCapMap.end()) {
818+
auto Key = std::make_pair(Condition, Cap);
819+
auto [It, Inserted] = ConditionalCapMap.try_emplace(Key);
820+
if (!Inserted) {
823821
return;
824822
}
825823

826-
auto *CapObj = new SPIRVConditionalCapabilityINTEL(this, Condition, Cap);
824+
auto CapObj =
825+
std::make_unique<SPIRVConditionalCapabilityINTEL>(this, Condition, Cap);
827826
if (AutoAddExtensions) {
828827
assert(false && "Auto adding conditional extensions is not supported.");
829828
}
830829

831-
ConditionalCapMap.insert(
832-
std::make_pair(std::make_pair(Condition, Cap), CapObj));
830+
It->second = std::move(CapObj);
833831
}
834832

835833
void SPIRVModuleImpl::eraseConditionalCapability(SPIRVId Condition,

lib/SPIRV/libSPIRV/SPIRVModule.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ struct SPIRVTypeImageDescriptor;
109109

110110
class SPIRVModule {
111111
public:
112-
typedef std::map<SPIRVCapabilityKind, SPIRVCapability *> SPIRVCapMap;
112+
typedef std::map<SPIRVCapabilityKind, std::unique_ptr<SPIRVCapability>>
113+
SPIRVCapMap;
113114
typedef std::map<std::pair<SPIRVId, SPIRVCapabilityKind>,
114-
SPIRVConditionalCapabilityINTEL *>
115+
std::unique_ptr<SPIRVConditionalCapabilityINTEL>>
115116
SPIRVConditionalCapMap;
116117
typedef std::vector<SPIRVConditionalEntryPointINTEL *>
117118
SPIRVConditionalEntryPointVec;

0 commit comments

Comments
 (0)