Re: [Lldb-commits] [lldb] r319597 - Fix warning in DynamicLoaderDarwinKernel.cpp, NFC

2017-12-02 Thread Davide Italiano via lldb-commits
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.

2017-12-02 Thread Davide Italiano via lldb-commits
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

2017-12-02 Thread Davide Italiano via Phabricator via lldb-commits
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.

2017-12-02 Thread Zachary Turner via lldb-commits
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

2017-12-02 Thread Jan Kratochvil via Phabricator via lldb-commits
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.

2017-12-02 Thread Zachary Turner via lldb-commits
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