[lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create

[vsk]

Summary

This fixes a UBSan error seen while debugging clang:

Member call on null pointer of type 'clang::TypeSourceInfo'

rdar://58783517

[shafik]

lldb/source/Symbol/TypeSystemClang.cpp

1283 ↗

(On Diff #241819)

There is another push_back with a nullptr a little further down. Is that one ok?

[teemperor]

Thanks for looking into this. I didn't get around to fix that myself yet. Out of curiosity, how did you get this test to fail? When I apply just your changes to the test (without the TypeSystemClang changes) then the test still passed in my build with LLVM_USE_SANITIZER:STRING=Address;Undefined ?

Otherwise this LGTM and is obviously better than just passing in a nullptr.

lldb/source/Symbol/TypeSystemClang.cpp

1283 ↗

(On Diff #241819)

I would say we no. Passing some valid type source info seems always better than a nullptr. I made D73946 for that

[vsk]

In D73808#1856503, @teemperor wrote:

Thanks for looking into this. I didn't get around to fix that myself yet. Out of curiosity, how did you get this test to fail? When I apply just your changes to the test (without the TypeSystemClang changes) then the test still passed in my build with LLVM_USE_SANITIZER:STRING=Address;Undefined ?

Otherwise this LGTM and is obviously better than just passing in a nullptr.

Hm, the modified test doesn't seem to trigger the sanitizer report. I expected it to but didn't verify: it looks like something more is needed to actually exercise the NonTypeTemplateParmDecl.

[vsk]

.. and I can no longer reproduce the sanitizer exception while debugging a Debug clang binary, as I rebased recently and some source change in clang hides the bug.

[vsk]

The original sanitizer report /does/ show UB in the NonTypeTemplateParmDecl static constructor:

* thread #1, queue = 'com.apple.main-thread', stop reason = Null pointer use                                                                                        
  * frame #0: 0x000000013ea61cb0 libclang_rt.asan_osx_dynamic.dylib`__ubsan_on_report                                                                               
    frame #1: 0x000000013ea5c76b libclang_rt.asan_osx_dynamic.dylib`__ubsan::Diag::~Diag() + 219                                                                    
    frame #2: 0x000000013ea5ded3 libclang_rt.asan_osx_dynamic.dylib`handleTypeMismatchImpl(__ubsan::TypeMismatchData*, unsigned long, __ubsan::ReportOptions) + 1155
    frame #3: 0x000000013ea5e00c libclang_rt.asan_osx_dynamic.dylib`__ubsan_handle_type_mismatch_v1_abort + 60                                                      
    frame #4: 0x000000011b383f5a liblldb.11.0.0git.dylib`clang::NonTypeTemplateParmDecl::Create(C=0x00006290000c3200, DC=0x0000621008793930, StartLoc=(ID = 0), IdLo
c=(ID = 0), D=0, P=1, Id=0x0000621008cfb668, T=QualType @ 0x00007ffeefbec980, ParameterPack=false, TInfo=0x0000000000000000) at DeclTemplate.cpp:692:25             
    frame #5: 0x00000001025a1d51 liblldb.11.0.0git.dylib`CreateTemplateParameterList(ast=0x00006290000c3200, template_param_infos=0x00007ffeefbedf50, template_param
_decls=0x00007ffeefbed500) at ClangASTContext.cpp:1272:38
    frame #6: 0x00000001025a4fcc liblldb.11.0.0git.dylib`lldb_private::ClangASTContext::CreateClassTemplateDecl(this=0x0000612000054c58, decl_ctx=0x00006210087942d0
, access_type=eAccessNone, class_name="BumpPtrAllocatorImpl", kind=3, template_param_infos=0x00007ffeefbedf50) at ClangASTContext.cpp:1373:48
    frame #7: 0x0000000102641e03 liblldb.11.0.0git.dylib`lldb_private::ClangASTContext::ParseClassTemplateDecl(this=0x0000612000054c58, decl_ctx=0x00006210087942d0,
 access_type=eAccessNone, parent_name="BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096, 4096>", tag_decl_kind=3, template_param_infos=0x00007ffeefbedf50) at ClangA
STContext.cpp:8792:12
    frame #8: 0x0000000104bb96e2 liblldb.11.0.0git.dylib`DWARFASTParserClang::ParseStructureLikeDIE(this=0x000060d00005bad0, sc=0x00007ffeefbefe80, die=0x00007ffeef
bf0520, attrs=0x00007ffeefbef180) at DWARFASTParserClang.cpp:1617:19
    frame #9: 0x0000000104baf779 liblldb.11.0.0git.dylib`DWARFASTParserClang::ParseTypeFromDWARF(this=0x000060d00005bad0, sc=0x00007ffeefbefe80, die=0x00007ffeefbf0
520, type_is_new_ptr=0x0000000000000000) at DWARFASTParserClang.cpp:491:15
    frame #10: 0x0000000104d3f396 liblldb.11.0.0git.dylib`SymbolFileDWARF::ParseType(this=0x000061b000089680, sc=0x00007ffeefbefe80, die=0x00007ffeefbf0520, type_is
_new_ptr=0x0000000000000000) at SymbolFileDWARF.cpp:3058:31
    frame #11: 0x0000000104d2049e liblldb.11.0.0git.dylib`SymbolFileDWARF::GetTypeForDIE(this=0x000061b000089680, die=0x00007ffeefbf0520, resolve_function_context=f
alse) at SymbolFileDWARF.cpp:2649:17
    frame #12: 0x0000000104d1bc1f liblldb.11.0.0git.dylib`SymbolFileDWARF::ResolveType(this=0x000061b000089680, die=0x00007ffeefbf0520, assert_not_being_parsed=true
, resolve_function_context=false) at SymbolFileDWARF.cpp:1456:18
    frame #13: 0x0000000104d1b42e liblldb.11.0.0git.dylib`SymbolFileDWARF::ResolveTypeUID(this=0x000061b000089680, type_uid=4295014635) at SymbolFileDWARF.cpp:1324:
21

Maybe it moved here?

AutoType *AT = TInfo->getType()->getContainedAutoType();

Or maybe Revert "[Concepts] Placeholder constraints and abbreviated templates" on 1/23 hides the bug?

[vsk]

It probably makes sense to land this and D73946 as a defensive measure, without any test change.

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

DeclContext *const decl_context =
       ast.getTranslationUnitDecl(); // Is this the right decl context?,
   for (size_t i = 0; i < num_template_params; ++i) {
     const char *name = template_param_infos.names[i];
 
     IdentifierInfo *identifier_info = nullptr;
     if (name && name[0])
       identifier_info = &ast.Idents.get(name);
     if (IsValueParam(template_param_infos.args[i])) {
+      QualType template_param_type =
+          template_param_infos.args[i].getIntegralType();
       template_param_decls.push_back(NonTypeTemplateParmDecl::Create(
           ast, decl_context, SourceLocation(), SourceLocation(), depth, i,
-          identifier_info, template_param_infos.args[i].getIntegralType(),
-          parameter_pack, nullptr));
-
+          identifier_info, template_param_type, parameter_pack,
+          ast.getTrivialTypeSourceInfo(template_param_type)));
     } else {
       template_param_decls.push_back(TemplateTypeParmDecl::Create(
           ast, decl_context, SourceLocation(), SourceLocation(), depth, i,
           identifier_info, is_typename, parameter_pack));
     }
   }
 
   if (template_param_infos.packed_args) {

lldb/test/API/lang/cpp/class-template-parameter-pack/main.cpp

template <> struct D<int, int> {
   int member;
   bool isIntBool() { return false; }
 };
 
 template <> struct D<int, int, bool> : D<int, int> {
   bool isIntBool() { return true; }
 };
 
+template<int Size> struct array {
+  int Arr[Size];
+  array() {}
+};
+
 int main (int argc, char const *argv[])
 {
     C<int,16,32> myC;
     C<int,16> myLesserC;
     myC.member = 64;
     (void)C<int,16,32>().isSixteenThirtyTwo();
     (void)C<int,16>().isSixteenThirtyTwo();
     (void)(myC.member != 64);   //% self.expect("expression -- myC", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
                                 //% self.expect("expression -- myLesserC.isSixteenThirtyTwo()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
                                 //% self.expect("expression -- myC.isSixteenThirtyTwo()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
 
                                 // Disabling until we do template lookup correctly: http://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180507/040689.html
                                 //#% self.expect("expression -- C<int, 16>().isSixteenThirtyTwo()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
                                 //#% self.expect("expression -- C<int, 16, 32>().isSixteenThirtyTwo()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
    
     D<int,int,bool> myD;
     D<int,int> myLesserD;
     myD.member = 64;
     (void)D<int,int,bool>().isIntBool();
-    (void)D<int,int>().isIntBool();
-    return myD.member != 64;  //% self.expect("expression -- myD", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
+    (void)D<int,int>().isIntBool(); //% self.expect("expression -- myD", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["64"])
                                 //% self.expect("expression -- myLesserD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
                                 //% self.expect("expression -- myD.isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
 
                                 // See comment above.
                                 //#% self.expect("expression -- D<int, int>().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
                                 //#% self.expect("expression -- D<int, int, bool>().isIntBool()", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["true"])
+
+    array<3> myArray; //% self.expect("expression -- myArray", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["Arr"])
+
+    return 1;
 }