Page MenuHomePhabricator

Fix memleak in VarTemplateSpecializationDecl
Changes PlannedPublic

Authored by MitalAshok on Jun 2 2022, 7:18 PM.

Details

Summary

Replace TemplateArgumentListInfo with ASTTemplateArgumentListInfo
and convert between them in some places

Diff Detail

Unit TestsFailed

TimeTest
330 msx64 debian > Clang-Unit.AST/_/ASTTests/24::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests-Clang-Unit-1636792-24-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=24 /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests
350 msx64 debian > Clang-Unit.AST/_/ASTTests/25::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests-Clang-Unit-1636792-25-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=25 /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests
350 msx64 debian > Clang-Unit.AST/_/ASTTests/26::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests-Clang-Unit-1636792-26-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=26 /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests
360 msx64 debian > Clang-Unit.AST/_/ASTTests/27::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests-Clang-Unit-1636792-27-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=27 /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/unittests/AST/./ASTTests
3,900 msx64 debian > Clang.ASTMerge/var-cpp::test.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -emit-pch -std=c++17 -o /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/ASTMerge/var-cpp/Output/test.cpp.tmp.1.ast /var/lib/buildkite-agent/builds/llvm-project/clang/test/ASTMerge/var-cpp/Inputs/var1.cpp
View Full Test Results (6 Failed)

Event Timeline

MitalAshok created this revision.Jun 2 2022, 7:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 7:18 PM
MitalAshok published this revision for review.Jun 2 2022, 7:28 PM
MitalAshok added a project: Restricted Project.

VarTemplateSpecializationDecl held a TemplateArgumentListInfo which holds a SmallVector<TemplateArgumentLoc, 8>, so in this test case https://github.com/llvm/llvm-project/blob/ee1cf1f64519c815025d962bdf9c9bb3d09d7699/clang/test/SemaCXX/has_unique_object_reps_bitint.cpp#L27 where there was a variable template with more than 8 template arguments, it leaked

Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 7:28 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MitalAshok added inline comments.Jun 2 2022, 7:33 PM
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
5561

The type of VarSpec->getTemplateArgsInfo() changed to ASTTemplateArgumentListInfo which is why the type of VisitVarTemplateSpecializationDecl needed to change

MitalAshok planned changes to this revision.Jun 2 2022, 9:54 PM

Appears that more things need to be changed, I'll get that done soon

I was also looking into fixing this: https://reviews.llvm.org/D126944

I'm not yet sure if my changes are correct.

I was also looking into fixing this: https://reviews.llvm.org/D126944

I'm not yet sure if my changes are correct.

I did a review on https://reviews.llvm.org/D126944, but I'll let the two of you decide which review to move forward with rather than review them both in tandem. :-)

I was also looking into fixing this: https://reviews.llvm.org/D126944

I'm not yet sure if my changes are correct.

I did a review on https://reviews.llvm.org/D126944, but I'll let the two of you decide which review to move forward with rather than review them both in tandem. :-)

FYI, I've accepted that other review (we needed to get the leak fixed ASAP).