[MC] SIGSEGV due to nullptr AsmParser.Out.CurrFrag

[sivan-shani]

Due to several recent changes, the function AsmParser::checkForValidSection() is now utilizing (among other functions) also CurFrag->getParent() instead of getCurrentSection().first

This seems to cause a SIGSEV in some cases when AsmParser::Run() is called via API.

AsmParser.Out.CurFrag is Edit: set to nullptr, AsmParser::Run() -> call -> AsmParser::parseStatement -> checkForValidSection Edit: which call CurFrag->getParent() and this terminates in SIGSEV.

gdb output:

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
MC Feature String: +v8a,+crc,+fp-armv8,+neon,+crypto
Running slice:
arch: core_v8A_64+vfpneon_v8A_fp_neon+extension_cryptography
mode: feature_AArch64
slice range: 0x15000000 - 0x15ffffff (provided (val & 0xff000000) == 0x15000000)
operation: assemble

Program received signal SIGSEGV, Segmentation fault.
0x0000555555b587ee in llvm::MCFragment::getParent (this=0x0) at /llvm-project/llvm/include/llvm/MC/MCFragment.h:93
93	  MCSection *getParent() const { return Parent; } // (this=0x0)
(gdb) bt
#0  0x0000555555b587ee in llvm::MCFragment::getParent (this=0x0) at /llvm-project/llvm/include/llvm/MC/MCFragment.h:93
#1  0x000055555615d545 in llvm::MCStreamer::getCurrentFragment (this=0x555557af6a60) at /llvm-project/llvm/include/llvm/MC/MCStreamer.h:412
#2  0x0000555556137125 in (anonymous namespace)::AsmParser::checkForValidSection (this=0x555557af74b0) at /llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1085
#3  0x000055555613c857 in (anonymous namespace)::AsmParser::parseStatement (this=0x555557af74b0, Info=..., SI=0x0) at /llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:2308
#4  0x0000555556136817 in (anonymous namespace)::AsmParser::Run (this=0x555557af74b0, NoInitialTextSection=false, NoFinalize=false)
    at /llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:999

[sivan-shani]

@MaskRay Could you kindly comment if this failure makes sense?
It seems as indeed CurFrag is not being assigned anywhere when AsmParser::Run is called directly.

[MaskRay]

Hi @sivan-shani, do you have a reproduce? Are you a library user for a downstream project? The upstream testsuite is asan and msan clean.

AsmParser.Out.CurFrag is not set, AsmParser::Run() -> call -> AsmParser::parseStatement -> checkForValidSection and this terminates in SIGSEV.

MCStreamer::CurFrag is initialized to null, so there shouldn't be an uninitialized failure. The first time switchSection is called, CurFrag is set to a non-null value (the current fragment of the associated section). Every section, when created, allocates an empty MCDataFragment, ensuring that CurFrag is non-null.

[sivan-shani]

Thank you for the reply.
You are correct, the issue is not an uninitialized value, but rather an attempt to access a nullptr (CurFrag->getParent() when CurFrag is nullptr)
The failures are being obtained by using an internal tool (the one that was presented in the LLVM 2012 European Conference:
slides video)
I do not have an reproducer at hand, but I will try to make one.

The jist of it is the usage of llvm::MCAsmParser in order to set a llvm::MemoryBuffer with an assembly string, passing it to the AsmParser and calling AsmParser::Run

I will follow the gdb run again to double check,but I do believe that switchSection was not called along the code path.

[MaskRay]

Thank you for the reply. You are correct, the issue is not an uninitialized value, but rather an attempt to access a nullptr (CurFrag->getParent() when CurFrag is nullptr) The failures are being obtained by using an internal tool (the one that was presented in the LLVM 2012 European Conference: slides video) I do not have an reproducer at hand, but I will try to make one.

Interesting!

The jist of it is the usage of llvm::MCAsmParser in order to set a llvm::MemoryBuffer with an assembly string, passing it to the AsmParser and calling AsmParser::Run

I will follow the gdb run again to double check,but I do believe that switchSection was not called along the code path.

The getParent call is probably from the following. When switchSection hasn't been called, getCurrentSection().first should be false and suppress the getParent call.

Perhaps your internal tool does something that adds an element to SectionStack but does not set CurFrag to a non-null pointer. This condition is not expected by the implicit invariant in the code (I think this unexpected case is unreachable with in-tree tools, llvm-mc, clang, etc.)

  MCFragment *getCurrentFragment() const {
    assert(!getCurrentSection().first ||
           CurFrag->getParent() == getCurrentSection().first);
    return CurFrag;
  }

[sivan-shani]

Apologizes, I was wrong. The execution does hit switchsection call, right at the very begiinng of AsmParser::Run.
NoInitialTextSection is set to false, Out.initSections is called, and it call switchSection

[MaskRay]

Apologizes, I was wrong. The execution does hit switchsection call, right at the very begiinng of AsmParser::Run.
NoInitialTextSection is set to false, Out.initSections is called, and it call switchSection

If there is a switchSection call, you might want to check whether *Sec->CurFragList has non-null Head and Tail. They will guarantee that CurFrag is non-null.

MCContext::getELFSection and other get*Section functions ensure that Head and Tail (and therefore CurFrag) are non-null. MCSectionELF has a private ctor, but a downstream can potentially change that.

[sivan-shani]

It seems as switchSection might be failing.

Will try and create a reproducer tomorrow. Meanwhile perhaps this data might be insightful:
After the call that invokes switchSection line 969 inside AsmParser:Run - Out.initSections(false, getTargetParser().getSTI() which call switchSection, and I have made sure that switchSection is indeed being executed:

• CurFrag = 0x0
• So far could not find CurFragList in (*this)->Out (MCStreamer) or (*this)->Ctx (MCContext)
• (*this)->Out:
  (gdb) print (*this)->Out $11 = (llvm::MCStreamer &) @0x555557af6a60: {_vptr.MCStreamer = 0x555557a12a80 <vtable for MCInstStreamer+16>, Context = @0x555557afc2d0, TargetStreamer = {_M_t = { _M_t = {<std::_Tuple_impl<0, llvm::MCTargetStreamer*, std::default_delete<llvm::MCTargetStreamer> >> = {<std::_Tuple_impl<1, std::default_delete<llvm::MCTargetStreamer> >> = {<std::_Head_base<1, std::default_delete<llvm::MCTargetStreamer>, true>> = {<std::default_delete<llvm::MCTargetStreamer>> = {<No data fields>}, <No data fields>}, <No data fields>}, <std::_Head_base<0, llvm::MCTargetStreamer*, false>> = { _M_head_impl = 0x555557afbfa0}, <No data fields>}, <No data fields>}}}, DwarfFrameInfos = {<std::_Vector_base<llvm::MCDwarfFrameInfo, std::allocator<llvm::MCDwarfFrameInfo> >> = { _M_impl = {<std::allocator<llvm::MCDwarfFrameInfo>> = {<__gnu_cxx::new_allocator<llvm::MCDwarfFrameInfo>> = {<No data fields>}, <No data fields>}, <std::_Vector_base<llvm::MCDwarfFrameInfo, std::allocator<llvm::MCDwarfFrameInfo> >::_Vector_impl_data> = {_M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}, <No data fields>}}, <No data fields>}, FrameInfoStack = {<llvm::SmallVectorImpl<std::pair<unsigned long, llvm::MCSection*> >> = {<llvm::SmallVectorTemplateBase<std::pair<unsigned long, llvm::MCSection*>, true>> = {<llvm::SmallVectorTemplateCommon<std::pair<unsigned long, llvm::MCSection*>, void>> = {<llvm::SmallVectorBase<unsigned int>> = {BeginX = 0x555557af6aa0, Size = 0, Capacity = 1}, <No data fields>}, static TakesParamByValue = true}, <No data fields>}, <llvm::SmallVectorStorage<std::pair<unsigned long, llvm::MCSection*>, 1>> = { InlineElts = "P\000\000\000\000\000\000\000P\000\000\000\000\000\000"}, <No data fields>}, WinFrameInfos = {<std::_Vector_base<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> >, std::allocator<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> > > >> = { _M_impl = {<std::allocator<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> > >> = {<__gnu_cxx::new_allocator<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> > >> = {<No data fields>}, <No data fields>}, <std::_Vector_base<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> >, std::allocator<std::unique_ptr<llvm::WinEH::FrameInfo, std::default_delete<llvm::WinEH::FrameInfo> > > >::_Vector_impl_data> = {_M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}, <No data fields>}}, <No data fields>}, CurrentWinFrameInfo = 0x0, CurrentProcWinFrameInfoStartIndex = 0, SectionStack = {<llvm::SmallVectorImpl<std::pair<std::pair<llvm::MCSection*, unsigned int>, std::pair<llvm::MCSection*, unsigned int> > >> = {<llvm::SmallVectorTemplateBase<std::pair<std::pair<llvm::MCSection*, unsigned int>, std::pair<llvm::MCSection*, unsigned int> >, true>> = {<llvm::SmallVectorTemplateCommon<std::pair<std::pair<llvm::MCSection*, unsigned int>, std::pair<llvm::MCSection*, unsigned int> >, void>> = {<llvm::SmallVectorBase<unsigned int>> = {BeginX = 0x555557af6ae8, Size = 1, Capacity = 4}, <No data fields>}, static TakesParamByValue = false}, <No data fields>}, <llvm::SmallVectorStorage<std::pair<std::pair<llvm::MCSection*, unsigned int>, std::pair<llvm::MCSection*, unsigned int> >, 4>> = { InlineElts = "\260\230\257WUU\000\000\000\000\000\000\377\177", '\000' <repeats 18 times>, "j\257WUU", '\000' <repeats 18 times>, "Pk\257WUU\000\000\027\000\000\000\000\000\000\000\027", '\000' <repeats 23 times>, "q\000\000\000\000\000\000\000PB\257WUU\000\000ଡ\367\377\177\000\000 \000\000\000\000\000\000"}, <No data fields>}, StartTokLocPtr = 0x555557af75c8, NextWinCFIID = 0, UseAssemblerInfoForParsing = true, AllowAutoPadding = false, CurFrag = 0x0}

[sivan-shani]

Hello, apparently the initSections and changeSection functions have been re-implemented within the tool that uses the llvm api and run the tests. It does not use the regular llvm functions and the re-implemented functions and it is not handling the CurFrag.
Thanks everyone, closing this report.

[sivan-shani]

Not an issue - happen due to an external re-implementation of the initSections and changeSection