[Lldb-commits] [PATCH] D77107: [intel-pt] Implement a basic test case

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am worried if this test will be flaky on loaded machines. Not sure how we can 
ever guarantee we will see processor traces with our stuff in it if the machine 
is busy running many tests or even doing other things.




Comment at: 
lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py:50-54
+"rand", # We expect to see a reference to the rand function
+# within the last instructions
+hex(fun_start_adddress),  # We expect to have seen the first
+  # instruction of 'fun'
+"at main.cpp:21" # We expect to see the exit condition of

can we guarantee we will see any of these on a fully loaded machine running 
many tests simultaneously? Maybe we need to settle for the header of the output 
only to know that it tried to display something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77107



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good as long as with we have "llvm::Optional 
request_path = {}" in the arguments for a function, that "{}" will turn into 
llvm::None and not an empty StringRef? Unclear to me. As long as this is what 
is happening and as long is this coding convention is used elsewhere in llvm 
(using "{}" instead of "llvm::None") I am ok. Pavel?




Comment at: lldb/tools/lldb-vscode/JSONUtils.h:204
+void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array ,
+  llvm::Optional request_path = {});
 

s/{}/llvm::None/ if {} doesn't construct to llvm::None?



Comment at: lldb/tools/lldb-vscode/JSONUtils.h:222
+CreateBreakpoint(lldb::SBBreakpoint ,
+ llvm::Optional request_path = {});
 

s/{}/llvm::None/ if {} doesn't construct to llvm::None


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253779.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1364,13 +1364,13 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1748,13 +1748,13 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendBreakpoint(src_bp.bp, response_breakpoints, path);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -184,28 +184,42 @@
 void SetValueForKey(lldb::SBValue , llvm::json::Object ,
 llvm::StringRef key);
 
-/// Converts \a bp to a JSON value and appends all locations to the
+/// Converts \a bp to a JSON value and appends the first valid location to the
 /// \a breakpoints array.
 ///
 /// \param[in] bp
-/// A LLDB breakpoint object which will get all locations extracted
-/// and converted into a JSON objects in the \a breakpoints array
+/// A LLDB breakpoint object which will get the first valid location
+/// extracted and converted into a JSON object in the \a breakpoints array
 ///
 /// \param[in] breakpoints
 /// A JSON array that will get a llvm::json::Value for \a bp
 /// appended to it.
-void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
+///
+/// \param[in] request_path
+/// An optional source path to use when creating the "Source" object of this
+/// breakpoint. If not specified, the "Source" object is created from the
+/// breakpoint's address' LineEntry. It is useful to ensure the same source
+/// paths provided by the setBreakpoints request are returned to the IDE.
+void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array ,
+  llvm::Optional request_path = {});
 
 /// Converts breakpoint location to a Visual Studio Code "Breakpoint"
-/// JSON object and appends it to the \a breakpoints array.
 ///
 /// \param[in] bp
 /// A LLDB breakpoint object to convert into a JSON value
 ///
+/// \param[in] request_path
+/// An optional source path to use when creating the "Source" object of this
+/// breakpoint. If not specified, the "Source" object is created from the
+/// breakpoint's address' LineEntry. It is useful to ensure the same source
+/// paths provided by the setBreakpoints request are returned to the IDE.
+///
 /// \return
 /// A "Breakpoint" JSON object with that follows the formal JSON
 /// definition outlined by Microsoft.
-llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint );
+llvm::json::Value
+CreateBreakpoint(lldb::SBBreakpoint ,
+ llvm::Optional request_path = {});
 
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
@@ -263,6 +277,16 @@
 /// definition outlined by Microsoft.
 llvm::json::Value CreateSource(lldb::SBLineEntry _entry);
 
+/// Create a "Source" object for a given source path.
+///
+/// \param[in] source_path
+/// The path to the source to use when creating the "Source" object.
+///
+/// \return
+/// A "Source" JSON object that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateSource(llvm::StringRef source_path);
+
 /// 

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace marked an inline comment as done.
wallace added inline comments.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:332
+
+  object.try_emplace("line", line);
+

clayborg wrote:
> This should do what it was doing before: grab the source line only from the 
> address. Why? If you set a breakpoint on a line that has no code like:
> 
> ```
> 1 int main(int argc, ...)
> 2 {  /// <-- set bp here
> 3   int foo = arg * 10; /// <-- bp will end up here
> ```
> 
> So we should still be looking up the line number from the Address of the 
> first resolved location so that the source line does get adjusted in the IDE.
TIL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

This is obviously good! Do you think that a similar error handling bug might 
exist in other cases that depend top-of-stack?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77108



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

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: aprantl.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch fixes a crash that happens on the DWARF expression evaluator
when trying to access the top of the stack while it's empty.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77108

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp


Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -234,6 +234,10 @@
   llvm::Failed());
 }
 
+TEST(DWARFExpression, DW_OP_stack_value) {
+  EXPECT_THAT_ERROR(Evaluate({DW_OP_stack_value}).takeError(), llvm::Failed());
+}
+
 TEST(DWARFExpression, DW_OP_piece) {
   EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x11, 0x22, DW_OP_piece, 2,
  DW_OP_const2u, 0x33, 0x44, DW_OP_piece, 2}),
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -2318,6 +2318,12 @@
 // rather is a constant value.  The value from the top of the stack is the
 // value to be used.  This is the actual object value and not the location.
 case DW_OP_stack_value:
+  if (stack.empty()) {
+if (error_ptr)
+  error_ptr->SetErrorString(
+  "Expression stack needs at least 1 item for DW_OP_stack_value.");
+return false;
+  }
   stack.back().SetValueType(Value::eValueTypeScalar);
   break;
 


Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -234,6 +234,10 @@
   llvm::Failed());
 }
 
+TEST(DWARFExpression, DW_OP_stack_value) {
+  EXPECT_THAT_ERROR(Evaluate({DW_OP_stack_value}).takeError(), llvm::Failed());
+}
+
 TEST(DWARFExpression, DW_OP_piece) {
   EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x11, 0x22, DW_OP_piece, 2,
  DW_OP_const2u, 0x33, 0x44, DW_OP_piece, 2}),
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -2318,6 +2318,12 @@
 // rather is a constant value.  The value from the top of the stack is the
 // value to be used.  This is the actual object value and not the location.
 case DW_OP_stack_value:
+  if (stack.empty()) {
+if (error_ptr)
+  error_ptr->SetErrorString(
+  "Expression stack needs at least 1 item for DW_OP_stack_value.");
+return false;
+  }
   stack.back().SetValueType(Value::eValueTypeScalar);
   break;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77107: [intel-pt] Implement a basic test case

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253748.
wallace added a comment.

Added a stop command invocation in the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77107

Files:
  lldb/test/API/tools/intel-features/intel-pt/test/Makefile
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
  lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp

Index: lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
===
--- lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
+++ lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
@@ -191,6 +191,7 @@
   result.SetStatus(lldb::eReturnStatusFailed);
   return false;
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -290,6 +291,7 @@
  s.GetData());
   result.AppendMessage(res.GetOutput());
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -428,6 +430,7 @@
   }
   result.AppendMessage(res.GetOutput());
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -480,6 +483,7 @@
   result.SetStatus(lldb::eReturnStatusFailed);
   return false;
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
Index: lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
@@ -0,0 +1,27 @@
+//===-- main.cpp *- C++ -*-===//
+
+ Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ See https://llvm.org/LICENSE.txt for license information.
+ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+===--===//
+//
+
+#include 
+#include 
+
+using namespace std;
+
+int fun(int a) {
+  return rand() + 1;
+}
+
+int main() {
+  int z = 0;
+  for(int i = 0; i < 1; i++) {
+z += fun(z);
+  }
+
+  cout << z << endl; // Break 1
+  return 0;
+}
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- /dev/null
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -0,0 +1,58 @@
+from __future__ import print_function
+
+import os
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestIntelPTSimpleBinary(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIf(oslist=no_match(['linux']))
+@skipIf(archs=no_match(['i386', 'x86_64']))
+@skipIfRemote
+def test_basic_flow(self):
+"""Test collection, decoding, and dumping instructions"""
+lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
+lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
+plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+if not os.path.isfile(plugin_file):
+self.skipTest("features plugin missing.")
+
+self.build()
+
+self.runCmd("plugin load " + plugin_file)
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+self.runCmd("b main")
+self.runCmd("b " + str(line_number('main.cpp', '// Break 1')))
+
+self.runCmd("r")
+# We start tracing from main
+self.runCmd("processor-trace start all")
+self.runCmd("c")
+# We check the trace after the for loop
+
+# We find the start address of the 'fun' function for a later check
+target = self.dbg.GetSelectedTarget()
+fun_start_adddress = target.FindFunctions("fun")[0].GetSymbol() \
+.GetStartAddress().GetLoadAddress(target)
+
+# We print the last instructions
+self.expect("processor-trace show-instr-log -c 100",
+patterns=[
+"rand", # We expect to see a reference to the rand function
+# within the last instructions
+hex(fun_start_adddress),  # We expect to have seen the first
+  # instruction of 'fun'
+"at main.cpp:21" # We expect to see the exit condition of
+ # the for loop
+])
+
+self.runCmd("processor-trace stop")
Index: lldb/test/API/tools/intel-features/intel-pt/test/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/intel-features/intel-pt/test/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include 

[Lldb-commits] [PATCH] D77107: [intel-pt] Implement a basic test case

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: clayborg, labath, kusmour, aadsm.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace updated this revision to Diff 253748.
wallace added a comment.

Added a stop command invocation in the test


Depends on D76872 .

There was no test for the Intel PT support on LLDB, so I'm creating one, which
will help making progress on solid grounds.

The test is skipped if the Intel PT plugin library is not built.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77107

Files:
  lldb/test/API/tools/intel-features/intel-pt/test/Makefile
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
  lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp

Index: lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
===
--- lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
+++ lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
@@ -191,6 +191,7 @@
   result.SetStatus(lldb::eReturnStatusFailed);
   return false;
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -290,6 +291,7 @@
  s.GetData());
   result.AppendMessage(res.GetOutput());
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -428,6 +430,7 @@
   }
   result.AppendMessage(res.GetOutput());
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
@@ -480,6 +483,7 @@
   result.SetStatus(lldb::eReturnStatusFailed);
   return false;
 }
+result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
 return true;
   }
 
Index: lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
@@ -0,0 +1,27 @@
+//===-- main.cpp *- C++ -*-===//
+
+ Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ See https://llvm.org/LICENSE.txt for license information.
+ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+===--===//
+//
+
+#include 
+#include 
+
+using namespace std;
+
+int fun(int a) {
+  return rand() + 1;
+}
+
+int main() {
+  int z = 0;
+  for(int i = 0; i < 1; i++) {
+z += fun(z);
+  }
+
+  cout << z << endl; // Break 1
+  return 0;
+}
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- /dev/null
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -0,0 +1,58 @@
+from __future__ import print_function
+
+import os
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestIntelPTSimpleBinary(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIf(oslist=no_match(['linux']))
+@skipIf(archs=no_match(['i386', 'x86_64']))
+@skipIfRemote
+def test_basic_flow(self):
+"""Test collection, decoding, and dumping instructions"""
+lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
+lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
+plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+if not os.path.isfile(plugin_file):
+self.skipTest("features plugin missing.")
+
+self.build()
+
+self.runCmd("plugin load " + plugin_file)
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+self.runCmd("b main")
+self.runCmd("b " + str(line_number('main.cpp', '// Break 1')))
+
+self.runCmd("r")
+# We start tracing from main
+self.runCmd("processor-trace start all")
+self.runCmd("c")
+# We check the trace after the for loop
+
+# We find the start address of the 'fun' function for a later check
+target = self.dbg.GetSelectedTarget()
+fun_start_adddress = target.FindFunctions("fun")[0].GetSymbol() \
+.GetStartAddress().GetLoadAddress(target)
+
+# We print the last instructions
+self.expect("processor-trace show-instr-log -c 100",
+patterns=[
+"rand", # We expect to see a reference to the rand function
+# within the last instructions
+hex(fun_start_adddress),  # We expect to have seen the first
+  # instruction of 'fun'
+"at main.cpp:21" # We expect to see the exit condition of
+ # 

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

After reviewing more, we should just re-use CreateBreakpoint and add a 
"llvm::Optional request_path" argument. Then all breakpoints use the 
same function and we avoid duplicated code. Inline comments should detail what 
should be done. Let me know if you have questions.




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:285
 // }
 llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint ) {
   // Each breakpoint location is treated as a separate breakpoint for VS code.

Read my comments below first, then read this:

we should add an extra llvm::Optional arg to this so we can have all 
code just use this function:

```
llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint , 
llvm::Optional request_path)
```

If "request_path" has a value, then we use this in the "source" response. If it 
doesn't we look it up in from the location address.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:318
   object.try_emplace("line", line);
 object.try_emplace("source", CreateSource(line_entry));
   }

we will need to add "llvm::Optional request_path" as an argument to 
CreateSource here, so it can use request_path if it has a value, else use the 
source file in the line_entry object.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:323
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint ,
+ llvm::StringRef sourcePath, int line) 
{

Actually the _only_ difference between this function and "llvm::json::Value 
CreateBreakpoint(lldb::SBBreakpoint )" is the fact that we pass along the 
path along. So maybe we can just add a new variable to CreateBreakpoint above 
so. See my inline comment on CreateBreakpoint().



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:332
+
+  object.try_emplace("line", line);
+

This should do what it was doing before: grab the source line only from the 
address. Why? If you set a breakpoint on a line that has no code like:

```
1 int main(int argc, ...)
2 {  /// <-- set bp here
3   int foo = arg * 10; /// <-- bp will end up here
```

So we should still be looking up the line number from the Address of the first 
resolved location so that the source line does get adjusted in the IDE.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:493
 // }
 llvm::json::Value CreateSource(lldb::SBLineEntry _entry) {
   llvm::json::Object object;

add llvm::Optional request_path argument for CreateBreakpoint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG38ddb49e5242: [lldb/Reproducers] Always collect the whole 
dSYM in the reproducer (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D76672?vs=252898=253718#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76672

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/source/Utility/Reproducer.cpp
  lldb/test/Shell/Reproducer/TestDSYM.test

Index: lldb/test/Shell/Reproducer/TestDSYM.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestDSYM.test
@@ -0,0 +1,11 @@
+# REQUIRES: system-darwin
+# Ensure that the reproducers captures the whole dSYM bundle.
+
+# RUN: rm -rf %t.repro
+# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
+# RUN: touch %t.out.dSYM/foo.bar
+
+# RUN: %lldb -x -b --capture --capture-path %t.repro %t.out -o 'b main' -o 'run' -o 'reproducer generate'
+
+# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
+# FILES: foo.bar
Index: lldb/source/Utility/Reproducer.cpp
===
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -321,6 +321,11 @@
   os << m_cwd << "\n";
 }
 
+void FileProvider::recordInterestingDirectory(const llvm::Twine ) {
+  if (m_collector)
+m_collector->addDirectory(dir);
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
@@ -145,6 +146,11 @@
 }
 
 if (dsym_fspec) {
+  // Compute dSYM root.
+  std::string dsym_root = dsym_fspec.GetPath();
+  const size_t pos = dsym_root.find("/Contents/Resources/");
+  dsym_root = pos != std::string::npos ? dsym_root.substr(0, pos) : "";
+
   DataBufferSP dsym_file_data_sp;
   lldb::offset_t dsym_file_data_offset = 0;
   dsym_objfile_sp =
@@ -154,136 +160,132 @@
   if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
 // We need a XML parser if we hope to parse a plist...
 if (XMLDocument::XMLEnabled()) {
-  char dsym_path[PATH_MAX];
-  if (module_sp->GetSourceMappingList().IsEmpty() &&
-  dsym_fspec.GetPath(dsym_path, sizeof(dsym_path))) {
+  if (module_sp->GetSourceMappingList().IsEmpty()) {
 lldb_private::UUID dsym_uuid = dsym_objfile_sp->GetUUID();
 if (dsym_uuid) {
   std::string uuid_str = dsym_uuid.GetAsString();
-  if (!uuid_str.empty()) {
-char *resources = strstr(dsym_path, "/Contents/Resources/");
-if (resources) {
-  char dsym_uuid_plist_path[PATH_MAX];
-  resources[strlen("/Contents/Resources/")] = '\0';
-  snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
-   "%s%s.plist", dsym_path, uuid_str.c_str());
-  FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
-  if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
-ApplePropertyList plist(dsym_uuid_plist_path);
-if (plist) {
-  std::string DBGBuildSourcePath;
-  std::string DBGSourcePath;
-
-  // DBGSourcePathRemapping is a dictionary in the plist
-  // with keys which are DBGBuildSourcePath file paths and
-  // values which are DBGSourcePath file paths
-
-  StructuredData::ObjectSP plist_sp =
-  plist.GetStructuredData();
-  if (plist_sp.get() && plist_sp->GetAsDictionary() &&
-  plist_sp->GetAsDictionary()->HasKey(
-  "DBGSourcePathRemapping") &&
-  plist_sp->GetAsDictionary()
-  ->GetValueForKey("DBGSourcePathRemapping")
-  ->GetAsDictionary()) {
-
-// If DBGVersion 1 or DBGVersion missing, ignore DBGSourcePathRemapping.
-// If DBGVersion 2, strip last two components of path remappings from
-//  entries to fix an issue with a specific 

[Lldb-commits] [PATCH] D77096: Correct the duplicate pragma marks in CommandObjectTarget.cpp

[Shivam Mittal via Phabricator via lldb-commits]

shivammittal99 created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Resolve the two duplicated pragma marks in 
lldb/source/Commands/CommandObjectTarget.cpp


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77096

Files:
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -571,7 +571,7 @@
   }
 };
 
-#pragma mark CommandObjectTargetSelect
+#pragma mark CommandObjectTargetDelete
 
 // "target delete"
 
@@ -2198,7 +2198,7 @@
   }
 };
 
-#pragma mark CommandObjectTargetModulesDumpSections
+#pragma mark CommandObjectTargetModulesDumpClangAST
 
 // Clang AST dumping command
 


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -571,7 +571,7 @@
   }
 };
 
-#pragma mark CommandObjectTargetSelect
+#pragma mark CommandObjectTargetDelete
 
 // "target delete"
 
@@ -2198,7 +2198,7 @@
   }
 };
 
-#pragma mark CommandObjectTargetModulesDumpSections
+#pragma mark CommandObjectTargetModulesDumpClangAST
 
 // Clang AST dumping command
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 38ddb49 - [lldb/Reproducers] Always collect the whole dSYM in the reproducer

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-30T15:16:50-07:00
New Revision: 38ddb49e5242920e44a982cff7bbe2e86bd23a69

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

LOG: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

The FileCollector in LLDB collects every files that's used during a
debug session when capture is enabled. This ensures that the reproducer
only contains the files necessary to reproduce. This approach is not a
good fit for the dSYM bundle, which is a directory on disk, but should
be treated as a single unit.

On macOS LLDB have automatically find the matching dSYM for a binary by
its UUID. Having a incomplete dSYM in a reproducer can break debugging
even when reproducers are disabled.

This patch adds a was to specify a directory of interest to the
reproducers. It is called from SymbolVendorMacOSX with the path of the
dSYMs used by LLDB.

Differential revision: https://reviews.llvm.org/D76672

Added: 
lldb/test/Shell/Reproducer/TestDSYM.test

Modified: 
lldb/include/lldb/Utility/Reproducer.h
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Utility/Reproducer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index bffb0f7c0647..b8b897d02ea2 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -98,6 +98,8 @@ class FileProvider : public Provider {
 return m_collector;
   }
 
+  void recordInterestingDirectory(const llvm::Twine );
+
   void Keep() override {
 auto mapping = GetRoot().CopyByAppendingPathComponent(Info::file);
 // Temporary files that are removed during execution can cause copy errors.

diff  --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp 
b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index 2b67fee70617..e819c342c6ec 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
@@ -145,6 +146,11 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP 
_sp,
 }
 
 if (dsym_fspec) {
+  // Compute dSYM root.
+  std::string dsym_root = dsym_fspec.GetPath();
+  const size_t pos = dsym_root.find("/Contents/Resources/");
+  dsym_root = pos != std::string::npos ? dsym_root.substr(0, pos) : "";
+
   DataBufferSP dsym_file_data_sp;
   lldb::offset_t dsym_file_data_offset = 0;
   dsym_objfile_sp =
@@ -154,136 +160,132 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP 
_sp,
   if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
 // We need a XML parser if we hope to parse a plist...
 if (XMLDocument::XMLEnabled()) {
-  char dsym_path[PATH_MAX];
-  if (module_sp->GetSourceMappingList().IsEmpty() &&
-  dsym_fspec.GetPath(dsym_path, sizeof(dsym_path))) {
+  if (module_sp->GetSourceMappingList().IsEmpty()) {
 lldb_private::UUID dsym_uuid = dsym_objfile_sp->GetUUID();
 if (dsym_uuid) {
   std::string uuid_str = dsym_uuid.GetAsString();
-  if (!uuid_str.empty()) {
-char *resources = strstr(dsym_path, "/Contents/Resources/");
-if (resources) {
-  char dsym_uuid_plist_path[PATH_MAX];
-  resources[strlen("/Contents/Resources/")] = '\0';
-  snprintf(dsym_uuid_plist_path, sizeof(dsym_uuid_plist_path),
-   "%s%s.plist", dsym_path, uuid_str.c_str());
-  FileSpec dsym_uuid_plist_spec(dsym_uuid_plist_path);
-  if (FileSystem::Instance().Exists(dsym_uuid_plist_spec)) {
-ApplePropertyList plist(dsym_uuid_plist_path);
-if (plist) {
-  std::string DBGBuildSourcePath;
-  std::string DBGSourcePath;
-
-  // DBGSourcePathRemapping is a dictionary in the plist
-  // with keys which are DBGBuildSourcePath file paths and
-  // values which are DBGSourcePath file paths
-
-  StructuredData::ObjectSP plist_sp =
-  plist.GetStructuredData();
-  if (plist_sp.get() && plist_sp->GetAsDictionary() &&
-  plist_sp->GetAsDictionary()->HasKey(
-  "DBGSourcePathRemapping") &&
-  

[Lldb-commits] [lldb] 075b610 - Recommit "[lldb] Make TestExprDiagnostics.py pass again after enabling Fix-Its in test"

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T14:24:06-07:00
New Revision: 075b610403a7d151ee9056aa490233bcd4248718

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

LOG: Recommit "[lldb] Make TestExprDiagnostics.py pass again after enabling 
Fix-Its in test"

This reverts commit 55ed09d32e2602eba1dbb8aba1985246739c3909 as
it was not responsible for breaking the bots. Sorry.

Added: 


Modified: 
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index da29d7b2c1af..b5eb552badb5 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -60,10 +60,10 @@ def test_source_and_caret_printing(self):
 self.assertIn(":1:10", 
value.GetError().GetCString())
 
 # Multiline top-level expressions.
-value = frame.EvaluateExpression("void x() {}\nvoid foo(unknown_type 
x) {}", top_level_opts)
+value = frame.EvaluateExpression("void x() {}\nvoid foo;", 
top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", 
value.GetError().GetCString())
-self.assertIn(":2:10", 
value.GetError().GetCString())
+self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn(":2:6", value.GetError().GetCString())
 
 # Test that we render Clang's 'notes' correctly.
 value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; 
};", top_level_opts)



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

[Lldb-commits] [lldb] 06bb7df - Recommit "[lldb] Make Fix-Its also apply to top-level expressions""

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T14:24:06-07:00
New Revision: 06bb7df81c0ba01f7efab76779e2eb7d76615e3d

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

LOG: Recommit "[lldb] Make Fix-Its also apply to top-level expressions""

This reverts commit fe5cb1c25fd6c07bbe3c0c698f36b74e6d04946f as it
 was not responsible for breaking the bots. Sorry.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index b246fc374d1c..2b75c4f75c63 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -616,15 +616,14 @@ bool ClangUserExpression::Parse(DiagnosticManager 
_manager,
   if (parser.RewriteExpression(diagnostic_manager)) {
 size_t fixed_start;
 size_t fixed_end;
-const std::string _expression =
-diagnostic_manager.GetFixedExpression();
+m_fixed_text = diagnostic_manager.GetFixedExpression();
 // Retrieve the original expression in case we don't have a top level
 // expression (which has no surrounding source code).
 if (m_source_code &&
-m_source_code->GetOriginalBodyBounds(fixed_expression, m_expr_lang,
+m_source_code->GetOriginalBodyBounds(m_fixed_text, m_expr_lang,
  fixed_start, fixed_end))
   m_fixed_text =
-  fixed_expression.substr(fixed_start, fixed_end - fixed_start);
+  m_fixed_text.substr(fixed_start, fixed_end - fixed_start);
   }
 }
 return false;

diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 273982c0c12f..eb1dd97aa9a9 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -33,6 +33,10 @@ def test_with_target(self):
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)
 
+top_level_options = lldb.SBExpressionOptions()
+top_level_options.SetAutoApplyFixIts(True)
+top_level_options.SetTopLevel(True)
+
 frame = self.thread.GetFrameAtIndex(0)
 
 # Try with one error:
@@ -41,6 +45,15 @@ def test_with_target(self):
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 10)
 
+# Try with one error in a top-level expression.
+# The Fix-It changes "ptr.m" to "ptr->m".
+expr = "struct X { int m; }; X x; X *ptr =  int m = ptr.m;"
+value = frame.EvaluateExpression(expr, top_level_options)
+# A successfully parsed top-level expression will yield an error
+# that there is 'no value'. If a parsing error would have happened we
+# would get a 
diff erent error kind, so let's check the error kind here.
+self.assertEquals(value.GetError().GetCString(), "error: No value")
+
 # Try with two errors:
 two_error_expression = "my_pointer.second->a"
 value = frame.EvaluateExpression(two_error_expression, options)



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

[Lldb-commits] [lldb] 50f7153 - Revert "[lldb][NFC] Refactor Fix-It filter for warnings"

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T14:12:11-07:00
New Revision: 50f7153ddb6e6dabcac6d16c8c908f1708f60d01

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

LOG: Revert "[lldb][NFC] Refactor Fix-It filter for warnings"

This reverts commit 11a5caee2aeae2546213366e7fc54095bb8163b9 as
it broke the bots.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index b3880ce03b08..e5de4b4651df 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -214,12 +214,16 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
   auto new_diagnostic = std::make_unique(
   stripped_output, severity, Info.getID());
 
+  // Don't store away warning fixits, since the compiler doesn't have
+  // enough context in an expression for the warning to be useful.
   // FIXME: Should we try to filter out FixIts that apply to our generated
   // code, and not the user's expression?
-  for (const clang::FixItHint  : Info.getFixItHints()) {
-if (fixit.isNull())
-  continue;
-new_diagnostic->AddFixitHint(fixit);
+  if (severity == eDiagnosticSeverityError) {
+for (const clang::FixItHint  : Info.getFixItHints()) {
+  if (fixit.isNull())
+continue;
+  new_diagnostic->AddFixitHint(fixit);
+}
   }
 
   m_manager->AddDiagnostic(std::move(new_diagnostic));
@@ -1123,10 +1127,6 @@ bool ClangExpressionParser::RewriteExpression(
   continue;
 if (!diagnostic->HasFixIts())
   continue;
-// Don't apply warning Fix-Its, since the compiler doesn't have enough
-// context in an expression for the warning to be useful.
-if (diagnostic->GetSeverity() != eDiagnosticSeverityError)
-  continue;
 for (const FixItHint  : diagnostic->FixIts())
   ApplyFixIt(fixit, commit);
   }



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253695.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1364,13 +1364,13 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1748,13 +1748,15 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendSourceBreakpoint(existing_bp->second.bp, path, src_bp.line,
+ response_breakpoints);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendSourceBreakpoint(src_bp.bp, path, src_bp.line,
+ response_breakpoints);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -196,6 +196,25 @@
 /// appended to it.
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
 
+/// Converts \a bp to a JSON value using the provided source line information to
+/// the \a breakpoints array.
+///
+/// \param[in] bp
+/// A LLDB breakpoint object which will provide basic information as ID and
+/// resolved status.
+///
+/// \param[in] sourcePath
+/// The full path to the source to store in the JSON value.
+///
+/// \param[in] line
+/// The line to store in the JSON value.
+///
+/// \param[in] breakpoints
+/// A JSON array that will get a llvm::json::Value for \a bp
+/// appended to it.
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array );
+
 /// Converts breakpoint location to a Visual Studio Code "Breakpoint"
 /// JSON object and appends it to the \a breakpoints array.
 ///
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -9,9 +9,11 @@
 #include 
 
 #include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/Path.h"
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
+#include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/Host/PosixApi.h"
 
@@ -319,10 +321,34 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint ,
+ llvm::StringRef sourcePath, int line) {
+  llvm::json::Object object;
+  if (!bp.IsValid())
+return llvm::json::Value(std::move(object));
+
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+
+  object.try_emplace("line", line);
+
+  llvm::json::Object source;
+  llvm::StringRef name = llvm::sys::path::filename(sourcePath);
+  EmplaceSafeString(source, "name", name);
+  EmplaceSafeString(source, "path", sourcePath);
+  object.try_emplace("source", llvm::json::Value(std::move(source)));
+  return llvm::json::Value(std::move(object));
+}
+
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array ) {
   breakpoints.emplace_back(CreateBreakpoint(bp));
 }
 
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array ) {
+  breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line));
+}
+
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 // 

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> In case of Apple platforms, this won't make a difference in practice, since 
> the support for that is implemented in PlatformDarwin (which all of these 
> inherit from), but it sounds like this will be a problem for the "linux" sdk 
> (assuming this is what I think it is), as there the selected platform will be 
> PlatformLinux, which has no clue about these sdk thingies.

The "Linux" SDK is a placeholder of sorts when debugging a Swift-on-Linux 
server application (e.g., something like https://www.kitura.io/) from *within* 
Xcode that points to a directory that contains the Swift resources/stdlib 
cross-compiled for Linux. So this is still an Xcode SDK that should be managed 
by PlatformDarwin despite the confusing name.

> So, it sounds to me like the sdk type should somehow play a role in the 
> selection of the platform instance, or this functionality should be moved 
> down into some Host function.

Since there is only going to be one place that does the SDK detection via 
xcrun, I would either move it to PlatformDarwin or HostMacOSX. Personally, I 
think I prefer the Host, since this is something that is really about the 
machine LLDB is running on and not about the remote platform.


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

https://reviews.llvm.org/D76471



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253696.
wallace added a comment.

nit 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1364,13 +1364,13 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1748,13 +1748,15 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendSourceBreakpoint(existing_bp->second.bp, path, src_bp.line,
+ response_breakpoints);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendSourceBreakpoint(src_bp.bp, path, src_bp.line,
+ response_breakpoints);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -196,6 +196,25 @@
 /// appended to it.
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
 
+/// Converts \a bp to a JSON value using the provided source line information to
+/// the \a breakpoints array.
+///
+/// \param[in] bp
+/// A LLDB breakpoint object which will provide basic information as ID and
+/// resolved status.
+///
+/// \param[in] sourcePath
+/// The full path to the source to store in the JSON value.
+///
+/// \param[in] line
+/// The line to store in the JSON value.
+///
+/// \param[in] breakpoints
+/// A JSON array that will get a llvm::json::Value for \a bp
+/// appended to it.
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array );
+
 /// Converts breakpoint location to a Visual Studio Code "Breakpoint"
 /// JSON object and appends it to the \a breakpoints array.
 ///
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -9,6 +9,7 @@
 #include 
 
 #include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/Path.h"
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
@@ -319,10 +320,34 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint ,
+ llvm::StringRef sourcePath, int line) {
+  llvm::json::Object object;
+  if (!bp.IsValid())
+return llvm::json::Value(std::move(object));
+
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+
+  object.try_emplace("line", line);
+
+  llvm::json::Object source;
+  llvm::StringRef name = llvm::sys::path::filename(sourcePath);
+  EmplaceSafeString(source, "name", name);
+  EmplaceSafeString(source, "path", sourcePath);
+  object.try_emplace("source", llvm::json::Value(std::move(source)));
+  return llvm::json::Value(std::move(object));
+}
+
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array ) {
   breakpoints.emplace_back(CreateBreakpoint(bp));
 }
 
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array ) {
+  breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line));
+}
+
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 // "type": "object",

[Lldb-commits] [lldb] 55ed09d - Revert "[lldb] Make TestExprDiagnostics.py pass again after enabling Fix-Its in test"

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T13:58:18-07:00
New Revision: 55ed09d32e2602eba1dbb8aba1985246739c3909

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

LOG: Revert "[lldb] Make TestExprDiagnostics.py pass again after enabling 
Fix-Its in test"

This reverts commit 502a06fcdafa637a9890da16c2734bc1a36010f6 as it
breaks the macOS bots. Raph will take a look and re-commit.

Added: 


Modified: 
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b5eb552badb5..da29d7b2c1af 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -60,10 +60,10 @@ def test_source_and_caret_printing(self):
 self.assertIn(":1:10", 
value.GetError().GetCString())
 
 # Multiline top-level expressions.
-value = frame.EvaluateExpression("void x() {}\nvoid foo;", 
top_level_opts)
+value = frame.EvaluateExpression("void x() {}\nvoid foo(unknown_type 
x) {}", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
-self.assertIn(":2:6", value.GetError().GetCString())
+self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", 
value.GetError().GetCString())
+self.assertIn(":2:10", 
value.GetError().GetCString())
 
 # Test that we render Clang's 'notes' correctly.
 value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; 
};", top_level_opts)



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

[Lldb-commits] [lldb] 10f633d - [TypeSystemClang] Add missing case in a switch. NFC'ish.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T13:53:53-07:00
New Revision: 10f633db86b523da17a820098e33f0caf70dbada

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

LOG: [TypeSystemClang] Add missing case in a switch. NFC'ish.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index c278aef8949e..a94e5a15fa68 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4661,6 +4661,7 @@ lldb::Encoding 
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
 case clang::BuiltinType::Kind::OCLReserveID:
 case clang::BuiltinType::Kind::OCLSampler:
 case clang::BuiltinType::Kind::OMPArraySection:
+case clang::BuiltinType::Kind::OMPArrayShaping:
 case clang::BuiltinType::Kind::Overload:
 case clang::BuiltinType::Kind::PseudoObject:
 case clang::BuiltinType::Kind::UnknownAny:



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

[Lldb-commits] [lldb] fe5cb1c - Revert "[lldb] Make Fix-Its also apply to top-level expressions"

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-30T13:23:58-07:00
New Revision: fe5cb1c25fd6c07bbe3c0c698f36b74e6d04946f

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

LOG: Revert "[lldb] Make Fix-Its also apply to top-level expressions"

This reverts commit 83c81c0a469482888482983c302c09c02680ae7c as
it broke the macOS lldb bots.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 2b75c4f75c63..b246fc374d1c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -616,14 +616,15 @@ bool ClangUserExpression::Parse(DiagnosticManager 
_manager,
   if (parser.RewriteExpression(diagnostic_manager)) {
 size_t fixed_start;
 size_t fixed_end;
-m_fixed_text = diagnostic_manager.GetFixedExpression();
+const std::string _expression =
+diagnostic_manager.GetFixedExpression();
 // Retrieve the original expression in case we don't have a top level
 // expression (which has no surrounding source code).
 if (m_source_code &&
-m_source_code->GetOriginalBodyBounds(m_fixed_text, m_expr_lang,
+m_source_code->GetOriginalBodyBounds(fixed_expression, m_expr_lang,
  fixed_start, fixed_end))
   m_fixed_text =
-  m_fixed_text.substr(fixed_start, fixed_end - fixed_start);
+  fixed_expression.substr(fixed_start, fixed_end - fixed_start);
   }
 }
 return false;

diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index eb1dd97aa9a9..273982c0c12f 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -33,10 +33,6 @@ def test_with_target(self):
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)
 
-top_level_options = lldb.SBExpressionOptions()
-top_level_options.SetAutoApplyFixIts(True)
-top_level_options.SetTopLevel(True)
-
 frame = self.thread.GetFrameAtIndex(0)
 
 # Try with one error:
@@ -45,15 +41,6 @@ def test_with_target(self):
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 10)
 
-# Try with one error in a top-level expression.
-# The Fix-It changes "ptr.m" to "ptr->m".
-expr = "struct X { int m; }; X x; X *ptr =  int m = ptr.m;"
-value = frame.EvaluateExpression(expr, top_level_options)
-# A successfully parsed top-level expression will yield an error
-# that there is 'no value'. If a parsing error would have happened we
-# would get a 
diff erent error kind, so let's check the error kind here.
-self.assertEquals(value.GetError().GetCString(), "error: No value")
-
 # Try with two errors:
 two_error_expression = "my_pointer.second->a"
 value = frame.EvaluateExpression(two_error_expression, options)



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

[Lldb-commits] [PATCH] D76872: [intel-pt] Fix existing support in LLDB

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253684.
wallace added a comment.

merge the commits of this diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76872

Files:
  lldb/tools/intel-features/CMakeLists.txt
  lldb/tools/intel-features/intel-pt/Decoder.cpp
  lldb/tools/intel-features/intel-pt/Decoder.h

Index: lldb/tools/intel-features/intel-pt/Decoder.h
===
--- lldb/tools/intel-features/intel-pt/Decoder.h
+++ lldb/tools/intel-features/intel-pt/Decoder.h
@@ -245,6 +245,22 @@
   lldb::SBError ) const;
   void DecodeTrace(struct pt_insn_decoder *decoder,
Instructions _list, lldb::SBError );
+  int HandlePTInstructionEvents(pt_insn_decoder *decoder, int errcode,
+Instructions _list,
+lldb::SBError );
+
+  int AppendErrorToInstructionList(int errcode, pt_insn_decoder *decoder,
+   Instructions _list,
+   lldb::SBError );
+
+  void AppendErrorWithOffsetToInstructionList(int errcode,
+  uint64_t decoder_offset,
+  Instructions _list,
+  lldb::SBError );
+
+  void AppendErrorWithoutOffsetToInstructionList(int errcode,
+ Instructions _list,
+ lldb::SBError );
 
   // Function to diagnose and indicate errors during raw trace decoding
   void Diagnose(struct pt_insn_decoder *decoder, int errcode,
Index: lldb/tools/intel-features/intel-pt/Decoder.cpp
===
--- lldb/tools/intel-features/intel-pt/Decoder.cpp
+++ lldb/tools/intel-features/intel-pt/Decoder.cpp
@@ -564,6 +564,61 @@
   }
 }
 
+void Decoder::AppendErrorWithOffsetToInstructionList(
+int errcode, uint64_t decoder_offset, Instructions _list,
+lldb::SBError ) {
+  sberror.SetErrorStringWithFormat(
+  "processor trace decoding library: \"%s\"  [decoder_offset] => "
+  "[0x%" PRIu64 "]",
+  pt_errstr(pt_errcode(errcode)), decoder_offset);
+  instruction_list.emplace_back(sberror.GetCString());
+}
+
+void Decoder::AppendErrorWithoutOffsetToInstructionList(
+int errcode, Instructions _list, lldb::SBError ) {
+  sberror.SetErrorStringWithFormat("processor trace decoding library: \"%s\"",
+   pt_errstr(pt_errcode(errcode)));
+  instruction_list.emplace_back(sberror.GetCString());
+}
+
+int Decoder::AppendErrorToInstructionList(int errcode, pt_insn_decoder *decoder,
+  Instructions _list,
+  lldb::SBError ) {
+  uint64_t decoder_offset = 0;
+  int errcode_off = pt_insn_get_offset(decoder, _offset);
+  if (errcode_off < 0) {
+AppendErrorWithoutOffsetToInstructionList(errcode, instruction_list,
+  sberror);
+return errcode_off;
+  }
+  AppendErrorWithOffsetToInstructionList(errcode, decoder_offset,
+ instruction_list, sberror);
+  return 0;
+}
+
+int Decoder::HandlePTInstructionEvents(pt_insn_decoder *decoder, int errcode,
+   Instructions _list,
+   lldb::SBError ) {
+  while (errcode & pts_event_pending) {
+pt_event event;
+errcode = pt_insn_event(decoder, , sizeof(event));
+if (errcode < 0)
+  return errcode;
+
+// The list of events are in
+// https://github.com/intel/libipt/blob/master/doc/man/pt_qry_event.3.md
+if (event.type == ptev_overflow) {
+  int append_errcode = AppendErrorToInstructionList(
+  errcode, decoder, instruction_list, sberror);
+  if (append_errcode < 0)
+return append_errcode;
+}
+// Other events don't signal stream errors
+  }
+
+  return 0;
+}
+
 // Start actual decoding of raw trace
 void Decoder::DecodeTrace(struct pt_insn_decoder *decoder,
   Instructions _list,
@@ -585,10 +640,8 @@
 
   int errcode_off = pt_insn_get_offset(decoder, _offset);
   if (errcode_off < 0) {
-sberror.SetErrorStringWithFormat(
-"processor trace decoding library: \"%s\"",
-pt_errstr(pt_errcode(errcode)));
-instruction_list.emplace_back(sberror.GetCString());
+AppendErrorWithoutOffsetToInstructionList(errcode, instruction_list,
+  sberror);
 return;
   }
 
@@ -619,16 +672,22 @@
   // progress further. Hence, returning in this situation.
   return;
 }
-sberror.SetErrorStringWithFormat(
-"processor trace decoding library: \"%s\"  [decoder_offset] => "
-  

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

The description confused me a bit as I thought we were going to be doing some 
"CommandObjectSource::DumpLinesInSymbolContexts()" stuff somewhere. But this 
path is really just "return the same source path from the setBreakpoints" 
request in the response and I am all for that. So a few nits and this will be 
good to go. See my inline comments.




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:335-336
+  llvm::json::Object source;
+  lldb::SBFileSpec file(sourcePath.str().c_str());
+  const char *name = file.GetFilename();
+  EmplaceSafeString(source, "name", name);

There might be a cheaper llvm::sys::path operation that can extract the 
basename from a StringRef instead of creating a SBFileSpec?



Comment at: lldb/tools/lldb-vscode/JSONUtils.h:207
+/// \param[in] sourcePath
+/// The full path to the source to store in the JSON value.
+///

Maybe reword a bit to make sure people understand this is the sourcePath that 
was pass in the setBreakpoint request?

```
The path that was specified in the setBreakpoint request.
```



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1375
   lldb::SBError status;
+
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));

remove white only change



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1377
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+
   if (status.Fail()) {

remove white only change




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D75750#1949678 , @fche2 wrote:

> In D75750#1949527 , @labath wrote:
>
> >
>
>
>
>
> > I am expecting that this feature will hook in very near to 
> > `DownloadObjectAndSymbolFile` for downloading the debug info, but it's not 
> > clear to me how would the source files fit in. Currently, debuginfod only 
> > provides an api to retrieve a single source file, so this code would have 
> > to parse all of the debug info, pry out the source files, and download them 
> > one by one -- a complicated and slow process.
>
> Yeah, as debuginfod does not support a batch type of source download, maybe 
> this particular lldb site is not an ideal fit..


We can make it ideal. debuginfod has nice stuff in it and we should adapt LLDB 
for sure! See my SymbolServer plug-in interface in my previous comments and let 
me know what you think.

> 
> 
>> (*) Though it seems very wasteful to download all files when we are going to 
>> need only a handful of them, it may not really be that way -- we're going to 
>> be downloading all of debug info anyway, and this is going to be much larger 
>> that all of source code put together.
> 
> I see your point, OTOH you only download the whole debuginfo because you 
> currently have no choice.  (Someday with debuginfod or such, you might be 
> able to offload the DWARF searches, and then you won't have to download the 
> whole thing.)  We do have the choice to download sources on demand.

We should be able to make this work lazily and not having to download all files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D75750#1949527 , @labath wrote:

> In D75750#1948273 , @clayborg wrote:
>
> > Currently we have a solution for macOS to locate symbol files in the 
> > "lldb/source/Symbol/LocateSymbolFile.cpp" file in the 
> > Symbols::LocateExecutableSymbolFile(...) function:
> >
> >   static FileSpec Symbols::LocateExecutableSymbolFile(const ModuleSpec 
> > _spec, const FileSpecList _search_paths);
> >
> >
> > This will locate any files that are already on the system and return the 
> > symbol file. When you don't have a symbol file, we can call:
> >
> >   static bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec _spec, 
> > bool force_lookup = true);
> >
> >
> > This might ping a build server and download the symbols.
> >
> > As for source file remappings, as Jim stated, on mac, each dSYM has path 
> > remappings already inside of it that are applied on the Module (not a 
> > target wide setting) itself and no modifications need to be done to the 
> > SourceManager.
> >
> > So my question is: can be use debuginfod to find the symbol file for a 
> > given build ID via Symbols::LocateExecutableSymbolFile(...), and when/if a 
> > symbol file is fetched from debuginfod, apply all path remappings to the 
> > module itself that we hand out? Then no changes would be needed in the 
> > SourceManager, we would just ask for a symbol file and get one back with 
> > all the remappings that are needed.
>
>
> I've been thinking about that a lot too. The thing that's not clear to me is, 
> does `DownloadObjectAndSymbolFile` download source files too? If so, how?


We should probably make a new SymbolServer plug-in and convert the Apple 
version to compile and be installed only for Apple. The API for this should be 
something like:

  class SymbolServer {
/// Get a cached symbol file that is already present on this machine.
/// This gets called by all LLDB during normal debugging to fetch 
/// and cached symbol files. 
virtual ModuleSP GetSymbolFile(ModuleSpec module_spec);
  
/// Download a symbol file when requested by the user. 
/// This only gets run in response to the use requesting the symbols, 
/// not as part of the normal debug work flow
virtual FileSpec DownloadSymbolFile(ModuleSpec module_spec);
  
/// New function that allows individual access to source files when 
/// they aren't available on disk.
virtual FileSpec GetSourceFile(FileSpec file, )
  };

Then debuginfod would fit right in there. The one thing that this interace 
doesn't cover is adding source remappings to modules, but it would be great if 
we can do this somehow with this new interface. Maybe 
SymbolServer::GetSymbolFile() can take a module_sp of the existing module so it 
can modify the source remappings if it has any?

> I am expecting that this feature will hook in very near to 
> `DownloadObjectAndSymbolFile` for downloading the debug info, but it's not 
> clear to me how would the source files fit in. Currently, debuginfod only 
> provides an api to retrieve a single source file, so this code would have to 
> parse all of the debug info, pry out the source files, and download them one 
> by one -- a complicated and slow process.

This would be taken care of by the SymbolServer plug-in described above. For 
the Apple version, it would download the symbol file and remap the paths as it 
already does and the SymbolServer::GetSourceFile(FileSpec file) would just 
return the FileSpec that was passed in since it already is an NFS mount path. 
For debuginfod it can grab the source file one by one.

One possibility is to apply a module level source remapping that is unique to 
the symbol file that is returned from SymbolServer::GetSymbolFile(). Maybe we 
prepend the UUID of the module to all paths in the debug info. Something like 
mapping:

  "/..." to "/ Now if debuginfod provided an api to download all source files in a single 
> request (*), that might be workable. However, in principle, I see nothing 
> wrong with being able to download the files on demand, if we have the option 
> to do that. (debuginfod's long term goal seems to be to provide an api to 
> download the debug info in chunks too -- that would be very interesting, 
> though I also expect it to be very complicated.)

Agreed, lazy is good. I don't see the need to download sources in chunks though.

So using SymbolServer with unique path mappings ("/..." to "/ (*) Though it seems very wasteful to download all files when we are going to 
> need only a handful of them, it may not really be that way -- we're going to 
> be downloading all of debug info anyway, and this is going to be much larger 
> that all of source code put together.

I think we should be able to figure out how to make this lazy with a new 
plug-in interface


Repository:
  rG LLVM Github Monorepo

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

[Lldb-commits] [PATCH] D76945: [lldb/CMake] Make check-lldb-* work for the standalone build.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG63aaecd5bebc: [lldb/CMake] Make check-lldb-* work for the 
standalone build. (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D76945?vs=253192=253656#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76945

Files:
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -1,6 +1,16 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+if(LLDB_BUILT_STANDALONE)
+  # In order to run check-lldb-* we need the correct map_config directives in
+  # llvm-lit. Because this is a standalone build, LLVM doesn't know about LLDB,
+  # and the lldb mappings are missing. We build our own llvm-lit, and tell LLVM
+  # to use the llvm-lit in the lldb build directory.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+set(LLVM_EXTERNAL_LIT ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/llvm-lit)
+  endif()
+endif()
+
 # Configure the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" 
CACHE PATH "The build root for building tests.")
 
@@ -180,3 +190,10 @@
 add_dependencies(check-lldb lldb-test-deps)
 set_target_properties(check-lldb PROPERTIES FOLDER "lldb misc")
 add_dependencies(check-lldb check-lldb-lit)
+
+if(LLDB_BUILT_STANDALONE)
+  # This has to happen *AFTER* add_lit_testsuite.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit 
${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
+  endif()
+endif()


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -1,6 +1,16 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+if(LLDB_BUILT_STANDALONE)
+  # In order to run check-lldb-* we need the correct map_config directives in
+  # llvm-lit. Because this is a standalone build, LLVM doesn't know about LLDB,
+  # and the lldb mappings are missing. We build our own llvm-lit, and tell LLVM
+  # to use the llvm-lit in the lldb build directory.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+set(LLVM_EXTERNAL_LIT ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/llvm-lit)
+  endif()
+endif()
+
 # Configure the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" CACHE PATH "The build root for building tests.")
 
@@ -180,3 +190,10 @@
 add_dependencies(check-lldb lldb-test-deps)
 set_target_properties(check-lldb PROPERTIES FOLDER "lldb misc")
 add_dependencies(check-lldb check-lldb-lit)
+
+if(LLDB_BUILT_STANDALONE)
+  # This has to happen *AFTER* add_lit_testsuite.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit ${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
+  endif()
+endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253651.
emrekultursay added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  int x125 = x124 * x123 + x122;
+  int x126 = x125 * x124 + x123;
+  int x127 = x126 * 

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 253649.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1364,15 +1364,17 @@
 llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
+
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+
   if (status.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", status.GetCString());
@@ -1748,13 +1750,15 @@
 const auto _bp = existing_source_bps->second.find(src_bp.line);
 if (existing_bp != existing_source_bps->second.end()) {
   existing_bp->second.UpdateBreakpoint(src_bp);
-  AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+  AppendSourceBreakpoint(existing_bp->second.bp, path, src_bp.line,
+ response_breakpoints);
   continue;
 }
   }
   // At this point the breakpoint is new
   src_bp.SetBreakpoint(path.data());
-  AppendBreakpoint(src_bp.bp, response_breakpoints);
+  AppendSourceBreakpoint(src_bp.bp, path, src_bp.line,
+ response_breakpoints);
   g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
 }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -196,6 +196,25 @@
 /// appended to it.
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array );
 
+/// Converts \a bp to a JSON value using the provided source line information to
+/// the \a breakpoints array.
+///
+/// \param[in] bp
+/// A LLDB breakpoint object which will provide basic information as ID and
+/// resolved status.
+///
+/// \param[in] sourcePath
+/// The full path to the source to store in the JSON value.
+///
+/// \param[in] line
+/// The line to store in the JSON value.
+///
+/// \param[in] breakpoints
+/// A JSON array that will get a llvm::json::Value for \a bp
+/// appended to it.
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array );
+
 /// Converts breakpoint location to a Visual Studio Code "Breakpoint"
 /// JSON object and appends it to the \a breakpoints array.
 ///
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -12,6 +12,7 @@
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
+#include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/Host/PosixApi.h"
 
@@ -319,10 +320,35 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint ,
+ llvm::StringRef sourcePath, int line) {
+  llvm::json::Object object;
+  if (!bp.IsValid())
+return llvm::json::Value(std::move(object));
+
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+
+  object.try_emplace("line", line);
+
+  llvm::json::Object source;
+  lldb::SBFileSpec file(sourcePath.str().c_str());
+  const char *name = file.GetFilename();
+  EmplaceSafeString(source, "name", name);
+  EmplaceSafeString(source, "path", sourcePath);
+  object.try_emplace("source", llvm::json::Value(std::move(source)));
+  return llvm::json::Value(std::move(object));
+}
+
 void AppendBreakpoint(lldb::SBBreakpoint , llvm::json::Array ) {
   breakpoints.emplace_back(CreateBreakpoint(bp));
 }
 
+void AppendSourceBreakpoint(lldb::SBBreakpoint , llvm::StringRef sourcePath,
+int line, llvm::json::Array ) {
+  breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line));
+}
+
 // "Event": {
 //   

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253648.
emrekultursay marked 6 inline comments as done.
emrekultursay added a comment.

PTAL. Applied suggested changes, thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  

[Lldb-commits] [lldb] 63aaecd - [lldb/CMake] Make check-lldb-* work for the standalone build.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-30T11:38:37-07:00
New Revision: 63aaecd5bebc8cf2cd4b4f2b42183978fd30ddef

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

LOG: [lldb/CMake] Make check-lldb-* work for the standalone build.

In order to run check-lldb-* we need the correct map_config directives
in llvm-lit. For the standalone build, LLVM doesn't know about LLDB, and
the lldb mappings are missing. In that case we build our own llvm-lit,
and tell LLVM to use the llvm-lit in the lldb build directory.

Differential revision: https://reviews.llvm.org/D76945

Added: 


Modified: 
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index d5b9fab1cd14..e86471609275 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -1,6 +1,16 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+if(LLDB_BUILT_STANDALONE)
+  # In order to run check-lldb-* we need the correct map_config directives in
+  # llvm-lit. Because this is a standalone build, LLVM doesn't know about LLDB,
+  # and the lldb mappings are missing. We build our own llvm-lit, and tell LLVM
+  # to use the llvm-lit in the lldb build directory.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+set(LLVM_EXTERNAL_LIT ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/llvm-lit)
+  endif()
+endif()
+
 # Configure the build directory.
 set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" 
CACHE PATH "The build root for building tests.")
 
@@ -180,3 +190,10 @@ add_custom_target(check-lldb)
 add_dependencies(check-lldb lldb-test-deps)
 set_target_properties(check-lldb PROPERTIES FOLDER "lldb misc")
 add_dependencies(check-lldb check-lldb-lit)
+
+if(LLDB_BUILT_STANDALONE)
+  # This has to happen *AFTER* add_lit_testsuite.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit 
${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
+  endif()
+endif()



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

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D76805#1949642 , @labath wrote:

> Ok, that makes kind of sense, though I am left wondering if we really need 
> this feature, given that we have survived so long without noticing it is 
> missing...
>
> Am I understanding it correctly that without this patch, we would only cache 
> the most recently accessed file (via `m_last_file_sp` member), and would 
> always reload when switching to a new file?


Yes, without this patch, only the most recently accessed file is cached inside 
that member, and switching to a new file replaces that entry. I guess this was 
designed for optimizing the case where the user hits "next line" while staying 
in the same file. Yet, if a breakpoint on a different file is hit during that 
"next line" command, we trigger a reload (twice).

This patch will increase memory usage, as mapped source files will stay in the 
cache, and thus, never be unmapped. The increase will be and proportional to 
the size of source files loaded by LLDB on-demand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805



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

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 2 inline comments as done.
wallace added inline comments.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py:25-43
+source_basename = 'main.cpp'
+source_path = os.path.join(os.getcwd(), source_basename)
+
+new_source_folder = os.path.join(os.path.dirname(os.getcwd()), 
'moved_location')
+new_source_path = os.path.join(new_source_folder, source_basename)
+
+def cleanup():

labath wrote:
> Please make sure none of these commands modify the source tree. You can look 
> at `test/API/source-manager/Makefile` for an example of how to build a binary 
> to reference files in the build tree.
thanks, will do



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1974
   // Add this breakpoint info to the response
   AppendBreakpoint(pair.second.bp, response_breakpoints);
 }

labath wrote:
> What about breakpoints in dynamically loaded shared libraries? Should you be 
> remembering the original path somewhere so that one you can set it here too?
This case is different, as function breakpoints are set by the IDE without 
referring to any source path, as they are equivalent to making 'b 
function_name'. Probably function breakpoints are not working correctly, so 
I'll have to fix it any of these days using the expensive query I mentioned 
above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

You also mentioned the `id` case failing as well. We should add that case to 
the test as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76964



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

[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:582
+static const ConstString Class_name("Class");
+if (name == id_name || name == Class_name) {
+  // Only disallow using "id" and "Class" if we are searching from the root

For these tiny strings a StringRef `==` comparison is going to be more 
efficient than constructing and storing a pointer to a ConstString.



Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:36
+"Make sure our expression evaluated without errors")
+self.assertTrue(expr_result.GetValue() == None,
+'Expression value is None')

`assertEqual(expr_result.GetValue(), None,  ...)` is better here, because it 
will print the result of `expr_result.GetValue()` in the failure case.



Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:38
+'Expression value is None')
+self.assertTrue(expr_result.GetType().GetName() == "a::Class",
+'Expression result type is "a::Class"')

same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76964



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

Re: [Lldb-commits] [lldb] 3806b38 - [LLDB] Initialize temporary token

[Shafik Yaghmour via lldb-commits]

Thank you for catching this and fixing it!

> On Mar 30, 2020, at 7:13 AM, Benjamin Kramer via lldb-commits 
>  wrote:
> 
> 
> Author: Benjamin Kramer
> Date: 2020-03-30T16:12:50+02:00
> New Revision: 3806b38045c08c674dc5db65bb06cf3dc34b9cc7
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/3806b38045c08c674dc5db65bb06cf3dc34b9cc7
> DIFF: 
> https://github.com/llvm/llvm-project/commit/3806b38045c08c674dc5db65bb06cf3dc34b9cc7.diff
> 
> LOG: [LLDB] Initialize temporary token
> 
> Found by msan.
> 
> Added: 
> 
> 
> Modified: 
>lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp 
> b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
> index a834be96444d..eca36fff18f8 100644
> --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
> +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
> @@ -347,7 +347,7 @@ bool CPlusPlusNameParser::ConsumeOperator() {
>   // If we find ( or < then this is indeed operator<< no need for fix.
>   if (n_token.getKind() != tok::l_paren && n_token.getKind() != 
> tok::less) {
> clang::Token tmp_tok;
> -
> +tmp_tok.startToken();
> tmp_tok.setLength(1);
> tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
> tmp_tok.setKind(tok::less);
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [lldb] 3788978 - Revert "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-30T17:32:42+02:00
New Revision: 37889786b040bd2bc48e5a01c6b92de777564a8f

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

LOG: Revert "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

This reverts commit because of test failures in TestHelloWorld.

It seems that this test (specifically running "ls" as a platform shell
command) depended on the implicit passing of the host environment.

The fix should be fairly simple (inherit the environment explicitly),
but it may take me a while to figure where exactly to do that. Revert
while I am figuring that out.

Added: 


Modified: 
lldb/source/Host/windows/ProcessLauncherWindows.cpp
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp 
b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index 00470f558e94..31101944d75c 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,9 +23,10 @@ using namespace lldb_private;
 namespace {
 void CreateEnvironmentBuffer(const Environment ,
  std::vector ) {
-  // The buffer is a list of null-terminated UTF-16 strings, followed by an
-  // extra L'\0' (two bytes of 0).  An empty environment must have one
-  // empty string, followed by an extra L'\0'.
+  if (env.size() == 0)
+return;
+
+  // Environment buffer is a null terminated list of null terminated strings
   for (const auto  : env) {
 std::wstring warg;
 if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -37,9 +38,6 @@ void CreateEnvironmentBuffer(const Environment ,
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
-  // Insert extra two bytes, just in case the environment was empty.
-  buffer.push_back(0);
-  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string ) {
@@ -96,7 +94,8 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo 
_info,
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  env_block = environment.data();
+  if (!environment.empty())
+env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index 29360856a735..c0cdc085f129 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -286,6 +286,7 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
 @skipIfRemote  # it doesn't make sense to send host env to remote target
+@skipIf(oslist=["windows"])
 def test_pass_host_env_vars(self):
 """Test that the host env vars are passed to the launched process."""
 self.build()



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[James Henderson via Phabricator via lldb-commits]

jhenderson added a comment.

In D74023#1949261 , @HsiangKai wrote:

> In D74023#1948388 , @MaskRay wrote:
>
> > The code generally looks good. For unittests, I think we can either make 
> > llvm-readobj -A canonical or the unittests canonical. If we decide to place 
> > tests on one place, we should delete most tests on the other side.
> >
> > My current preference is that we use more of unittests and leave the 
> > minimum to `test/llvm-readobj/ELF/{ARM,RISCV}/`
>
>
> Agree. I will remove redundant tests in 
> test/tools/llvm-readobj/ELF/{ARM,RISCV}/.


I also agree with testing primarily through the unit-tests where possible. 
However, there are probably aspects that still need to be tested via lit, 
namely code behaviour in ELFDumper.cpp, so just be slightly wary. High-level 
examples that might or might not be relevant for this patch are making sure 
that the output is printed correctly, and making sure errors/warnings can be 
threaded up to the final output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 253553.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,70 @@
+//===- unittests/RISCVAttributeParserTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value)
+  : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+  unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,63 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//
+//
+// Part of the LLVM Project, under the 

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

The code generally looks good. For unittests, I think we can either make 
llvm-readobj -A canonical or the unittests canonical. If we decide to place 
tests on one place, we should delete most tests on the other side.

My current preference is that we use more of unittests and leave the minimum to 
`test/llvm-readobj/ELF/{ARM,RISCV}/`




Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1689
+/// parseDirectiveAttribute
+///  ::= .attribute int, int [, "str"]
+///  ::= .attribute Tag_name, int [, "str"]

No space before `[`



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1723
+
+  StringRef StringValue = "";
+  int64_t IntegerValue = 0;

Delete `= ""`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai added a comment.

In D74023#1948388 , @MaskRay wrote:

> The code generally looks good. For unittests, I think we can either make 
> llvm-readobj -A canonical or the unittests canonical. If we decide to place 
> tests on one place, we should delete most tests on the other side.
>
> My current preference is that we use more of unittests and leave the minimum 
> to `test/llvm-readobj/ELF/{ARM,RISCV}/`


Agree. I will remove redundant tests in 
test/tools/llvm-readobj/ELF/{ARM,RISCV}/.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai added a comment.

@MaskRay, do you have any other comments about this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:102
+
+Streamer.emitIntValue(ELFAttrs::Format_Version, 1);
+  }

`emitInt8`



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:113
+
+  Streamer.emitIntValue(VendorHeaderSize + TagHeaderSize + ContentsSize, 4);
+  Streamer.emitBytes(CurrentVendor);

emitInt32



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:115
+  Streamer.emitBytes(CurrentVendor);
+  Streamer.emitIntValue(0, 1); // '\0'
+

emitInt8



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:42
+Arch = (Twine(Arch) + "_m2p0").str();
+
+  if (STI.hasFeature(RISCV::FeatureStdExtA))

These empty lines seem unneeded.



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:44
+  if (STI.hasFeature(RISCV::FeatureStdExtA))
+Arch = (Twine(Arch) + "_a2p0").str();
+

Just `Arch += "_a2p0";`



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:93
+   StringRef String) {
+  OS << "\t.attribute\t" << Attribute << ", \"" << String << "\"\n";
+}

Does `String` need to be escaped?



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:186
+void RISCVAsmPrinter::emitEndOfAsmFile(Module ) {
+  MCTargetStreamer  = *OutStreamer->getTargetStreamer();
+  RISCVTargetStreamer  = static_cast(TS);

`TS` can be inlined.



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:195
+  MCTargetStreamer  = *OutStreamer->getTargetStreamer();
+  RISCVTargetStreamer  = static_cast(TS);
+

TS can be inlined.



Comment at: llvm/test/CodeGen/RISCV/attributes.ll:25
+
+define i32 @addi(i32 %a) nounwind {
+  %1 = add i32 %a, 1

Delete `nounwind` (unneeded detail)



Comment at: llvm/test/CodeGen/RISCV/attributes.ll:29
+}
+

Delete empty line



Comment at: llvm/test/MC/RISCV/attribute-arch.s:38
+# CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
+

Delete empty line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai marked an inline comment as done.
HsiangKai added inline comments.



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:93
+   StringRef String) {
+  OS << "\t.attribute\t" << Attribute << ", \"" << String << "\"\n";
+}

MaskRay wrote:
> Does `String` need to be escaped?
There is no need to escape the string. The string will be generated in 
RISCVTargetStreamer::emitTargetAttributes().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D76835: [lldb] Fix TestSettings.test_pass_host_env_vars on windows

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rG908f78f3c198: [lldb] Fix 
TestSettings.test_pass_host_env_vars on windows (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D76835?vs=252785=253594#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76835

Files:
  lldb/source/Host/windows/ProcessLauncherWindows.cpp
  lldb/test/API/commands/settings/TestSettings.py


Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@
 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
 @skipIfRemote  # it doesn't make sense to send host env to remote target
-@skipIf(oslist=["windows"])
 def test_pass_host_env_vars(self):
 """Test that the host env vars are passed to the launched process."""
 self.build()
Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,10 +23,9 @@
 namespace {
 void CreateEnvironmentBuffer(const Environment ,
  std::vector ) {
-  if (env.size() == 0)
-return;
-
-  // Environment buffer is a null terminated list of null terminated strings
+  // The buffer is a list of null-terminated UTF-16 strings, followed by an
+  // extra L'\0' (two bytes of 0).  An empty environment must have one
+  // empty string, followed by an extra L'\0'.
   for (const auto  : env) {
 std::wstring warg;
 if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -38,6 +37,9 @@
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
+  // Insert extra two bytes, just in case the environment was empty.
+  buffer.push_back(0);
+  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string ) {
@@ -94,8 +96,7 @@
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-env_block = environment.data();
+  env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);


Index: lldb/test/API/commands/settings/TestSettings.py
===
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@
 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
 @skipIfRemote  # it doesn't make sense to send host env to remote target
-@skipIf(oslist=["windows"])
 def test_pass_host_env_vars(self):
 """Test that the host env vars are passed to the launched process."""
 self.build()
Index: lldb/source/Host/windows/ProcessLauncherWindows.cpp
===
--- lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,10 +23,9 @@
 namespace {
 void CreateEnvironmentBuffer(const Environment ,
  std::vector ) {
-  if (env.size() == 0)
-return;
-
-  // Environment buffer is a null terminated list of null terminated strings
+  // The buffer is a list of null-terminated UTF-16 strings, followed by an
+  // extra L'\0' (two bytes of 0).  An empty environment must have one
+  // empty string, followed by an extra L'\0'.
   for (const auto  : env) {
 std::wstring warg;
 if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -38,6 +37,9 @@
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
+  // Insert extra two bytes, just in case the environment was empty.
+  buffer.push_back(0);
+  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string ) {
@@ -94,8 +96,7 @@
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-env_block = environment.data();
+  env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76835: [lldb] Fix TestSettings.test_pass_host_env_vars on windows

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added inline comments.



Comment at: lldb/source/Host/windows/ProcessLauncherWindows.cpp:41
+  // double null.
+  if (env.empty()) {
+buffer.push_back(0);

amccarth wrote:
> There would be no harm in always adding the extra terminator.
Yeah, I wondered myself what would be less confusing... inserting a seemingly 
needless terminator, or cluttering the code with an extra if...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76835



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

[Lldb-commits] [lldb] 3806b38 - [LLDB] Initialize temporary token

[Benjamin Kramer via lldb-commits]

Author: Benjamin Kramer
Date: 2020-03-30T16:12:50+02:00
New Revision: 3806b38045c08c674dc5db65bb06cf3dc34b9cc7

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

LOG: [LLDB] Initialize temporary token

Found by msan.

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
index a834be96444d..eca36fff18f8 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -347,7 +347,7 @@ bool CPlusPlusNameParser::ConsumeOperator() {
   // If we find ( or < then this is indeed operator<< no need for fix.
   if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less) 
{
 clang::Token tmp_tok;
-
+tmp_tok.startToken();
 tmp_tok.setLength(1);
 tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
 tmp_tok.setKind(tok::less);



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

[Lldb-commits] [lldb] 908f78f - [lldb] Fix TestSettings.test_pass_host_env_vars on windows

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-30T16:06:52+02:00
New Revision: 908f78f3c19805ee7386186072407a541c9fb167

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

LOG: [lldb] Fix TestSettings.test_pass_host_env_vars on windows

Summary:
A defensive check in ProcessLauncherWindows meant that we would never
attempt to launch a process with a completely empty environment -- the
host environment would be used instead. Instead, I make the function add
an extra null wchar_t at the end of an empty environment. The
documentation on this is a bit fuzzy, but it seems to be what is needed
to make windows accept these kinds of environments.

Reviewers: amccarth, friss

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Host/windows/ProcessLauncherWindows.cpp
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp 
b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index 31101944d75c..00470f558e94 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,10 +23,9 @@ using namespace lldb_private;
 namespace {
 void CreateEnvironmentBuffer(const Environment ,
  std::vector ) {
-  if (env.size() == 0)
-return;
-
-  // Environment buffer is a null terminated list of null terminated strings
+  // The buffer is a list of null-terminated UTF-16 strings, followed by an
+  // extra L'\0' (two bytes of 0).  An empty environment must have one
+  // empty string, followed by an extra L'\0'.
   for (const auto  : env) {
 std::wstring warg;
 if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -38,6 +37,9 @@ void CreateEnvironmentBuffer(const Environment ,
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
+  // Insert extra two bytes, just in case the environment was empty.
+  buffer.push_back(0);
+  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string ) {
@@ -94,8 +96,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo 
_info,
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-env_block = environment.data();
+  env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index c0cdc085f129..29360856a735 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
 @skipIfRemote  # it doesn't make sense to send host env to remote target
-@skipIf(oslist=["windows"])
 def test_pass_host_env_vars(self):
 """Test that the host env vars are passed to the launched process."""
 self.build()



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

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

[Daniel Kiss via Phabricator via lldb-commits]

danielkiss added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2051
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[256]; // big enough to support up to 256 byte AArch64 SVE
+  // registers

Could we use the kMaxRegisterByteSize here? 


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

https://reviews.llvm.org/D77044



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

[Lldb-commits] [PATCH] D76840: [lldb] Fix another crash in covariant type handling

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b00eeb53de0: [lldb] Fix another crash in covariant type 
handling (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76840

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
  lldb/test/API/lang/cpp/covariant-return-types/main.cpp


Index: lldb/test/API/lang/cpp/covariant-return-types/main.cpp
===
--- lldb/test/API/lang/cpp/covariant-return-types/main.cpp
+++ lldb/test/API/lang/cpp/covariant-return-types/main.cpp
@@ -28,6 +28,23 @@
   OtherDerived& getOtherRef() override { return other_derived; }
 };
 
+// A regression test for a class with at least two members containing a
+// covariant function, which is referenced through another covariant function.
+struct BaseWithMembers {
+  int a = 42;
+  int b = 47;
+  virtual BaseWithMembers *get() { return this; }
+};
+struct DerivedWithMembers: BaseWithMembers {
+  DerivedWithMembers *get() override { return this; }
+};
+struct ReferencingBase {
+  virtual BaseWithMembers *getOther() { return new BaseWithMembers(); }
+};
+struct ReferencingDerived: ReferencingBase {
+  DerivedWithMembers *getOther() { return new DerivedWithMembers(); }
+};
+
 int main() {
   Derived derived;
   Base base;
@@ -36,5 +53,7 @@
   (void)base_ptr_to_derived->getRef();
   (void)base_ptr_to_derived->getOtherPtr();
   (void)base_ptr_to_derived->getOtherRef();
+
+  ReferencingDerived referencing_derived;
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
===
--- lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
+++ lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
@@ -38,3 +38,5 @@
 self.expect_expr("derived.getOtherRef().value()", 
result_summary='"derived"')
 self.expect_expr("base_ptr_to_derived->getOtherRef().value()", 
result_summary='"derived"')
 self.expect_expr("base.getOtherRef().value()", result_summary='"base"')
+
+self.expect_expr("referencing_derived.getOther()->get()->a", 
result_value='42')
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -997,6 +997,8 @@
   clang::RecordDecl *rd = return_type->getPointeeType()->getAsRecordDecl();
   if (!rd)
 return;
+  if (rd->getDefinition())
+return;
 
   importer.CompleteTagDecl(rd);
 }


Index: lldb/test/API/lang/cpp/covariant-return-types/main.cpp
===
--- lldb/test/API/lang/cpp/covariant-return-types/main.cpp
+++ lldb/test/API/lang/cpp/covariant-return-types/main.cpp
@@ -28,6 +28,23 @@
   OtherDerived& getOtherRef() override { return other_derived; }
 };
 
+// A regression test for a class with at least two members containing a
+// covariant function, which is referenced through another covariant function.
+struct BaseWithMembers {
+  int a = 42;
+  int b = 47;
+  virtual BaseWithMembers *get() { return this; }
+};
+struct DerivedWithMembers: BaseWithMembers {
+  DerivedWithMembers *get() override { return this; }
+};
+struct ReferencingBase {
+  virtual BaseWithMembers *getOther() { return new BaseWithMembers(); }
+};
+struct ReferencingDerived: ReferencingBase {
+  DerivedWithMembers *getOther() { return new DerivedWithMembers(); }
+};
+
 int main() {
   Derived derived;
   Base base;
@@ -36,5 +53,7 @@
   (void)base_ptr_to_derived->getRef();
   (void)base_ptr_to_derived->getOtherPtr();
   (void)base_ptr_to_derived->getOtherRef();
+
+  ReferencingDerived referencing_derived;
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
===
--- lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
+++ lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
@@ -38,3 +38,5 @@
 self.expect_expr("derived.getOtherRef().value()", result_summary='"derived"')
 self.expect_expr("base_ptr_to_derived->getOtherRef().value()", result_summary='"derived"')
 self.expect_expr("base.getOtherRef().value()", result_summary='"base"')
+
+self.expect_expr("referencing_derived.getOther()->get()->a", result_value='42')
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ 

[Lldb-commits] [PATCH] D76840: [lldb] Fix another crash in covariant type handling

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the review, and a pointer to `CompleteTagDeclsScope`. While looking 
at this, I got some ideas on how to reduce the number of chained imports here 
(nothing magical -- just avoid importing the type if the base and derived 
return types are identical (no covariance). I'll try to remove the recursive 
imports before hacking on this further (but I don't know when exactly that will 
happen).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76840



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

[Lldb-commits] [lldb] 7b00eeb - [lldb] Fix another crash in covariant type handling

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-30T16:00:21+02:00
New Revision: 7b00eeb53de06c7979fd2a88436c3387d0492ee8

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

LOG: [lldb] Fix another crash in covariant type handling

Summary:
D73024 seems to have fixed one set crash, but it introduced another.
Namely, if a class contains a covariant method returning itself, the
logic in MaybeCompleteReturnType could cause us to attempt a recursive
import, which would result in an assertion failure in
clang::DeclContext::removeDecl.

For some reason, this only manifested itself if the class contained at
least two member variables, and the class itself was imported as a
result of a recursive covariant import.

This patch fixes the crash by not attempting to import classes which are
already completed in MaybeCompleteReturnType. However, it's not clear to
me if this is the right fix, or if this should be handled automatically
by functions lower in the stack.

Reviewers: teemperor, shafik

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
lldb/test/API/lang/cpp/covariant-return-types/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 479cb64deefd..4b3e237dc62c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -997,6 +997,8 @@ static void MaybeCompleteReturnType(ClangASTImporter 
,
   clang::RecordDecl *rd = return_type->getPointeeType()->getAsRecordDecl();
   if (!rd)
 return;
+  if (rd->getDefinition())
+return;
 
   importer.CompleteTagDecl(rd);
 }

diff  --git 
a/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py 
b/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
index 86a6ddd6e47b..6f2b3eafd2e9 100644
--- a/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
+++ b/lldb/test/API/lang/cpp/covariant-return-types/TestCovariantReturnTypes.py
@@ -38,3 +38,5 @@ def test(self):
 self.expect_expr("derived.getOtherRef().value()", 
result_summary='"derived"')
 self.expect_expr("base_ptr_to_derived->getOtherRef().value()", 
result_summary='"derived"')
 self.expect_expr("base.getOtherRef().value()", result_summary='"base"')
+
+self.expect_expr("referencing_derived.getOther()->get()->a", 
result_value='42')

diff  --git a/lldb/test/API/lang/cpp/covariant-return-types/main.cpp 
b/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
index 2a6fc682c39a..ea05043d6cf9 100644
--- a/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
+++ b/lldb/test/API/lang/cpp/covariant-return-types/main.cpp
@@ -28,6 +28,23 @@ struct Derived : public Base {
   OtherDerived& getOtherRef() override { return other_derived; }
 };
 
+// A regression test for a class with at least two members containing a
+// covariant function, which is referenced through another covariant function.
+struct BaseWithMembers {
+  int a = 42;
+  int b = 47;
+  virtual BaseWithMembers *get() { return this; }
+};
+struct DerivedWithMembers: BaseWithMembers {
+  DerivedWithMembers *get() override { return this; }
+};
+struct ReferencingBase {
+  virtual BaseWithMembers *getOther() { return new BaseWithMembers(); }
+};
+struct ReferencingDerived: ReferencingBase {
+  DerivedWithMembers *getOther() { return new DerivedWithMembers(); }
+};
+
 int main() {
   Derived derived;
   Base base;
@@ -36,5 +53,7 @@ int main() {
   (void)base_ptr_to_derived->getRef();
   (void)base_ptr_to_derived->getOtherPtr();
   (void)base_ptr_to_derived->getOtherRef();
+
+  ReferencingDerived referencing_derived;
   return 0; // break here
 }



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

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Target/TargetProperties.td:183
 Desc<"A path to a python OS plug-in module file that contains a 
OperatingSystemPlugIn class.">;
+  def PluginReportsAllThreads: Property<"plugin-reports-all-threads", 
"Boolean">,
+Global,

jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > If this is relevant only for os plugins, then it would be good to 
> > > > reflect that in the name as well.
> > > I thought about that.  In the future a regular Process plugin might 
> > > decide it was too expensive to report all threads as well.  There's 
> > > nothing in this patch that wouldn't "just work" with that case as well.  
> > > Leaving the OS out was meant to indicate this was about how the Process 
> > > plugin OR any of its helpers (e.g. the OS Plugin) produces threads.
> > Well, I am hoping that if we ever extend this support to the regular 
> > process plugins, we will implement it in a way where the plugin itself can 
> > tell us whether it is operating/supporting this mode (which I guess would 
> > involve a new gdb-remote packet and some specification of what exactly 
> > should happen when it gets sent), instead of relying on the user to set 
> > this correctly.
> > 
> > I mean, in an ideal world this is I would want to happen with the python 
> > plugins as well, but it seems that we are stuck with some existing plugins 
> > which already do that. However, I wouldn't want to plan on the same thing 
> > happening again. :)
> Right, I put this in mostly to help backwards compatibility.  For instance, 
> another OS Plugin I know about handles some older cooperative threading 
> scheme.  That one does report all threads on each stop.  I didn't want to 
> force them to do anything to keep their plugin working as well as it did 
> before.  That's also why I set the default to true here.
> 
> Even when we have plugins that actually support not reporting all threads, 
> you could imagine somebody having a Process plugin that supports both modes - 
> particularly early in the development of its support for this feature, and in 
> some corner cases the "doesn't report all threads" mode has some subtle 
> problem.  Having this setting will allow people to get the slower but more 
> assuredly correct behavior till it works 100% reliably.  So I still think the 
> setting has some value.
> 
> But I agree, going forward there should be some kind of handshake between the 
> ThreadPlanStackMap and the Process Plugin, either a "I've reported all 
> threads now" which could trigger a prune, or a "Is TID X still alive" which 
> the generic code could use to balance the cost of keeping outdated stacks 
> alive against when we want to ask about all threads.
All of this sounds fine, and I wouldn't mind having a setting like that, even 
after the support for partial thread lists is considered "stable". However, 
that sounds like a different setting to me -- that's like a directive to the 
process plugin about how it should behave, whereas this setting is more like a 
statement of fact about what the plugin does.

The setting could be later repurposed to do both, but it's not clear to me 
whether that is useful. Like, since we already support plugin-specific 
settings, the plugin which decides to implement both modes of operation could 
expose that as a custom setting. That way, one wouldn't get the impression that 
this setting applies to any process plugin...



Comment at: 
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp:40
+  std::thread thread_1(thread_func);
+  g_cv.wait(main_lock);
+  g_value = 1;

spurious wake up danger here too



Comment at: 
lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py:23
+
+def check_list_output(self, command, active_plans = [], completed_plans = 
[], discarded_plans = []):
+# Check the "thread plan list" output against a list of active & 
completed and discarded plans.

jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > this looks like a model example for using `self.filecheck`
> > > I don't see that.  Do you mean write a file check matching file with the 
> > > contents of the three arrays in this function, and then run file check on 
> > > that?  Or did you mean convert the call sites into embedded patterns and 
> > > then call file check on myself?  But then I'd have to also embed the 
> > > headers in the body such that they came in the right order for all the 
> > > tests.  Neither of these seem like attractive options to me.  It doesn't 
> > > seem like it will be hard to maintain this little checker, and I don't 
> > > see what I would gain by using file check instead.
> > What I meant was doing doing something like:
> > ```
> > self.filecheck("thread plan list %d"%(current_id), __file__, 
> > 

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76471#1947250 , @aprantl wrote:

> I've reworked this a little based on your feedback.
>
> First, I've renamed `SDK` to `XcodeSDK`. An Xcode SDK is a fairly specific 
> concept and I'm not going to pretend that it makes sense to generalize it for 
> something else, so I thought the name should reflect this. I've kept it in 
> Utility though, since the functionality mirrors `ArchSpec` and one platform 
> typically hosts many SDKs at the same time. I entertained the idea of 
> creating a base class and each platform would implement its own SDK, which 
> sounds neat, but with all the merging functionality needed, this doesn't 
> really work out.


Thanks.

That sounds fine. I don't have a problem with a class like this residing in 
Utility. I also don't think it's time to try to generalize this just yet, as 
its very likely that the generalized concept would not fit what some other 
platform wants to do. I just think some of the apis should reflect the 
specificity of this more. Some inline comments reflect that.

> I did incorporate the suggestion of requesting a platform (because I do think 
> it should be Platform or Host that is calling `xcrun`) via the Module's 
> ArchSpec.

That sounds fine to me, though I'd like to hear @jingham's take on Platform in 
this way. The part that I'm not super-clear on is the `type` argument to the 
`GetSDKPath` function. I mean, if I look at the `XcodeSDK::Type` enum, then the 
list pretty much mirrors the list if Platform classes we have. So asking a 
`PlatformRemoteAppleWatch` for an "AppleTV" sdk sounds nonsensical, and one 
gets the impression that this information should be already encoded in the 
selection of the platform object. It probably already is, via the ArchSpec 
argument, no?

In case of Apple platforms, this won't make a difference in practice, since the 
support for that is implemented in PlatformDarwin (which all of these inherit 
from), but it sounds like this will be a problem for the "linux" sdk (assuming 
this is what I think it is), as there the selected platform will be 
PlatformLinux, which has no clue about these sdk thingies.

So, it sounds to me like the sdk type should somehow play a role in the 
selection of the platform instance, or this functionality should be moved down 
into some Host function.

> Finally I added unit tests for all of the SDK functionality.
> 
> I have no great idea for how to test the remapping functionality other than 
> completely end-to-end, because its primary effect is showing when debugging 
> the OS itself (you need to resolve files *inside* the SDK).

I guess this is down to our current inability to "mock" `xcrun` responses (?) 
It might be possible to do something about that in a unit test with a custom 
platform plugin, but it seems that the amount of untested functionality here is 
not very big (Module::RegisterSDK, basically), so we could let that slide.




Comment at: lldb/include/lldb/Core/Module.h:517
+  /// \param sysroot will be added to the path remapping dictionary.
+  void RegisterSDK(llvm::StringRef sdk, llvm::StringRef sysroot);
+

RegisterXCodeSDK?



Comment at: lldb/include/lldb/Core/Module.h:983
+  /// The SDK this module was compiled with.
+  XcodeSDK m_sdk;
+  

m_xcode_sdk



Comment at: lldb/include/lldb/Target/Platform.h:437
 
+  virtual ConstString GetSDKPath(int type) { return {}; }
+

GetXCodeSDKPath? (and maybe the argument can be the proper enum type now.



Comment at: lldb/include/lldb/Utility/XcodeSDK.h:24
+  XcodeSDK() = default;
+  XcodeSDK(std::string &) : m_name(name) {}
+

the fancy `&&` is useless here without `std::move` (and it's not even very 
useful with it).


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

https://reviews.llvm.org/D76471



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

[Lldb-commits] [lldb] 11a5cae - [lldb][NFC] Refactor Fix-It filter for warnings

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T14:01:16+02:00
New Revision: 11a5caee2aeae2546213366e7fc54095bb8163b9

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

LOG: [lldb][NFC] Refactor Fix-It filter for warnings

LLDB only automatically applies Fix-Its from errors, but not from warnings.

Currently we only store Fix-Its from errors and then later apply all Fix-Its
we stored. This moves the filter to the application phase, so that we now
store *all* Fix-Its but only apply Fix-Its from errors later on.

This is NFC preparation for an upcoming patch.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index e5de4b4651df..b3880ce03b08 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -214,16 +214,12 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
   auto new_diagnostic = std::make_unique(
   stripped_output, severity, Info.getID());
 
-  // Don't store away warning fixits, since the compiler doesn't have
-  // enough context in an expression for the warning to be useful.
   // FIXME: Should we try to filter out FixIts that apply to our generated
   // code, and not the user's expression?
-  if (severity == eDiagnosticSeverityError) {
-for (const clang::FixItHint  : Info.getFixItHints()) {
-  if (fixit.isNull())
-continue;
-  new_diagnostic->AddFixitHint(fixit);
-}
+  for (const clang::FixItHint  : Info.getFixItHints()) {
+if (fixit.isNull())
+  continue;
+new_diagnostic->AddFixitHint(fixit);
   }
 
   m_manager->AddDiagnostic(std::move(new_diagnostic));
@@ -1127,6 +1123,10 @@ bool ClangExpressionParser::RewriteExpression(
   continue;
 if (!diagnostic->HasFixIts())
   continue;
+// Don't apply warning Fix-Its, since the compiler doesn't have enough
+// context in an expression for the warning to be useful.
+if (diagnostic->GetSeverity() != eDiagnosticSeverityError)
+  continue;
 for (const FixItHint  : diagnostic->FixIts())
   ApplyFixIt(fixit, commit);
   }



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

[Lldb-commits] [lldb] 502a06f - [lldb] Make TestExprDiagnostics.py pass again after enabling Fix-Its in test

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T13:52:09+02:00
New Revision: 502a06fcdafa637a9890da16c2734bc1a36010f6

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

LOG: [lldb] Make TestExprDiagnostics.py pass again after enabling Fix-Its in 
test

Commit 83c81c0a469482888482983c302c09c02680ae7c enabled Fix-Its for top-level
expressions which change the error message of this test here as Clang comes
up with a strange Fix-It for this expression. This patch just changes the
test to declare a void variable so that Clang doesn't see a way to
recover with a Fix-It and change the error message.

Added: 


Modified: 
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py 
b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index da29d7b2c1af..b5eb552badb5 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -60,10 +60,10 @@ def test_source_and_caret_printing(self):
 self.assertIn(":1:10", 
value.GetError().GetCString())
 
 # Multiline top-level expressions.
-value = frame.EvaluateExpression("void x() {}\nvoid foo(unknown_type 
x) {}", top_level_opts)
+value = frame.EvaluateExpression("void x() {}\nvoid foo;", 
top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", 
value.GetError().GetCString())
-self.assertIn(":2:10", 
value.GetError().GetCString())
+self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn(":2:6", value.GetError().GetCString())
 
 # Test that we render Clang's 'notes' correctly.
 value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; 
};", top_level_opts)



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Frank Ch. Eigler via Phabricator via lldb-commits]

fche2 added a comment.

In D75750#1949527 , @labath wrote:

>




> I am expecting that this feature will hook in very near to 
> `DownloadObjectAndSymbolFile` for downloading the debug info, but it's not 
> clear to me how would the source files fit in. Currently, debuginfod only 
> provides an api to retrieve a single source file, so this code would have to 
> parse all of the debug info, pry out the source files, and download them one 
> by one -- a complicated and slow process.

Yeah, as debuginfod does not support a batch type of source download, maybe 
this particular lldb site is not an ideal fit..

> (*) Though it seems very wasteful to download all files when we are going to 
> need only a handful of them, it may not really be that way -- we're going to 
> be downloading all of debug info anyway, and this is going to be much larger 
> that all of source code put together.

I see your point, OTOH you only download the whole debuginfo because you 
currently have no choice.  (Someday with debuginfod or such, you might be able 
to offload the DWARF searches, and then you won't have to download the whole 
thing.)  We do have the choice to download sources on demand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py:17
+
+@skipIf(oslist=no_match(["windows"]))
+def test_set_use_source_cache_false(self):

Could you remove this decorator? The test is not extremely interesting on 
non-windows hosts, but the flow should still work, so there's no harm in 
testing it.



Comment at: 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py:22
+
+@skipIf(oslist=no_match(["windows"]))
+def test_set_use_source_cache_true(self):

technically, this should be `hostoslist=no_match(...)`



Comment at: 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py:30-70
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Enable/Disable source cache
+self.runCmd(
+"settings set use-source-cache " +

All of this could be done via a single `lldbutil.run_to_source_breakpoint` call 
(that's a fairly new thing, so it's possible the test you based this on is not 
using it yet).



Comment at: 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py:40
+# Get paths for source files: header.h, header2.h
+src_file = self.getSourcePath("header.h")
+self.assertTrue(src_file)

Please avoid modifying the source tree. You can take a look at 
`test/API/source-manager/Makefile` to see how to build binaries with sources in 
the build tree.



Comment at: 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py:86-87
+
+if is_cache_enabled and is_file_removed:
+raise Exception("Source cache is enabled, but delete file 
succeeded")
+

maybe:
```
if is_cache_enabled:
  self.assertFalse(is_file_removed)
```
(and similarly for the other case).



Comment at: lldb/test/API/commands/settings/use_source_cache/header.h:1
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:

Does this need to be in a separate file? Could you just put it into main.cpp ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D73206#1949516 , @labath wrote:

> The main thing I don't like about the current state is the introduction of 
> user_id_t into the index classes. It forces a bunch of conversions for a flow 
> that really should be simple.
>
> I guess that was to avoid main-cu-ificating the DIERef class?


Yes, that was the purpose of D74637 . To have 
two different small (8 bytes) DIE references, one with MainCU (chosen to be 
`user_id_t`) and one without MainCU (chosen to be `DIERef`).

> I think (and I think I've already said this) the right thing to do here would 
> be to change the index classes to be callback-based. Then the memory 
> footprint is out of the equation (no vector, and the indexes can 
> return a "main cu pair" or something of sorts, as that is what their callers 
> will convert this to anyway.

OK, I can make an unrelated trunk-based refactorization for `DWARFIndex` 
descendants to use callbacks.

Internally `ManualDWARFIndex` and `DebugNamesDWARFIndex` need to store also 
MainCU so `user_id_t` looks to me as their best choice for their internal 
storage. It is true `AppleDWARFIndex` does not need MainCU so that would 
prevent its `DIERef`->`user_id_t` conversion, that is the only win there I can 
see. One could also refactor `AppleDWARFIndex` even more to use `user_id_t` 
natively. Would then be there still a win with the callback?

Or is the callback preferred unrelated to any `DIERef` vs. `user_id_t`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



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

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76805#1944198 , @emrekultursay 
wrote:

> > Does this actually depend on the other patch? It looks like an independent 
> > fix we could commit separately.
>
> This bug seems to have existed forever. Fixing it means we will have source 
> file cache enabled for the first time. If it causes any unforeseen issues, 
> I'd like users to have the ability to disable the cache, which is why I made 
> this change depend on the other change.


Ok, that makes kind of sense, though I am left wondering if we really need this 
feature, given that we have survived so long without noticing it is missing...

Am I understanding it correctly that without this patch, we would only cache 
the most recently accessed file (via `m_last_file_sp` member), and would always 
reload when switching to a new file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805



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

[Lldb-commits] [PATCH] D77000: [LLDB] [PECOFF] Only use PECallFrameInfo on the one supported architecture

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D77000#1949247 , @aleksandr.urakov 
wrote:

> Hello! But does the format of an exception directory for aarch64 differ 
> completely from x86-64 one?


As far as I know, it's pretty different yes. There's one format for arm32 as 
well (which isn't quite as thoroughly supported within llvm iirc) which I think 
is pretty close to the one for aarch64.

> Can we somehow adopt the existing parser to support it?

Not sure if it'd be an enhancement to the existing class or would warrant a 
separate class.

> If we can, I think that this check looks good (we encapsulate the difference 
> inside `PECallFrameInfo`). Or may be it is worth to make a different parser 
> for aarch64? Then I think the check would look better in 
> `ObjectFilePECOFF::CreateEHCallFrameInfo` (and then we will choose a right 
> exception directory parser depending on an architecture). What do you think 
> on this?



In D77000#1949428 , @labath wrote:

> I think it would make sense to put the function there (I guess you meant 
> `ObjectFilePECOFF::CreateCallFrameInfo`) even if we later do end up adding 
> arm support to the `PECallFrameInfo` class).


Yeah, that place does indeed look much nicer than where I placed the check 
initially - will update the patch.

>> Not sure how easy it is to make a sensible test for this; it makes a 
>> difference when the aarch64 RuntimeFunction struct, when interpreted as an 
>> x86_64 RuntimeFunction, happens to falsely match address ranges that the 
>> unwinder looks for
> 
> One could construct a yaml file with a "falsely matching record" and then run 
> "image show-unwind" to demonstrate that the record is _not_ parsed. The 
> tricky part there is that this command only works on  "live" processes (one 
> day i'd like to change that, but right now it's hard because of... reasons). 
> The "breakpad" unwinding tests work around that by creating a process out of 
> a minidump core file. That might be overkill for a change of this type...

The issue where I noticed this was actually much more complicated than that. 
There was no visible traces of PECallFrameInfo in `image show-unwind` anywhere.

The situation I looked at was two nearly identical binaries, one with a few 
other libraries (not involved in the unwind) linked dynamically, one with them 
linked statically. Backtraces from one of them worked nicely, while the other 
one didn't give any useful backtrace. `image show-unwind -a ` on the 
crash location gave the exact same output on both of them, saying `Asynchronous 
(not restricted to call-sites) UnwindPlan is 'EmulateInstructionARM64'` and the 
same emulated interpretation of the function. The call frame info based unwind 
plan didn't actually show up anywhere there.

However even if `image show-unwind` indicated that `EmulateInstructionARM64` 
would be used, in practice it fell back to using the arch default unwind plan, 
on the binary where backtraces failed. The reason for this was that 
`PECallFrameInfo::GetAddressRange` reported a bogus length for the function, 
which then caused 
`UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly` to fail 
because `process_sp->GetTarget().ReadMemory` failed, because it tried to read a 
too long range.


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

https://reviews.llvm.org/D77000



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

[Lldb-commits] [PATCH] D77000: [LLDB] [PECOFF] Only use PECallFrameInfo on the one supported architecture

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 253548.
mstorsjo retitled this revision from "[lldb] [PECOFF] Check that 
PECallFrameInfo operates on the expected architecture before interpreting 
RuntimeFunction structs" to "[LLDB] [PECOFF] Only use PECallFrameInfo on the 
one supported architecture".
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Moved the check up to ObjectFilePECOFF.


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

https://reviews.llvm.org/D77000

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -782,6 +782,9 @@
   if (!data_dir_exception.vmaddr)
 return {};
 
+  if (m_coff_header.machine != llvm::COFF::IMAGE_FILE_MACHINE_AMD64)
+return {};
+
   return std::make_unique(*this, data_dir_exception.vmaddr,
data_dir_exception.vmsize);
 }


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -782,6 +782,9 @@
   if (!data_dir_exception.vmaddr)
 return {};
 
+  if (m_coff_header.machine != llvm::COFF::IMAGE_FILE_MACHINE_AMD64)
+return {};
+
   return std::make_unique(*this, data_dir_exception.vmaddr,
data_dir_exception.vmsize);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and ptrace support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: labath, clayborg, jankratochvil, jasonmolenda.
Herald added subscribers: danielkiss, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.

This patch adds support for AArch64 SVE register infos description and register 
access via ptrace.

AArch64 SVE is a an optional extension of Arm v8.3-a architecture which 
introduces 32 vector registers Z, 16 predicate P registers and FFR predicate 
register. These registers have fixed names but can dynamically be configured to 
different size based on underlying OS configuration.

This patch adds register info struct that describes SVE register infos. Also 
provides native register context routines to access SVE registers. It 
introduces a mechanism to configure SVE register size and offsets at startup 
before exchanging register information across gdb-remote process.

It makes use of linux kernel definitions copied into 
lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h for backward 
compatibility with sysroots which might yet not support SVE definitions.

There are two test cases added to the testsuite one of them checks register 
size configuration based on vg (vector granule register). Also a test is added 
which verifies registers can be read and written.

There is no physical hardware currently available to test SVE and we make use 
of QEMU for the purpose of testing this feature.


https://reviews.llvm.org/D77047

Files:
  lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
  lldb/source/Utility/ARM64_DWARF_Registers.h
  lldb/source/Utility/ARM64_ehframe_Registers.h
  lldb/test/API/commands/register/register/aarch64_sve_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/main.c
@@ -0,0 +1,5 @@
+int main() {
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.
+}
\ No newline at end of file
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
@@ -0,0 +1,128 @@
+"""
+Test the AArch64 SVE registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Verify "%s" == %i' % (name, expected))
+
+mydir = TestBase.compute_mydir(__file__)
+@skipIf
+def test_sve_registers_configuration(self):
+"""Test AArch64 SVE registers size configuration."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs = ["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+for registerSet in currentFrame.GetRegisters():
+if 'sve registers' in registerSet.GetName().lower():
+has_sve = True
+
+if not has_sve:
+self.skipTest('SVE registers must be supported.')
+   

[Lldb-commits] [PATCH] D77042: [lldb] Make Fix-Its also apply to top-level expressions

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83c81c0a4694: [lldb] Make Fix-Its also apply to top-level 
expressions (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77042

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/test/API/commands/expression/fixits/TestFixIts.py


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -33,6 +33,10 @@
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)
 
+top_level_options = lldb.SBExpressionOptions()
+top_level_options.SetAutoApplyFixIts(True)
+top_level_options.SetTopLevel(True)
+
 frame = self.thread.GetFrameAtIndex(0)
 
 # Try with one error:
@@ -41,6 +45,15 @@
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 10)
 
+# Try with one error in a top-level expression.
+# The Fix-It changes "ptr.m" to "ptr->m".
+expr = "struct X { int m; }; X x; X *ptr =  int m = ptr.m;"
+value = frame.EvaluateExpression(expr, top_level_options)
+# A successfully parsed top-level expression will yield an error
+# that there is 'no value'. If a parsing error would have happened we
+# would get a different error kind, so let's check the error kind here.
+self.assertEquals(value.GetError().GetCString(), "error: No value")
+
 # Try with two errors:
 two_error_expression = "my_pointer.second->a"
 value = frame.EvaluateExpression(two_error_expression, options)
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -616,15 +616,14 @@
   if (parser.RewriteExpression(diagnostic_manager)) {
 size_t fixed_start;
 size_t fixed_end;
-const std::string _expression =
-diagnostic_manager.GetFixedExpression();
+m_fixed_text = diagnostic_manager.GetFixedExpression();
 // Retrieve the original expression in case we don't have a top level
 // expression (which has no surrounding source code).
 if (m_source_code &&
-m_source_code->GetOriginalBodyBounds(fixed_expression, m_expr_lang,
+m_source_code->GetOriginalBodyBounds(m_fixed_text, m_expr_lang,
  fixed_start, fixed_end))
   m_fixed_text =
-  fixed_expression.substr(fixed_start, fixed_end - fixed_start);
+  m_fixed_text.substr(fixed_start, fixed_end - fixed_start);
   }
 }
 return false;


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -33,6 +33,10 @@
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)
 
+top_level_options = lldb.SBExpressionOptions()
+top_level_options.SetAutoApplyFixIts(True)
+top_level_options.SetTopLevel(True)
+
 frame = self.thread.GetFrameAtIndex(0)
 
 # Try with one error:
@@ -41,6 +45,15 @@
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 10)
 
+# Try with one error in a top-level expression.
+# The Fix-It changes "ptr.m" to "ptr->m".
+expr = "struct X { int m; }; X x; X *ptr =  int m = ptr.m;"
+value = frame.EvaluateExpression(expr, top_level_options)
+# A successfully parsed top-level expression will yield an error
+# that there is 'no value'. If a parsing error would have happened we
+# would get a different error kind, so let's check the error kind here.
+self.assertEquals(value.GetError().GetCString(), "error: No value")
+
 # Try with two errors:
 two_error_expression = "my_pointer.second->a"
 value = frame.EvaluateExpression(two_error_expression, options)
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -616,15 +616,14 @@
   if (parser.RewriteExpression(diagnostic_manager)) {
 

[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Overall, I think this is workable. The main thing I don't like about the 
current state is the introduction of user_id_t into the index classes. It 
forces a bunch of conversions for a flow that really should be simple.

I guess that was to avoid main-cu-ificating the DIERef class? I think (and I 
think I've already said this) the right thing to do here would be to change the 
index classes to be callback-based. Then the memory footprint is out of the 
equation (no vector, and the indexes can return a "main cu pair" or 
something of sorts, as that is what their callers will convert this to anyway.

I still see other issues (too much knowledge of "main cus" inside DWARFDie, 
inconsistent DWARFCompileUnit/CompileUnit usage in DWARFASTParser), but 
handling these can wait a bit...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D75750#1948273 , @clayborg wrote:

> Currently we have a solution for macOS to locate symbol files in the 
> "lldb/source/Symbol/LocateSymbolFile.cpp" file in the 
> Symbols::LocateExecutableSymbolFile(...) function:
>
>   static FileSpec Symbols::LocateExecutableSymbolFile(const ModuleSpec 
> _spec, const FileSpecList _search_paths);
>
>
> This will locate any files that are already on the system and return the 
> symbol file. When you don't have a symbol file, we can call:
>
>   static bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec _spec, 
> bool force_lookup = true);
>
>
> This might ping a build server and download the symbols.
>
> As for source file remappings, as Jim stated, on mac, each dSYM has path 
> remappings already inside of it that are applied on the Module (not a target 
> wide setting) itself and no modifications need to be done to the 
> SourceManager.
>
> So my question is: can be use debuginfod to find the symbol file for a given 
> build ID via Symbols::LocateExecutableSymbolFile(...), and when/if a symbol 
> file is fetched from debuginfod, apply all path remappings to the module 
> itself that we hand out? Then no changes would be needed in the 
> SourceManager, we would just ask for a symbol file and get one back with all 
> the remappings that are needed.


I've been thinking about that a lot too. The thing that's not clear to me is, 
does `DownloadObjectAndSymbolFile` download source files too? If so, how?

I am expecting that this feature will hook in very near to 
`DownloadObjectAndSymbolFile` for downloading the debug info, but it's not 
clear to me how would the source files fit in. Currently, debuginfod only 
provides an api to retrieve a single source file, so this code would have to 
parse all of the debug info, pry out the source files, and download them one by 
one -- a complicated and slow process.

Now if debuginfod provided an api to download all source files in a single 
request (*), that might be workable. However, in principle, I see nothing wrong 
with being able to download the files on demand, if we have the option to do 
that. (debuginfod's long term goal seems to be to provide an api to download 
the debug info in chunks too -- that would be very interesting, though I also 
expect it to be very complicated.)

(*) Though it seems very wasteful to download all files when we are going to 
need only a handful of them, it may not really be that way -- we're going to be 
downloading all of debug info anyway, and this is going to be much larger that 
all of source code put together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [lldb] 83c81c0 - [lldb] Make Fix-Its also apply to top-level expressions

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T11:50:57+02:00
New Revision: 83c81c0a469482888482983c302c09c02680ae7c

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

LOG: [lldb] Make Fix-Its also apply to top-level expressions

Summary:
Currently top-level expressions won't automatically get Fix-Its applied. The 
reason
for that is that we only set the `m_fixed_text` member if we have a wrapping
source code (I.e. `m_source_code` is not zero and is wrapping some expressions).

This patch just always sets `m_fixed_text` to get this working.

Reviewers: labath, jingham

Reviewed By: labath

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index b246fc374d1c..2b75c4f75c63 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -616,15 +616,14 @@ bool ClangUserExpression::Parse(DiagnosticManager 
_manager,
   if (parser.RewriteExpression(diagnostic_manager)) {
 size_t fixed_start;
 size_t fixed_end;
-const std::string _expression =
-diagnostic_manager.GetFixedExpression();
+m_fixed_text = diagnostic_manager.GetFixedExpression();
 // Retrieve the original expression in case we don't have a top level
 // expression (which has no surrounding source code).
 if (m_source_code &&
-m_source_code->GetOriginalBodyBounds(fixed_expression, m_expr_lang,
+m_source_code->GetOriginalBodyBounds(m_fixed_text, m_expr_lang,
  fixed_start, fixed_end))
   m_fixed_text =
-  fixed_expression.substr(fixed_start, fixed_end - fixed_start);
+  m_fixed_text.substr(fixed_start, fixed_end - fixed_start);
   }
 }
 return false;

diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 273982c0c12f..eb1dd97aa9a9 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -33,6 +33,10 @@ def test_with_target(self):
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)
 
+top_level_options = lldb.SBExpressionOptions()
+top_level_options.SetAutoApplyFixIts(True)
+top_level_options.SetTopLevel(True)
+
 frame = self.thread.GetFrameAtIndex(0)
 
 # Try with one error:
@@ -41,6 +45,15 @@ def test_with_target(self):
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 10)
 
+# Try with one error in a top-level expression.
+# The Fix-It changes "ptr.m" to "ptr->m".
+expr = "struct X { int m; }; X x; X *ptr =  int m = ptr.m;"
+value = frame.EvaluateExpression(expr, top_level_options)
+# A successfully parsed top-level expression will yield an error
+# that there is 'no value'. If a parsing error would have happened we
+# would get a 
diff erent error kind, so let's check the error kind here.
+self.assertEquals(value.GetError().GetCString(), "error: No value")
+
 # Try with two errors:
 two_error_expression = "my_pointer.second->a"
 value = frame.EvaluateExpression(two_error_expression, options)



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

[Lldb-commits] [PATCH] D77045: Add invalidate list to primary regs in arm64 register infos

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: danielkiss, kristof.beyls.

AArch64 reigster X and V registers are primary GPR and vector registers 
respectively. If these registers are modified their corresponding children w 
regs or s/d regs should be invalidated. Specially when a register write fails 
it is important that failure gets reflected to all the registers which draw 
their value from a particular value register.


https://reviews.llvm.org/D77045

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -297,6 +297,36 @@
   k_num_registers
 };
 
+static uint32_t g_x0_invalidates[] = {gpr_w0, LLDB_INVALID_REGNUM};
+static uint32_t g_x1_invalidates[] = {gpr_w1, LLDB_INVALID_REGNUM};
+static uint32_t g_x2_invalidates[] = {gpr_w2, LLDB_INVALID_REGNUM};
+static uint32_t g_x3_invalidates[] = {gpr_w3, LLDB_INVALID_REGNUM};
+static uint32_t g_x4_invalidates[] = {gpr_w4, LLDB_INVALID_REGNUM};
+static uint32_t g_x5_invalidates[] = {gpr_w5, LLDB_INVALID_REGNUM};
+static uint32_t g_x6_invalidates[] = {gpr_w6, LLDB_INVALID_REGNUM};
+static uint32_t g_x7_invalidates[] = {gpr_w7, LLDB_INVALID_REGNUM};
+static uint32_t g_x8_invalidates[] = {gpr_w8, LLDB_INVALID_REGNUM};
+static uint32_t g_x9_invalidates[] = {gpr_w9, LLDB_INVALID_REGNUM};
+static uint32_t g_x10_invalidates[] = {gpr_w10, LLDB_INVALID_REGNUM};
+static uint32_t g_x11_invalidates[] = {gpr_w11, LLDB_INVALID_REGNUM};
+static uint32_t g_x12_invalidates[] = {gpr_w12, LLDB_INVALID_REGNUM};
+static uint32_t g_x13_invalidates[] = {gpr_w13, LLDB_INVALID_REGNUM};
+static uint32_t g_x14_invalidates[] = {gpr_w14, LLDB_INVALID_REGNUM};
+static uint32_t g_x15_invalidates[] = {gpr_w15, LLDB_INVALID_REGNUM};
+static uint32_t g_x16_invalidates[] = {gpr_w16, LLDB_INVALID_REGNUM};
+static uint32_t g_x17_invalidates[] = {gpr_w17, LLDB_INVALID_REGNUM};
+static uint32_t g_x18_invalidates[] = {gpr_w18, LLDB_INVALID_REGNUM};
+static uint32_t g_x19_invalidates[] = {gpr_w19, LLDB_INVALID_REGNUM};
+static uint32_t g_x20_invalidates[] = {gpr_w20, LLDB_INVALID_REGNUM};
+static uint32_t g_x21_invalidates[] = {gpr_w21, LLDB_INVALID_REGNUM};
+static uint32_t g_x22_invalidates[] = {gpr_w22, LLDB_INVALID_REGNUM};
+static uint32_t g_x23_invalidates[] = {gpr_w23, LLDB_INVALID_REGNUM};
+static uint32_t g_x24_invalidates[] = {gpr_w24, LLDB_INVALID_REGNUM};
+static uint32_t g_x25_invalidates[] = {gpr_w25, LLDB_INVALID_REGNUM};
+static uint32_t g_x26_invalidates[] = {gpr_w26, LLDB_INVALID_REGNUM};
+static uint32_t g_x27_invalidates[] = {gpr_w27, LLDB_INVALID_REGNUM};
+static uint32_t g_x28_invalidates[] = {gpr_w28, LLDB_INVALID_REGNUM};
+
 static uint32_t g_contained_x0[] = {gpr_x0, LLDB_INVALID_REGNUM};
 static uint32_t g_contained_x1[] = {gpr_x1, LLDB_INVALID_REGNUM};
 static uint32_t g_contained_x2[] = {gpr_x2, LLDB_INVALID_REGNUM};
@@ -456,6 +486,38 @@
 static uint32_t g_d30_invalidates[] = {fpu_v30, fpu_s30, LLDB_INVALID_REGNUM};
 static uint32_t g_d31_invalidates[] = {fpu_v31, fpu_s31, LLDB_INVALID_REGNUM};
 
+static uint32_t g_v0_invalidates[] = {fpu_d0, fpu_s0, LLDB_INVALID_REGNUM};
+static uint32_t g_v1_invalidates[] = {fpu_d1, fpu_s1, LLDB_INVALID_REGNUM};
+static uint32_t g_v2_invalidates[] = {fpu_d2, fpu_s2, LLDB_INVALID_REGNUM};
+static uint32_t g_v3_invalidates[] = {fpu_d3, fpu_s3, LLDB_INVALID_REGNUM};
+static uint32_t g_v4_invalidates[] = {fpu_d4, fpu_s4, LLDB_INVALID_REGNUM};
+static uint32_t g_v5_invalidates[] = {fpu_d5, fpu_s5, LLDB_INVALID_REGNUM};
+static uint32_t g_v6_invalidates[] = {fpu_d6, fpu_s6, LLDB_INVALID_REGNUM};
+static uint32_t g_v7_invalidates[] = {fpu_d7, fpu_s7, LLDB_INVALID_REGNUM};
+static uint32_t g_v8_invalidates[] = {fpu_d8, fpu_s8, LLDB_INVALID_REGNUM};
+static uint32_t g_v9_invalidates[] = {fpu_d9, fpu_s9, LLDB_INVALID_REGNUM};
+static uint32_t g_v10_invalidates[] = {fpu_d10, fpu_s10, LLDB_INVALID_REGNUM};
+static uint32_t g_v11_invalidates[] = {fpu_d11, fpu_s11, LLDB_INVALID_REGNUM};
+static uint32_t g_v12_invalidates[] = {fpu_d12, fpu_s12, LLDB_INVALID_REGNUM};
+static uint32_t g_v13_invalidates[] = {fpu_d13, fpu_s13, LLDB_INVALID_REGNUM};
+static uint32_t g_v14_invalidates[] = {fpu_d14, fpu_s14, LLDB_INVALID_REGNUM};
+static uint32_t g_v15_invalidates[] = {fpu_d15, fpu_s15, LLDB_INVALID_REGNUM};
+static uint32_t g_v16_invalidates[] = {fpu_d16, fpu_s16, LLDB_INVALID_REGNUM};
+static uint32_t g_v17_invalidates[] = {fpu_d17, fpu_s17, LLDB_INVALID_REGNUM};
+static uint32_t g_v18_invalidates[] = {fpu_d18, fpu_s18, LLDB_INVALID_REGNUM};
+static uint32_t g_v19_invalidates[] = {fpu_d19, fpu_s19, LLDB_INVALID_REGNUM};
+static uint32_t g_v20_invalidates[] = {fpu_d20, fpu_s20, LLDB_INVALID_REGNUM};
+static uint32_t g_v21_invalidates[] = 

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: labath, jankratochvil.
Herald added subscribers: danielkiss, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.

This patch increases maximum register size to 256 bytes to accommodate AArch64 
SVE registers maximum possible size of 256 bytes.


https://reviews.llvm.org/D77044

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Utility/RegisterValue.cpp


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint32_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2048,7 +2048,8 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[256]; // big enough to support up to 256 byte AArch64 SVE
+  // registers
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,7 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +261,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint32_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint32_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2048,7 +2048,8 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[256]; // big enough to support up to 256 byte AArch64 SVE
+  // registers
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,7 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +261,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint32_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 253531.
kwk added a comment.

- Use file:// and require debuginfod 0.179
- simplify FindDebuginfod.cmake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,114 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. We then set DEBUGINFOD_URLS to the mock
+//  directory using file://. This avoids the need for a
+//  debuginfod server to be run. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod; \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-2
+
+// TEST-2: (lldb) source list -n main
+// TEST-2: File: /my/new/path/test.cpp
+// TEST-2-EMPTY:
+
+
+//-- TEST 3 -- debuginfod URL pointing corectly 
+
+
+// RUN: DEBUGINFOD_URLS="file://%t.mock/" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-3
+
+// TEST-3: (lldb) source list -n main
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: {{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}   // Some context lines before
+// TEST-3-NEXT:{{[0-9]+}}   // the function.
+// TEST-3-NEXT:{{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}   int main(int argc, char **argv) {
+// 

[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: labath, jankratochvil.
Herald added subscribers: danielkiss, kristof.beyls.
Herald added a reviewer: rengolin.

Native register descriptions in LLDB specify lldb register numbers in 
value_regs and invalidate_regs lists. These register numbers may not match with 
Process gdb-remote register numbers which are generated through native process 
by counting all registers in its register sets.

It was coincidentally not causing any problems as we never came across a native 
target with dynamically changing register sets and register numbers generated 
by counter matched with LLDB native register numbers. This came up while 
testing target AArch64 SVE which can choose register sets based on underlying 
hardware.

This patch fixes this behavior and tries to send lldb register number as extra 
information in registerinfo and targetxml packets. This patch also updates 
Read/Write RegisterBytes function of process gdb-remote to look for LLDB 
register numbers if they are available.

I have tested this on arm, aarch64, x86_64.


https://reviews.llvm.org/D77043

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

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
@@ -553,6 +553,10 @@
   } else if (name.equals("generic")) {
 reg_info.kinds[eRegisterKindGeneric] =
 Args::StringToGenericRegister(value);
+  } else if (name.equals("regnum")) {
+if (value.getAsInteger(0,
+   reg_info.kinds[eRegisterKindProcessPlugin]))
+  reg_info.kinds[eRegisterKindProcessPlugin] = reg_num;
   } else if (name.equals("container-regs")) {
 SplitCommaSeparatedRegisterNumberString(value, value_regs, 16);
   } else if (name.equals("invalidate-regs")) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -242,11 +242,15 @@
   // Index of the primordial register.
   bool success = true;
   for (uint32_t idx = 0; success; ++idx) {
-const uint32_t prim_reg = reg_info->value_regs[idx];
+uint32_t prim_reg = reg_info->value_regs[idx];
 if (prim_reg == LLDB_INVALID_REGNUM)
   break;
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
+uint32_t regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, prim_reg);
+if (regnum != LLDB_INVALID_REGNUM)
+  prim_reg = regnum;
 const RegisterInfo *prim_reg_info = GetRegisterInfoAtIndex(prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
@@ -375,11 +379,15 @@
   // Invalidate this composite register first.
 
   for (uint32_t idx = 0; success; ++idx) {
-const uint32_t reg = reg_info->value_regs[idx];
+uint32_t reg = reg_info->value_regs[idx];
 if (reg == LLDB_INVALID_REGNUM)
   break;
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
+uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, reg);
+if (lldb_regnum != LLDB_INVALID_REGNUM)
+  reg = lldb_regnum;
 const RegisterInfo *value_reg_info = GetRegisterInfoAtIndex(reg);
 if (value_reg_info == nullptr)
   success = false;
@@ -397,6 +405,10 @@
   for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0];
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx]) {
+uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, reg);
+if (lldb_regnum != LLDB_INVALID_REGNUM)
+  reg = lldb_regnum;
 SetRegisterIsValid(reg, false);
   }
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

[Lldb-commits] [PATCH] D76968: [lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py:25-43
+source_basename = 'main.cpp'
+source_path = os.path.join(os.getcwd(), source_basename)
+
+new_source_folder = os.path.join(os.path.dirname(os.getcwd()), 
'moved_location')
+new_source_path = os.path.join(new_source_folder, source_basename)
+
+def cleanup():

Please make sure none of these commands modify the source tree. You can look at 
`test/API/source-manager/Makefile` for an example of how to build a binary to 
reference files in the build tree.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:324
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint ,
+ const llvm::StringRef ,
+ int line) {

StringRefs are normally passed by value.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1974
   // Add this breakpoint info to the response
   AppendBreakpoint(pair.second.bp, response_breakpoints);
 }

What about breakpoints in dynamically loaded shared libraries? Should you be 
remembering the original path somewhere so that one you can set it here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



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

[Lldb-commits] [PATCH] D77000: [lldb] [PECOFF] Check that PECallFrameInfo operates on the expected architecture before interpreting RuntimeFunction structs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77000#1949247 , @aleksandr.urakov 
wrote:

> Hello! But does the format of an exception directory for aarch64 differ 
> completely from x86-64 one? Can we somehow adopt the existing parser to 
> support it? If we can, I think that this check looks good (we encapsulate the 
> difference inside `PECallFrameInfo`). Or may be it is worth to make a 
> different parser for aarch64? Then I think the check would look better in 
> `ObjectFilePECOFF::CreateEHCallFrameInfo` (and then we will choose a right 
> exception directory parser depending on an architecture). What do you think 
> on this?


I think it would make sense to put the function there (I guess you meant 
`ObjectFilePECOFF::CreateCallFrameInfo`) even if we later do end up adding arm 
support to the `PECallFrameInfo` class).

> Not sure how easy it is to make a sensible test for this; it makes a 
> difference when the aarch64 RuntimeFunction struct, when interpreted as an 
> x86_64 RuntimeFunction, happens to falsely match address ranges that the 
> unwinder looks for

One could construct a yaml file with a "falsely matching record" and then run 
"image show-unwind" to demonstrate that the record is _not_ parsed. The tricky 
part there is that this command only works on  "live" processes (one day i'd 
like to change that, but right now it's hard because of... reasons). The 
"breakpad" unwinding tests work around that by creating a process out of a 
minidump core file. That might be overkill for a change of this type...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77000



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

[Lldb-commits] [lldb] 767a97b - [lldb][NFC] Cleanup Fix-It code

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T11:02:44+02:00
New Revision: 767a97b22339350d1d1368ca5548f967e49a4c00

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

LOG: [lldb][NFC] Cleanup Fix-It code

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 698fea4c2d3c..e5de4b4651df 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -219,11 +219,10 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
   // FIXME: Should we try to filter out FixIts that apply to our generated
   // code, and not the user's expression?
   if (severity == eDiagnosticSeverityError) {
-size_t num_fixit_hints = Info.getNumFixItHints();
-for (size_t i = 0; i < num_fixit_hints; i++) {
-  const clang::FixItHint  = Info.getFixItHint(i);
-  if (!fixit.isNull())
-new_diagnostic->AddFixitHint(fixit);
+for (const clang::FixItHint  : Info.getFixItHints()) {
+  if (fixit.isNull())
+continue;
+  new_diagnostic->AddFixitHint(fixit);
 }
   }
 
@@ -1071,6 +1070,28 @@ ClangExpressionParser::GetClangTargetABI(const ArchSpec 
_arch) {
   return abi;
 }
 
+/// Applies the given Fix-It hint to the given commit.
+static void ApplyFixIt(const FixItHint , clang::edit::Commit ) {
+  // This is cobbed from clang::Rewrite::FixItRewriter.
+  if (fixit.CodeToInsert.empty()) {
+if (fixit.InsertFromRange.isValid()) {
+  commit.insertFromRange(fixit.RemoveRange.getBegin(),
+ fixit.InsertFromRange, /*afterToken=*/false,
+ fixit.BeforePreviousInsertions);
+  return;
+}
+commit.remove(fixit.RemoveRange);
+return;
+  }
+  if (fixit.RemoveRange.isTokenRange() ||
+  fixit.RemoveRange.getBegin() != fixit.RemoveRange.getEnd()) {
+commit.replace(fixit.RemoveRange, fixit.CodeToInsert);
+return;
+  }
+  commit.insert(fixit.RemoveRange.getBegin(), fixit.CodeToInsert,
+/*afterToken=*/false, fixit.BeforePreviousInsertions);
+}
+
 bool ClangExpressionParser::RewriteExpression(
 DiagnosticManager _manager) {
   clang::SourceManager _manager = m_compiler->getSourceManager();
@@ -1102,26 +1123,12 @@ bool ClangExpressionParser::RewriteExpression(
 
   for (const auto  : diagnostic_manager.Diagnostics()) {
 const auto *diagnostic = llvm::dyn_cast(diag.get());
-if (diagnostic && diagnostic->HasFixIts()) {
-  for (const FixItHint  : diagnostic->FixIts()) {
-// This is cobbed from clang::Rewrite::FixItRewriter.
-if (fixit.CodeToInsert.empty()) {
-  if (fixit.InsertFromRange.isValid()) {
-commit.insertFromRange(fixit.RemoveRange.getBegin(),
-   fixit.InsertFromRange, /*afterToken=*/false,
-   fixit.BeforePreviousInsertions);
-  } else
-commit.remove(fixit.RemoveRange);
-} else {
-  if (fixit.RemoveRange.isTokenRange() ||
-  fixit.RemoveRange.getBegin() != fixit.RemoveRange.getEnd())
-commit.replace(fixit.RemoveRange, fixit.CodeToInsert);
-  else
-commit.insert(fixit.RemoveRange.getBegin(), fixit.CodeToInsert,
-  /*afterToken=*/false, 
fixit.BeforePreviousInsertions);
-}
-  }
-}
+if (!diagnostic)
+  continue;
+if (!diagnostic->HasFixIts())
+  continue;
+for (const FixItHint  : diagnostic->FixIts())
+  ApplyFixIt(fixit, commit);
   }
 
   // FIXME - do we want to try to propagate specific errors here?



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

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76955#1947160 , @JDevlieghere 
wrote:

> Right, I didn't check whether out and err were strings or bytes. I'll see if 
> I can find a better solution or alternatively wrap it in a try-except.


It might be simplest to wrap that in `if PY2` (when can we drop python2 support 
again?). I don't believe this can happen with py3 due to the try-catch here 
.

A lot of this stuff could be cleaned up when python2 is gone. Right now it's 
very confusing to reason about what is the type a thing in which python version.




Comment at: lldb/test/API/lldbtest.py:85
 'import sys; print(sys.executable)'
-]).decode('utf-8').strip()
+]).decode('utf-8', errors='ignore').strip()
 shutil.copy(python, copied_python)

`replace` or `backslashreplace` [[ 
https://docs.python.org/3/howto/unicode.html#the-string-type | error ]] 
strategy might be better, so that the presence/value of the erroneous character 
is not completely lost.


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

https://reviews.llvm.org/D76955



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

[Lldb-commits] [PATCH] D76945: [lldb/CMake] Make check-lldb-* work for the standalone build.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

sounds reasonable


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

https://reviews.llvm.org/D76945



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

[Lldb-commits] [lldb] 064ab22 - [lldb] Run TestFixIts on non-Darwin platforms

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T09:53:51+02:00
New Revision: 064ab22fb29be673fa2ac4ab3b28cc30f9371e9c

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

LOG: [lldb] Run TestFixIts on non-Darwin platforms

This test also passes on my Linux machine, so this seems too strict.

Added: 


Modified: 
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 1e8e7dda68c0..273982c0c12f 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -23,7 +23,6 @@ def test_with_dummy_target(self):
 self.assertEqual(result, lldb.eReturnStatusSuccessFinishResult, "The 
expression was successful.")
 self.assertTrue("Fix-it applied" in ret_val.GetError(), "Found the 
applied FixIt.")
 
-@skipUnlessDarwin
 def test_with_target(self):
 """Test calling expressions with errors that can be fixed by the 
FixIts."""
 self.build()



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

[Lldb-commits] [lldb] 53e7c8f - [lldb][NFC] Cleanup TestFixIts.py

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-30T09:40:03+02:00
New Revision: 53e7c8fdfaaa1172f7658aa5bb61a3282fb09a84

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

LOG: [lldb][NFC] Cleanup TestFixIts.py

Added: 


Modified: 
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 7aca1c5aa863..1e8e7dda68c0 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -2,8 +2,6 @@
 Test calling an expression with errors that a FixIt can fix.
 """
 
-
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -14,19 +12,6 @@ class ExprCommandWithFixits(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-def setUp(self):
-# Call super's setUp().
-TestBase.setUp(self)
-
-self.main_source = "main.cpp"
-self.main_source_spec = lldb.SBFileSpec(self.main_source)
-
-@skipUnlessDarwin
-def test_with_target(self):
-"""Test calling expressions with errors that can be fixed by the 
FixIts."""
-self.build()
-self.try_expressions()
-
 def test_with_dummy_target(self):
 """Test calling expressions in the dummy target with errors that can 
be fixed by the FixIts."""
 
@@ -38,10 +23,13 @@ def test_with_dummy_target(self):
 self.assertEqual(result, lldb.eReturnStatusSuccessFinishResult, "The 
expression was successful.")
 self.assertTrue("Fix-it applied" in ret_val.GetError(), "Found the 
applied FixIt.")
 
-def try_expressions(self):
+@skipUnlessDarwin
+def test_with_target(self):
 """Test calling expressions with errors that can be fixed by the 
FixIts."""
+self.build()
 (target, process, self.thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
-'Stop here to evaluate expressions', 
self.main_source_spec)
+'Stop here to evaluate expressions',
+ lldb.SBFileSpec("main.cpp"))
 
 options = lldb.SBExpressionOptions()
 options.SetAutoApplyFixIts(True)



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

[Lldb-commits] [PATCH] D77000: [lldb] [PECOFF] Check that PECallFrameInfo operates on the expected architecture before interpreting RuntimeFunction structs

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Hello! But does the format of an exception directory for aarch64 differ 
completely from x86-64 one? Can we somehow adopt the existing parser to support 
it? If we can, I think that this check looks good (we encapsulate the 
difference inside `PECallFrameInfo`). Or may be it is worth to make a different 
parser for aarch64? Then I think the check would look better in 
`ObjectFilePECOFF::CreateEHCallFrameInfo` (and then we will choose a right 
exception directory parser depending on an architecture). What do you think on 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77000



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