[Lldb-commits] [PATCH] D13125: Improve error reporting for failing to find argdumper.
brucem created this revision. brucem added reviewers: tfiala, granata.enrico, clayborg. brucem added a subscriber: lldb-commits. http://reviews.llvm.org/D13125 Files: source/Host/macosx/Host.mm Index: source/Host/macosx/Host.mm === --- source/Host/macosx/Host.mm +++ source/Host/macosx/Host.mm @@ -1345,13 +1345,13 @@ FileSpec expand_tool_spec; if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, expand_tool_spec)) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorString("could not get support executable directory for argdumper tool"); return error; } expand_tool_spec.AppendPathComponent("argdumper"); if (!expand_tool_spec.Exists()) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorStringWithFormat("could not find argdumper tool: %s", expand_tool_spec.GetPath().c_str()); return error; } Index: source/Host/macosx/Host.mm === --- source/Host/macosx/Host.mm +++ source/Host/macosx/Host.mm @@ -1345,13 +1345,13 @@ FileSpec expand_tool_spec; if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, expand_tool_spec)) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorString("could not get support executable directory for argdumper tool"); return error; } expand_tool_spec.AppendPathComponent("argdumper"); if (!expand_tool_spec.Exists()) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorStringWithFormat("could not find argdumper tool: %s", expand_tool_spec.GetPath().c_str()); return error; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248466 - Improve error reporting for failing to find argdumper.
Author: brucem Date: Thu Sep 24 02:08:29 2015 New Revision: 248466 URL: http://llvm.org/viewvc/llvm-project?rev=248466&view=rev Log: Improve error reporting for failing to find argdumper. Reviewers: tfiala, granata.enrico, clayborg Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D13125 Modified: lldb/trunk/source/Host/macosx/Host.mm Modified: lldb/trunk/source/Host/macosx/Host.mm URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Host.mm?rev=248466&r1=248465&r2=248466&view=diff == --- lldb/trunk/source/Host/macosx/Host.mm (original) +++ lldb/trunk/source/Host/macosx/Host.mm Thu Sep 24 02:08:29 2015 @@ -1345,13 +1345,13 @@ Host::ShellExpandArguments (ProcessLaunc FileSpec expand_tool_spec; if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, expand_tool_spec)) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorString("could not get support executable directory for argdumper tool"); return error; } expand_tool_spec.AppendPathComponent("argdumper"); if (!expand_tool_spec.Exists()) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorStringWithFormat("could not find argdumper tool: %s", expand_tool_spec.GetPath().c_str()); return error; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13125: Improve error reporting for failing to find argdumper.
This revision was automatically updated to reflect the committed changes. Closed by commit rL248466: Improve error reporting for failing to find argdumper. (authored by brucem). Changed prior to commit: http://reviews.llvm.org/D13125?vs=35590&id=35591#toc Repository: rL LLVM http://reviews.llvm.org/D13125 Files: lldb/trunk/source/Host/macosx/Host.mm Index: lldb/trunk/source/Host/macosx/Host.mm === --- lldb/trunk/source/Host/macosx/Host.mm +++ lldb/trunk/source/Host/macosx/Host.mm @@ -1345,13 +1345,13 @@ FileSpec expand_tool_spec; if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, expand_tool_spec)) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorString("could not get support executable directory for argdumper tool"); return error; } expand_tool_spec.AppendPathComponent("argdumper"); if (!expand_tool_spec.Exists()) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorStringWithFormat("could not find argdumper tool: %s", expand_tool_spec.GetPath().c_str()); return error; } Index: lldb/trunk/source/Host/macosx/Host.mm === --- lldb/trunk/source/Host/macosx/Host.mm +++ lldb/trunk/source/Host/macosx/Host.mm @@ -1345,13 +1345,13 @@ FileSpec expand_tool_spec; if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, expand_tool_spec)) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorString("could not get support executable directory for argdumper tool"); return error; } expand_tool_spec.AppendPathComponent("argdumper"); if (!expand_tool_spec.Exists()) { -error.SetErrorString("could not find argdumper tool"); +error.SetErrorStringWithFormat("could not find argdumper tool: %s", expand_tool_spec.GetPath().c_str()); return error; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13094: LLDB-MI: Fix assignment operator in CMIUtilString
evgeny777 added inline comments. Comment at: tools/lldb-mi/MIUtilString.cpp:41 @@ -40,3 +40,3 @@ CMIUtilString::CMIUtilString(const char *vpData) -: std::string(vpData) +: std::string(vpData != nullptr ? vpData : "") { ki.stfu wrote: > Not sure about usefulness of these changes. The NULL usually means an error > in contrast to "" which means "there is nothing to show". In your case, you > can check whether it's NULL or not, and then construct an object. > > Besides that, you can use operator=: > ``` > SBValue val; > CMIUtilString s; > if (const char* s_cstr = val.GetSummary()) > s = s_cstr; > ``` As I already said, in MI both NULL and "" mean nothing to output. Regarding your proposal compare this ``` CMIUtilString s; if (const char* ss = val.GetSummary()) { if ((s=ss).size()) { } } ``` with this ``` if ( (s = val.GetSummary()).size() ) { } ``` http://reviews.llvm.org/D13094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
labath added a comment. I don't want to stand in the way of progress (and I do think that getting rid of the timeout dependency is progress), but this implementation regresses in a couple of features compared to using timeout: - timeout tries (with moderate success) to cleanup children spawned by the main process. your implementation will (afaik) kill only the main process. This is especially important for build bots, since leaking processes will starve the bot's resources after a while (and we have had problems with this on our darwin build bot). - we intentionally had timeout terminate the child processes with SIGQUIT, because this produces core files of the terminated processes. I have found this very useful when diagnosing the sources of hanging tests. I'd like to have the possibility to do this, even if it will not be enabled by default. Do you think that the benefits of this change outweigh these issues? If so, and they don't cause our bots to break, then we can put it in (and i'll probably implement the code dumping feature when I find some time), but I though you should be aware of these downsides. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13058: LLDB-MI: Bug when evaluating strings containing characters from non-ascii range
evgeny777 updated this revision to Diff 35607. evgeny777 added a comment. Revised according to suggestions from granta.enrico and ki.stfu http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp test/tools/lldb-mi/variable/TestMiGdbSetShowPrint.py test/tools/lldb-mi/variable/TestMiVar.py test/tools/lldb-mi/variable/main.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.h Index: tools/lldb-mi/MICmnLLDBUtilSBValue.h === --- tools/lldb-mi/MICmnLLDBUtilSBValue.h +++ tools/lldb-mi/MICmnLLDBUtilSBValue.h @@ -55,7 +55,7 @@ CMIUtilString GetSimpleValueCStringPointer() const; CMIUtilString GetSimpleValueCStringArray() const; bool GetCompositeValue(const bool vbPrintFieldNames, CMICmnMIValueTuple &vwrMiValueTuple, const MIuint vnDepth = 1) const; - +bool TryGetValueSummary(CMIUtilString &vrValue) const; // Statics: private: static bool IsCharBasicType(lldb::BasicType eType); Index: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp === --- tools/lldb-mi/MICmnLLDBUtilSBValue.cpp +++ tools/lldb-mi/MICmnLLDBUtilSBValue.cpp @@ -17,6 +17,7 @@ #include "MICmnMIValueTuple.h" #include "MIUtilString.h" +#include "lldb/API/SBTypeSummary.h" //++ // Details: CMICmnLLDBUtilSBValue constructor. // Type:Method. @@ -165,6 +166,13 @@ return MIstatus::success; } } +else +{ +// Treat composite value which has registered summary +// (for example with AddCXXSummary) as simple value +if (TryGetValueSummary(vwrValue)) +return MIstatus::success; +} // Composite variable type i.e. struct return MIstatus::failure; @@ -180,6 +188,10 @@ CMIUtilString CMICmnLLDBUtilSBValue::GetSimpleValueChar() const { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + const uint64_t value = m_rValue.GetValueAsUnsigned(); if (value == 0) { @@ -227,6 +239,10 @@ if (value == nullptr) return m_pUnkwn; +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + lldb::SBValue child = m_rValue.GetChildAtIndex(0); const lldb::BasicType eType = child.GetType().GetBasicType(); switch (eType) @@ -266,6 +282,10 @@ CMIUtilString CMICmnLLDBUtilSBValue::GetSimpleValueCStringArray() const { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + const MIuint nChildren = m_rValue.GetNumChildren(); lldb::SBValue child = m_rValue.GetChildAtIndex(0); const lldb::BasicType eType = child.GetType().GetBasicType(); @@ -347,6 +367,30 @@ return MIstatus::success; } +bool +CMICmnLLDBUtilSBValue::TryGetValueSummary(CMIUtilString &vrValue) const +{ +if (!m_rValue.IsValid()) +return false; + +vrValue = m_rValue.GetSummary(); +if (!vrValue.empty()) +{ +lldb::SBTypeSummary summary = m_rValue.GetTypeSummary(); +if (summary.IsValid() && summary.DoesPrintValue(m_rValue)) +{ +CMIUtilString valStr = m_rValue.GetValue(); +if (!valStr.empty()) +{ +vrValue.insert(0, valStr + " "); +} +} +return true; +} + +return false; +} + //++ // Details: Check that basic type is a char type. Char type can be signed or unsigned. // Type:Static. Index: test/tools/lldb-mi/variable/main.cpp === --- test/tools/lldb-mi/variable/main.cpp +++ test/tools/lldb-mi/variable/main.cpp @@ -8,6 +8,7 @@ //===--===// #include +#include struct complex_type { @@ -64,9 +65,21 @@ const char32_t *u32p = U"\t\"hello\"\n"; const char32_t u32a[] = U"\t\"hello\"\n"; +const char16_t* u16p_rus = u"\\Аламо-сквер"; +const char16_t u16a_rus[] = u"\\Бейвью"; +const char32_t* u32p_rus = U"\\Чайнатаун"; +const char32_t u32a_rus[] = U"\\Догпатч"; + // BP_gdb_set_show_print_char_array_as_string_test } +void +cpp_stl_types_test(void) +{ +std::string std_string = "hello"; +// BP_cpp_stl_types_test +} + struct not_str { not_str(char _c, int _f) @@ -105,6 +118,7 @@ var_update_test(); var_list_children_test(); gdb_set_show_print_char_array_as_string_test(); +cpp_stl_types_test(); gdb_set_show_print_expand_aggregates(); gdb_set_show_print_aggregate_field_names(); return 0; // BP_return Index: test/tools/lldb-mi/variable/TestMiVar.py === --- test/t
Re: [Lldb-commits] [PATCH] D13094: LLDB-MI: Fix assignment operator in CMIUtilString
brucem added a subscriber: brucem. brucem added a comment. http://reviews.llvm.org/D13094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13094: LLDB-MI: Fix assignment operator in CMIUtilString
brucem added a comment. It doesn't seem right to have a strong construction and copy just to determine the length or if it is empty or not as in your examples. That would be appropriate for a helper function. http://reviews.llvm.org/D13094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13058: LLDB-MI: Bug when evaluating strings containing characters from non-ascii range
evgeny777 updated this revision to Diff 35618. evgeny777 added a comment. Revised, removed dependencies from http://reviews.llvm.org/D13094 http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp test/tools/lldb-mi/variable/TestMiGdbSetShowPrint.py test/tools/lldb-mi/variable/TestMiVar.py test/tools/lldb-mi/variable/main.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.h Index: tools/lldb-mi/MICmnLLDBUtilSBValue.h === --- tools/lldb-mi/MICmnLLDBUtilSBValue.h +++ tools/lldb-mi/MICmnLLDBUtilSBValue.h @@ -55,7 +55,7 @@ CMIUtilString GetSimpleValueCStringPointer() const; CMIUtilString GetSimpleValueCStringArray() const; bool GetCompositeValue(const bool vbPrintFieldNames, CMICmnMIValueTuple &vwrMiValueTuple, const MIuint vnDepth = 1) const; - +bool TryGetValueSummary(CMIUtilString &vrValue) const; // Statics: private: static bool IsCharBasicType(lldb::BasicType eType); Index: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp === --- tools/lldb-mi/MICmnLLDBUtilSBValue.cpp +++ tools/lldb-mi/MICmnLLDBUtilSBValue.cpp @@ -17,6 +17,7 @@ #include "MICmnMIValueTuple.h" #include "MIUtilString.h" +#include "lldb/API/SBTypeSummary.h" //++ // Details: CMICmnLLDBUtilSBValue constructor. // Type:Method. @@ -165,6 +166,13 @@ return MIstatus::success; } } +else +{ +// Treat composite value which has registered summary +// (for example with AddCXXSummary) as simple value +if (TryGetValueSummary(vwrValue)) +return MIstatus::success; +} // Composite variable type i.e. struct return MIstatus::failure; @@ -180,6 +188,10 @@ CMIUtilString CMICmnLLDBUtilSBValue::GetSimpleValueChar() const { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + const uint64_t value = m_rValue.GetValueAsUnsigned(); if (value == 0) { @@ -227,6 +239,10 @@ if (value == nullptr) return m_pUnkwn; +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + lldb::SBValue child = m_rValue.GetChildAtIndex(0); const lldb::BasicType eType = child.GetType().GetBasicType(); switch (eType) @@ -266,6 +282,10 @@ CMIUtilString CMICmnLLDBUtilSBValue::GetSimpleValueCStringArray() const { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + const MIuint nChildren = m_rValue.GetNumChildren(); lldb::SBValue child = m_rValue.GetChildAtIndex(0); const lldb::BasicType eType = child.GetType().GetBasicType(); @@ -347,6 +367,30 @@ return MIstatus::success; } +bool +CMICmnLLDBUtilSBValue::TryGetValueSummary(CMIUtilString &vrValue) const +{ +if (!m_rValue.IsValid()) +return false; + +const char* c_str = m_rValue.GetSummary(); +if (c_str && c_str[0]) +{ +vrValue = c_str; +lldb::SBTypeSummary summary = m_rValue.GetTypeSummary(); +if (summary.IsValid() && summary.DoesPrintValue(m_rValue)) +{ +c_str = m_rValue.GetValue(); +if (c_str && c_str[0]) +{ +vrValue.insert(0, std::string(c_str) + " "); +} +} +return true; +} +return false; +} + //++ // Details: Check that basic type is a char type. Char type can be signed or unsigned. // Type:Static. Index: test/tools/lldb-mi/variable/main.cpp === --- test/tools/lldb-mi/variable/main.cpp +++ test/tools/lldb-mi/variable/main.cpp @@ -8,6 +8,7 @@ //===--===// #include +#include struct complex_type { @@ -64,9 +65,21 @@ const char32_t *u32p = U"\t\"hello\"\n"; const char32_t u32a[] = U"\t\"hello\"\n"; +const char16_t* u16p_rus = u"\\Аламо-сквер"; +const char16_t u16a_rus[] = u"\\Бейвью"; +const char32_t* u32p_rus = U"\\Чайнатаун"; +const char32_t u32a_rus[] = U"\\Догпатч"; + // BP_gdb_set_show_print_char_array_as_string_test } +void +cpp_stl_types_test(void) +{ +std::string std_string = "hello"; +// BP_cpp_stl_types_test +} + struct not_str { not_str(char _c, int _f) @@ -105,6 +118,7 @@ var_update_test(); var_list_children_test(); gdb_set_show_print_char_array_as_string_test(); +cpp_stl_types_test(); gdb_set_show_print_expand_aggregates(); gdb_set_show_print_aggregate_field_names(); return 0; // BP_return Index: test/tools/lldb-mi/variable/TestMiVar.py ==
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On 23 September 2015 at 13:47, Siva Chandra via lldb-commits wrote: > Author: sivachandra > Date: Wed Sep 23 12:47:08 2015 > New Revision: 248401 > > URL: http://llvm.org/viewvc/llvm-project?rev=248401&view=rev > Log: > DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or > child Great, thank you for tackling this. The test case fails on FreeBSD right now because there's no error on "limit" and "nolimit": (lldb) expr f (Foo) $0 = (s = "qwerty") (lldb) expr s (MyString) $1 = (std::__1::string = "qwerty") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On Thu, Sep 24, 2015 at 6:40 AM, Ed Maste wrote: > The test case fails on FreeBSD right now because there's no error on > "limit" and "nolimit": > > (lldb) expr f > (Foo) $0 = (s = "qwerty") > (lldb) expr s > (MyString) $1 = (std::__1::string = "qwerty") > Does -flimit-debug-info have any effect on FreeBSD? For example, if you try to print a std::string value when compiled with -flimit-debug-info, what does it say? ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On Thu, Sep 24, 2015 at 6:47 AM, Siva Chandra wrote: > > > On Thu, Sep 24, 2015 at 6:40 AM, Ed Maste wrote: > >> The test case fails on FreeBSD right now because there's no error on >> "limit" and "nolimit": >> >> (lldb) expr f >> (Foo) $0 = (s = "qwerty") >> (lldb) expr s >> (MyString) $1 = (std::__1::string = "qwerty") >> > > Does -flimit-debug-info have any effect on FreeBSD? For example, if you > try to print a std::string value when compiled with -flimit-debug-info, > what does it say? > Actually, the test fails on Darwin as well for the same reason. ISTM that -flimit-debug-info does not have any effect? ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
I don't have access to a FreeBSD or a mac at the moment. Can you, or someone else, check how -fstandalone-debug and -fno-standalone-debug behave? On Thu, Sep 24, 2015 at 6:57 AM, Siva Chandra wrote: > On Thu, Sep 24, 2015 at 6:47 AM, Siva Chandra > wrote: > >> >> >> On Thu, Sep 24, 2015 at 6:40 AM, Ed Maste wrote: >> >>> The test case fails on FreeBSD right now because there's no error on >>> "limit" and "nolimit": >>> >>> (lldb) expr f >>> (Foo) $0 = (s = "qwerty") >>> (lldb) expr s >>> (MyString) $1 = (std::__1::string = "qwerty") >>> >> >> Does -flimit-debug-info have any effect on FreeBSD? For example, if you >> try to print a std::string value when compiled with -flimit-debug-info, >> what does it say? >> > > Actually, the test fails on Darwin as well for the same reason. ISTM that > -flimit-debug-info does not have any effect? > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13094: LLDB-MI: Fix assignment operator in CMIUtilString
evgeny777 updated this revision to Diff 35620. evgeny777 added a comment. Revised, now contains fix for just assignment operation The "if(vpRhs != nullptr)" was removed to provide the same behaviour for a) CMIUtilString s = (char*)nullptr; b) s = (char*)nullptr; http://reviews.llvm.org/D13094 Files: tools/lldb-mi/MIUtilString.cpp Index: tools/lldb-mi/MIUtilString.cpp === --- tools/lldb-mi/MIUtilString.cpp +++ tools/lldb-mi/MIUtilString.cpp @@ -76,14 +76,7 @@ //-- CMIUtilString &CMIUtilString::operator=(const char *vpRhs) { -if (*this == vpRhs) -return *this; - -if (vpRhs != nullptr) -{ -assign(vpRhs); -} - +assign(vpRhs); return *this; } Index: tools/lldb-mi/MIUtilString.cpp === --- tools/lldb-mi/MIUtilString.cpp +++ tools/lldb-mi/MIUtilString.cpp @@ -76,14 +76,7 @@ //-- CMIUtilString &CMIUtilString::operator=(const char *vpRhs) { -if (*this == vpRhs) -return *this; - -if (vpRhs != nullptr) -{ -assign(vpRhs); -} - +assign(vpRhs); return *this; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
If I understand correctly, the process hierarchy used to look like this: python - dotest.py |__ python - multiprocessing fork |__ python - lldbtest |__ inferior executable And now looks like this: python - dotest.py |__ python - lldbtest |__ inferior executable In either case, the process.terminate() runs in the python - lldbtest process, and terminates the inferior. So I don't see how the behavior is any different than before. Do you mean with gtimeout it's creating a separate process group for the inferior so that if the inferior itself spawns children, those will get cleaned up too? Do any inferiors actually do that? And if so, we could always use the same logic from python of creating the inferior in a separate process group On Thu, Sep 24, 2015 at 3:36 AM Pavel Labath wrote: > labath added a comment. > > I don't want to stand in the way of progress (and I do think that getting > rid of the timeout dependency is progress), but this implementation > regresses in a couple of features compared to using timeout: > > - timeout tries (with moderate success) to cleanup children spawned by the > main process. your implementation will (afaik) kill only the main process. > This is especially important for build bots, since leaking processes will > starve the bot's resources after a while (and we have had problems with > this on our darwin build bot). > - we intentionally had timeout terminate the child processes with SIGQUIT, > because this produces core files of the terminated processes. I have found > this very useful when diagnosing the sources of hanging tests. I'd like to > have the possibility to do this, even if it will not be enabled by default. > > Do you think that the benefits of this change outweigh these issues? If > so, and they don't cause our bots to break, then we can put it in (and i'll > probably implement the code dumping feature when I find some time), but I > though you should be aware of these downsides. > > > http://reviews.llvm.org/D13124 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. In http://reviews.llvm.org/D13124#252564, @labath wrote: > I don't want to stand in the way of progress (and I do think that getting rid > of the timeout dependency is progress), but this implementation regresses in > a couple of features compared to using timeout: > > - timeout tries (with moderate success) to cleanup children spawned by the > main process. your implementation will (afaik) kill only the main process. > This is especially important for build bots, since leaking processes will > starve the bot's resources after a while (and we have had problems with this > on our darwin build bot). Fair enough. Either yesterday or the day before, I put up a change that ensures the dotest inferiors are in a separate process group. I could (on non-Windows) send that process group a SIGQUIT. The reason I prefer not to use SIGQUIT is that it is catchable, and thus can be ignored. Once something can be ignored, the runner needs to be able to handle the timed out child really not ending with a SIGQUIT. That means either more logic around timing out on the "wait for death", or possibly not reaping. In the case of a test runner, I'd probably put the focus on making sure the test runner makes it through ahead of getting the coredump from a process that times out. That said, my initial implementation of this did do the following: - Start with a SIGQUIT (via subprocess.Process.terminate(), so does something reasonable on Windows). Wait for 5 seconds to see if it wraps up (by way of waiting for the communicate() thread to die). - If still not dead, send a SIGKILL (via subprocess.Process.kill()). I thought the added complication wasn't worth it, but it's not that complicated. And would not regress behavior on the Linux side. The only diff would be, on non-Windows, send a kill to the process group (which has already been created for the inferior dotest), and just use the terminate() call on Windows. I can give that a whirl. It's a bit more code but is not going to be too bad. > - we intentionally had timeout terminate the child processes with SIGQUIT, > because this produces core files of the terminated processes. I have found > this very useful when diagnosing the sources of hanging tests. I'd like to > have the possibility to do this, even if it will not be enabled by default. > > Do you think that the benefits of this change outweigh these issues? If so, > and they don't cause our bots to break, then we can put it in (and i'll > probably implement the code dumping feature when I find some time), but I > though you should be aware of these downsides. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
labath added a comment. I haven't followed the recent changes too closely, but at some point the hierarchy was: dosep.py dosep.py fork timeout dotest.py (lldb client) lldb-server inferior other executables which were sometimes spawned by the tests Timeout was making sure none of its children survived after the test timed out. I don't know how it did that, but I presume it ran them in a separate process group. This was not always successful (some of our tests like to create process groups as well), but it was pretty good at compartmenalizing the tests and making sure we leave no processes hanging around. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
labath added a comment. In http://reviews.llvm.org/D13124#252791, @tfiala wrote: > In http://reviews.llvm.org/D13124#252564, @labath wrote: > > > I don't want to stand in the way of progress (and I do think that getting > > rid of the timeout dependency is progress), but this implementation > > regresses in a couple of features compared to using timeout: > > > > - timeout tries (with moderate success) to cleanup children spawned by the > > main process. your implementation will (afaik) kill only the main process. > > This is especially important for build bots, since leaking processes will > > starve the bot's resources after a while (and we have had problems with > > this on our darwin build bot). > > > Fair enough. Either yesterday or the day before, I put up a change that > ensures the dotest inferiors are in a separate process group. I could (on > non-Windows) send that process group a SIGQUIT. The reason I prefer not to > use SIGQUIT is that it is catchable, and thus can be ignored. Once something > can be ignored, the runner needs to be able to handle the timed out child > really not ending with a SIGQUIT. That means either more logic around timing > out on the "wait for death", or possibly not reaping. In the case of a test > runner, I'd probably put the focus on making sure the test runner makes it > through ahead of getting the coredump from a process that times out. None of the processes we run catch SIGQUIT, so it's mostly safe, but we can do the QUIT+sleep+KILL combo to be safe. The problem with the race conditions is that they are very hard to reproduce. I often need to run the test suite at full speed dozens of times to be able to catch it happening, and then the only way of diagnosing the issue is digging through the core files. Thanks for working on this once again :) http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. In http://reviews.llvm.org/D13124#252784, @zturner wrote: > If I understand correctly, the process hierarchy used to look like this: > > python - dotest.py > |__ python - multiprocessing fork > > |__ python - lldbtest > |__ inferior executable > > > And now looks like this: > > python - dotest.py > > |__ python - lldbtest > |__ inferior executable > > > In either case, the process.terminate() runs in the python - lldbtest > process, and terminates the inferior. So I don't see how the behavior is > any different than before. Do you mean with gtimeout it's creating a > separate process group for the inferior so that if the inferior itself > spawns children, those will get cleaned up too? The man page for the timeout (this one here: http://linux.die.net/man/1/timeout) doesn't explicitly call out that it creates a process group, but that wouldn't at all be out of character for a command like that. Interestingly, it does call out the issue that a using a catchable signal means it can't guarantee that it really shuts down that app if it handles the signal and essentially ignores it. > do that? And if so, we could always use the same logic from python of > creating the inferior in a separate process group We have that logic in for non-Windows. That's the change I sent you a ping on to see if you wanted to add the Windows flag for creating a separate process group. Since I no longer recall the semantics of that on the Windows side, I wasn't sure if that was useful to you. I think sending the signal to the process group (since we have one) is probably the right way to go here. But I also think we get into the realm of "did we really kill it" if we start playing around with catchable signals. Also, there is going to be a divergence on how that works on Unix-like vs. Windows. The process group kill command is os.killpg(), which is listed as Unix-only. I'll come up with the alternative and we can see where we go from there. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. > Thanks for working on this once again :) Sure thing! http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. I filed this: https://llvm.org/bugs/show_bug.cgi?id=24926 And I'm starting to look at it now. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. > What is happening is that it is looking for argdumper and not finding it. > This happens because it is looking in the lib directory within the build > directory, but argdumper is in the bin directory (where it should be). I'm not sure I would say that is accurate. argdumper is an internal implementation detail binary,, and isn't meant to be a stand-alone utility. In Linux, it would most properly be located in libexec. (Older pre-layout-standardization would put it in lib). So I'd expect it in lib as well on OS X since OS X doesn't use a libexec. Sounds like this is probably just a cmake configuration issue. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248513 - Don't enter interactive loop when finished running the test suite.
Author: gclayton Date: Thu Sep 24 11:37:41 2015 New Revision: 248513 URL: http://llvm.org/viewvc/llvm-project?rev=248513&view=rev Log: Don't enter interactive loop when finished running the test suite. Modified: lldb/trunk/test/curses_results.py Modified: lldb/trunk/test/curses_results.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/curses_results.py?rev=248513&r1=248512&r2=248513&view=diff == --- lldb/trunk/test/curses_results.py (original) +++ lldb/trunk/test/curses_results.py Thu Sep 24 11:37:41 2015 @@ -207,7 +207,7 @@ class Curses(test_results.ResultsFormatt self.status_panel.add_status_item(name="unexpected_success", title="Unexpected Success", format="%u", width=30, value=0, update=False) self.main_window.refresh() elif event == 'terminate': -self.main_window.key_event_loop() +#self.main_window.key_event_loop() lldbcurses.terminate_curses() check_for_one_key = False self.using_terminal = False ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
I'm ok with leaving some straggling processes on Windows for now, because it's obviously better than what we have currently, which is nothing. I can implement some sort of helper module at some point that exposes a new createProcess function that supports this for Windows and Unix with a single interface once it becomes a problem. Which reminds me of another thing: Since you're working heavily on the tet suite, one thing I've always wanted is what I just mentioned: Something like an lldbxplat module which is essentially a helper module that exposes a single interface for doing things that differ depending on the platform. This would essentially let us remove almost every single occurrence of `if sys.name.startswith('win32')` from the entire test suite, which would really help maintainability. It's something I'll get to myself one day if nobody else beats me, but since you're making a lot of great improvements to the test suite recently, figured I'd throw it out there. On Thu, Sep 24, 2015 at 8:54 AM Todd Fiala wrote: > tfiala added a comment. > > > Thanks for working on this once again :) > > > Sure thing! > > > http://reviews.llvm.org/D13124 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. > Which reminds me of another thing: Since you're working heavily on the tet > suite, one thing I've always wanted is what I just mentioned: Something > like an lldbxplat module which is essentially a helper module that exposes > a single interface for doing things that differ depending on the platform. > This would essentially let us remove almost every single occurrence of `if > sys.name.startswith('win32')` from the entire test suite, which would > really help maintainability. That's a good idea. I'm (somewhat slowly) attempting to work in some clean up each time I touch it. I'll hit that at some point. I'll start winding down on changes to the test infrastructure soon-ish. If I don't hit that by the time I stop heavy changes, definitely feel free to jump on it! http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12968: Fix for lldb-mi crash in Listener code if -exec-abort MI command was invoked without getting process stopped
KLapshin added a comment. @clayborg, @labath, After more deep investigation I think setting listener for hijacking also looks like workaround. Setting additional listener just change code flow path and buggy path not executed, thus no crash. At top level - crash appeared in Process::m_listener involved - as no hijacked listener was set in Destroy(). See code below: StateType Process::WaitForProcessToStop (const TimeValue *timeout, EventSP *event_sp_ptr, bool wait_always, Listener *hijack_listener, Stream *stream) { // We can't just wait for a "stopped" event, because the stopped event may have restarted the target. // We have to actually check each event, and in the case of a stopped event check the restarted flag // on the event. if (event_sp_ptr) event_sp_ptr->reset(); StateType state = GetState(); // If we are exited or detached, we won't ever get back to any // other valid state... if (state == eStateDetached || state == eStateExited) return state; Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS)); if (log) log->Printf ("Process::%s (timeout = %p)", __FUNCTION__, static_cast(timeout)); if (!wait_always && StateIsStoppedState(state, true) && StateIsStoppedState(GetPrivateState(), true)) { if (log) log->Printf("Process::%s returning without waiting for events; process private and public states are already 'stopped'.", __FUNCTION__); // We need to toggle the run lock as this won't get done in // SetPublicState() if the process is hijacked. if (hijack_listener) m_public_run_lock.SetStopped(); return state; } while (state != eStateInvalid) { EventSP event_sp; state = WaitForStateChangedEvents (timeout, event_sp, hijack_listener); <--- if (event_sp_ptr && event_sp) *event_sp_ptr = event_sp; StateType Process::WaitForStateChangedEvents (const TimeValue *timeout, EventSP &event_sp, Listener *hijack_listener) { Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS)); if (log) log->Printf ("Process::%s (timeout = %p, event_sp)...", __FUNCTION__, static_cast(timeout)); Listener *listener = hijack_listener; if (listener == NULL) listener = &m_listener; <--- what if m_listener was set as unitialized or deallocated Listener instance, "dummy" listener ? StateType state = eStateInvalid; if (listener->WaitForEventForBroadcasterWithType (timeout, this, eBroadcastBitStateChanged | eBroadcastBitInterrupt, event_sp)) { if (event_sp && event_sp->GetType() == eBroadcastBitStateChanged) state = Process::ProcessEventData::GetStateFromEvent(event_sp.get()); else if (log) log->Printf ("Process::%s got no event or was interrupted.", __FUNCTION__); } ... You can reproduce crash even without lldb-MI driver - just with remote iOS target and "process launch", then "process kill". Corresponding log ("log enable lldb events"): (lldb) process launch ... Process::ShouldBroadcastEvent (0x7ff1fa4a1260) => new state: running, last broadcast state: running - NO 0x7ff1fad1b110 Listener::WaitForEventsInternal (timeout = { 0x0 }) for lldb.process.internal_state_listener (lldb) process kill 0x7ff1fad1afa0 Broadcaster("lldb.process.internal_state_broadcaster")::BroadcastEvent (event_sp = {0x7ff1fce160d0 Event: broadcaster = 0x7ff1fad1afa0 (lldb.process.internal_state_broadcaster), type = 0x0002, data = }, unique =0) hijack = 0x0 0x7ff1fad1b110 Listener('lldb.process.internal_state_listener')::AddEvent (event_sp = {0x7ff1fce160d0}) 0x7ff1fa48e300 Listener::WaitForEventsInternal (timeout = { 0x7fff57d47240 }) for <-- WHAT ?! Listener without name - Process::m_listener may be ? 0x7ff1fad1b110 'lldb.process.internal_state_listener' Listener::FindNextEventInternal(broadcaster=0x0, broadcaster_names=0x0[0], event_type_mask=0x, remove=1) event 0x7ff1fce160d0 0x7ff1fad1ae38 Broadcaster("lldb.process")::HijackBroadcaster (listener("lldb.process.halt_listener")=0x7ff1fe01ca00) CRASH ! Will continue investigation. Repository: rL LLVM http://reviews.llvm.org/D12968 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
clayborg added a comment. Each shared library is an object that can be shared between multiple targets. We do not want to injecting types from another shared library into the static notion of what a type is within a shared library. Why? What if one target in lldb loads liba.so which has a forward declaration to "class Bar;", and this same target loads libb.so which contains a full definition for "Bar". Another target also loads liba.so, but it loads a different version of "libb.so" which has a newer more up to date version of "Bar" because you are doing a edit/compile/debug cycle. We can't take a full definition of "Bar" and place it into the debug info for liba.so because it can be wrong. So each shared library always just parses the types _as_they_see_them_ in their debug info. We do allow variable introspection to always grab the complete type from another shared library when displaying the type, we just don't allow the static notion of a type that comes from a specific shared library to be augmented within that shared library. So lets say you have liba.so that contains Foo class which has a "Bar *m_bar;" member variable. liba.so doesn't have a full definition for "Bar" which is OK. "Bar", in liba.so, is known to be "class Bar;", nothing else. When we are debugging a a.out binary later that loads both liba.so and libb.so (which contains the full definition for "Bar"), in the variable display code we see that we want to expand "Bar" in the variables view and we note that we have a full definition of "Bar" in libb.so, so we end up using the full definition from libb.so in place of the forward declaration. Again, we just switch the types. We can do this because have have a type that comes from a specific target, and within that target (a collection of shared libraries) we can come up with the right definition for "Bar" without modifying the type itself from the debug info within liba.so. So there should be not problems when viewing variables because we have that covered. If we have any problems in the expression parser, we will need to solve them in the say kind of way: given a target context, determine the right version of the type to use and copy into the ASTContext for the expression. We currently import types from multiple different ASTContexts into an expression ASTContext for each expression that we run, so it would be possible to identify any classes that were forward declarations and try to complete them during the ASTContext -> ASTContext copy cycle. We would need to somehow mark these incomplete types to allow them to be completed later. Like for example the forward declaration to a base class. We used to just complete the definition for the base class and say it had no member and no methods. We have hooks in to allow the DWARF to assist in laying out a class so that we can correct any alignment issues we run into (since DWARF doesn't always capture any #pragma pack and other compiler directives that can affect how a class is laid out), but we could allow these base classes to be created and mark them somehow in the ClangASTContext so we can identify them later during any expression use. We could also use this for the Variable display code and find the full definition of the forward declaration base class... So, any changes you make in future fixes must adhere to: - a type is parsed exactly as it is represented within the object file itself, no outside definitions from other shared libraries can be merged into these types - it is fine to switch over to using a different version of a type (the full definition of "Bar" from libb.so when you have a forward declaration for "Bar" in liba.so) when displaying variables or evaluating expressions that have a Target in their execution context since the target has a list of all the shared libraries that are currently loaded. This allows process a.out to load one version of "Bar" from libb.so, and process b.out to load another version of "Bar" from libc.so - We can modify the ClangASTContext ASTImporter to do the same kind of type switching when copying types into the expression ASTContext http://reviews.llvm.org/D13066 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13143: [TestCppIncompleteTypes] Remove the dependence on std::string.
sivachandra created this revision. sivachandra added reviewers: dblaikie, clayborg. sivachandra added a subscriber: lldb-commits. http://reviews.llvm.org/D13143 Files: test/lang/cpp/incomplete-types/Makefile test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py test/lang/cpp/incomplete-types/a.cpp test/lang/cpp/incomplete-types/a.h test/lang/cpp/incomplete-types/length.cpp test/lang/cpp/incomplete-types/length.h test/lang/cpp/incomplete-types/main.cpp Index: test/lang/cpp/incomplete-types/main.cpp === --- test/lang/cpp/incomplete-types/main.cpp +++ test/lang/cpp/incomplete-types/main.cpp @@ -3,21 +3,16 @@ class Foo { public: -Foo(std::string x) : s(x) {} - -private: -std::string s; +A a; }; -class MyString : public std::string { -public: -MyString(std::string x) : std::string(x) {} +class MyA : public A { }; int main() { -Foo f("qwerty"); -MyString s("qwerty"); +Foo f; +MyA a; -return length(s); // break here +return length(a); // break here } Index: test/lang/cpp/incomplete-types/length.h === --- test/lang/cpp/incomplete-types/length.h +++ test/lang/cpp/incomplete-types/length.h @@ -1,8 +1,8 @@ #ifndef __LENGTH_H__ #define __LENGTH_H__ -#include +#include "a.h" -size_t length (const std::string &str); +int length (A &a); #endif Index: test/lang/cpp/incomplete-types/length.cpp === --- test/lang/cpp/incomplete-types/length.cpp +++ test/lang/cpp/incomplete-types/length.cpp @@ -1,8 +1,8 @@ #include "length.h" -size_t -length (const std::string &str) +int +length (A &a) { - return str.length(); + return a.length(); } Index: test/lang/cpp/incomplete-types/a.h === --- /dev/null +++ test/lang/cpp/incomplete-types/a.h @@ -0,0 +1,11 @@ +#ifndef __A_H__ +#define __A_H__ + +class A +{ +public: + A(); + virtual int length(); +}; + +#endif Index: test/lang/cpp/incomplete-types/a.cpp === --- /dev/null +++ test/lang/cpp/incomplete-types/a.cpp @@ -0,0 +1,10 @@ + +#include "a.h" + +A::A () { } + +int +A::length () +{ + return 123; +} Index: test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py === --- test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py +++ test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py @@ -16,9 +16,9 @@ self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object") self.assertFalse(value_f.GetError().Success(), "'expr f' results in an error, but LLDB does not crash") -value_s = frame.EvaluateExpression("s") -self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object") -self.assertFalse(value_s.GetError().Success(), "'expr s' results in an error, but LLDB does not crash") +value_a = frame.EvaluateExpression("a") +self.assertTrue(value_a.IsValid(), "'expr a' results in a valid SBValue object") +self.assertFalse(value_a.GetError().Success(), "'expr a' results in an error, but LLDB does not crash") @dwarf_test @skipIfGcc @@ -30,9 +30,9 @@ self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object") self.assertTrue(value_f.GetError().Success(), "'expr f' is successful") -value_s = frame.EvaluateExpression("s") -self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object") -self.assertTrue(value_s.GetError().Success(), "'expr s' is successful") +value_a = frame.EvaluateExpression("a") +self.assertTrue(value_a.IsValid(), "'expr a' results in a valid SBValue object") +self.assertTrue(value_a.GetError().Success(), "'expr a' is successful") def setUp(self): TestBase.setUp(self) Index: test/lang/cpp/incomplete-types/Makefile === --- test/lang/cpp/incomplete-types/Makefile +++ test/lang/cpp/incomplete-types/Makefile @@ -1,6 +1,6 @@ LEVEL = ../../../make -CXX_SOURCES = main.cpp length.cpp +CXX_SOURCES = main.cpp length.cpp a.cpp CFLAGS_LIMIT = -c $(CXXFLAGS) CFLAGS_NO_LIMIT = -c $(CXXFLAGS) @@ -12,11 +12,11 @@ all: limit nolimit -limit: main.o length_limit.o - $(CXX) $(LDFLAGS) main.o length_limit.o -o limit +limit: main.o length_limit.o a.o + $(CXX) $(LDFLAGS) main.o length_limit.o a.o -o limit -nolimit: main.o length_nolimit.o - $(CXX) $(LDFLAGS) main.o length_nolimit.o -o nolimit +nolimit: main.o length_nolimit.o a.o + $(CXX) $(LDFLAGS) main.o length_nolimit.o a.o -o nolimit main.o: main.cpp $(CXX) $(CFLAGS_LIMIT) main.cpp -o main.o @@ -27,6 +27,9 @@ length_nolimit.o: length.cpp $(CXX) $(CFLAGS_NO_LIMIT) l
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
I don't want compiler options in the test Makefiles. Please move the logic to Makefile.rules, and create a variable and set that instead. On Thu, Sep 24, 2015 at 11:43 AM Greg Clayton via lldb-commits < lldb-commits@lists.llvm.org> wrote: > clayborg added a comment. > > Each shared library is an object that can be shared between multiple > targets. We do not want to injecting types from another shared library into > the static notion of what a type is within a shared library. Why? What if > one target in lldb loads liba.so which has a forward declaration to "class > Bar;", and this same target loads libb.so which contains a full definition > for "Bar". Another target also loads liba.so, but it loads a different > version of "libb.so" which has a newer more up to date version of "Bar" > because you are doing a edit/compile/debug cycle. We can't take a full > definition of "Bar" and place it into the debug info for liba.so because it > can be wrong. So each shared library always just parses the types > _as_they_see_them_ in their debug info. > > We do allow variable introspection to always grab the complete type from > another shared library when displaying the type, we just don't allow the > static notion of a type that comes from a specific shared library to be > augmented within that shared library. > > So lets say you have liba.so that contains Foo class which has a "Bar > *m_bar;" member variable. liba.so doesn't have a full definition for "Bar" > which is OK. "Bar", in liba.so, is known to be "class Bar;", nothing else. > When we are debugging a a.out binary later that loads both liba.so and > libb.so (which contains the full definition for "Bar"), in the variable > display code we see that we want to expand "Bar" in the variables view and > we note that we have a full definition of "Bar" in libb.so, so we end up > using the full definition from libb.so in place of the forward declaration. > Again, we just switch the types. We can do this because have have a type > that comes from a specific target, and within that target (a collection of > shared libraries) we can come up with the right definition for "Bar" > without modifying the type itself from the debug info within liba.so. > > So there should be not problems when viewing variables because we have > that covered. > > If we have any problems in the expression parser, we will need to solve > them in the say kind of way: given a target context, determine the right > version of the type to use and copy into the ASTContext for the expression. > We currently import types from multiple different ASTContexts into an > expression ASTContext for each expression that we run, so it would be > possible to identify any classes that were forward declarations and try to > complete them during the ASTContext -> ASTContext copy cycle. We would need > to somehow mark these incomplete types to allow them to be completed later. > Like for example the forward declaration to a base class. We used to just > complete the definition for the base class and say it had no member and no > methods. We have hooks in to allow the DWARF to assist in laying out a > class so that we can correct any alignment issues we run into (since DWARF > doesn't always capture any #pragma pack and other compiler directives that > can affect how a class is laid out), but we could allow these base classes > to be created and mark them somehow in the ClangASTContext so we can > identify them later during any expression use. We could also use this for > the Variable display code and find the full definition of the forward > declaration base class... > > So, any changes you make in future fixes must adhere to: > > - a type is parsed exactly as it is represented within the object file > itself, no outside definitions from other shared libraries can be merged > into these types > - it is fine to switch over to using a different version of a type (the > full definition of "Bar" from libb.so when you have a forward declaration > for "Bar" in liba.so) when displaying variables or evaluating expressions > that have a Target in their execution context since the target has a list > of all the shared libraries that are currently loaded. This allows process > a.out to load one version of "Bar" from libb.so, and process b.out to load > another version of "Bar" from libc.so > - We can modify the ClangASTContext ASTImporter to do the same kind of > type switching when copying types into the expression ASTContext > > > http://reviews.llvm.org/D13066 > > > > ___ > 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] LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be restarted after 6 PM Pacific time today. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
zturner added a comment. I know, I've seen them in a few places myself. But I think we should move away from that, because it's a large source of portability problems http://reviews.llvm.org/D13066 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
dawn added a comment. Is there a workaround for this? Ok to revert this commit until it is fixed? It's blocker for me. Thanks. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. Hi Dawn, See https://llvm.org/bugs/show_bug.cgi?id=24926 for a workaround. I'm developing a fix for it. Still testing but what is there in that patch (in the bug) works for your build scenario as long as you add '--executable /path/to/your/just/build/lldb' when calling dotest.py. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
dawn added a comment. Hi Todd, yes, I read the bug report and updated my comment while you were writing yours :) Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. > Hi Todd, yes, I read the bug report and updated my comment while you were > writing yours :) Great! Once I validate that the standard OS X build isn't busted (in progress), I'll get it checked in. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. I do intend to get back to this after resolving the cmake build issue on OS X. Hopefully by tonight. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added inline comments. Comment at: test/curses_results.py:223 @@ -223,1 +222,3 @@ +# Greg - not sure why this is here. It locks up on exit. +# self.main_window.key_event_loop() lldbcurses.terminate_curses() tfiala wrote: > Greg, let me know if there's something more sensible I can do here rather > than comment it out. It never seemed to return and just kept looping. Greg got rid of this call this morning. There was a 'q' command intended to be there that wasn't yet, which prevented us from exiting at the end. For now we're just taking this out. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On 24 September 2015 at 10:04, Siva Chandra wrote: > I don't have access to a FreeBSD or a mac at the moment. Can you, or someone > else, check how -fstandalone-debug and -fno-standalone-debug behave? Both OS X and FreeBSD default to full debug info, but the enable / disable options should still have an effect. I'll try to investigate soon, unless someone on OS X gets to it first. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248541 - [TestCppIncompleteTypes] Remove the dependence on std::string.
Author: sivachandra Date: Thu Sep 24 16:29:54 2015 New Revision: 248541 URL: http://llvm.org/viewvc/llvm-project?rev=248541&view=rev Log: [TestCppIncompleteTypes] Remove the dependence on std::string. Reviewers: dblaikie, clayborg Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D13143 Added: lldb/trunk/test/lang/cpp/incomplete-types/a.cpp lldb/trunk/test/lang/cpp/incomplete-types/a.h Modified: lldb/trunk/test/lang/cpp/incomplete-types/Makefile lldb/trunk/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py lldb/trunk/test/lang/cpp/incomplete-types/length.cpp lldb/trunk/test/lang/cpp/incomplete-types/length.h lldb/trunk/test/lang/cpp/incomplete-types/main.cpp Modified: lldb/trunk/test/lang/cpp/incomplete-types/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lang/cpp/incomplete-types/Makefile?rev=248541&r1=248540&r2=248541&view=diff == --- lldb/trunk/test/lang/cpp/incomplete-types/Makefile (original) +++ lldb/trunk/test/lang/cpp/incomplete-types/Makefile Thu Sep 24 16:29:54 2015 @@ -1,6 +1,6 @@ LEVEL = ../../../make -CXX_SOURCES = main.cpp length.cpp +CXX_SOURCES = main.cpp length.cpp a.cpp CFLAGS_LIMIT = -c $(CXXFLAGS) CFLAGS_NO_LIMIT = -c $(CXXFLAGS) @@ -12,11 +12,11 @@ endif all: limit nolimit -limit: main.o length_limit.o - $(CXX) $(LDFLAGS) main.o length_limit.o -o limit +limit: main.o length_limit.o a.o + $(CXX) $(LDFLAGS) main.o length_limit.o a.o -o limit -nolimit: main.o length_nolimit.o - $(CXX) $(LDFLAGS) main.o length_nolimit.o -o nolimit +nolimit: main.o length_nolimit.o a.o + $(CXX) $(LDFLAGS) main.o length_nolimit.o a.o -o nolimit main.o: main.cpp $(CXX) $(CFLAGS_LIMIT) main.cpp -o main.o @@ -27,6 +27,9 @@ length_limit.o: length.cpp length_nolimit.o: length.cpp $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o +a.o: a.cpp + $(CXX) -c a.cpp -o a.o + clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o include $(LEVEL)/Makefile.rules Modified: lldb/trunk/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py?rev=248541&r1=248540&r2=248541&view=diff == --- lldb/trunk/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py (original) +++ lldb/trunk/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py Thu Sep 24 16:29:54 2015 @@ -16,9 +16,9 @@ class TestCppIncompleteTypes(TestBase): self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object") self.assertFalse(value_f.GetError().Success(), "'expr f' results in an error, but LLDB does not crash") -value_s = frame.EvaluateExpression("s") -self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object") -self.assertFalse(value_s.GetError().Success(), "'expr s' results in an error, but LLDB does not crash") +value_a = frame.EvaluateExpression("a") +self.assertTrue(value_a.IsValid(), "'expr a' results in a valid SBValue object") +self.assertFalse(value_a.GetError().Success(), "'expr a' results in an error, but LLDB does not crash") @dwarf_test @skipIfGcc @@ -30,9 +30,9 @@ class TestCppIncompleteTypes(TestBase): self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object") self.assertTrue(value_f.GetError().Success(), "'expr f' is successful") -value_s = frame.EvaluateExpression("s") -self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object") -self.assertTrue(value_s.GetError().Success(), "'expr s' is successful") +value_a = frame.EvaluateExpression("a") +self.assertTrue(value_a.IsValid(), "'expr a' results in a valid SBValue object") +self.assertTrue(value_a.GetError().Success(), "'expr a' is successful") def setUp(self): TestBase.setUp(self) Added: lldb/trunk/test/lang/cpp/incomplete-types/a.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lang/cpp/incomplete-types/a.cpp?rev=248541&view=auto == --- lldb/trunk/test/lang/cpp/incomplete-types/a.cpp (added) +++ lldb/trunk/test/lang/cpp/incomplete-types/a.cpp Thu Sep 24 16:29:54 2015 @@ -0,0 +1,10 @@ + +#include "a.h" + +A::A () { } + +int +A::length () +{ + return 123; +} Added: lldb/trunk/test/lang/cpp/incomplete-types/a.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lang/cpp/incomplete-types/a.h?rev=248541&view=auto == --- lldb/trunk/test/lang/cpp/incomplete-types/a.h (added) +++ lldb/trunk/test/lang/cpp/incomplete-types/a.h Thu Sep 24 16:29:54 2015
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On Thu, Sep 24, 2015 at 2:15 PM, Ed Maste wrote: > On 24 September 2015 at 10:04, Siva Chandra > wrote: > > I don't have access to a FreeBSD or a mac at the moment. Can you, or > someone > > else, check how -fstandalone-debug and -fno-standalone-debug behave? > > Both OS X and FreeBSD default to full debug info, but the enable / > disable options should still have an effect. I'll try to investigate > soon, unless someone on OS X gets to it first. > As blaikie pointed out on the review, it could be because libc++ (which is the default on FreeBSD and Darwin?) does not have an explicit declaration for std::basic_string<...>? I have now landed a change which removes the dependence on std::string. Hope it fixes the test for FreeBSD and darwin. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248545 - Fix tests on cmake-based OS X after rL248338
Author: tfiala Date: Thu Sep 24 16:59:48 2015 New Revision: 248545 URL: http://llvm.org/viewvc/llvm-project?rev=248545&view=rev Log: Fix tests on cmake-based OS X after rL248338 See: https://llvm.org/bugs/show_bug.cgi?id=24926 for details. On OS X, when LLDB.framework is not part of the lldb.dylib path, the supporting executable path is resolved to be the bin directory sitting next to the lib directory with the dylib lives. Not a perfect solution, but we also can't base it on the executable path since both Python and the lldb driver can be the executable. Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=248545&r1=248544&r2=248545&view=diff == --- lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm (original) +++ lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Thu Sep 24 16:59:48 2015 @@ -13,6 +13,7 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Host/macosx/HostInfoMacOSX.h" +#include "lldb/Core/Log.h" #include "lldb/Interpreter/Args.h" #include "lldb/Utility/SafeMachO.h" @@ -23,7 +24,9 @@ #include // C inclues +#include #include +#include #include // Objective C/C++ includes @@ -152,6 +155,40 @@ HostInfoMacOSX::ComputeSupportExeDirecto raw_path.append("/Resources"); #endif } +else +{ +// Find the bin path relative to the lib path where the cmake-based +// OS X .dylib lives. This is not going to work if the bin and lib +// dir are not both in the same dir. +// +// It is not going to work to do it by the executable path either, +// as in the case of a python script, the executable is python, not +// the lldb driver. +raw_path.append("/../bin"); +FileSpec support_dir_spec(raw_path, true); +if (!support_dir_spec.Exists() || !support_dir_spec.IsDirectory()) +{ +Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); +if (log) +log->Printf("HostInfoMacOSX::%s(): failed to find support directory", +__FUNCTION__); +return false; +} + +// Get normalization from support_dir_spec. Note the FileSpec resolve +// does not remove '..' in the path. +char *const dir_realpath = realpath(support_dir_spec.GetPath().c_str(), NULL); +if (dir_realpath) +{ +raw_path = dir_realpath; +free(dir_realpath); +} +else +{ +raw_path = support_dir_spec.GetPath(); +} +} + file_spec.GetDirectory().SetString(llvm::StringRef(raw_path.c_str(), raw_path.size())); return (bool)file_spec.GetDirectory(); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. The cmake OS X fix went in here: Sendingsource/Host/macosx/HostInfoMacOSX.mm Transmitting file data . Committed revision 248545. Let me know if you still have trouble after that. Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13147: [TestCppIncompleteTypes] Handle different archs when building a.o.
sivachandra created this revision. sivachandra added a reviewer: chying. sivachandra added a subscriber: lldb-commits. http://reviews.llvm.org/D13147 Files: test/lang/cpp/incomplete-types/Makefile Index: test/lang/cpp/incomplete-types/Makefile === --- test/lang/cpp/incomplete-types/Makefile +++ test/lang/cpp/incomplete-types/Makefile @@ -28,7 +28,7 @@ $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o a.o: a.cpp - $(CXX) -c a.cpp -o a.o + $(CXX) $(CFLAGS_NO_DEBUG) -c a.cpp -o a.o clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o Index: test/lang/cpp/incomplete-types/Makefile === --- test/lang/cpp/incomplete-types/Makefile +++ test/lang/cpp/incomplete-types/Makefile @@ -28,7 +28,7 @@ $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o a.o: a.cpp - $(CXX) -c a.cpp -o a.o + $(CXX) $(CFLAGS_NO_DEBUG) -c a.cpp -o a.o clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248547 - [TestCppIncompleteTypes] Handle different archs when building a.o.
Author: sivachandra Date: Thu Sep 24 17:13:36 2015 New Revision: 248547 URL: http://llvm.org/viewvc/llvm-project?rev=248547&view=rev Log: [TestCppIncompleteTypes] Handle different archs when building a.o. Reviewers: chying Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D13147 Modified: lldb/trunk/test/lang/cpp/incomplete-types/Makefile Modified: lldb/trunk/test/lang/cpp/incomplete-types/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lang/cpp/incomplete-types/Makefile?rev=248547&r1=248546&r2=248547&view=diff == --- lldb/trunk/test/lang/cpp/incomplete-types/Makefile (original) +++ lldb/trunk/test/lang/cpp/incomplete-types/Makefile Thu Sep 24 17:13:36 2015 @@ -28,7 +28,7 @@ length_nolimit.o: length.cpp $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o a.o: a.cpp - $(CXX) -c a.cpp -o a.o + $(CXX) $(CFLAGS_NO_DEBUG) -c a.cpp -o a.o clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13147: [TestCppIncompleteTypes] Handle different archs when building a.o.
This revision was automatically updated to reflect the committed changes. Closed by commit rL248547: [TestCppIncompleteTypes] Handle different archs when building a.o. (authored by sivachandra). Changed prior to commit: http://reviews.llvm.org/D13147?vs=35678&id=35679#toc Repository: rL LLVM http://reviews.llvm.org/D13147 Files: lldb/trunk/test/lang/cpp/incomplete-types/Makefile Index: lldb/trunk/test/lang/cpp/incomplete-types/Makefile === --- lldb/trunk/test/lang/cpp/incomplete-types/Makefile +++ lldb/trunk/test/lang/cpp/incomplete-types/Makefile @@ -28,7 +28,7 @@ $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o a.o: a.cpp - $(CXX) -c a.cpp -o a.o + $(CXX) $(CFLAGS_NO_DEBUG) -c a.cpp -o a.o clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o Index: lldb/trunk/test/lang/cpp/incomplete-types/Makefile === --- lldb/trunk/test/lang/cpp/incomplete-types/Makefile +++ lldb/trunk/test/lang/cpp/incomplete-types/Makefile @@ -28,7 +28,7 @@ $(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o a.o: a.cpp - $(CXX) -c a.cpp -o a.o + $(CXX) $(CFLAGS_NO_DEBUG) -c a.cpp -o a.o clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13149: Create GoLanguageRuntime
ribrdb created this revision. ribrdb added a reviewer: clayborg. ribrdb added a subscriber: lldb-commits. ribrdb set the repository for this revision to rL LLVM. GoLanguageRuntime supports finding the runtime type for Go interfaces. Repository: rL LLVM http://reviews.llvm.org/D13149 Files: lldb.xcodeproj/project.pbxproj source/API/SystemInitializerFull.cpp source/Plugins/LanguageRuntime/CMakeLists.txt source/Plugins/LanguageRuntime/Go/CMakeLists.txt source/Plugins/LanguageRuntime/Go/GoLanguageRuntime.cpp source/Plugins/LanguageRuntime/Go/GoLanguageRuntime.h source/Plugins/LanguageRuntime/Go/Makefile source/Symbol/GoASTContext.cpp test/lang/go/runtime/TestGoASTContext.py test/lang/go/runtime/main.go Index: test/lang/go/runtime/main.go === --- /dev/null +++ test/lang/go/runtime/main.go @@ -0,0 +1,38 @@ +package main + +import "fmt" + +type Fooer interface { + Foo() int +} + +type SomeFooer struct { + val int +} + +func (s SomeFooer) Foo() int { + return s.val +} + +type AnotherFooer struct { +a, b, c int +} + +func (s AnotherFooer) Foo() int { + return s.a +} + + +func printEface(a, b, c, d interface{}) { +fmt.Println(a, b, c, d) // Set breakpoint 1 +} + +func printIface(a, b Fooer) { +fmt.Println(a, b) // Set breakpoint 2 +} +func main() { +sf := SomeFooer{9} +af := AnotherFooer{-1, -2, -3} +printEface(1,2.0, sf, af) +printIface(sf, af) +} \ No newline at end of file Index: test/lang/go/runtime/TestGoASTContext.py === --- /dev/null +++ test/lang/go/runtime/TestGoASTContext.py @@ -0,0 +1,79 @@ +"""Test the go dynamic type handling.""" + +import os, time +import unittest2 +import lldb +import lldbutil +from lldbtest import * + +class TestGoLanguageRuntime(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +@python_api_test +@skipIfRemote # Not remote test suite ready +@skipUnlessGoInstalled +def test_with_dsym_and_python_api(self): +"""Test GoASTContext dwarf parsing.""" +self.buildGo() +self.launchProcess() +self.go_interface_types() + +def setUp(self): +# Call super's setUp(). +TestBase.setUp(self) +# Find the line numbers to break inside main(). +self.main_source = "main.go" +self.break_line1 = line_number(self.main_source, '// Set breakpoint 1') +self.break_line2 = line_number(self.main_source, '// Set breakpoint 2') + + +def launchProcess(self): +exe = os.path.join(os.getcwd(), "a.out") + +target = self.dbg.CreateTarget(exe) +self.assertTrue(target, VALID_TARGET) + +bpt1 = target.BreakpointCreateByLocation(self.main_source, self.break_line1) +self.assertTrue(bpt1, VALID_BREAKPOINT) +bpt2 = target.BreakpointCreateByLocation(self.main_source, self.break_line2) +self.assertTrue(bpt2, VALID_BREAKPOINT) + +# Now launch the process, and do not stop at entry point. +process = target.LaunchSimple (None, None, self.get_process_working_directory()) + +self.assertTrue(process, PROCESS_IS_VALID) + +# The stop reason of the thread should be breakpoint. +thread_list = lldbutil.get_threads_stopped_at_breakpoint (process, bpt1) + +# Make sure we stopped at the first breakpoint. +self.assertTrue (len(thread_list) != 0, "No thread stopped at our breakpoint.") +self.assertTrue (len(thread_list) == 1, "More than one thread stopped at our breakpoint.") + +frame = thread_list[0].GetFrameAtIndex(0) +self.assertTrue (frame, "Got a valid frame 0 frame.") + +def go_interface_types(self): +f = self.frame() +v = f.FindVariable("a", lldb.eDynamicCanRunTarget) +self.assertEqual("*int", v.GetType().name) +self.assertEqual(1, v.Dereference().GetValueAsSigned()) +v = f.FindVariable("b", lldb.eDynamicCanRunTarget) +self.assertEqual("*float64", v.GetType().name) +err = lldb.SBError() +self.assertEqual(2.0, v.Dereference().GetData().GetDouble(err, 0)) +v = f.FindVariable("c", lldb.eDynamicCanRunTarget) +self.assertEqual("*main.SomeFooer", v.GetType().name) +self.assertEqual(9, v.Dereference().GetChildAtIndex(0).GetValueAsSigned()) +v = f.FindVariable("d", lldb.eDynamicCanRunTarget) +self.assertEqual("*main.AnotherFooer", v.GetType().name) +self.assertEqual(-1, v.Dereference().GetChildAtIndex(0).GetValueAsSigned()) +self.assertEqual(-2, v.Dereference().GetChildAtIndex(1).GetValueAsSigned()) +self.assertEqual(-3, v.Dereference().GetChildAtIndex(2).GetValueAsSigned()) + +if __name__ == '__main__': +import atexit +lldb.SBDebugger.Initialize() +atexit.register(lambda: lldb.SBDebugger.Terminate()) +unittest2.main() Index: source/Symbol/GoASTContext.cpp =
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
To be clear, I would like this Makefile to turn into the following: LEVEL = ../../../make CXX_SOURCES = main.cpp length.cpp DEBUG_INFO_FULL = True DEBUG_INFO_LIMITED = True And that's it. You shouldn't need anything else. Whatever needs to happen in Makefile.rules to make this work should be done. By default, DEBUG_INFO_FULL should be True and DEBUG_INFO_LIMITED should be false. This will allow all other makefiles that don't care about the debug info to work as they normally do. If neither variable is True, it won't use -g. If only DEBUG_INFO_FULL is True, it will build one target and use -fno-limit-debug-info. If only DEBUG_INFO_LIMITED is true, it will build one target and use -flimit-debug-info. If both are true, it will build two targets, one for each case. Does this seem reasonable? On Thu, Sep 24, 2015 at 12:57 PM Zachary Turner wrote: > zturner added a comment. > > I know, I've seen them in a few places myself. But I think we should move > away from that, because it's a large source of portability problems > > > http://reviews.llvm.org/D13066 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
Also,when you say the next iteration, is this something that is going to happen at an unknown time in the future whenever you happen to touch it again, or are you working on another iteration now? On Thu, Sep 24, 2015 at 4:25 PM Zachary Turner wrote: > To be clear, I would like this Makefile to turn into the following: > > LEVEL = ../../../make > > CXX_SOURCES = main.cpp length.cpp > > DEBUG_INFO_FULL = True > DEBUG_INFO_LIMITED = True > > And that's it. You shouldn't need anything else. Whatever needs to > happen in Makefile.rules to make this work should be done. > > By default, DEBUG_INFO_FULL should be True and DEBUG_INFO_LIMITED should > be false. This will allow all other makefiles that don't care about the > debug info to work as they normally do. If neither variable is True, it > won't use -g. If only DEBUG_INFO_FULL is True, it will build one target > and use -fno-limit-debug-info. If only DEBUG_INFO_LIMITED is true, it will > build one target and use -flimit-debug-info. If both are true, it will > build two targets, one for each case. > > Does this seem reasonable? > > On Thu, Sep 24, 2015 at 12:57 PM Zachary Turner > wrote: > >> zturner added a comment. >> >> I know, I've seen them in a few places myself. But I think we should move >> away from that, because it's a large source of portability problems >> >> >> http://reviews.llvm.org/D13066 >> >> >> >> ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
zturner added a comment. Also,when you say the next iteration, is this something that is going to happen at an unknown time in the future whenever you happen to touch it again, or are you working on another iteration now? http://reviews.llvm.org/D13066 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
zturner added a comment. To be clear, I would like this Makefile to turn into the following: LEVEL = ../../../make CXX_SOURCES = main.cpp length.cpp DEBUG_INFO_FULL = True DEBUG_INFO_LIMITED = True And that's it. You shouldn't need anything else. Whatever needs to happen in Makefile.rules to make this work should be done. By default, DEBUG_INFO_FULL should be True and DEBUG_INFO_LIMITED should be false. This will allow all other makefiles that don't care about the debug info to work as they normally do. If neither variable is True, it won't use -g. If only DEBUG_INFO_FULL is True, it will build one target and use -fno-limit-debug-info. If only DEBUG_INFO_LIMITED is true, it will build one target and use -flimit-debug-info. If both are true, it will build two targets, one for each case. Does this seem reasonable? http://reviews.llvm.org/D13066 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On 24 September 2015 at 17:35, Siva Chandra wrote: > > As blaikie pointed out on the review, it could be because libc++ (which is > the default on FreeBSD and Darwin?) does not have an explicit declaration > for std::basic_string<...>? I have now landed a change which removes the > dependence on std::string. Hope it fixes the test for FreeBSD and darwin. Ah yes, I did not see that before replying. The test indeed now passes for me on FreeBSD. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r248401 - DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
On Thu, Sep 24, 2015 at 4:57 PM, Ed Maste wrote: > On 24 September 2015 at 17:35, Siva Chandra > wrote: > > > > As blaikie pointed out on the review, it could be because libc++ (which > is > > the default on FreeBSD and Darwin?) does not have an explicit declaration > > for std::basic_string<...>? I have now landed a change which removes the > > dependence on std::string. Hope it fixes the test for FreeBSD and darwin. > > Ah yes, I did not see that before replying. The test indeed now passes > for me on FreeBSD. > Darwin as well \o/ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
dawn accepted this commit. dawn added a comment. Fixed by Todd in r248545. Todd rocks!!! :) Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [Diffusion] rL248048: Added support for resolving symbolic links to FileSpec.
dawn added a comment. The LaunchWithShellExpandTestCase tests are now fixed, but I'm still seeing a failure in the LaunchInTerminalTestCase test. Users: spyffe (Author) dawn (Auditor) http://reviews.llvm.org/rL248048 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r248555 - Fix evaluation of unicode character arrays (char16_t[] and char32_t[])
Author: dperchik Date: Thu Sep 24 21:16:52 2015 New Revision: 248555 URL: http://llvm.org/viewvc/llvm-project?rev=248555&view=rev Log: Fix evaluation of unicode character arrays (char16_t[] and char32_t[]) Suppose we have the UTF-16 string: char16_t[] s = u"hello"; Before this patch, evaluating the string in lldb would get: (char16_t [6]) $0 = ([0] = U+0068 u'h', [1] = U+0065 u'e', [2] = U+006c u'l', [3] = U+006c u'l', [4] = U+006f u'o', [5] = U+ u'\0') After applying the patch, we now get: (char16_t [6]) $0 = u"hello" Patch from evgeny.levi...@gmail.com Reviewed by: granata.enrico Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D13053 Modified: lldb/trunk/include/lldb/DataFormatters/FormattersHelpers.h lldb/trunk/source/DataFormatters/FormattersHelpers.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp lldb/trunk/test/lang/cpp/char1632_t/TestChar1632T.py lldb/trunk/test/lang/cpp/char1632_t/main.cpp Modified: lldb/trunk/include/lldb/DataFormatters/FormattersHelpers.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/FormattersHelpers.h?rev=248555&r1=248554&r2=248555&view=diff == --- lldb/trunk/include/lldb/DataFormatters/FormattersHelpers.h (original) +++ lldb/trunk/include/lldb/DataFormatters/FormattersHelpers.h Thu Sep 24 21:16:52 2015 @@ -106,6 +106,9 @@ namespace lldb_private { size_t ExtractIndexFromString (const char* item_name); +lldb::addr_t +GetArrayAddressOrPointerValue (ValueObject& valobj); + time_t GetOSXEpoch (); } // namespace formatters Modified: lldb/trunk/source/DataFormatters/FormattersHelpers.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/FormattersHelpers.cpp?rev=248555&r1=248554&r2=248555&view=diff == --- lldb/trunk/source/DataFormatters/FormattersHelpers.cpp (original) +++ lldb/trunk/source/DataFormatters/FormattersHelpers.cpp Thu Sep 24 21:16:52 2015 @@ -306,3 +306,16 @@ lldb_private::formatters::ExtractIndexFr return UINT32_MAX; return idx; } + +lldb::addr_t +lldb_private::formatters::GetArrayAddressOrPointerValue (ValueObject& valobj) +{ +lldb::addr_t data_addr = LLDB_INVALID_ADDRESS; + +if (valobj.IsPointerType()) +data_addr = valobj.GetValueAsUnsigned(0); +else if (valobj.IsArrayType()) +data_addr = valobj.GetAddressOf(); + +return data_addr; +} Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp?rev=248555&r1=248554&r2=248555&view=diff == --- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (original) +++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp Thu Sep 24 21:16:52 2015 @@ -628,8 +628,20 @@ LoadSystemFormatters(lldb::TypeCategoryI #ifndef LLDB_DISABLE_PYTHON // FIXME because of a bug in the FormattersContainer we need to add a summary for both X* and const X* () AddCXXSummary(cpp_category_sp, lldb_private::formatters::Char16StringSummaryProvider, "char16_t * summary provider", ConstString("char16_t *"), string_flags); +AddCXXSummary(cpp_category_sp, + lldb_private::formatters::Char16StringSummaryProvider, + "char16_t [] summary provider", + ConstString("char16_t \\[[0-9]+\\]"), + string_array_flags, + true); AddCXXSummary(cpp_category_sp, lldb_private::formatters::Char32StringSummaryProvider, "char32_t * summary provider", ConstString("char32_t *"), string_flags); +AddCXXSummary(cpp_category_sp, + lldb_private::formatters::Char32StringSummaryProvider, + "char32_t [] summary provider", + ConstString("char32_t \\[[0-9]+\\]"), + string_array_flags, + true); AddCXXSummary(cpp_category_sp, lldb_private::formatters::WCharStringSummaryProvider, "wchar_t * summary provider", ConstString("wchar_t *"), string_flags); AddCXXSummary(cpp_category_sp, lldb_private::formatters::WCharStringSummaryProvider, "wchar_t * summary provider", ConstString("wchar_t \\[[0-9]+\\]"), string_array_flags, true); Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp?rev=248555&r1=248554&r2=248555&view=diff == --- lldb/trunk/source/Plugins/Language/CPlusPlus/C
Re: [Lldb-commits] [Diffusion] rL248338: Move the "run" alias from process launch --shell to process launch --shell…
tfiala added a comment. > Fixed by Todd in r248545. Oh good! > Todd rocks!!! :) You are far too kind :-) Users: dawn (Auditor) http://reviews.llvm.org/rL248338 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13154: [MIPS] Use Address::GetAddressClass() instead of elf flags to decide address space of an address
bhushan created this revision. bhushan added a reviewer: clayborg. bhushan added subscribers: lldb-commits, nitesh.jain, mohit.bhakkad, sagar, jaydeep. bhushan set the repository for this revision to rL LLVM. In MIPS, an application elf can contain mixed code (mips + micromips) i.e some functions in the application can be "micromips" and some functions can be MIPS-only (non-micromips). Micromips functions has compressed addresses (bit #0 set) and MIPS functions has un-compressed addresses (bit #0 clear). Such mixed-mode elf will have micromips specific bits set in its flags. That means "IsMicromips" will be true even for non-micromips address. This patch fixes this by using Address::GetAddressClass() to decide which address space the address belongs to (instead of deciding this from elf's flags). Repository: rL LLVM http://reviews.llvm.org/D13154 Files: source/Target/Target.cpp Index: source/Target/Target.cpp === --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -2269,10 +2269,12 @@ uint32_t loop_count = 0; Address resolved_addr; uint32_t arch_flags = m_arch.GetFlags (); -bool IsMips16 = arch_flags & ArchSpec::eMIPSAse_mips16; -bool IsMicromips = arch_flags & ArchSpec::eMIPSAse_micromips; SectionLoadList §ion_load_list = GetSectionLoadList(); +// Get opcode address +addr = GetOpcodeLoadAddress (addr, eAddressClassCode); +breakable_addr = addr; + if (section_load_list.IsEmpty()) // No sections are loaded, so we must assume we are not running yet // and need to operate only on file address. @@ -2310,7 +2312,7 @@ if (loop_count > 3) { // Scan previous 6 bytes -if (IsMips16 | IsMicromips) +if (resolved_addr.GetAddressClass() == eAddressClassCodeAlternateISA) loop_count = 3; // For mips-only, instructions are always 4 bytes, so scan previous 4 bytes only. else Index: source/Target/Target.cpp === --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -2269,10 +2269,12 @@ uint32_t loop_count = 0; Address resolved_addr; uint32_t arch_flags = m_arch.GetFlags (); -bool IsMips16 = arch_flags & ArchSpec::eMIPSAse_mips16; -bool IsMicromips = arch_flags & ArchSpec::eMIPSAse_micromips; SectionLoadList §ion_load_list = GetSectionLoadList(); +// Get opcode address +addr = GetOpcodeLoadAddress (addr, eAddressClassCode); +breakable_addr = addr; + if (section_load_list.IsEmpty()) // No sections are loaded, so we must assume we are not running yet // and need to operate only on file address. @@ -2310,7 +2312,7 @@ if (loop_count > 3) { // Scan previous 6 bytes -if (IsMips16 | IsMicromips) +if (resolved_addr.GetAddressClass() == eAddressClassCodeAlternateISA) loop_count = 3; // For mips-only, instructions are always 4 bytes, so scan previous 4 bytes only. else ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism
tfiala added a comment. > That's a good idea. I'm (somewhat slowly) attempting to work in some clean up > each time I touch it. I'll hit that at some point. I'll start winding down on > changes to the test infrastructure soon-ish. If I don't hit that by the time > I stop heavy changes, definitely feel free to jump on it! Okay, I couldn't help myself. I'm taking a bit more time on this as I set it up much more nicely. I'm pulling all the process management into another module and breaking out some platform-specific bits into a per-platform helper. I won't have something up until tomorrow later on at the earliest. http://reviews.llvm.org/D13124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D13158: Allow to construct CMIUtilString using std::string directly (MI)
ki.stfu created this revision. ki.stfu added reviewers: abidh, brucem. ki.stfu added subscribers: abidh, brucem, lldb-commits. Allow to construct CMIUtilString using std::string directly (MI) This patch cleans up lldb-mi code, and serves to simplify the following case: ``` std::string strGoodbye = "!Hello"; CMIUtilString strHello = strGoodbye.substr(1).c_str(); ``` With CMIUtilString(std::string) we can omit .c_str(): ``` std::string strGoodbye = "!Hello"; CMIUtilString strHello = strGoodbye.substr(1); ``` http://reviews.llvm.org/D13158 Files: tools/lldb-mi/MICmdArgContext.cpp tools/lldb-mi/MICmdArgValOptionLong.cpp tools/lldb-mi/MICmdArgValOptionShort.cpp tools/lldb-mi/MICmdArgValThreadGrp.cpp tools/lldb-mi/MICmdCmdData.cpp tools/lldb-mi/MICmdCmdVar.cpp tools/lldb-mi/MICmdInterpreter.cpp tools/lldb-mi/MICmnLLDBDebugger.cpp tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.cpp tools/lldb-mi/MIDriver.cpp tools/lldb-mi/MIUtilString.cpp tools/lldb-mi/MIUtilString.h Index: tools/lldb-mi/MIUtilString.h === --- tools/lldb-mi/MIUtilString.h +++ tools/lldb-mi/MIUtilString.h @@ -45,6 +45,7 @@ /* ctor */ CMIUtilString(const char *vpData); /* ctor */ CMIUtilString(const char *const *vpData); /* ctor */ CMIUtilString(const char *vpData, size_t nLen); +/* ctor */ CMIUtilString(const std::string& vrStr); // bool ExtractNumber(MIint64 &vwrNumber) const; CMIUtilString FindAndReplace(const CMIUtilString &vFind, const CMIUtilString &vReplaceWith) const; Index: tools/lldb-mi/MIUtilString.cpp === --- tools/lldb-mi/MIUtilString.cpp +++ tools/lldb-mi/MIUtilString.cpp @@ -68,6 +68,18 @@ } //++ +// Details: CMIUtilString constructor. +// Type:Method. +// Args:vpStr - Text data. +// Return: None. +// Throws: None. +//-- +CMIUtilString::CMIUtilString(const std::string& vrStr) +: std::string(vrStr) +{ +} + +//++ // Details: CMIUtilString assignment operator. // Type:Method. // Args:vpRhs - Pointer to UTF8 text data. @@ -243,7 +255,7 @@ // Extract string between delimiters const size_t nSectionLen(nNextDelimiterPos - nSectionPos); const std::string strSection(substr(nSectionPos, nSectionLen)); -vwVecSplits.push_back(strSection.c_str()); +vwVecSplits.push_back(strSection); // Next nOffset = nNextDelimiterPos + 1; @@ -299,7 +311,7 @@ // Extract string between delimiters const size_t nSectionLen(nNextDelimiterPos - nSectionPos); const std::string strSection(substr(nSectionPos, nSectionLen)); -vwVecSplits.push_back(strSection.c_str()); +vwVecSplits.push_back(strSection); // Next nOffset = nNextDelimiterPos + 1; @@ -337,7 +349,7 @@ if (nPos == std::string::npos) return *this; -const CMIUtilString strNew(substr(0, nPos).c_str()); +const CMIUtilString strNew(substr(0, nPos)); return strNew; } @@ -542,12 +554,12 @@ const size_t nPos = find_last_not_of(pWhiteSpace); if (nPos != std::string::npos) { -strNew = substr(0, nPos + 1).c_str(); +strNew = substr(0, nPos + 1); } const size_t nPos2 = strNew.find_first_not_of(pWhiteSpace); if (nPos2 != std::string::npos) { -strNew = strNew.substr(nPos2).c_str(); +strNew = strNew.substr(nPos2); } return strNew; @@ -568,7 +580,7 @@ if (nLen > 1) { if ((strNew[0] == vChar) && (strNew[nLen - 1] == vChar)) -strNew = strNew.substr(1, nLen - 2).c_str(); +strNew = strNew.substr(1, nLen - 2); } return strNew; Index: tools/lldb-mi/MIDriver.cpp === --- tools/lldb-mi/MIDriver.cpp +++ tools/lldb-mi/MIDriver.cpp @@ -893,7 +893,7 @@ const std::string vToken(vTextLine.begin(), vTextLine.begin() + nCommandOffset); // 001target create "/path/to/file" //^ -- CLI command - const CMIUtilString vCliCommand(std::string(vTextLine, nCommandOffset).c_str()); + const CMIUtilString vCliCommand(std::string(vTextLine, nCommandOffset)); // 5. Escape special characters and embed the command in a string // Result: it looks like -- target create \"/path/to/file\". Index: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp === --- tools/lldb-mi/MICmnLLDBUtilSBValue.cpp +++ tools/lldb-mi/MICmnLLDBUtilSBValue.cpp @@ -505,7 +505,7 @@ addr += sizeof(ch); } -return result.c_str(); +return result; } //++
Re: [Lldb-commits] [PATCH] D13158: Allow to construct CMIUtilString using std::string directly (MI)
brucem added inline comments. Comment at: tools/lldb-mi/MICmdInterpreter.cpp:166 @@ -165,3 +165,3 @@ m_miCmdData.strMiCmdToken = strNum.c_str(); } Can this one be changed too? http://reviews.llvm.org/D13158 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13158: Allow to construct CMIUtilString using std::string directly (MI)
ki.stfu added inline comments. Comment at: tools/lldb-mi/MICmdInterpreter.cpp:166 @@ -165,3 +165,3 @@ m_miCmdData.strMiCmdToken = strNum.c_str(); } brucem wrote: > Can this one be changed too? Sure. http://reviews.llvm.org/D13158 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13158: Allow to construct CMIUtilString using std::string directly (MI)
ki.stfu updated this revision to Diff 35703. ki.stfu added a comment. A little fix as @brucem requested; Remove CMIUtilString(const char *const *vpData) and CMIUtilString(const char *vpData, size_t nLen) ctors; Cleanup CMIUtilString::operator=(const std::string &vrRhs) http://reviews.llvm.org/D13158 Files: tools/lldb-mi/MICmdArgContext.cpp tools/lldb-mi/MICmdArgValOptionLong.cpp tools/lldb-mi/MICmdArgValOptionShort.cpp tools/lldb-mi/MICmdArgValThreadGrp.cpp tools/lldb-mi/MICmdCmdData.cpp tools/lldb-mi/MICmdCmdVar.cpp tools/lldb-mi/MICmdInterpreter.cpp tools/lldb-mi/MICmnLLDBDebugger.cpp tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp tools/lldb-mi/MICmnLLDBUtilSBValue.cpp tools/lldb-mi/MIDriver.cpp tools/lldb-mi/MIUtilString.cpp tools/lldb-mi/MIUtilString.h Index: tools/lldb-mi/MIUtilString.h === --- tools/lldb-mi/MIUtilString.h +++ tools/lldb-mi/MIUtilString.h @@ -43,8 +43,7 @@ public: /* ctor */ CMIUtilString(); /* ctor */ CMIUtilString(const char *vpData); -/* ctor */ CMIUtilString(const char *const *vpData); -/* ctor */ CMIUtilString(const char *vpData, size_t nLen); +/* ctor */ CMIUtilString(const std::string& vrStr); // bool ExtractNumber(MIint64 &vwrNumber) const; CMIUtilString FindAndReplace(const CMIUtilString &vFind, const CMIUtilString &vReplaceWith) const; Index: tools/lldb-mi/MIUtilString.cpp === --- tools/lldb-mi/MIUtilString.cpp +++ tools/lldb-mi/MIUtilString.cpp @@ -45,25 +45,12 @@ //++ // Details: CMIUtilString constructor. // Type:Method. -// Args:vpData - Pointer to UTF8 text data. -// Return: None. -// Throws: None. -//-- -CMIUtilString::CMIUtilString(const char *const *vpData) -: std::string((const char *)vpData) -{ -} - -//++ -// Details: CMIUtilString constructor. -// Type:Method. -// Args:vpData - Pointer to UTF8 text data. -// nLen- Length of string. +// Args:vpStr - Text data. // Return: None. // Throws: None. //-- -CMIUtilString::CMIUtilString(const char *vpData, size_t nLen) -: std::string(vpData, nLen) +CMIUtilString::CMIUtilString(const std::string& vrStr) +: std::string(vrStr) { } @@ -96,11 +83,7 @@ //-- CMIUtilString &CMIUtilString::operator=(const std::string &vrRhs) { -if (*this == vrRhs) -return *this; - assign(vrRhs); - return *this; } @@ -243,7 +226,7 @@ // Extract string between delimiters const size_t nSectionLen(nNextDelimiterPos - nSectionPos); const std::string strSection(substr(nSectionPos, nSectionLen)); -vwVecSplits.push_back(strSection.c_str()); +vwVecSplits.push_back(strSection); // Next nOffset = nNextDelimiterPos + 1; @@ -299,7 +282,7 @@ // Extract string between delimiters const size_t nSectionLen(nNextDelimiterPos - nSectionPos); const std::string strSection(substr(nSectionPos, nSectionLen)); -vwVecSplits.push_back(strSection.c_str()); +vwVecSplits.push_back(strSection); // Next nOffset = nNextDelimiterPos + 1; @@ -337,7 +320,7 @@ if (nPos == std::string::npos) return *this; -const CMIUtilString strNew(substr(0, nPos).c_str()); +const CMIUtilString strNew(substr(0, nPos)); return strNew; } @@ -542,12 +525,12 @@ const size_t nPos = find_last_not_of(pWhiteSpace); if (nPos != std::string::npos) { -strNew = substr(0, nPos + 1).c_str(); +strNew = substr(0, nPos + 1); } const size_t nPos2 = strNew.find_first_not_of(pWhiteSpace); if (nPos2 != std::string::npos) { -strNew = strNew.substr(nPos2).c_str(); +strNew = strNew.substr(nPos2); } return strNew; @@ -568,7 +551,7 @@ if (nLen > 1) { if ((strNew[0] == vChar) && (strNew[nLen - 1] == vChar)) -strNew = strNew.substr(1, nLen - 2).c_str(); +strNew = strNew.substr(1, nLen - 2); } return strNew; Index: tools/lldb-mi/MIDriver.cpp === --- tools/lldb-mi/MIDriver.cpp +++ tools/lldb-mi/MIDriver.cpp @@ -893,7 +893,7 @@ const std::string vToken(vTextLine.begin(), vTextLine.begin() + nCommandOffset); // 001target create "/path/to/file" //^ -- CLI command - const CMIUtilString vCliCommand(std::string(vTextLine, nCommandOffset).c_str()); + const CMIUtilString vCliCommand(std::string(vTextLine, nCommandOffset)); // 5. Escape special characters and embed the command in a string // Result: it looks like -- target create \"/path/to/file\". Index: tools/lldb-mi/MICmnLLDBUtil