Re: [Lldb-commits] [lldb] r319597 - Fix warning in DynamicLoaderDarwinKernel.cpp, NFC
On Fri, Dec 1, 2017 at 3:53 PM, Vedant Kumar via lldb-commits wrote: > Author: vedantk > Date: Fri Dec 1 15:53:01 2017 > New Revision: 319597 > > URL: http://llvm.org/viewvc/llvm-project?rev=319597&view=rev > Log: > Fix warning in DynamicLoaderDarwinKernel.cpp, NFC > > Modified: > > lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp > > Modified: > lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp?rev=319597&r1=319596&r2=319597&view=diff > == > --- > lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp > (original) > +++ > lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp > Fri Dec 1 15:53:01 2017 > @@ -1407,7 +1407,7 @@ bool DynamicLoaderDarwinKernel::ReadAllK > void DynamicLoaderDarwinKernel::KextImageInfo::PutToLog(Log *log) const { >if (log == NULL) > return; > - const uint8_t *u = (uint8_t *)m_uuid.GetBytes(); > + const uint8_t *u = (const uint8_t *)m_uuid.GetBytes(); > Nit: I'd rather use static_cast<> :) >if (m_load_address == LLDB_INVALID_ADDRESS) { > if (u) { > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r319598 - Don't use llvm::EnablePrettyStackTrace on macOS.
Maybe we should remove this feature altogether? On Fri, Dec 1, 2017 at 4:11 PM, Jim Ingham via lldb-commits wrote: > Author: jingham > Date: Fri Dec 1 16:11:18 2017 > New Revision: 319598 > > URL: http://llvm.org/viewvc/llvm-project?rev=319598&view=rev > Log: > Don't use llvm::EnablePrettyStackTrace on macOS. > > LLDB.framework gets loaded into Xcode and other > frameworks, and this is inserting a signal handler into > the process even when lldb isn't used. I have a bunch > of reports of this SignalHandler blowing out the stack, > which renders crash reports for the crash useless. > > And in any case libraries really shouldn't be installing > signal handlers. > > I only turned this off for APPLE platforms, I'll let > the maintainers of other platforms decide what policy > they want to have w.r.t. this. > > Modified: > lldb/trunk/source/Initialization/SystemInitializerCommon.cpp > > Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=319598&r1=319597&r2=319598&view=diff > == > --- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp (original) > +++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Fri Dec 1 > 16:11:18 2017 > @@ -69,7 +69,9 @@ void SystemInitializerCommon::Initialize >} > #endif > > +#if not defined(__APPLE__) >llvm::EnablePrettyStackTrace(); > +#endif >Log::Initialize(); >HostInfo::Initialize(); >static Timer::Category func_cat(LLVM_PRETTY_FUNCTION); > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, I stumbled upon the same issue :) An alternative would be that of naming (as you suggested), but if this is consistent, no worries. https://reviews.llvm.org/D40757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r319598 - Don't use llvm::EnablePrettyStackTrace on macOS.
I'm curious why it's not working as it's supposed to work on these platforms. When it does work, it's quite helpful On Sat, Dec 2, 2017 at 12:24 PM Davide Italiano wrote: > Maybe we should remove this feature altogether? > > On Fri, Dec 1, 2017 at 4:11 PM, Jim Ingham via lldb-commits > wrote: > > Author: jingham > > Date: Fri Dec 1 16:11:18 2017 > > New Revision: 319598 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=319598&view=rev > > Log: > > Don't use llvm::EnablePrettyStackTrace on macOS. > > > > LLDB.framework gets loaded into Xcode and other > > frameworks, and this is inserting a signal handler into > > the process even when lldb isn't used. I have a bunch > > of reports of this SignalHandler blowing out the stack, > > which renders crash reports for the crash useless. > > > > And in any case libraries really shouldn't be installing > > signal handlers. > > > > I only turned this off for APPLE platforms, I'll let > > the maintainers of other platforms decide what policy > > they want to have w.r.t. this. > > > > Modified: > > lldb/trunk/source/Initialization/SystemInitializerCommon.cpp > > > > Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp > > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=319598&r1=319597&r2=319598&view=diff > > > == > > --- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp > (original) > > +++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Fri > Dec 1 16:11:18 2017 > > @@ -69,7 +69,9 @@ void SystemInitializerCommon::Initialize > >} > > #endif > > > > +#if not defined(__APPLE__) > >llvm::EnablePrettyStackTrace(); > > +#endif > >Log::Initialize(); > >HostInfo::Initialize(); > >static Timer::Category func_cat(LLVM_PRETTY_FUNCTION); > > > > > > ___ > > lldb-commits mailing list > > lldb-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40466: DWZ 03/12: DWARFCompileUnit split to DWARFCompileUnitData
jankratochvil added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + jankratochvil wrote: > clayborg wrote: > > jankratochvil wrote: > > > clayborg wrote: > > > > Is there a reason this is a member variable that I am not seeing? Seems > > > > we could have this class inherit from DWARFCompileUnitData. I am > > > > guessing this will be needed for a future patch? > > > Yes, future patch D40474 contains a new constructor so multiple > > > `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure > > > that happens only in the case ot `DW_TAG_partial_unit` (one > > > `DWARFCompileUnit` is read from a file while other `DWARFCompileUnit` are > > > remapped instances with unique offset as used from units which did use > > > `DW_TAG_imported_unit` for them). > > > `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);` > > > > > We should just have an instance of this in this class and add a virtual > > function to retrieve it. Then we make a DWARFPartialUnit that subclasses > > this and has its own extra member variable and can use either one when it > > makes sense. Not a fan of just having a dangling pointer with no clear > > ownership. There is no "delete m_data" anywhere? > The `DWARFCompileUnitData *m_data` content is being held in: > `std::forward_list > SymbolFileDWARF::m_compile_unit_data_list` as I hope/believe a > `DWARFCompileUnit` is never deleted until whole `SymbolFileDWARF` is deleted. > I did not use `std::shared_ptr > DWARFCompileUnit::m_data` as `shared_ptr` is both memory and > lock-instruction-performance expensive. > > Then we make a DWARFPartialUnit that subclasses this We cannot because DWARFCompileUnit is constructed by `DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded`->`DWARFCompileUnit::Extract` and it currently does not know whether it is `DW_TAG_compile_unit` or `DW_TAG_partial_unit`. But then `DWARFCompileUnit::Extract` could read even the very first DIE and save it for later use. Then `DWARFCompileUnit::ExtractDIEsIfNeeded` would not need the `bool cu_die_only` parameter and altogether I could also drop my D40471. Do you agree? (This whole DWARF units parsing across the file is not too efficient anyway, there should be some index in use; and I would prefer the DWARF-5 one over the Apple one.) https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r319598 - Don't use llvm::EnablePrettyStackTrace on macOS.
That said, it does seem to make more sense as something you would do in the main lldb executable, and not in library code. On Sat, Dec 2, 2017 at 12:26 PM Zachary Turner wrote: > I'm curious why it's not working as it's supposed to work on these > platforms. When it does work, it's quite helpful > > On Sat, Dec 2, 2017 at 12:24 PM Davide Italiano > wrote: > >> Maybe we should remove this feature altogether? >> >> On Fri, Dec 1, 2017 at 4:11 PM, Jim Ingham via lldb-commits >> wrote: >> > Author: jingham >> > Date: Fri Dec 1 16:11:18 2017 >> > New Revision: 319598 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=319598&view=rev >> > Log: >> > Don't use llvm::EnablePrettyStackTrace on macOS. >> > >> > LLDB.framework gets loaded into Xcode and other >> > frameworks, and this is inserting a signal handler into >> > the process even when lldb isn't used. I have a bunch >> > of reports of this SignalHandler blowing out the stack, >> > which renders crash reports for the crash useless. >> > >> > And in any case libraries really shouldn't be installing >> > signal handlers. >> > >> > I only turned this off for APPLE platforms, I'll let >> > the maintainers of other platforms decide what policy >> > they want to have w.r.t. this. >> > >> > Modified: >> > lldb/trunk/source/Initialization/SystemInitializerCommon.cpp >> > >> > Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp >> > URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=319598&r1=319597&r2=319598&view=diff >> > >> == >> > --- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp >> (original) >> > +++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Fri >> Dec 1 16:11:18 2017 >> > @@ -69,7 +69,9 @@ void SystemInitializerCommon::Initialize >> >} >> > #endif >> > >> > +#if not defined(__APPLE__) >> >llvm::EnablePrettyStackTrace(); >> > +#endif >> >Log::Initialize(); >> >HostInfo::Initialize(); >> >static Timer::Category func_cat(LLVM_PRETTY_FUNCTION); >> > >> > >> > ___ >> > lldb-commits mailing list >> > lldb-commits@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits