This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by vsk on Jan 31 2020, 2:38 PM.

Details

Summary

This fixes a UBSan error seen while debugging clang:

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

rdar://58783517

Diff Detail

Event Timeline

vsk created this revision.Jan 31 2020, 2:38 PM
vsk retitled this revision from [lldb/ClangASTContext] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create to [lldb/TypeSystemClang] Supply trivial TypeSourceInfo to NonTypeTemplateParmDecl::Create.Feb 2 2020, 8:44 PM
shafik added inline comments.Feb 3 2020, 4:04 PM
lldb/source/Symbol/TypeSystemClang.cpp
1283

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

teemperor accepted this revision.Feb 4 2020, 1:26 AM

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

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

This revision is now accepted and ready to land.Feb 4 2020, 1:26 AM
vsk added a comment.Feb 4 2020, 1:42 PM

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 added a comment.Feb 4 2020, 2:00 PM

.. 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 added a comment.Feb 4 2020, 2:22 PM

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 added a comment.Feb 4 2020, 2:23 PM

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

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2020, 11:29 AM