[lldb][TypeSystemClang] Initialize ClassTemplateSpecializationDecl's StrictPackMatch field #126215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…StrictPackMatch field This addresses the MSAN failure reported in llvm#125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() llvm#1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
llvmbot
added
clang
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-clang Author: Michael Buch (Michael137) ChangesThis addresses the MSAN failure reported The ASTImporter reads This patch just initializes the field to a fixed value of An alternative would be to always initialize it in the Full diff: https://github.com/llvm/llvm-project/pull/126215.diff 2 Files Affected:
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index a30ae798a99bc69..b82f75dd63fa508 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1960,6 +1960,8 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
bool hasStrictPackMatch() const { return StrictPackMatch; }
+ void setStrictPackMatch(bool Val) { StrictPackMatch = Val; }
+
/// Get the point of instantiation (if any), or null if none.
SourceLocation getPointOfInstantiation() const {
return PointOfInstantiation;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1da8fbe0bcd6dda..ecb571b1161bbc6 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1666,6 +1666,12 @@ TypeSystemClang::CreateClassTemplateSpecializationDecl(
ast.getTypeDeclType(class_template_specialization_decl, nullptr);
class_template_specialization_decl->setDeclName(
class_template_decl->getDeclName());
+
+ // FIXME: set to fixed value for now so it's not uninitialized.
+ // One way to determine StrictPackMatch would be
+ // Sema::CheckTemplateTemplateArgument.
+ class_template_specialization_decl->setStrictPackMatch(false);
+
SetOwningModule(class_template_specialization_decl, owning_module);
decl_ctx->addDecl(class_template_specialization_decl);
|
mizvekov
approved these changes
Feb 7, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/14274 Here is the relevant piece of the build log for the reference |
|
@mizvekov Do you want this backported to the release branch? |
Yes, there might be other patches missing though. I am in the c++ committee meeting, so hadn't the time to sort it out yet. |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…StrictPackMatch field (llvm#126215) This addresses the MSAN failure reported in llvm#125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() llvm#1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
|
Cherry picking this onto the 20.x release branch requires #125791 to be cherry-picked first. |
But this is just a bugfix for that PR, right? (IOW, if there's no reason to cherry-pick that PR, then there's also no reason cherry-pick this fix) |
|
But there is reason to cherry-pick #125791, as it fixes a regression which would otherwise be introduced in 20.x. |
|
Okay, nevermind me then. :) Thanks for the explanation. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses the MSAN failure reported
in #125791 (comment):
The ASTImporter reads
D->hasStrictPackMatch()and forwards it to the constructor of the destinationClassTemplateSpecializationDecl. But ifDis a decl that LLDB created from debug-info, it would've been created usingClassTemplateSpecializationDecl::CreateDeserialized, which doesn't initialize theStrictPackMatchfield.This patch just initializes the field to a fixed value of
false, to preserve previous behaviour and avoid the use-of-uninitialized-value.An alternative would be to always initialize it in the
ClassTemplateSpecializationDeclconstructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.