[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-09 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna updated this revision to Diff 385711.
lawrence_danna added a comment.

updated according to reviewer feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112973/new/

https://reviews.llvm.org/D112973

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/lldb-python
  lldb/docs/man/lldb.rst
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBHostOS.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/test/API/functionalities/paths/TestPaths.py
  lldb/test/Shell/Driver/TestHelp.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Driver.h
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -48,6 +48,10 @@
   HelpText<"Alias for --python-path">,
   Group;
 
+def print_script_interpreter_info: F<"print-script-interpreter-info">,
+  HelpText<"Prints out a json dictionary with information about the scripting language interpreter.">,
+  Group;
+
 def script_language: Separate<["--", "-"], "script-language">,
   MetaVarName<"">,
   HelpText<"Tells the debugger to use the specified scripting language for user-defined scripts.">,
Index: lldb/tools/driver/Driver.h
===
--- lldb/tools/driver/Driver.h
+++ lldb/tools/driver/Driver.h
@@ -79,6 +79,7 @@
 bool m_source_quietly = false;
 bool m_print_version = false;
 bool m_print_python_path = false;
+bool m_print_script_interpreter_info = false;
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -201,6 +201,9 @@
   if (args.hasArg(OPT_python_path)) {
 m_option_data.m_print_python_path = true;
   }
+  if (args.hasArg(OPT_print_script_interpreter_info)) {
+m_option_data.m_print_script_interpreter_info = true;
+  }
 
   if (args.hasArg(OPT_batch)) {
 m_option_data.m_batch = true;
@@ -398,6 +401,18 @@
 return error;
   }
 
+  if (m_option_data.m_print_script_interpreter_info) {
+const char *info = m_debugger.GetScriptInterpreterInfo(m_debugger.GetScriptLanguage());
+if (info) {
+  llvm::outs() << info << '\n';
+} else {
+  llvm::errs() << "no script interpreter.\n";
+  error.SetErrorString("no script interpreter.");
+}
+exiting = true;
+return error;
+  }
+
   return error;
 }
 
Index: lldb/test/Shell/Driver/TestHelp.test
===
--- lldb/test/Shell/Driver/TestHelp.test
+++ lldb/test/Shell/Driver/TestHelp.test
@@ -62,6 +62,7 @@
 
 CHECK: SCRIPTING
 CHECK: -l
+CHECK: --print-script-interpreter-info
 CHECK: --python-path
 CHECK: -P
 CHECK: --script-language
Index: lldb/test/API/functionalities/paths/TestPaths.py
===
--- lldb/test/API/functionalities/paths/TestPaths.py
+++ lldb/test/API/functionalities/paths/TestPaths.py
@@ -5,6 +5,8 @@
 
 import lldb
 import os
+import sys
+import json
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -42,6 +44,14 @@
 self.assertTrue(any([os.path.exists(os.path.join(shlib_dir, f)) for f in
 filenames]), "shlib_dir = " + shlib_dir)
 
+@no_debug_info_test
+def test_interpreter_info(self):
+info = json.loads(self.dbg.GetScriptInterpreterInfo(self.dbg.GetScriptingLanguage("python")))
+prefix = info['prefix']
+self.assertEqual(os.path.realpath(sys.prefix), os.path.realpath(prefix))
+self.assertEqual(
+os.path.realpath(os.path.join(info['lldb-pythonpath'], 'lldb')),
+os.path.realpath(os.path.dirname(lldb.__file__)))
 
 @no_debug_info_test
 def test_directory_doesnt_end_with_slash(self):
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -46,6 +46,7 @@
   : ScriptInterpreter(debugger, lldb::eScriptLanguagePython),

[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-09 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna updated this revision to Diff 385712.
lawrence_danna edited the summary of this revision.
lawrence_danna added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112973/new/

https://reviews.llvm.org/D112973

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/lldb-python
  lldb/docs/man/lldb.rst
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBHostOS.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/test/API/functionalities/paths/TestPaths.py
  lldb/test/Shell/Driver/TestHelp.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Driver.h
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -48,6 +48,10 @@
   HelpText<"Alias for --python-path">,
   Group;
 
+def print_script_interpreter_info: F<"print-script-interpreter-info">,
+  HelpText<"Prints out a json dictionary with information about the scripting language interpreter.">,
+  Group;
+
 def script_language: Separate<["--", "-"], "script-language">,
   MetaVarName<"">,
   HelpText<"Tells the debugger to use the specified scripting language for user-defined scripts.">,
Index: lldb/tools/driver/Driver.h
===
--- lldb/tools/driver/Driver.h
+++ lldb/tools/driver/Driver.h
@@ -79,6 +79,7 @@
 bool m_source_quietly = false;
 bool m_print_version = false;
 bool m_print_python_path = false;
+bool m_print_script_interpreter_info = false;
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -201,6 +201,9 @@
   if (args.hasArg(OPT_python_path)) {
 m_option_data.m_print_python_path = true;
   }
+  if (args.hasArg(OPT_print_script_interpreter_info)) {
+m_option_data.m_print_script_interpreter_info = true;
+  }
 
   if (args.hasArg(OPT_batch)) {
 m_option_data.m_batch = true;
@@ -398,6 +401,19 @@
 return error;
   }
 
+  if (m_option_data.m_print_script_interpreter_info) {
+const char *info =
+m_debugger.GetScriptInterpreterInfo(m_debugger.GetScriptLanguage());
+if (info) {
+  llvm::outs() << info << '\n';
+} else {
+  llvm::errs() << "no script interpreter.\n";
+  error.SetErrorString("no script interpreter.");
+}
+exiting = true;
+return error;
+  }
+
   return error;
 }
 
Index: lldb/test/Shell/Driver/TestHelp.test
===
--- lldb/test/Shell/Driver/TestHelp.test
+++ lldb/test/Shell/Driver/TestHelp.test
@@ -62,6 +62,7 @@
 
 CHECK: SCRIPTING
 CHECK: -l
+CHECK: --print-script-interpreter-info
 CHECK: --python-path
 CHECK: -P
 CHECK: --script-language
Index: lldb/test/API/functionalities/paths/TestPaths.py
===
--- lldb/test/API/functionalities/paths/TestPaths.py
+++ lldb/test/API/functionalities/paths/TestPaths.py
@@ -5,6 +5,8 @@
 
 import lldb
 import os
+import sys
+import json
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -42,6 +44,14 @@
 self.assertTrue(any([os.path.exists(os.path.join(shlib_dir, f)) for f in
 filenames]), "shlib_dir = " + shlib_dir)
 
+@no_debug_info_test
+def test_interpreter_info(self):
+info = json.loads(self.dbg.GetScriptInterpreterInfo(self.dbg.GetScriptingLanguage("python")))
+prefix = info['prefix']
+self.assertEqual(os.path.realpath(sys.prefix), os.path.realpath(prefix))
+self.assertEqual(
+os.path.realpath(os.path.join(info['lldb-pythonpath'], 'lldb')),
+os.path.realpath(os.path.dirname(lldb.__file__)))
 
 @no_debug_info_test
 def test_directory_doesnt_end_with_slash(self):
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -46,6 +46,7 @@
   : ScriptInterpreter(debug

[Lldb-commits] [PATCH] D113362: [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread Danil Stefaniuc via Phabricator via lldb-commits
danilashtefan updated this revision to Diff 385744.
danilashtefan marked 10 inline comments as done.
danilashtefan added a comment.

All the requested changes are done :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113362/new/

https://reviews.llvm.org/D113362

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterStdForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main() {
+  std::forward_list empty{}, one_elt{47},
+  five_elts{1, 22, 333, , 5};
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterStdForwardList.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterStdForwardList.py
@@ -0,0 +1,51 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestDataFormatterLibcxxForwardList(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.line = line_number('main.cpp', '// break here')
+self.namespace = 'std'
+
+@add_test_categories(["libstdcxx"])
+def test(self):
+"""Test that std::forward_list is displayed correctly"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+forward_list = self.namespace + '::forward_list'
+self.expect("frame variable empty",
+substrs=[forward_list,
+ 'size=0',
+ '{}'])
+
+self.expect("frame variable one_elt",
+substrs=[forward_list,
+ 'size=1',
+ '{',
+ '[0] = 47',
+ '}'])
+
+self.expect("frame variable five_elts",
+substrs=[forward_list,
+ 'size=5',
+ '{',
+ '[0] = 1',
+ '[1] = 22',
+ '[2] = 333',
+ '[3] = ',
+ '[4] = 5',
+ '}'])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBSTDCPP := 1
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -923,6 +923,11 @@
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+  SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  stl_synth_flags,
+  "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
@@ -953,6 +958,10 @@
   RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
   TypeSummaryImplSP(
   new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
+  cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
+  RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+  TypeSummaryImplSP(
+  new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
 
   AddCXXSynthetic(
   cpp_category_sp,
Index: lldb/examples/synthetic/gnu_libstdcpp.py
===
--- lldb/examples/s

[Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added a comment.

In D113400#3117500 , @wallace wrote:

> could you share a screenshot of how this looks like?

Sure! Here are the screenshots of before/after in Visual Studio 2022.

Before everything is in one place (in Locals Window):
F20179455: visual-studio-before.png 
After, Registers Window (marked in red -- changed since previous step):
F20179456: registers-after.png 
After, Locals Window (locals are displayed directly, without "Locals" expand 
block):
F20179454: locals-after.png 

---

There are no visible changes in Visual Studio Code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113400/new/

https://reviews.llvm.org/D113400

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113362: [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread Danil Stefaniuc via Phabricator via lldb-commits
danilashtefan added inline comments.



Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:92
+return 1   
+size = self.node_value_pointer_offset
 current = self.next

wallace wrote:
> let's better initialize this with a simpler value
Initial size does also depend on the has_prev value, so I decided to initialize 
it is this simple way



Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:95
 current = current.GetChildMemberWithName('_M_next')
-return (size - 1)
+return (size - size_correction)
 except:

wallace wrote:
> what is this? try to come up with a better name with documentation to make 
> this easier to understand
Size to return also depends on the has_prev. I have simplified the previously 
introduced if case 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113362/new/

https://reviews.llvm.org/D113362

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113163: [LLDB][Breakpad] Create a function for each compilation unit.

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Sorry for the delay. Just a couple of (hopefully final) requests.




Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:245
+addr_t address = record->Address + base;
+SectionSP section_sp = list->FindSectionContainingFileAddress(address);
+AddressRange func_range(section_sp, address - section_sp->GetFileAddress(),

It would be nice to handle the case when this fails to find any sections. The 
most likely way for this to happen is a mismatched symbol file.



Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:337
+  std::lock_guard guard(GetModuleMutex());
+  if (!(name_type_mask & eFunctionNameTypeMethod))
+return;

How did you come to pick this? I get that this is not critical functionality 
for your use case, but it seems rather strange to be claiming that you're 
looking up methods here. If you don't care what you look up exactly, then maybe 
you could just skip this check completely, possibly leaving a TODO to implement 
this in a better way. (The Symtab class has code (heuristics) which tries to 
classify symbols into functions, methods, etc., but I'm not sure if it actually 
works with breakpad symbols (them being demangled), nor am I sure that we 
should replicate that here.)



Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:381
   std::vector symbols;
   auto add_symbol = [&](addr_t address, llvm::Optional size,
 llvm::StringRef name) {

It'd probably be better to inline this lambda now. You can do that as a 
follow-up commit. No need for review.



Comment at: lldb/test/Shell/Minidump/breakpad-symbols.test:17
 # CHECK-LABEL: image dump symtab /tmp/test/linux-x86_64
-# CHECK: Symtab, file = /tmp/test/linux-x86_64, num_symbols = 2:
-# CHECK: [0]  0   X Code0x004003d0 
0x004003d0 0x0018 0x crash()
-# CHECK: [1]  0   X Code0x004003f0 
0x004003f0 0x0010 0x _start
+# CHECK: Symtab, file = /tmp/test/linux-x86_64, num_symbols = 0
 

It'd be better to modify the test inputs here to replace FUNC with PUBLIC 
records. This test checks that breakpad symbols files work with minidump cores, 
and checking for `num_symbols = 0` is not very helpful, as that's precisely 
what would happen if the symbol files did _not_ work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113163/new/

https://reviews.llvm.org/D113163

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113330: [LLDB][Breakpad] Make lldb understand INLINE and INLINE_ORIGIN records in breakpad.

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Looks pretty good. I might have split the record parsing functions into a 
separate patch, but this is not bad either.

Just one comment about error handling.




Comment at: 
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:313-314
+BlockSP block_sp = std::make_shared(It.GetBookmark().offset);
+FileSpec callsite_file = (*m_files)[record->CallSiteFileNum];
+llvm::StringRef name = (*m_inline_origins)[record->OriginNum];
+Declaration callsite(callsite_file, record->CallSiteLineNum);

In lldb, we treat (or at least try to) debug info [[ 
https://lldb.llvm.org/resources/contributing.html#error-handling-and-use-of-assertions-in-lldb
 | as "user input"]], meaning invalid/inconsistent debug info should not crash 
the debugger. While the situation is not as critical for breakpad as it is for 
dwarf (due to it not having as many producers), I think the principle remains.

It doesn't really matter much how you handle invalid records (like, it'd be 
fine if you just throw away all inline info for the affected function), but 
it'd be nice if it does not crash lldb.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113330/new/

https://reviews.llvm.org/D113330

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113174: [lldb] Summary provider for char flexible array members

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D113174#3112464 , @shafik wrote:

> In D113174#3109664 , @jingham wrote:
>
>> Regex Type summary matching happens after the ConstString matching.  Enrico 
>> did it that way because the ConstString matching is so much quicker, so if 
>> we can find a match there we'll get to it more quickly...
>>
>> So this patch moves the char * recognition from the beginning of the type 
>> summary matching to the end, and potentially makes it a slower match.
>>
>> I doubt that this will be noticeable on modern systems, however, just 
>> something to keep in mind.
>>
>> It also changes the order of search slightly.  I think this is observable: 
>> it would mean a regex that happens to match "char *" as well as other things 
>> used to not be chosen for "char *" because it would have hit the ConstString 
>> summary first.  Now it will match, because the built-in regex summary will 
>> be checked after the user added ones.
>>
>> Again, I don't think this is a reason not to do the patch.  But something to 
>> keep in mind.  The code itself looks fine.
>
> I was looking at these char formatters a while ago and IIRC I did see some 
> impact making these regex matches but I don't remember the details and did 
> not look closely (so maybe there was another effect at play) b/c I went with 
> a different approach.

This is still a bounded set, so it is possible to replace this regex with 6 
strings. I did a regex because it makes the code shorter, but I have no problem 
with spelling this out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113174/new/

https://reviews.llvm.org/D113174

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112709/new/

https://reviews.llvm.org/D112709

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

It would be nice if the commit message/summary included more details about the 
differences in signal handling (the things we talked about on irc), but 
generally, I think this patch looks good.

The only thing that's missing is interoperability with older lldb-compatible 
stubs (debugservers mainly). I believe we already have some "standard" solution 
for that, like checking for QThreadSuffixSupported or some similar lldb-only 
packet (?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108078/new/

https://reviews.llvm.org/D108078

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D95710: [lldb/Commands] Add command options for ScriptedProcess to ProcessLaunch

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Sorry for grave digging, but my compiler just became smart enough to start 
complaining about this.




Comment at: lldb/source/API/SBLaunchInfo.cpp:392
+  llvm::json::OStream s(stream.ref().AsRawOstream());
+  dict_sp->Serialize(s);
+

you're dereferencing a null pointer here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95710/new/

https://reviews.llvm.org/D95710

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] a40929d - [lldb] Fix cross-platform kills

2021-11-09 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-11-09T15:31:07+01:00
New Revision: a40929dcd295997736caf066758dd359216443c2

URL: 
https://github.com/llvm/llvm-project/commit/a40929dcd295997736caf066758dd359216443c2
DIFF: 
https://github.com/llvm/llvm-project/commit/a40929dcd295997736caf066758dd359216443c2.diff

LOG: [lldb] Fix cross-platform kills

This patch fixes an amusing bug where a Platform::Kill operation would
happily terminate a proces on a completely different platform, as long
as they have the same process ID. This was due to the fact that the
implementation was iterating through all known (debugged) processes in
order terminate them directly.

This patch just deletes that logic, and makes everything go through the
OS process termination APIs. While it would be possible to fix the logic
to check for a platform match, it seemed to me that the implementation
was being too smart for its own good -- accessing random Process
objects without knowing anything about their state is risky at best.
Going through the os ensures we avoid any races.

I also "upgrade" the termination signal to a SIGKILL to ensure the
process really dies after this operation.

Differential Revision: https://reviews.llvm.org/D113184

Added: 
lldb/test/API/functionalities/gdb_remote_client/Makefile
lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
lldb/test/API/functionalities/gdb_remote_client/sleep.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Target/Platform.cpp
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 7477905a131d0..924e1fed4f38c 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -422,6 +422,9 @@ def terminate(self):
 def poll(self):
 return self._proc.poll()
 
+def wait(self, timeout=None):
+return self._proc.wait(timeout)
+
 
 class _RemoteProcess(_BaseProcess):
 

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index b7c691b058be5..be91a90158e8c 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1044,25 +1044,11 @@ Status Platform::KillProcess(const lldb::pid_t pid) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "Platform::%s, pid %" PRIu64, __FUNCTION__, pid);
 
-  // Try to find a process plugin to handle this Kill request.  If we can't,
-  // fall back to the default OS implementation.
-  size_t num_debuggers = Debugger::GetNumDebuggers();
-  for (size_t didx = 0; didx < num_debuggers; ++didx) {
-DebuggerSP debugger = Debugger::GetDebuggerAtIndex(didx);
-lldb_private::TargetList &targets = debugger->GetTargetList();
-for (int tidx = 0; tidx < targets.GetNumTargets(); ++tidx) {
-  ProcessSP process = targets.GetTargetAtIndex(tidx)->GetProcessSP();
-  if (process->GetID() == pid)
-return process->Destroy(true);
-}
-  }
-
   if (!IsHost()) {
 return Status(
-"base lldb_private::Platform class can't kill remote processes unless "
-"they are controlled by a process plugin");
+"base lldb_private::Platform class can't kill remote processes");
   }
-  Host::Kill(pid, SIGTERM);
+  Host::Kill(pid, SIGKILL);
   return Status();
 }
 

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/Makefile 
b/lldb/test/API/functionalities/gdb_remote_client/Makefile
new file mode 100644
index 0..22f1051530f87
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/Makefile
@@ -0,0 +1 @@
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
new file mode 100644
index 0..2ae9f8ac6f5f2
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
@@ -0,0 +1,47 @@
+import lldb
+import time
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+class TestPlatformKill(GDBRemoteTestBase):
+
+@skipIfRemote
+def test_kill_
diff erent_platform(self):
+"""Test connecting to a remote linux platform"""
+
+self.build(dictionary={"CXX_SOURCES":"sleep.cpp"})
+host_process = self.spawnSubprocess(self.getBuildArtifact())
+
+# Create a fake remote process with the same PID as host_process
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.got_kill = False
+
+def qC(self):
+return "QC%x"%host_process.pid
+
+def k(self):
+self.got_kill = True
+   

[Lldb-commits] [PATCH] D113184: [lldb] Fix cross-platform kills

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa40929dcd295: [lldb] Fix cross-platform kills (authored by 
labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113184/new/

https://reviews.llvm.org/D113184

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Target/Platform.cpp
  lldb/test/API/functionalities/gdb_remote_client/Makefile
  lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/test/API/functionalities/gdb_remote_client/sleep.cpp

Index: lldb/test/API/functionalities/gdb_remote_client/sleep.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/sleep.cpp
@@ -0,0 +1,6 @@
+#include 
+
+int main() {
+  std::this_thread::sleep_for(std::chrono::minutes(1));
+  return 0;
+}
Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -203,6 +203,8 @@
 if packet.startswith("qRegisterInfo"):
 regnum = int(packet[len("qRegisterInfo"):], 16)
 return self.qRegisterInfo(regnum)
+if packet == "k":
+return self.k()
 
 return self.other(packet)
 
@@ -331,6 +333,9 @@
 def qRegisterInfo(self, num):
 return ""
 
+def k(self):
+return ""
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable for the test at
Index: lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
@@ -0,0 +1,47 @@
+import lldb
+import time
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+class TestPlatformKill(GDBRemoteTestBase):
+
+@skipIfRemote
+def test_kill_different_platform(self):
+"""Test connecting to a remote linux platform"""
+
+self.build(dictionary={"CXX_SOURCES":"sleep.cpp"})
+host_process = self.spawnSubprocess(self.getBuildArtifact())
+
+# Create a fake remote process with the same PID as host_process
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.got_kill = False
+
+def qC(self):
+return "QC%x"%host_process.pid
+
+def k(self):
+self.got_kill = True
+return "X09"
+
+self.server.responder = MyResponder()
+
+error = lldb.SBError()
+target = self.dbg.CreateTarget("", "x86_64-pc-linux", "remote-linux",
+False, error)
+self.assertSuccess(error)
+process = self.connect(target)
+self.assertEqual(process.GetProcessID(), host_process.pid)
+
+host_platform = lldb.SBPlatform("host")
+self.assertSuccess(host_platform.Kill(host_process.pid))
+
+# Host dies, remote process lives.
+self.assertFalse(self.server.responder.got_kill)
+self.assertIsNotNone(host_process.wait(timeout=10))
+
+# Now kill the remote one as well
+self.assertSuccess(process.Kill())
+self.assertTrue(self.server.responder.got_kill)
Index: lldb/test/API/functionalities/gdb_remote_client/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1044,25 +1044,11 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "Platform::%s, pid %" PRIu64, __FUNCTION__, pid);
 
-  // Try to find a process plugin to handle this Kill request.  If we can't,
-  // fall back to the default OS implementation.
-  size_t num_debuggers = Debugger::GetNumDebuggers();
-  for (size_t didx = 0; didx < num_debuggers; ++didx) {
-DebuggerSP debugger = Debugger::GetDebuggerAtIndex(didx);
-lldb_private::TargetList &targets = debugger->GetTargetList();
-for (int tidx = 0; tidx < targets.GetNumTargets(); ++tidx) {
-  ProcessSP process = targets.GetTargetAtIndex(tidx)->GetProcessSP();
-  if (process->GetID() == pid)
-return process->Destroy(true);
-}
-  }
-
   if (!IsHost()) {
 return Status(
-"base lldb_private::Platform class can't kill remote processes unless 

[Lldb-commits] [PATCH] D95710: [lldb/Commands] Add command options for ScriptedProcess to ProcessLaunch

2021-11-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D95710#3118346 , @labath wrote:

> Sorry for grave digging, but my compiler just became smart enough to start 
> complaining about this.

Haha, not at all! This wasn't called when I implemented it, but I stumbled into 
it in following patches and fixed it in D112109 
.

The fix hasn't landed yet because I'm still trying to understand why would the 
python-initialized `SBStructuredData` be pointing the corrupted memory when 
trying accessing it in C++, instead of deserializing it into a std::string, and 
serializing it back, which is more a workaround that an actual fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95710/new/

https://reviews.llvm.org/D95710

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113487: [lldb] Refactor Platform::ResolveExecutable

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, jingham, clayborg, PatriosTheGreat.
labath requested review of this revision.
Herald added a project: LLDB.

Module resolution is probably the most complex piece of lldb [citation
needed], with numerous levels of abstraction, each one implementing
various retry and fallback strategies.

It is also a very repetitive, with only small differences between
"host", "remote-and-connected" and "remote-but-not-(yet)-connected"
scenarios.

The goal of this patch (first in series) is to reduce the number of
abstractions, and deduplicate the code.

One of the reasons for this complexity is the tension between the desire
to offload the process of module resolution to the remote platform
instance (that's how most other platform methods work), and the desire
to keep it local to the outer platform class (its easier to subclass the
outer class, and it generally makes more sense).

This patch resolves that conflict in favour of doing everything in the
outer class. The gdb-remote (our only remote platform) implementation of
ResolveExecutable was not doing anything gdb-specific, and was rather
similar to the other implementations of that method (any divergence is
most likely the result of fixes not being applied everywhere rather than
intentional).

It does this by excising the remote platform out of the resolution
codepath. The gdb-remote implementation of ResolveExecutable is moved to
Platform::ResolveRemoteExecutable, and the (only) call site is
redirected to that. On its own, this does not achieve (much), but it
creates new opportunities for layer peeling and code sharing, since all
of the code now lives closer together.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113487

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Index: lldb/unittests/Target/RemoteAwarePlatformTest.cpp
===
--- lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -31,6 +31,17 @@
ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
   MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
 
+  MOCK_METHOD2(ResolveRemoteExecutable,
+   std::pair(const ModuleSpec &,
+   const FileSpecList *));
+  Status ResolveRemoteExecutable(
+  const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
+  const FileSpecList *module_search_paths_ptr) /*override*/ {
+auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
+exe_module_sp = pair.second;
+return pair.first;
+  }
+
   void SetRemotePlatform(lldb::PlatformSP platform) {
 m_remote_platform_sp = platform;
   }
@@ -47,18 +58,6 @@
ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
   MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
   MOCK_METHOD0(GetUserIDResolver, UserIDResolver &());
-
-  MOCK_METHOD2(ResolveExecutable,
-   std::pair(const ModuleSpec &,
-   const FileSpecList *));
-  Status
-  ResolveExecutable(const ModuleSpec &module_spec,
-lldb::ModuleSP &exe_module_sp,
-const FileSpecList *module_search_paths_ptr) /*override*/ {
-auto pair = ResolveExecutable(module_spec, module_search_paths_ptr);
-exe_module_sp = pair.second;
-return pair.first;
-  }
 };
 
 namespace {
@@ -73,15 +72,13 @@
   ModuleSpec executable_spec;
   ModuleSP expected_executable(new Module(executable_spec));
 
-  auto platform_sp = std::make_shared(false);
-  EXPECT_CALL(*platform_sp, ResolveExecutable(_, _))
-  .WillRepeatedly(Return(std::make_pair(Status(), expected_executable)));
-
   RemoteAwarePlatformTester platform(false);
   EXPECT_CALL(platform, GetSupportedArchitectureAtIndex(_, _))
   .WillRepeatedly(Return(false));
+  EXPECT_CALL(platform, ResolveRemoteExecutable(_, _))
+  .WillRepeatedly(Return(std::make_pair(Status(), expected_executable)));
 
-  platform.SetRemotePlatform(platform_sp);
+  platform.SetRemotePlatform(std::make_shared(false));
 
   ModuleSP resolved_sp;
   lldb_private::Status status =
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -72,8 +72,7 @@
   } else {
 if (m_remote_platform_sp) {
   return GetCachedExecutable(resolved_module_spec, exe_module_sp,
- module_search_paths_ptr,
- *m_remote_platform_sp);
+   

[Lldb-commits] [PATCH] D113487: [lldb] Refactor Platform::ResolveExecutable

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Target/Platform.cpp:865
+
+Status
+Platform::ResolveRemoteExecutable(const ModuleSpec &module_spec,

Next step is to merge this with the `if (!m_remote_platform_sp)` path in 
RemoteAwarePlatform::ResolveExecutable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113487/new/

https://reviews.llvm.org/D113487

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

2021-11-09 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat added a comment.

Thanks for review!

Could you or someone else take this commit to master?
I don't have a commit permissions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104413/new/

https://reviews.llvm.org/D104413

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Walter via lldb-commits
Cool. I hope that vscode catches up.

Do you have push permissions or should I land this for you?

Il Mar 9 Nov 2021, 2:16 AM Andy Yankovsky via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> werat added a comment.
>
> In D113400#3117500 , @wallace
> wrote:
>
> > could you share a screenshot of how this looks like?
>
> Sure! Here are the screenshots of before/after in Visual Studio 2022.
>
> Before everything is in one place (in Locals Window):
> F20179455: visual-studio-before.png 
> After, Registers Window (marked in red -- changed since previous step):
> F20179456: registers-after.png 
> After, Locals Window (locals are displayed directly, without "Locals"
> expand block):
> F20179454: locals-after.png 
>
> ---
>
> There are no visible changes in Visual Studio Code.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D113400/new/
>
> https://reviews.llvm.org/D113400
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added a comment.

Thanks! I have push permission, will land it myself in a bit


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113400/new/

https://reviews.llvm.org/D113400

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] dc8f003 - [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Andy Yankovsky via lldb-commits

Author: Andy Yankovsky
Date: 2021-11-09T17:50:46+01:00
New Revision: dc8f0035ca990fc5587bd508fed609f995e7c842

URL: 
https://github.com/llvm/llvm-project/commit/dc8f0035ca990fc5587bd508fed609f995e7c842
DIFF: 
https://github.com/llvm/llvm-project/commit/dc8f0035ca990fc5587bd508fed609f995e7c842.diff

LOG: [lldb-vscode] Add presentation hints for scopes

Scopes can have an optional hint for how to present this scope in the UI:
https://microsoft.github.io/debug-adapter-protocol/specification#Types_Scope

The IDEs can use the hint to present the data accordingly. For example,
Visual Studio has a separate Registers window, which is populated with the
data from the scope with `presentationHint: "registers"`.

Reviewed By: wallace

Differential Revision: https://reviews.llvm.org/D113400

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/tools/lldb-vscode/JSONUtils.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py 
b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index 30a85ec5eb28e..9a4a5de40e718 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -394,3 +394,17 @@ def test_scopes_and_evaluate_expansion(self):
 self.verify_variables(
 expandable_expression["children"], response["body"]["variables"]
 )
+
+# Test that frame scopes have corresponding presentation hints.
+frame_id = self.vscode.get_stackFrame()["id"]
+scopes = self.vscode.request_scopes(frame_id)["body"]["scopes"]
+
+scope_names = [scope["name"] for scope in scopes]
+self.assertIn("Locals", scope_names)
+self.assertIn("Registers", scope_names)
+
+for scope in scopes:
+if scope["name"] == "Locals":
+self.assertEquals(scope.get("presentationHint"), "locals")
+if scope["name"] == "Registers":
+self.assertEquals(scope.get("presentationHint"), "registers")

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 0e6227f3b15d4..58a08796e738d 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -175,6 +175,13 @@ void FillResponse(const llvm::json::Object &request,
 //   "type": "string",
 //   "description": "Name of the scope such as 'Arguments', 'Locals'."
 // },
+// "presentationHint": {
+//   "type": "string",
+//   "description": "An optional hint for how to present this scope in the
+//   UI. If this attribute is missing, the scope is shown
+//   with a generic UI.",
+//   "_enum": [ "arguments", "locals", "registers" ],
+// },
 // "variablesReference": {
 //   "type": "integer",
 //   "description": "The variables of this scope can be retrieved by
@@ -229,6 +236,15 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
   int64_t namedVariables, bool expensive) {
   llvm::json::Object object;
   EmplaceSafeString(object, "name", name.str());
+
+  // TODO: Support "arguments" scope. At the moment lldb-vscode includes the
+  // arguments into the "locals" scope.
+  if (variablesReference == VARREF_LOCALS) {
+object.try_emplace("presentationHint", "locals");
+  } else if (variablesReference == VARREF_REGS) {
+object.try_emplace("presentationHint", "registers");
+  }
+
   object.try_emplace("variablesReference", variablesReference);
   object.try_emplace("expensive", expensive);
   object.try_emplace("namedVariables", namedVariables);



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Andy Yankovsky via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc8f0035ca99: [lldb-vscode] Add presentation hints for 
scopes (authored by werat).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113400/new/

https://reviews.llvm.org/D113400

Files:
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -175,6 +175,13 @@
 //   "type": "string",
 //   "description": "Name of the scope such as 'Arguments', 'Locals'."
 // },
+// "presentationHint": {
+//   "type": "string",
+//   "description": "An optional hint for how to present this scope in the
+//   UI. If this attribute is missing, the scope is shown
+//   with a generic UI.",
+//   "_enum": [ "arguments", "locals", "registers" ],
+// },
 // "variablesReference": {
 //   "type": "integer",
 //   "description": "The variables of this scope can be retrieved by
@@ -229,6 +236,15 @@
   int64_t namedVariables, bool expensive) {
   llvm::json::Object object;
   EmplaceSafeString(object, "name", name.str());
+
+  // TODO: Support "arguments" scope. At the moment lldb-vscode includes the
+  // arguments into the "locals" scope.
+  if (variablesReference == VARREF_LOCALS) {
+object.try_emplace("presentationHint", "locals");
+  } else if (variablesReference == VARREF_REGS) {
+object.try_emplace("presentationHint", "registers");
+  }
+
   object.try_emplace("variablesReference", variablesReference);
   object.try_emplace("expensive", expensive);
   object.try_emplace("namedVariables", namedVariables);
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -394,3 +394,17 @@
 self.verify_variables(
 expandable_expression["children"], response["body"]["variables"]
 )
+
+# Test that frame scopes have corresponding presentation hints.
+frame_id = self.vscode.get_stackFrame()["id"]
+scopes = self.vscode.request_scopes(frame_id)["body"]["scopes"]
+
+scope_names = [scope["name"] for scope in scopes]
+self.assertIn("Locals", scope_names)
+self.assertIn("Registers", scope_names)
+
+for scope in scopes:
+if scope["name"] == "Locals":
+self.assertEquals(scope.get("presentationHint"), "locals")
+if scope["name"] == "Registers":
+self.assertEquals(scope.get("presentationHint"), "registers")


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -175,6 +175,13 @@
 //   "type": "string",
 //   "description": "Name of the scope such as 'Arguments', 'Locals'."
 // },
+// "presentationHint": {
+//   "type": "string",
+//   "description": "An optional hint for how to present this scope in the
+//   UI. If this attribute is missing, the scope is shown
+//   with a generic UI.",
+//   "_enum": [ "arguments", "locals", "registers" ],
+// },
 // "variablesReference": {
 //   "type": "integer",
 //   "description": "The variables of this scope can be retrieved by
@@ -229,6 +236,15 @@
   int64_t namedVariables, bool expensive) {
   llvm::json::Object object;
   EmplaceSafeString(object, "name", name.str());
+
+  // TODO: Support "arguments" scope. At the moment lldb-vscode includes the
+  // arguments into the "locals" scope.
+  if (variablesReference == VARREF_LOCALS) {
+object.try_emplace("presentationHint", "locals");
+  } else if (variablesReference == VARREF_REGS) {
+object.try_emplace("presentationHint", "registers");
+  }
+
   object.try_emplace("variablesReference", variablesReference);
   object.try_emplace("expensive", expensive);
   object.try_emplace("namedVariables", namedVariables);
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -394,3 +394,17 @@
 self.verify_variables(
 expandable_expression["children"], response["body"]["variables"]
 )
+
+# Test that frame scopes have corresponding presentation hints.
+frame_id = self.vscode.get_stackFrame(

[Lldb-commits] [PATCH] D109101: [lldb] Add an option to specify a VFS overlay

2021-11-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere abandoned this revision.
JDevlieghere added a comment.

Looks like there's an easier way to test this, so no need to add another SB API.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109101/new/

https://reviews.llvm.org/D109101

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] c9881c7 - Support looking up absolute symbols

2021-11-09 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-11-09T09:44:37-08:00
New Revision: c9881c7d99c6e4073ed8de11cd3450ef23bd66fc

URL: 
https://github.com/llvm/llvm-project/commit/c9881c7d99c6e4073ed8de11cd3450ef23bd66fc
DIFF: 
https://github.com/llvm/llvm-project/commit/c9881c7d99c6e4073ed8de11cd3450ef23bd66fc.diff

LOG: Support looking up absolute symbols

The Swift stdlib uses absolute symbols in the dylib to communicate
feature flags to the process. LLDB's expression evaluator needs to be
able to find them. This wires up absolute symbols so they show up in
the symtab lookup command, which is also all that's needed for them to
be visible to the expression evaluator JIT.

rdar://85093828

Differential Revision: https://reviews.llvm.org/D113445

Added: 
lldb/test/Shell/SymbolFile/absolute-symbol.s

Modified: 
lldb/source/Symbol/Symbol.cpp
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp
index 251f9104ad54d..a8c81ee3082f2 100644
--- a/lldb/source/Symbol/Symbol.cpp
+++ b/lldb/source/Symbol/Symbol.cpp
@@ -115,7 +115,8 @@ void Symbol::Clear() {
 }
 
 bool Symbol::ValueIsAddress() const {
-  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr;
+  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr ||
+ m_type == eSymbolTypeAbsolute;
 }
 
 ConstString Symbol::GetDisplayName() const {

diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 61003981f18ef..69887034a9fb0 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -1100,6 +1100,7 @@ void Symtab::FindFunctionSymbols(ConstString name, 
uint32_t name_type_mask,
   case eSymbolTypeCode:
   case eSymbolTypeResolver:
   case eSymbolTypeReExported:
+  case eSymbolTypeAbsolute:
 symbol_indexes.push_back(temp_symbol_indexes[i]);
 break;
   default:

diff  --git a/lldb/test/Shell/SymbolFile/absolute-symbol.s 
b/lldb/test/Shell/SymbolFile/absolute-symbol.s
new file mode 100644
index 0..912703fd38283
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/absolute-symbol.s
@@ -0,0 +1,7 @@
+# RUN: %clang %s -g -c -o %t.o
+# RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
+# CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Address: 0x12345678 (0x12345678)
+# CHECK:   Summary: 0x12345678
+.globl absolute_symbol
+absolute_symbol = 0x12345678



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113445: Support looking up absolute symbols

2021-11-09 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9881c7d99c6: Support looking up absolute symbols (authored 
by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113445/new/

https://reviews.llvm.org/D113445

Files:
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/test/Shell/SymbolFile/absolute-symbol.s


Index: lldb/test/Shell/SymbolFile/absolute-symbol.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/absolute-symbol.s
@@ -0,0 +1,7 @@
+# RUN: %clang %s -g -c -o %t.o
+# RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
+# CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Address: 0x12345678 (0x12345678)
+# CHECK:   Summary: 0x12345678
+.globl absolute_symbol
+absolute_symbol = 0x12345678
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -1100,6 +1100,7 @@
   case eSymbolTypeCode:
   case eSymbolTypeResolver:
   case eSymbolTypeReExported:
+  case eSymbolTypeAbsolute:
 symbol_indexes.push_back(temp_symbol_indexes[i]);
 break;
   default:
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -115,7 +115,8 @@
 }
 
 bool Symbol::ValueIsAddress() const {
-  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr;
+  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr ||
+ m_type == eSymbolTypeAbsolute;
 }
 
 ConstString Symbol::GetDisplayName() const {


Index: lldb/test/Shell/SymbolFile/absolute-symbol.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/absolute-symbol.s
@@ -0,0 +1,7 @@
+# RUN: %clang %s -g -c -o %t.o
+# RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck %s
+# CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Address: 0x12345678 (0x12345678)
+# CHECK:   Summary: 0x12345678
+.globl absolute_symbol
+absolute_symbol = 0x12345678
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -1100,6 +1100,7 @@
   case eSymbolTypeCode:
   case eSymbolTypeResolver:
   case eSymbolTypeReExported:
+  case eSymbolTypeAbsolute:
 symbol_indexes.push_back(temp_symbol_indexes[i]);
 break;
   default:
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -115,7 +115,8 @@
 }
 
 bool Symbol::ValueIsAddress() const {
-  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr;
+  return m_addr_range.GetBaseAddress().GetSection().get() != nullptr ||
+ m_type == eSymbolTypeAbsolute;
 }
 
 ConstString Symbol::GetDisplayName() const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 56f7da6 - Add a requires line to test.

2021-11-09 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-11-09T10:16:03-08:00
New Revision: 56f7da6e0d29139d7684b2dc08901fefb64e4fa1

URL: 
https://github.com/llvm/llvm-project/commit/56f7da6e0d29139d7684b2dc08901fefb64e4fa1
DIFF: 
https://github.com/llvm/llvm-project/commit/56f7da6e0d29139d7684b2dc08901fefb64e4fa1.diff

LOG: Add a requires line to test.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/absolute-symbol.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/absolute-symbol.s 
b/lldb/test/Shell/SymbolFile/absolute-symbol.s
index 912703fd3828..08e7eeb818f7 100644
--- a/lldb/test/Shell/SymbolFile/absolute-symbol.s
+++ b/lldb/test/Shell/SymbolFile/absolute-symbol.s
@@ -1,4 +1,5 @@
-# RUN: %clang %s -g -c -o %t.o
+# REQUIRES: system-darwin
+# RUN: %clang %s -c -o %t.o
 # RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
 # CHECK: 1 symbols match 'absolute_symbol'
 # CHECK:   Address: 0x12345678 (0x12345678)



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

2021-11-09 Thread Andy Yankovsky via Phabricator via lldb-commits
werat created this revision.
werat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Operands to `getelementptr` can be constants or constant expressions. Check
that all operands can be constant-resolved and resolve them during the
evaluation. If some operands can't be resolved as constants -- the expression
evaluation will fallback to JIT.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=52449


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113498

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
  lldb/test/API/lang/cpp/static_members/main.cpp

Index: lldb/test/API/lang/cpp/static_members/main.cpp
===
--- lldb/test/API/lang/cpp/static_members/main.cpp
+++ lldb/test/API/lang/cpp/static_members/main.cpp
@@ -11,6 +11,14 @@
 long A::s_b = 2;
 int A::s_c = 3;
 
+// class Foo
+// {
+// public:
+// static int Bar;
+// };
+
+// int Foo::Bar = 10;
+
 int main() {
   A my_a;
   my_a.m_a = 1;
Index: lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
===
--- lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -41,3 +41,31 @@
 self.createTestTarget()
 self.expect("expression s_c", error=True,
 startstr="error: use of undeclared identifier 's_d'")
+
+def test_no_crash_in_IR_arithmetic(self):
+"""
+Test that LLDB doesn't crash on evaluating specific expression involving
+pointer arithmetic and taking the address of a static class member.
+See https://bugs.llvm.org/show_bug.cgi?id=52449
+"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// stop in main", lldb.SBFileSpec("main.cpp"))
+
+# This expression contains the following IR code:
+# ... i64 ptrtoint (i32* @_ZN1A3s_cE to i64)) ...
+expr = "(int*)100 + (long long)(&A::s_c)"
+
+# The IR interpreter doesn't support non-const operands to the
+# `GetElementPtr` IR instruction, so verify that it correctly fails to
+# evaluate expression.
+opts = lldb.SBExpressionOptions()
+opts.SetAllowJIT(False)
+value = self.target().EvaluateExpression(expr, opts)
+self.assertTrue(value.GetError().Fail())
+self.assertIn(
+"Can't evaluate the expression without a running target",
+value.GetError().GetCString())
+
+# Evaluating the expression via JIT should work fine.
+value = self.target().EvaluateExpression(expr)
+self.assertTrue(value.GetError().Success())
Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -283,11 +283,30 @@
 return true; // no offset to apply!
 
   SmallVector indices(op_cursor, op_end);
+  SmallVector const_indices;
+
+  for (Value *idx : indices) {
+Constant *constant_idx = dyn_cast(idx);
+if (!constant_idx)
+  return false;
+
+ConstantInt *constant_int = dyn_cast(constant_idx);
+if (!constant_int) {
+  APInt v;
+  if (!ResolveConstantValue(v, constant_idx))
+return false;
+
+  constant_int =
+  cast(ConstantInt::get(idx->getType(), v));
+}
+
+const_indices.push_back(constant_int);
+  }
 
   Type *src_elem_ty =
   cast(constant_expr)->getSourceElementType();
   uint64_t offset =
-  m_target_data.getIndexedOffsetInType(src_elem_ty, indices);
+  m_target_data.getIndexedOffsetInType(src_elem_ty, const_indices);
 
   const bool is_signed = true;
   value += APInt(value.getBitWidth(), offset, is_signed);
@@ -465,12 +484,18 @@
   case Instruction::BitCast:
 return CanResolveConstant(constant_expr->getOperand(0));
   case Instruction::GetElementPtr: {
-ConstantExpr::const_op_iterator op_cursor = constant_expr->op_begin();
-Constant *base = dyn_cast(*op_cursor);
-if (!base)
-  return false;
-
-return CanResolveConstant(base);
+// Check that all operands of `getelementptr` can be constant-resolved.
+SmallVector operands(constant_expr->op_begin(),
+ constant_expr->op_end());
+for (Value *op : operands) {
+  Constant *constant_op = dyn_cast(op);
+  if (!constant_op)
+return false;
+  if (!CanResolveConstant(constant_op)) {
+return false;
+  }
+}
+return true;
   }
   }
 } 

[Lldb-commits] [PATCH] D113498: [lldb] Constant-resolve operands to `getelementptr`

2021-11-09 Thread Andy Yankovsky via Phabricator via lldb-commits
werat updated this revision to Diff 385892.
werat added a comment.
Herald added a subscriber: JDevlieghere.

Remove accidental code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113498/new/

https://reviews.llvm.org/D113498

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py


Index: lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
===
--- lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -41,3 +41,31 @@
 self.createTestTarget()
 self.expect("expression s_c", error=True,
 startstr="error: use of undeclared identifier 's_d'")
+
+def test_no_crash_in_IR_arithmetic(self):
+"""
+Test that LLDB doesn't crash on evaluating specific expression 
involving
+pointer arithmetic and taking the address of a static class member.
+See https://bugs.llvm.org/show_bug.cgi?id=52449
+"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// stop in main", 
lldb.SBFileSpec("main.cpp"))
+
+# This expression contains the following IR code:
+# ... i64 ptrtoint (i32* @_ZN1A3s_cE to i64)) ...
+expr = "(int*)100 + (long long)(&A::s_c)"
+
+# The IR interpreter doesn't support non-const operands to the
+# `GetElementPtr` IR instruction, so verify that it correctly fails to
+# evaluate expression.
+opts = lldb.SBExpressionOptions()
+opts.SetAllowJIT(False)
+value = self.target().EvaluateExpression(expr, opts)
+self.assertTrue(value.GetError().Fail())
+self.assertIn(
+"Can't evaluate the expression without a running target",
+value.GetError().GetCString())
+
+# Evaluating the expression via JIT should work fine.
+value = self.target().EvaluateExpression(expr)
+self.assertTrue(value.GetError().Success())
Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -283,11 +283,30 @@
 return true; // no offset to apply!
 
   SmallVector indices(op_cursor, op_end);
+  SmallVector const_indices;
+
+  for (Value *idx : indices) {
+Constant *constant_idx = dyn_cast(idx);
+if (!constant_idx)
+  return false;
+
+ConstantInt *constant_int = dyn_cast(constant_idx);
+if (!constant_int) {
+  APInt v;
+  if (!ResolveConstantValue(v, constant_idx))
+return false;
+
+  constant_int =
+  cast(ConstantInt::get(idx->getType(), v));
+}
+
+const_indices.push_back(constant_int);
+  }
 
   Type *src_elem_ty =
   cast(constant_expr)->getSourceElementType();
   uint64_t offset =
-  m_target_data.getIndexedOffsetInType(src_elem_ty, indices);
+  m_target_data.getIndexedOffsetInType(src_elem_ty, const_indices);
 
   const bool is_signed = true;
   value += APInt(value.getBitWidth(), offset, is_signed);
@@ -465,12 +484,18 @@
   case Instruction::BitCast:
 return CanResolveConstant(constant_expr->getOperand(0));
   case Instruction::GetElementPtr: {
-ConstantExpr::const_op_iterator op_cursor = constant_expr->op_begin();
-Constant *base = dyn_cast(*op_cursor);
-if (!base)
-  return false;
-
-return CanResolveConstant(base);
+// Check that all operands of `getelementptr` can be constant-resolved.
+SmallVector operands(constant_expr->op_begin(),
+ constant_expr->op_end());
+for (Value *op : operands) {
+  Constant *constant_op = dyn_cast(op);
+  if (!constant_op)
+return false;
+  if (!CanResolveConstant(constant_op)) {
+return false;
+  }
+}
+return true;
   }
   }
 } else {


Index: lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
===
--- lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -41,3 +41,31 @@
 self.createTestTarget()
 self.expect("expression s_c", error=True,
 startstr="error: use of undeclared identifier 's_d'")
+
+def test_no_crash_in_IR_arithmetic(self):
+"""
+Test that LLDB doesn't crash on evaluating specific expression involving
+pointer arithmetic and taking the address of a static class member.
+See https://bugs.llvm.org

[Lldb-commits] [lldb] 68a4d17 - Use yaml2obj instead of relying on invoking the Darwin system assembler.

2021-11-09 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-11-09T10:47:26-08:00
New Revision: 68a4d179c2ac4c882f2d242b81748ceed66827ff

URL: 
https://github.com/llvm/llvm-project/commit/68a4d179c2ac4c882f2d242b81748ceed66827ff
DIFF: 
https://github.com/llvm/llvm-project/commit/68a4d179c2ac4c882f2d242b81748ceed66827ff.diff

LOG: Use yaml2obj instead of relying on invoking the Darwin system assembler.

Added: 
lldb/test/Shell/SymbolFile/absolute-symbol.test

Modified: 


Removed: 
lldb/test/Shell/SymbolFile/absolute-symbol.s



diff  --git a/lldb/test/Shell/SymbolFile/absolute-symbol.s 
b/lldb/test/Shell/SymbolFile/absolute-symbol.s
deleted file mode 100644
index 08e7eeb818f7..
--- a/lldb/test/Shell/SymbolFile/absolute-symbol.s
+++ /dev/null
@@ -1,8 +0,0 @@
-# REQUIRES: system-darwin
-# RUN: %clang %s -c -o %t.o
-# RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
-# CHECK: 1 symbols match 'absolute_symbol'
-# CHECK:   Address: 0x12345678 (0x12345678)
-# CHECK:   Summary: 0x12345678
-.globl absolute_symbol
-absolute_symbol = 0x12345678

diff  --git a/lldb/test/Shell/SymbolFile/absolute-symbol.test 
b/lldb/test/Shell/SymbolFile/absolute-symbol.test
new file mode 100644
index ..1d234cb55e05
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/absolute-symbol.test
@@ -0,0 +1,95 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
+# CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Address: 0x12345678 (0x12345678)
+# CHECK:   Summary: 0x12345678
+# Created from:
+#   .globl absolute_symbol
+#   absolute_symbol = 0x12345678
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0x1
+  ncmds:   4
+  sizeofcmds:  280
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 152
+segname: ''
+vmaddr:  0
+vmsize:  0
+fileoff: 312
+filesize:0
+maxprot: 7
+initprot:7
+nsects:  1
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x0
+size:0
+offset:  0x138
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x8000
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: ''
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 0
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  312
+nsyms:   2
+stroff:  344
+strsize: 24
+  - cmd: LC_DYSYMTAB
+cmdsize: 80
+ilocalsym:   0
+nlocalsym:   1
+iextdefsym:  1
+nextdefsym:  1
+iundefsym:   2
+nundefsym:   0
+tocoff:  0
+ntoc:0
+modtaboff:   0
+nmodtab: 0
+extrefsymoff:0
+nextrefsyms: 0
+indirectsymoff:  0
+nindirectsyms:   0
+extreloff:   0
+nextrel: 0
+locreloff:   0
+nlocrel: 0
+LinkEditData:
+  NameList:
+- n_strx:  17
+  n_type:  0xE
+  n_sect:  1
+  n_desc:  0
+  n_value: 0
+- n_strx:  1
+  n_type:  0x3
+  n_sect:  0
+  n_desc:  0
+  n_value: 305419896
+  StringTable:
+- ''
+- absolute_symbol
+- ltmp0
+- ''
+...
+



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] adc7d63 - [lldb] XFAIL TestPlatformKill on windows

2021-11-09 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-11-09T19:50:14+01:00
New Revision: adc7d63f46a70e68c60ecd4ad17f09eb6f4abbca

URL: 
https://github.com/llvm/llvm-project/commit/adc7d63f46a70e68c60ecd4ad17f09eb6f4abbca
DIFF: 
https://github.com/llvm/llvm-project/commit/adc7d63f46a70e68c60ecd4ad17f09eb6f4abbca.diff

LOG: [lldb] XFAIL TestPlatformKill on windows

-> PR52451

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
index 2ae9f8ac6f5f..2e15c8a27260 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py
@@ -7,6 +7,7 @@
 class TestPlatformKill(GDBRemoteTestBase):
 
 @skipIfRemote
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr52451")
 def test_kill_
diff erent_platform(self):
 """Test connecting to a remote linux platform"""
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113163: [LLDB][Breakpad] Create a function for each compilation unit.

2021-11-09 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 385896.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113163/new/

https://reviews.llvm.org/D113163

Files:
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/test/Shell/Minidump/Inputs/linux-x86_64.syms
  lldb/test/Shell/Minidump/breakpad-symbols.test
  lldb/test/Shell/SymbolFile/Breakpad/line-table.test
  lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Index: lldb/test/Shell/SymbolFile/Breakpad/symtab.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/symtab.test
+++ lldb/test/Shell/SymbolFile/Breakpad/symtab.test
@@ -3,17 +3,17 @@
 # RUN:   -s %s | FileCheck %s
 
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
-# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 5:
+# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 4:
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
 # CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}
-# CHECK: [1]  0   X Code0x004000b00x000c 0x f1_func
-# CHECK: [2]  0   X Code0x004000a00x000d 0x func_only
-# CHECK: [3]  0   X Code0x004000c00x0010 0x f2
-# CHECK: [4]  0   X Code0x004000d00x0022 0x _start
+# CHECK: [1]  0   X Code0x004000b00x0010 0x f1
+# CHECK: [2]  0   X Code0x004000c00x0010 0x f2
+# CHECK: [3]  0   X Code0x004000d00x0022 0x _start
 
 # CHECK-LABEL: (lldb) image lookup -a 0x4000b0 -v
 # CHECK: Address: symtab.out[0x004000b0] (symtab.out.PT_LOAD[0]..text2 + 0)
-# CHECK: Symbol: id = {0x}, range = [0x004000b0-0x004000bc), name="f1_func"
+# CHECK: Function: id = {0x0001}, name = "f1_func", range = [0x004000b0-0x004000bc)
+# CHECK: Symbol: id = {0x}, range = [0x004000b0-0x004000c0), name="f1"
 
 # CHECK-LABEL: (lldb) image lookup -n f2 -v
 # CHECK: Address: symtab.out[0x004000c0] (symtab.out.PT_LOAD[0]..text2 + 16)
Index: lldb/test/Shell/SymbolFile/Breakpad/line-table.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/line-table.test
+++ lldb/test/Shell/SymbolFile/Breakpad/line-table.test
@@ -39,7 +39,16 @@
 image lookup -a 0x4000b2 -v
 # CHECK-LABEL: image lookup -a 0x4000b2 -v
 # CHECK: Summary: line-table.out`func + 2
+# CHECK: Function: id = {0x}, name = "func", range = [0x004000b0-0x004000c0)
+
+image dump symfile
+# CHECK-LABEL: Compile units:
+# CHECK-NEXT:  CompileUnit{0x}, language = "", file = '/tmp/a.c'
+# CHECK-NEXT:   Function{0x}, demangled = func, type_uid = 0x
+# CHECK:  CompileUnit{0x0001}, language = "", file = '/tmp/c.c'
+# CHECK-NEXT:  CompileUnit{0x0002}, language = "", file = '/tmp/d.c'
+# CHECK-NEXT:  CompileUnit{0x0003}, language = "", file = '/tmp/d.c'
 
 breakpoint set -f c.c -l 2
 # CHECK-LABEL: breakpoint set -f c.c -l 2
-# CHECK: Breakpoint 1: where = line-table.out`func + 2, address = 0x004000b2
+# CHECK: Breakpoint 1: where = line-table.out`func + 2 at c.c:2, address = 0x004000b2
Index: lldb/test/Shell/Minidump/breakpad-symbols.test
===
--- lldb/test/Shell/Minidump/breakpad-symbols.test
+++ lldb/test/Shell/Minidump/breakpad-symbols.test
@@ -15,8 +15,8 @@
 image dump symtab /tmp/test/linux-x86_64
 # CHECK-LABEL: image dump symtab /tmp/test/linux-x86_64
 # CHECK: Symtab, file = /tmp/test/linux-x86_64, num_symbols = 2:
-# CHECK: [0]  0   X Code0x004003d0 0x004003d0 0x0018 0x crash()
-# CHECK: [1]  0   X Code0x004003f0 0x004003f0 0x0010 0x _start
+# CHECK: [0]  0   X Code0x004003d0 0x004003d0 0x0020 0x crash()
+# CHECK: [1]  0   X Code0x004003f0 0x004003f0 0x0c10 0x _start
 
 image lookup -a 0x4003f3
 # CHECK-LABEL: image lookup -a 0x4003f3
Index: lldb/test/Shell/Minidump/Inputs/linux-x86_64.syms
===
--- lldb/test/Shell/Minidu

[Lldb-commits] [PATCH] D113163: [LLDB][Breakpad] Create a function for each compilation unit.

2021-11-09 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:337
+  std::lock_guard guard(GetModuleMutex());
+  if (!(name_type_mask & eFunctionNameTypeMethod))
+return;

labath wrote:
> How did you come to pick this? I get that this is not critical functionality 
> for your use case, but it seems rather strange to be claiming that you're 
> looking up methods here. If you don't care what you look up exactly, then 
> maybe you could just skip this check completely, possibly leaving a TODO to 
> implement this in a better way. (The Symtab class has code (heuristics) which 
> tries to classify symbols into functions, methods, etc., but I'm not sure if 
> it actually works with breakpad symbols (them being demangled), nor am I sure 
> that we should replicate that here.)
I just found that name_type_mask always has `eFunctionNameTypeMethod`. 
Removed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113163/new/

https://reviews.llvm.org/D113163

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113330: [LLDB][Breakpad] Make lldb understand INLINE and INLINE_ORIGIN records in breakpad.

2021-11-09 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 385903.
zequanwu marked an inline comment as done.
zequanwu added a comment.

Use empty callsite_file or name if index is out of range.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113330/new/

https://reviews.llvm.org/D113330

Files:
  lldb/include/lldb/Symbol/Block.h
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Symbol/Block.cpp
  lldb/test/Shell/SymbolFile/Breakpad/Inputs/inline-record.syms
  lldb/test/Shell/SymbolFile/Breakpad/inline-record.test
  lldb/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

Index: lldb/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
===
--- lldb/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
+++ lldb/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp
@@ -18,6 +18,8 @@
   EXPECT_EQ(Record::Info, Record::classify("INFO"));
   EXPECT_EQ(Record::File, Record::classify("FILE"));
   EXPECT_EQ(Record::Func, Record::classify("FUNC"));
+  EXPECT_EQ(Record::Inline, Record::classify("INLINE"));
+  EXPECT_EQ(Record::InlineOrigin, Record::classify("INLINE_ORIGIN"));
   EXPECT_EQ(Record::Public, Record::classify("PUBLIC"));
   EXPECT_EQ(Record::StackCFI, Record::classify("STACK CFI"));
   EXPECT_EQ(Record::StackWin, Record::classify("STACK WIN"));
@@ -76,6 +78,27 @@
   EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC"));
 }
 
+TEST(InlineOriginRecord, parse) {
+  EXPECT_EQ(InlineOriginRecord(47, "foo"),
+InlineOriginRecord::parse("INLINE_ORIGIN 47 foo"));
+  EXPECT_EQ(llvm::None, InlineOriginRecord::parse("INLINE_ORIGIN 47"));
+  EXPECT_EQ(llvm::None, InlineOriginRecord::parse("INLINE_ORIGIN"));
+  EXPECT_EQ(llvm::None, InlineOriginRecord::parse(""));
+}
+
+TEST(InlineRecord, parse) {
+  InlineRecord record1 = InlineRecord(0, 1, 2, 3);
+  record1.Ranges.emplace_back(4, 5);
+  EXPECT_EQ(record1, InlineRecord::parse("INLINE 0 1 2 3 4 5"));
+  record1.Ranges.emplace_back(6, 7);
+  EXPECT_EQ(record1, InlineRecord::parse("INLINE 0 1 2 3 4 5 6 7"));
+  EXPECT_EQ(llvm::None, InlineRecord::parse("INLINE 0 1 2 3"));
+  EXPECT_EQ(llvm::None, InlineRecord::parse("INLINE 0 1 2 3 4 5 6"));
+  EXPECT_EQ(llvm::None, InlineRecord::parse("INLNIE 0"));
+  EXPECT_EQ(llvm::None, InlineRecord::parse(""));
+  EXPECT_EQ(llvm::None, InlineRecord::parse("FUNC"));
+}
+
 TEST(LineRecord, parse) {
   EXPECT_EQ(LineRecord(0x47, 0x74, 47, 74), LineRecord::parse("47 74 47 74"));
   EXPECT_EQ(llvm::None, LineRecord::parse("47 74 47"));
Index: lldb/test/Shell/SymbolFile/Breakpad/inline-record.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/Breakpad/inline-record.test
@@ -0,0 +1,29 @@
+# RUN: yaml2obj %S/Inputs/basic-elf.yaml -o %T/inline-record.out
+# RUN: %lldb %T/inline-record.out -o "target symbols add -s inline-record.out %S/Inputs/inline-record.syms" \
+# RUN:   -s %s | FileCheck %s
+
+# CHECK-LABEL: (lldb) image lookup -a 0x400010 -v
+# CHECK:  Function: id = {0x}, name = "f1", range = [0x00400010-0x00400020)
+# CHECK-NEXT:   Blocks: id = {0x}, range = [0x00400010-0x00400020)
+# CHECK-NEXT:   id = {0x0010}, ranges = [0x00400010-0x00400015)[0x00400017-0x0040001b), name = "inlined_f1"
+
+# CHECK-LABEL: (lldb) image lookup -a 0x400016 -v
+# CHECK:  Function: id = {0x}, name = "f1", range = [0x00400010-0x00400020)
+# CHECK-NEXT:   Blocks: id = {0x}, range = [0x00400010-0x00400020)
+
+# CHECK-LABEL: (lldb) image lookup -a 0x400023 -v
+# CHECK:  Function: id = {0x0001}, name = "f2", range = [0x00400020-0x00400030)
+# CHECK-NEXT:   Blocks: id = {0x0001}, range = [0x00400020-0x00400030)
+# CHECK-NEXT:   id = {0x0043}, range = [0x00400023-0x0040002d), name = "inlined_f1"
+# CHECK-NEXT:   id = {0x0057}, range = [0x00400023-0x00400028), name = "inlined_f2"
+
+# CHECK-LABEL: (lldb) image lookup -a 0x400029 -v
+# CHECK:  Function: id = {0x0001}, name = "f2", range = [0x00400020-0x00400030)
+# CHECK-NEXT:   Blocks: id = {0x0001}, range = [0x00400020-0x00400030)
+# CHECK-NEXT:   id = {0x0043}, range = [0x00400023-0x0040002d), name = "inlined_f1"
+
+image lookup -a 0x400010 -v
+image lookup -a 0x400016 -v
+image lookup -a 0x400023 -v
+image lookup -a 0x400029 -v
+exit
Index: lldb/test/Shell/SymbolFile/Breakpad/Inputs/inline-record.syms
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/Breakpad/Inputs/inline-record.syms
@@ -0,0 +1,13 @@
+MODULE Linux x86_64 761550E0808633

[Lldb-commits] [PATCH] D113362: [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread Danil Stefaniuc via Phabricator via lldb-commits
danilashtefan updated this revision to Diff 385904.
danilashtefan added a comment.

Right size is returned and tests are unified! :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113362/new/

https://reviews.llvm.org/D113362

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
+++ /dev/null
@@ -1,7 +0,0 @@
-#include 
-
-int main()
-{
-  std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5};
-  return 0; // break here
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main() {
+  std::forward_list empty{}, one_elt{47},
+  five_elts{1, 22, 333, , 5};
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
@@ -9,8 +9,10 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
-class TestDataFormatterLibcxxForwardList(TestBase):
+class TestDataFormatterGenericForwardList(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -19,10 +21,10 @@
 self.line = line_number('main.cpp', '// break here')
 self.namespace = 'std'
 
-@add_test_categories(["libc++"])
-def test(self):
+
+def do_test(self, stdlib_type):
 """Test that std::forward_list is displayed correctly"""
-self.build()
+self.build(dictionary={stdlib_type: "1"})
 lldbutil.run_to_source_breakpoint(self, '// break here',
 lldb.SBFileSpec("main.cpp", False))
 
@@ -49,3 +51,12 @@
  '[3] = ',
  '[4] = 5',
  '}'])
+
+@add_test_categories(["libstdcxx"])
+def test_libstdcpp(self):
+self.do_test(USE_LIBSTDCPP)
+
+@add_test_categories(["libc++"])
+def test_libcpp(self):
+ self.do_test(USE_LIBCPP)
+
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
@@ -1,4 +1,3 @@
 CXX_SOURCES := main.cpp
 
-USE_LIBCPP := 1
 include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -923,6 +923,11 @@
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+  SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  stl_synth_flags,
+  "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
@@ -953,6 +958,10 @@
   RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
   TypeSummaryImplSP(
   new StringSummaryFormat(s

[Lldb-commits] [PATCH] D113362: [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread walter erquinigo via Phabricator via lldb-commits
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113362/new/

https://reviews.llvm.org/D113362

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112147: [lldb] Fix lookup for global constants in namespaces

2021-11-09 Thread Tonko Sabolčec via Phabricator via lldb-commits
tonkosi added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3281-3282
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit ||
+ parent_tag == DW_TAG_namespace) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(

clayborg wrote:
> I think we should just always call this function in regardless of what the 
> parent variable tag is since what happens if the parent tag is another value 
> decl context like DW_TAG_class_type, DW_TAG_structure_type? 
> 
> The call to GetDWARFDeclContext(die) below will calculate the 
> DWARFDeclContext for a DIE and then construct an appropriate qualified name, 
> so we can just always do this so I would suggest just making this:
> 
> ```
> if (Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
>   mangled = 
> GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();
> ```
> Then we should always get this right all the time. It doesn't actually make 
> sense to call this if the parent is the DW_TAG_compile_unit or 
> DW_TAG_partial_unit because then there is no decl context to add to the 
> variable name.
I tried to call `GetDWARFDeclContext(die)` in a general case, but it seems that 
`GetQualifiedName` will append `::` 
([source](https://github.com/llvm/llvm-project/blob/5015f250894d3d97917d858850fae7960923a4ae/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp#L22))
 in front of the variable name if there's no other declaration context.

As a result, LLDB will display local variables and function arguments as 
`::var`. Any suggestion to deal with that?

I also noticed that with the current change it would be possible to get 
variables in anonymous namespaces, e.g. for the following source

```
namespace ns {
namespace {
const int var = 1;
}
}
```

call to `SBTarget::FindGlobalVariables("ns::(anonymous namespace)::var")` will 
succeed. Is that OK?



Comment at: lldb/test/API/lang/cpp/global_variables/main.cpp:5
+int g_file_global_int = 42;
+const int g_file_global_const_int = 1337;
 }

clayborg wrote:
> might be nice to see if we can create constants inside of a class or struct 
> as well as mentioned in the above inline comment
I couldn't come up with a struct or class example that triggers changes in this 
diff. Members were either `DW_TAG_member` which is not handled by 
`ParseVariableDIE` or they already contained `DW_AT_linkage_name`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112147/new/

https://reviews.llvm.org/D112147

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

2021-11-09 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:762
+static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = src <= std::numeric_limits::max() ? src
+   : fallback;

I am building on a macOS and I seeing the following diagnostic for this line:

```
warning: comparison of integers of different signs: 'int' and 
'std::numeric_limits::type' (aka 'unsigned int') [-Wsign-compare]
  dest = src <= std::numeric_limits::max() ? src
 ~~~ ^  ~~
```

and it is triggered by:

```
fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
```

In this case it will do the right thing but allowing mixed types is 
problematic, usually I see clamp done with homogeneous types. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107840/new/

https://reviews.llvm.org/D107840

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

2021-11-09 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 385917.
mgorny edited the summary of this revision.
mgorny added a comment.

Add fallback to `QThreadSuffixSupported` and a respective test. Improve the 
summary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108078/new/

https://reviews.llvm.org/D108078

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.cpp
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3462,7 +3462,8 @@
   uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
  // size--debugger can always use less
   char buf[256];
-  snprintf(buf, sizeof(buf), "qXfer:features:read+;PacketSize=%x;qEcho+",
+  snprintf(buf, sizeof(buf),
+   "qXfer:features:read+;PacketSize=%x;qEcho+;native-signals+",
max_packet_size);
 
   bool enable_compression = false;
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -412,3 +412,77 @@
 process = self.connect(target)
 process.Detach()
 self.assertRegex(self.server.responder.detached, r"D;0*400")
+
+def test_signal_gdb(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def haltReason(self):
+return "S0a"
+
+def cont(self):
+return self.haltReason()
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+self.assertEqual(process.threads[0].GetStopReason(),
+ lldb.eStopReasonSignal)
+self.assertEqual(process.threads[0].GetStopDescription(100),
+ 'signal SIGBUS')
+
+def test_signal_lldb_old(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qHostInfo(self):
+return "triple:61726d76372d756e6b6e6f776e2d6c696e75782d676e75;"
+
+def QThreadSuffixSupported(self):
+return "OK"
+
+def haltReason(self):
+return "S0a"
+
+def cont(self):
+return self.haltReason()
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+self.assertEqual(process.threads[0].GetStopReason(),
+ lldb.eStopReasonSignal)
+# NB: this may need adjusting per current platform
+self.assertEqual(process.threads[0].GetStopDescription(100),
+ 'signal SIGUSR1')
+
+def test_signal_lldb(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;native-signals+"
+
+def qHostInfo(self):
+return "triple:61726d76372d756e6b6e6f776e2d6c696e75782d676e75;"
+
+def haltReason(self):
+return "S0a"
+
+def cont(self):
+return self.haltReason()
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+self.assertEqual(process.threads[0].GetStopReason(),
+ lldb.eStopReasonSignal)
+# NB: this may need adjusting per current platform
+self.assertEqual(process.threads[0].GetStopDescription(100),
+ 'signal SIGUSR1')
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -607,14 +607,6 @@
 __FUNCTION__, GetID(),
 GetTarget().GetArchitecture().GetTriple().getTriple().c_str());
 
-  if (err

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

2021-11-09 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:762
+static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = src <= std::numeric_limits::max() ? src
+   : fallback;

shafik wrote:
> I am building on a macOS and I seeing the following diagnostic for this line:
> 
> ```
> warning: comparison of integers of different signs: 'int' and 
> 'std::numeric_limits::type' (aka 'unsigned int') 
> [-Wsign-compare]
>   dest = src <= std::numeric_limits::max() ? src
>  ~~~ ^  ~~
> ```
> 
> and it is triggered by:
> 
> ```
> fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
> ```
> 
> In this case it will do the right thing but allowing mixed types is 
> problematic, usually I see clamp done with homogeneous types. 
Could you suggest how to fix it? Some platforms have different signedness than 
the GDB struct type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107840/new/

https://reviews.llvm.org/D107840

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

2021-11-09 Thread Luís Ferreira via Phabricator via lldb-commits
ljmf00 added a comment.

Since D111414  got merged, @teemperor can you 
land this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110578/new/

https://reviews.llvm.org/D110578

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

2021-11-09 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:762
+static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = src <= std::numeric_limits::max() ? src
+   : fallback;

mgorny wrote:
> shafik wrote:
> > I am building on a macOS and I seeing the following diagnostic for this 
> > line:
> > 
> > ```
> > warning: comparison of integers of different signs: 'int' and 
> > 'std::numeric_limits::type' (aka 'unsigned int') 
> > [-Wsign-compare]
> >   dest = src <= std::numeric_limits::max() ? src
> >  ~~~ ^  ~~
> > ```
> > 
> > and it is triggered by:
> > 
> > ```
> > fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
> > ```
> > 
> > In this case it will do the right thing but allowing mixed types is 
> > problematic, usually I see clamp done with homogeneous types. 
> Could you suggest how to fix it? Some platforms have different signedness 
> than the GDB struct type.
Since we don't have C++20 I think you would have to roll your own `cmp_less` 
see the [possible implementation on 
cppreference](https://en.cppreference.com/w/cpp/utility/intcmp).

For reference see [Safe integral comparisons 
proposal](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0586r2.html).

I looked and I don't see something similar in llvm anywhere but I could be 
missing it.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107840/new/

https://reviews.llvm.org/D107840

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

2021-11-09 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

Given that this patch has been in tree for half a year, it'd be good to get 
confirmation here this can be reverted given there is now a test case for 
causing a crash. I got an offline comment that this is OK to revert, so if 
nobody has objections, I'll land sometime tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113449/new/

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

2021-11-09 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113449/new/

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

2021-11-09 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp:21
+};
+B b;

FWIW, I think probably should be an API test (for a bunch of reasons from not 
relying on formatting output to remote device testing), but given this is just 
a revert I won't insist on that. Thanks again!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113449/new/

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113519: [lldb] [gdb-server] Fix fill_clamp to handle signed src types

2021-11-09 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, shafik.
mgorny requested review of this revision.

Fix the fill_clamp() function to handle signed source types.  Make sure
that the source value is always non-negative, and cast it to unsigned
when verifying the upper bound.  This fixes compiler warnings about
comparing unsigned and signed types.


https://reviews.llvm.org/D113519

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -771,8 +771,9 @@
 
 template 
 static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
-  dest = src <= std::numeric_limits::max() ? src
-   : fallback;
+  using UU = typename std::make_unsigned::type;
+  constexpr auto T_max = std::numeric_limits::max();
+  dest = src >= 0 && static_cast(src) <= T_max ? src : fallback;
 }
 
 GDBRemoteCommunication::PacketResult


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -771,8 +771,9 @@
 
 template 
 static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
-  dest = src <= std::numeric_limits::max() ? src
-   : fallback;
+  using UU = typename std::make_unsigned::type;
+  constexpr auto T_max = std::numeric_limits::max();
+  dest = src >= 0 && static_cast(src) <= T_max ? src : fallback;
 }
 
 GDBRemoteCommunication::PacketResult
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112147: [lldb] Fix lookup for global constants in namespaces

2021-11-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3281-3282
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit ||
+ parent_tag == DW_TAG_namespace) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(

tonkosi wrote:
> clayborg wrote:
> > I think we should just always call this function in regardless of what the 
> > parent variable tag is since what happens if the parent tag is another 
> > value decl context like DW_TAG_class_type, DW_TAG_structure_type? 
> > 
> > The call to GetDWARFDeclContext(die) below will calculate the 
> > DWARFDeclContext for a DIE and then construct an appropriate qualified 
> > name, so we can just always do this so I would suggest just making this:
> > 
> > ```
> > if (Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
> >   mangled = 
> > GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();
> > ```
> > Then we should always get this right all the time. It doesn't actually make 
> > sense to call this if the parent is the DW_TAG_compile_unit or 
> > DW_TAG_partial_unit because then there is no decl context to add to the 
> > variable name.
> I tried to call `GetDWARFDeclContext(die)` in a general case, but it seems 
> that `GetQualifiedName` will append `::` 
> ([source](https://github.com/llvm/llvm-project/blob/5015f250894d3d97917d858850fae7960923a4ae/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp#L22))
>  in front of the variable name if there's no other declaration context.
> 
> As a result, LLDB will display local variables and function arguments as 
> `::var`. Any suggestion to deal with that?
> 
> I also noticed that with the current change it would be possible to get 
> variables in anonymous namespaces, e.g. for the following source
> 
> ```
> namespace ns {
> namespace {
> const int var = 1;
> }
> }
> ```
> 
> call to `SBTarget::FindGlobalVariables("ns::(anonymous namespace)::var")` 
> will succeed. Is that OK?
GetQualifiedNameAsConstString could take an extra argument that specifies if we 
should prepend the "::" to the root namespace.

I am fine with a lookup of "ns::(anonymous namespace)::var" succeeding.



Comment at: lldb/test/API/lang/cpp/global_variables/main.cpp:5
+int g_file_global_int = 42;
+const int g_file_global_const_int = 1337;
 }

tonkosi wrote:
> clayborg wrote:
> > might be nice to see if we can create constants inside of a class or struct 
> > as well as mentioned in the above inline comment
> I couldn't come up with a struct or class example that triggers changes in 
> this diff. Members were either `DW_TAG_member` which is not handled by 
> `ParseVariableDIE` or they already contained `DW_AT_linkage_name`.
ok, thanks for trying


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112147/new/

https://reviews.llvm.org/D112147

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113400: [lldb-vscode] Add presentation hints for scopes

2021-11-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113400/new/

https://reviews.llvm.org/D113400

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113521: Allow lldb to launch a remote executable when there isn't a local copy

2021-11-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: clayborg, labath, jasonmolenda, JDevlieghere.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

lldb doesn't need to have a local copy of the binary that it is going to ask 
the remote stub to run for it, it can send the remote stub a path to the 
binary, and then read the executable module out of memory once the binary has 
been launched.

There is a workflow for providing an alternate remote path, but it hangs the 
information off the executable module, which if you don't have a local copy of 
the file you can't make.

So I came up with an alternate workflow for the case where you don't have a 
local executable copy, using LaunchInfo::SetExecutableFile.  If you tell a 
target or connected process to Launch, and the LaunchInfo has an executable 
file but the target doesn't, then send the LaunchInfo's executable file to the 
remote stub, to see if it can launch it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113521

Files:
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestNoLocalFile.py
@@ -0,0 +1,90 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestNoLocalFile(GDBRemoteTestBase):
+""" Test the case where there is NO local copy of the file
+being debugged.  We shouldn't immediately error out, but
+rather lldb should ask debugserver if it knows about the file. """
+
+
+@skipIfXmlSupportMissing
+def test(self):
+self.absent_file = '/nosuch_dir/nosuch_subdir/nosuch_executable'
+
+class MyResponder(MockGDBServerResponder):
+def __init__(self, testcase):
+MockGDBServerResponder.__init__(self)
+self.after_launch = False
+self.testcase = testcase
+self.current_thread = 0
+
+def A(self, packet):
+# This is the main test, we want to see that lldb DID send the
+# A packet to get debugserver to load the file.
+# Skip the length and second length:
+a_arr = packet.split(",")
+ascii = bytearray.fromhex(a_arr[2]).decode()
+self.testcase.assertEqual(ascii, self.testcase.absent_file, "We sent the right path")
+return "OK"
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return """
+
+  i386:x86-64
+  
+
+  
+""", False
+else:
+return None, False
+
+def qC(self):
+if not self.after_launch:
+return "QC0"
+return "0"
+
+def qfThreadInfo(self):
+if not self.after_launch:
+return "OK"
+return "m0"
+
+def qsThreadInfo(self):
+if not self.after_launch:
+return "OK"
+return "l"
+
+def qLaunchSuccess(self):
+return "OK"
+
+def qProcessInfo(self):
+return "$pid:10b70;parent-pid:10b20;real-uid:1f6;real-gid:14;effective-uid:1f6;effective-gid:14;cputype:107;cpusubtype:8;ptrsize:8;ostype:macosx;vendor:apple;endian:little;"
+
+
+
+self.server.responder = MyResponder(self)
+target = self.dbg.CreateTargetWithFileAndArch(None, "x86_64")
+launch_info = target.GetLaunchInfo()
+launch_info.SetExecutableFile(lldb.SBFileSpec(self.absent_file), True)
+flags = launch_info.GetLaunchFlags()
+flags |= lldb.eLaunchFlagStopAtEntry
+launch_info.SetLaunchFlags(flags)
+
+process = self.connect(target)
+self.assertTrue(process.IsValid(), "Process is valid")
+
+# We need to fetch the connected event:
+event = lldb.SBEvent()
+got_event = self.dbg.GetListener().WaitForEventForBroadcaster(30, process.GetBroadcaster(), event)
+self.assertTrue(got_event, "Got an initial event after connect")
+self.assertEqual(process.GetState(), lldb.eStateConnected, "Unexpected state.")
+
+error = lldb.SBError()
+self.server.responder.after_launch = True
+
+process = target.Launch(launch_info, error)
+self.assertSuccess(error, "Successfull

[Lldb-commits] [PATCH] D113487: [lldb] Refactor Platform::ResolveExecutable

2021-11-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

LGTM. I will ok soon if no one else chimes in!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113487/new/

https://reviews.llvm.org/D113487

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

2021-11-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg abandoned this revision.
clayborg added a comment.

This was submitted as a series of patches that updated "statistics dump" to 
emit JSON:

https://reviews.llvm.org/D111686
https://reviews.llvm.org/D112279
https://reviews.llvm.org/D112501
https://reviews.llvm.org/D112683
https://reviews.llvm.org/D112587


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110804/new/

https://reviews.llvm.org/D110804

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112973: [lldb] make it easier to find LLDB's python

2021-11-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Rather than percolating up a JSON string, we should use `StructuredData` (and 
`SBStructuredData` at the SB API layer) and only convert it to JSON at the very 
last moment.




Comment at: lldb/tools/driver/Driver.cpp:410
+} else {
+  llvm::errs() << "no script interpreter.\n";
+  error.SetErrorString("no script interpreter.");




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112973/new/

https://reviews.llvm.org/D112973

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112107: [lldb] Fix Scripted ProcessLaunchInfo Argument nullptr deref

2021-11-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 386005.
mib retitled this revision from "[lldb/Plugins] Make 
`ScriptedInterface::CreatePluginObject` more generic" to "[lldb] Fix Scripted 
ProcessLaunchInfo Argument nullptr deref".
mib edited the summary of this revision.
mib added a comment.

Address @JDevlieghere comments from D112109 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112107/new/

https://reviews.llvm.org/D112107

Files:
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -55,7 +55,7 @@
   StructuredData::GenericSP object_sp =
   scripted_thread_interface->CreatePluginObject(
   class_name->c_str(), exe_ctx,
-  process.m_scripted_process_info.GetDictionarySP());
+  process.m_scripted_process_info.GetArgsSP());
   if (!object_sp || !object_sp->IsValid()) {
 error.SetErrorString("Failed to create valid script object");
 return;
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -25,17 +25,15 @@
   public:
 ScriptedProcessInfo(const ProcessLaunchInfo &launch_info) {
   m_class_name = launch_info.GetScriptedProcessClassName();
-  m_dictionary_sp = launch_info.GetScriptedProcessDictionarySP();
+  m_args_sp = launch_info.GetScriptedProcessDictionarySP();
 }
 
 std::string GetClassName() const { return m_class_name; }
-StructuredData::DictionarySP GetDictionarySP() const {
-  return m_dictionary_sp;
-}
+StructuredData::DictionarySP GetArgsSP() const { return m_args_sp; }
 
   private:
 std::string m_class_name;
-StructuredData::DictionarySP m_dictionary_sp;
+StructuredData::DictionarySP m_args_sp = nullptr;
   };
 
 public:
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -106,7 +106,7 @@
 
   StructuredData::GenericSP object_sp = GetInterface().CreatePluginObject(
   m_scripted_process_info.GetClassName().c_str(), exe_ctx,
-  m_scripted_process_info.GetDictionarySP());
+  m_scripted_process_info.GetArgsSP());
 
   if (!object_sp || !object_sp->IsValid()) {
 error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
Index: lldb/source/API/SBLaunchInfo.cpp
===
--- lldb/source/API/SBLaunchInfo.cpp
+++ lldb/source/API/SBLaunchInfo.cpp
@@ -380,16 +380,18 @@
 void SBLaunchInfo::SetScriptedProcessDictionary(lldb::SBStructuredData dict) {
   LLDB_RECORD_METHOD(void, SBLaunchInfo, SetScriptedProcessDictionary,
  (lldb::SBStructuredData), dict);
+  if (!dict.IsValid() || !dict.m_impl_up)
+return;
 
-  SBStream stream;
-  SBError error = dict.GetAsJSON(stream);
+  StructuredData::ObjectSP obj_sp = dict.m_impl_up->GetObjectSP();
 
-  if (error.Fail())
+  if (!obj_sp)
 return;
 
-  StructuredData::DictionarySP dict_sp;
-  llvm::json::OStream s(stream.ref().AsRawOstream());
-  dict_sp->Serialize(s);
+  StructuredData::DictionarySP dict_sp =
+  std::make_shared(obj_sp);
+  if (!dict_sp || dict_sp->GetType() == lldb::eStructuredDataTypeInvalid)
+return;
 
   m_opaque_sp->SetScriptedProcessDictionarySP(dict_sp);
 }
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -353,6 +353,17 @@
   public:
 Dictionary() : Object(lldb::eStructuredDataTypeDictionary), m_dict() {}
 
+Dictionary(ObjectSP obj_sp)
+: Object(lldb::eStructuredDataTypeDictionary), m_dict() {
+  if (!obj_sp || obj_sp->GetType() != lldb::eStructuredDataTypeDictionary) {
+SetType(lldb::eStructuredDataTypeInvalid);
+return;
+  }
+
+  Dictionary *dict = obj_sp->GetAsDictionary();
+  m_dict = dict->m_dict;
+}
+
 ~Dictionary() override = default;
 
 size_t GetSize() const { return m_dict.size(); }
Index: lldb/include/lldb/Core/StructuredDataImpl.h
===
--- lldb/include/lldb/Core/StructuredDataImpl.h
+++ lld

[Lldb-commits] [PATCH] D112109: [lldb/Plugins] Serialize ProcessLaunchInfo ScriptedProcess Dictionary

2021-11-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib abandoned this revision.
mib added a comment.

Dropping this in favour of D112107 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112109/new/

https://reviews.llvm.org/D112109

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112147: [lldb] Fix lookup for global constants in namespaces

2021-11-09 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3281-3282
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit ||
+ parent_tag == DW_TAG_namespace) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(

clayborg wrote:
> tonkosi wrote:
> > clayborg wrote:
> > > I think we should just always call this function in regardless of what 
> > > the parent variable tag is since what happens if the parent tag is 
> > > another value decl context like DW_TAG_class_type, DW_TAG_structure_type? 
> > > 
> > > The call to GetDWARFDeclContext(die) below will calculate the 
> > > DWARFDeclContext for a DIE and then construct an appropriate qualified 
> > > name, so we can just always do this so I would suggest just making this:
> > > 
> > > ```
> > > if (Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
> > >   mangled = 
> > > GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();
> > > ```
> > > Then we should always get this right all the time. It doesn't actually 
> > > make sense to call this if the parent is the DW_TAG_compile_unit or 
> > > DW_TAG_partial_unit because then there is no decl context to add to the 
> > > variable name.
> > I tried to call `GetDWARFDeclContext(die)` in a general case, but it seems 
> > that `GetQualifiedName` will append `::` 
> > ([source](https://github.com/llvm/llvm-project/blob/5015f250894d3d97917d858850fae7960923a4ae/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp#L22))
> >  in front of the variable name if there's no other declaration context.
> > 
> > As a result, LLDB will display local variables and function arguments as 
> > `::var`. Any suggestion to deal with that?
> > 
> > I also noticed that with the current change it would be possible to get 
> > variables in anonymous namespaces, e.g. for the following source
> > 
> > ```
> > namespace ns {
> > namespace {
> > const int var = 1;
> > }
> > }
> > ```
> > 
> > call to `SBTarget::FindGlobalVariables("ns::(anonymous namespace)::var")` 
> > will succeed. Is that OK?
> GetQualifiedNameAsConstString could take an extra argument that specifies if 
> we should prepend the "::" to the root namespace.
> 
> I am fine with a lookup of "ns::(anonymous namespace)::var" succeeding.
You should document the anonymous namespace case in a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112147/new/

https://reviews.llvm.org/D112147

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113449: Revert "[lldb] Disable minimal import mode for RecordDecls that back FieldDecls"

2021-11-09 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp:21
+};
+B b;

teemperor wrote:
> FWIW, I think probably should be an API test (for a bunch of reasons from not 
> relying on formatting output to remote device testing), but given this is 
> just a revert I won't insist on that. Thanks again!
Makes sense -- this was just the easiest way to copy/paste the repro from the 
bug. I'll convert it to an API test when landing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113449/new/

https://reviews.llvm.org/D113449

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112047: [lldb/test] Update TestScriptedProcess to use skinny corefiles

2021-11-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 3 inline comments as done.
mib added inline comments.



Comment at: 
lldb/examples/python/scripted_process/stack_core_scripted_process.py:1
+import os,struct,signal
+

JDevlieghere wrote:
> This should live next to the test. I don't see a point of shipping this to 
> users. 
Do you mean the python script or the import statement ? In both case, I don't 
think this we ship these to the users.



Comment at: 
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py:124
 
-scripted_process_example_relpath = 
['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
-self.runCmd("command script import " + 
os.path.join(self.getSourceDir(),
-
*scripted_process_example_relpath))
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + 
os.path.join(self.scripted_process_example_abspath,

JDevlieghere wrote:
> We should really find a better way to do this. Can you remind me why we can't 
> pass that in the structured data? Other than for you test, I assume we rarely 
> want a process launch for a scripted process, so maybe the default should be 
> inverted. 
This is an environment variable used to decide whether we should launch the 
process right after importing the python script in lldb or leave it to the 
user' s discretion.

This is also used to launch the process from the script interpreter using the 
SBAPI, to test launching the scripted process in both a synchronous and 
asynchronous mode.

It can also come quite handy for debugging and troubleshooting scripted 
processes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112047/new/

https://reviews.llvm.org/D112047

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112047: [lldb/test] Update TestScriptedProcess to use skinny corefiles

2021-11-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 386021.
mib marked 2 inline comments as done.
mib added a comment.

Address @JDevlieghere comments and skip test for non x86_64 archs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112047/new/

https://reviews.llvm.org/D112047

Files:
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/examples/python/scripted_process/stack_core_scripted_process.py
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -2,7 +2,7 @@
 Test python scripted process in lldb
 """
 
-import os
+import os, json, tempfile
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -10,14 +10,15 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test import lldbtest
 
-
 class ScriptedProcesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 def setUp(self):
 TestBase.setUp(self)
-self.source = "main.c"
+self.scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process']
+self.scripted_process_example_abspath = os.path.join(self.getSourceDir(),
+*self.scripted_process_example_relpath)
 
 def tearDown(self):
 TestBase.tearDown(self)
@@ -43,7 +44,7 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
-@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
+@skipIf(archs=no_match(['x86_64']))
 def test_scripted_process_and_scripted_thread(self):
 """Test that we can launch an lldb scripted process using the SBAPI,
 check its process ID, read string from memory, check scripted thread
@@ -78,19 +79,29 @@
 self.assertGreater(thread.GetNumFrames(), 0)
 
 frame = thread.GetFrameAtIndex(0)
+GPRs = None
 register_set = frame.registers # Returns an SBValueList.
 for regs in register_set:
-if 'GPR' in regs.name:
-registers  = regs
+if 'general purpose' in regs.name.lower():
+GPRs = regs
 break
 
-self.assertTrue(registers, "Invalid General Purpose Registers Set")
-self.assertEqual(registers.GetNumChildren(), 21)
-for idx, reg in enumerate(registers, start=1):
+self.assertTrue(GPRs, "Invalid General Purpose Registers Set")
+self.assertEqual(GPRs.GetNumChildren(), 21)
+for idx, reg in enumerate(GPRs, start=1):
 self.assertEqual(idx, int(reg.value, 16))
 
-@skipIfDarwin
+def create_stack_skinny_corefile(self, file):
+self.build()
+target, process, thread, _ = lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+self.assertTrue(process.IsValid(), "Process is invalid.")
+# FIXME: Use SBAPI to save the process corefile.
+self.runCmd("process save-core -s stack  " + file)
+self.assertTrue(os.path.exists(file), "No stack-only corefile found.")
+self.assertTrue(self.dbg.DeleteTarget(target), "Couldn't delete target")
+
 @skipUnlessDarwin
+@skipIf(archs=no_match(['x86_64']))
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
@@ -101,26 +112,44 @@
 for module in target.modules:
 if 'a.out' in module.GetFileSpec().GetFilename():
 main_module = module
+break
 
 self.assertTrue(main_module, "Invalid main module.")
 error = target.SetModuleLoadAddress(main_module, 0)
 self.assertTrue(error.Success(), "Reloading main module at offset 0 failed.")
 
-scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
-self.runCmd("command script import " + os.path.join(self.getSourceDir(),
-*scripted_process_example_relpath))
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + os.path.join(self.scripted_process_example_abspath,
+"stack_core_scripted_process.py"))
+
+corefile_process = None
+with tempfile.NamedTemporaryFile() as file:
+self.create_stack_skinny_corefile(file.name)
+corefile_target = self.dbg.CreateTarget(None)
+co

[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

2021-11-09 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu created this revision.
zequanwu added reviewers: labath, shafik.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As the deleted comment says, we don't care about the access specifier.
I also encountered a crash at here (called from 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L2141)
 when fixing a bug on SymbolFileNativePDB.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113533

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -89,11 +89,6 @@
 namespace {
 static void VerifyDecl(clang::Decl *decl) {
   assert(decl && "VerifyDecl called with nullptr?");
-#ifndef NDEBUG
-  // We don't care about the actual access value here but only want to trigger
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif
 }
 
 static inline bool


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -89,11 +89,6 @@
 namespace {
 static void VerifyDecl(clang::Decl *decl) {
   assert(decl && "VerifyDecl called with nullptr?");
-#ifndef NDEBUG
-  // We don't care about the actual access value here but only want to trigger
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif
 }
 
 static inline bool
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

2021-11-09 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:95
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif

No, we don't care about the return value but we care about the `assert` which 
means the AST node is not well formed. In this case IIRC we are verifying that 
a member has an access specifier that is not set to `AS_none`. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113533/new/

https://reviews.llvm.org/D113533

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 577c1ee - [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread Walter Erquinigo via lldb-commits

Author: Danil Stefaniuc
Date: 2021-11-09T21:33:08-08:00
New Revision: 577c1eecf8c4b078eecb57e1c5b8d86adfc3c08a

URL: 
https://github.com/llvm/llvm-project/commit/577c1eecf8c4b078eecb57e1c5b8d86adfc3c08a
DIFF: 
https://github.com/llvm/llvm-project/commit/577c1eecf8c4b078eecb57e1c5b8d86adfc3c08a.diff

LOG: [formatters] Add a libstdcpp formatter for forward_list and refactor list 
formatter

This diff adds a data formatter for libstdcpp's forward_list. Besides, it 
refactors the existing code by extracting the common functionality between 
libstdcpp forward_list and list formatters into the AbstractListSynthProvider 
class.

Reviewed By: wallace

Differential Revision: https://reviews.llvm.org/D113362

Added: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp

Modified: 
lldb/examples/synthetic/gnu_libstdcpp.py
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Removed: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp



diff  --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index 30149d17b68af..a5b24273489f2 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -9,12 +9,17 @@
 # before relying on these formatters to do the right thing for your setup
 
 
-class StdListSynthProvider:
-
-def __init__(self, valobj, dict):
+class AbstractListSynthProvider:
+def __init__(self, valobj, dict, has_prev):
+'''
+:param valobj: The value object of the list
+:param dict: A dict with metadata provided by LLDB
+:param has_prev: Whether the list supports a 'prev' pointer besides a 
'next' one
+'''
 logger = lldb.formatters.Logger.Logger()
 self.valobj = valobj
 self.count = None
+self.has_prev = has_prev
 logger >> "Providing synthetic children for a list named " + \
 str(valobj.GetName())
 
@@ -24,13 +29,13 @@ def next_node(self, node):
 
 def is_valid(self, node):
 logger = lldb.formatters.Logger.Logger()
-valid = self.value(self.next_node(node)) != self.node_address
+valid = self.value(self.next_node(node)) != 
self.get_end_of_list_address()
 if valid:
 logger >> "%s is valid" % str(self.valobj.GetName())
 else:
 logger >> "synthetic value is not valid"
 return valid
-
+
 def value(self, node):
 logger = lldb.formatters.Logger.Logger()
 value = node.GetValueAsUnsigned()
@@ -73,26 +78,30 @@ def num_children(self):
 def num_children_impl(self):
 logger = lldb.formatters.Logger.Logger()
 try:
-next_val = self.next.GetValueAsUnsigned(0)
-prev_val = self.prev.GetValueAsUnsigned(0)
 # After a std::list has been initialized, both next and prev will
 # be non-NULL
-if next_val == 0 or prev_val == 0:
-return 0
-if next_val == self.node_address:
+next_val = self.next.GetValueAsUnsigned(0)
+if next_val == 0: 
 return 0
-if next_val == prev_val:
-return 1
 if self.has_loop():
 return 0
-size = 2
+if self.has_prev:
+prev_val = self.prev.GetValueAsUnsigned(0)
+if prev_val == 0:
+return 0
+if next_val == self.node_address:
+return 0
+if next_val == prev_val:
+return 1   
+size = 1
 current = self.next
 while current.GetChildMemberWithName(
-'_M_next').GetValueAsUnsigned(0) != self.node_address:
+'_M_next').GetValueAsUnsigned(0) != 
self.get_end_of_list_address():
 size = size + 1
 current = current.GetChildMemberWithName('_M_next')
-return (size - 1)
+return size 
 except:
+logger >> "Error determining the size"
 return 0
 
 def get_child_index(self, name):
@@ -101,7 +110,7 @@ def get_child_index(self, name):
 return int(name.lstrip('[').rstrip(']'))
 except:
 return -1
-
+  
 def get_child_at_index(self, index):
 logger = lldb.formatters.Logger.Logge

[Lldb-commits] [PATCH] D113362: [formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

2021-11-09 Thread Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG577c1eecf8c4: [formatters] Add a libstdcpp formatter for 
forward_list and refactor list… (authored by danilashtefan, committed by Walter 
Erquinigo ).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113362/new/

https://reviews.llvm.org/D113362

Files:
  lldb/examples/synthetic/gnu_libstdcpp.py
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
+++ /dev/null
@@ -1,7 +0,0 @@
-#include 
-
-int main()
-{
-  std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5};
-  return 0; // break here
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main() {
+  std::forward_list empty{}, one_elt{47},
+  five_elts{1, 22, 333, , 5};
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
@@ -9,8 +9,10 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
-class TestDataFormatterLibcxxForwardList(TestBase):
+class TestDataFormatterGenericForwardList(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -19,10 +21,10 @@
 self.line = line_number('main.cpp', '// break here')
 self.namespace = 'std'
 
-@add_test_categories(["libc++"])
-def test(self):
+
+def do_test(self, stdlib_type):
 """Test that std::forward_list is displayed correctly"""
-self.build()
+self.build(dictionary={stdlib_type: "1"})
 lldbutil.run_to_source_breakpoint(self, '// break here',
 lldb.SBFileSpec("main.cpp", False))
 
@@ -49,3 +51,12 @@
  '[3] = ',
  '[4] = 5',
  '}'])
+
+@add_test_categories(["libstdcxx"])
+def test_libstdcpp(self):
+self.do_test(USE_LIBSTDCPP)
+
+@add_test_categories(["libc++"])
+def test_libcpp(self):
+ self.do_test(USE_LIBCPP)
+
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile
@@ -1,4 +1,3 @@
 CXX_SOURCES := main.cpp
 
-USE_LIBCPP := 1
 include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -923,6 +923,11 @@
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+  SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  stl_synth_flags,
+  "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
@@ -953,6 +958,10 @@

[Lldb-commits] [PATCH] D112047: [lldb/test] Update TestScriptedProcess to use skinny corefiles

2021-11-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: 
lldb/examples/python/scripted_process/stack_core_scripted_process.py:1
+import os,struct,signal
+

mib wrote:
> JDevlieghere wrote:
> > This should live next to the test. I don't see a point of shipping this to 
> > users. 
> Do you mean the python script or the import statement ? In both case, I don't 
> think this we ship these to the users.
I mean that `stack_core_scripted_process.py` shouldn't be under `lldb/examples` 
but should live in the test directory where it's used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112047/new/

https://reviews.llvm.org/D112047

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D112107: [lldb] Fix Scripted ProcessLaunchInfo Argument nullptr deref

2021-11-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Thanks. This LGTM!




Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.h:36
 std::string m_class_name;
-StructuredData::DictionarySP m_dictionary_sp;
+StructuredData::DictionarySP m_args_sp = nullptr;
   };

A shared pointer is default constructed with a `nullptr` so you can omit that 
here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112107/new/

https://reviews.llvm.org/D112107

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py:486
+ lldb.eStopReasonSignal)
+# NB: this may need adjusting per current platform
+self.assertEqual(process.threads[0].GetStopDescription(100),

I believe this will always use remote-linux (or something), as the platform is 
determined by the binary. And even in case it isn't , it would probably be 
better to hardcode the platform by using the appropriate CreateTarget overload


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108078/new/

https://reviews.llvm.org/D108078

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113519: [lldb] [gdb-server] Fix fill_clamp to handle signed src types

2021-11-09 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:776
+  constexpr auto T_max = std::numeric_limits::max();
+  dest = src >= 0 && static_cast(src) <= T_max ? src : fallback;
 }

`T` will always be unsigned here, right? May not hurt to add a `static_assert` 
for that, mainly to avoid people wondering if this logic is correct...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113519/new/

https://reviews.llvm.org/D113519

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D113519: [lldb] [gdb-server] Fix fill_clamp to handle signed src types

2021-11-09 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked an inline comment as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:776
+  constexpr auto T_max = std::numeric_limits::max();
+  dest = src >= 0 && static_cast(src) <= T_max ? src : fallback;
 }

labath wrote:
> `T` will always be unsigned here, right? May not hurt to add a 
> `static_assert` for that, mainly to avoid people wondering if this logic is 
> correct...
Yes, and that makes the logic much simpler. I'll add the assert. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113519/new/

https://reviews.llvm.org/D113519

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits