Skip to content

Commit ea12288

Browse files
jmmartinezjsji
authored andcommitted
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. Original commit: KhronosGroup/SPIRV-LLVM-Translator@c11cd5ba1bf8bdc
1 parent 219729b commit ea12288

File tree

3 files changed

+18
-19
lines changed

3 files changed

+18
-19
lines changed

llvm-spirv/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;

llvm-spirv/lib/SPIRV/libSPIRV/SPIRVModule.cpp

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

665-
for (auto C : CapMap)
666-
delete C.second;
667-
668-
for (auto C : ConditionalCapMap)
669-
delete C.second;
670-
671665
for (auto *M : ModuleProcessedVec)
672666
delete M;
673667
}
@@ -786,10 +780,12 @@ void SPIRVModuleImpl::addCapability(SPIRVCapabilityKind Cap) {
786780
addCapabilities(SPIRV::getCapability(Cap));
787781
SPIRVDBG(spvdbgs() << "addCapability: " << SPIRVCapabilityNameMap::map(Cap)
788782
<< '\n');
789-
if (hasCapability(Cap))
783+
784+
auto [It, Inserted] = CapMap.try_emplace(Cap);
785+
if (!Inserted)
790786
return;
791787

792-
auto *CapObj = new SPIRVCapability(this, Cap);
788+
auto CapObj = std::make_unique<SPIRVCapability>(this, Cap);
793789
if (AutoAddExtensions) {
794790
// While we are reading existing SPIR-V we need to read it as-is and don't
795791
// add required extensions for each entry automatically
@@ -798,15 +794,16 @@ void SPIRVModuleImpl::addCapability(SPIRVCapabilityKind Cap) {
798794
addExtension(Ext.value());
799795
}
800796

801-
CapMap.insert(std::make_pair(Cap, CapObj));
797+
It->second = std::move(CapObj);
802798
}
803799

804800
void SPIRVModuleImpl::addCapabilityInternal(SPIRVCapabilityKind Cap) {
805801
if (AutoAddCapability) {
806-
if (hasCapability(Cap))
802+
auto [It, Inserted] = CapMap.try_emplace(Cap);
803+
if (!Inserted)
807804
return;
808805

809-
CapMap.insert(std::make_pair(Cap, new SPIRVCapability(this, Cap)));
806+
It->second = std::make_unique<SPIRVCapability>(this, Cap);
810807
}
811808
}
812809

@@ -815,18 +812,19 @@ void SPIRVModuleImpl::addConditionalCapability(SPIRVId Condition,
815812
SPIRVDBG(spvdbgs() << "addConditionalCapability: "
816813
<< SPIRVCapabilityNameMap::map(Cap)
817814
<< ", condition: " << Condition << '\n');
818-
if (ConditionalCapMap.find(std::make_pair(Condition, Cap)) !=
819-
ConditionalCapMap.end()) {
815+
auto Key = std::make_pair(Condition, Cap);
816+
auto [It, Inserted] = ConditionalCapMap.try_emplace(Key);
817+
if (!Inserted) {
820818
return;
821819
}
822820

823-
auto *CapObj = new SPIRVConditionalCapabilityINTEL(this, Condition, Cap);
821+
auto CapObj =
822+
std::make_unique<SPIRVConditionalCapabilityINTEL>(this, Condition, Cap);
824823
if (AutoAddExtensions) {
825824
assert(false && "Auto adding conditional extensions is not supported.");
826825
}
827826

828-
ConditionalCapMap.insert(
829-
std::make_pair(std::make_pair(Condition, Cap), CapObj));
827+
It->second = std::move(CapObj);
830828
}
831829

832830
void SPIRVModuleImpl::eraseConditionalCapability(SPIRVId Condition,

llvm-spirv/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)