[Lldb-commits] [lldb] 1f780c9 - [LLDB] Disable flaky lldb-vscode tests on arm

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-07-08T07:07:10+05:00
New Revision: 1f780c997c3616465a4180ffb20a5db4ec9d7776

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

LOG: [LLDB] Disable flaky lldb-vscode tests on arm

Summary:
These two tests are flaky on lldb Arm buildbot as well. They are already
being skipped for aarch64. I am going to mark them skipped for Arm.

Tags: #lldb

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py 
b/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
index ac74c7c51c87..e49c9267d971 100644
--- a/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
+++ b/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
@@ -117,7 +117,7 @@ def test_by_name_waitFor(self):
 @skipIfWindows
 @skipIfDarwin
 @skipIfNetBSD # Hangs on NetBSD as well
-@skipIf(archs="aarch64") # Example of a flaky run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5527/steps/test/logs/stdio
+@skipIf(archs=["arm", "aarch64"]) # Example of a flaky run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5527/steps/test/logs/stdio
 def test_commands(self):
 '''
 Tests the "initCommands", "preRunCommands", "stopCommands",
@@ -198,7 +198,7 @@ def test_commands(self):
 @skipIfWindows
 @skipIfDarwin
 @skipIfNetBSD # Hangs on NetBSD as well
-@skipIf(archs="aarch64") # Example of a flaky run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5517/steps/test/logs/stdio
+@skipIf(archs=["arm", "aarch64"]) # Example of a flaky run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5517/steps/test/logs/stdio
 def test_terminate_commands(self):
 '''
 Tests that the "terminateCommands", that can be passed during



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

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

[Benson Li via Phabricator via lldb-commits]

bbli added a comment.

F12311388: image.png 

Hi, so I think I got the fix working. Attached is a screenshot of the new 
output, with title "Build Command Stdout Ouput". Should I submit a new pull 
request for this?

Also just wondering,  it seems you guys have added a lldb_extensions dictionary 
to the `CalledProcessError` class. Was this monkey-patched in, because I don't 
see `subprocess` as one of the vendored third party libraries in the repo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81697



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

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 276294.
aelitashen added a comment.

Add Path Verification in Test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/module/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/VSCode.cpp
  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
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
@@ -434,6 +435,30 @@
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
+  } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
+event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
+event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+  for (int i = 0; i < num_modules; i++) {
+auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+auto module_event = CreateEventObject("module");
+llvm::json::Value module_value = CreateModule(module);
+llvm::json::Object body;
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
+  body.try_emplace("reason", "new");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+  body.try_emplace("reason", "removed");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  body.try_emplace("reason", "changed");
+}
+body.try_emplace("module", module_value);
+module_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
+  }
+}
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
 if (event_mask & eBroadcastBitStopEventThread) {
   done = true;
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -358,6 +358,11 @@
 lldb::SBTarget::eBroadcastBitBreakpointChanged);
 listener.StartListeningForEvents(this->broadcaster,
  eBroadcastBitStopEventThread);
+listener.StartListeningForEvents(
+  this->target.GetBroadcaster(),
+  lldb::SBTarget::eBroadcastBitModulesLoaded |
+  lldb::SBTarget::eBroadcastBitModulesUnloaded |
+  lldb::SBTarget::eBroadcastBitSymbolsLoaded);
   }
 }
 
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "VSCodeForward.h"
+#include "lldb/API/SBModule.h"
 
 namespace lldb_vscode {
 
@@ -237,6 +238,16 @@
  llvm::Optional request_path = llvm::None,
  llvm::Optional request_line = llvm::None);
 
+/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
+///
+/// \param[in] module
+/// A LLDB module object to convert into a JSON value
+///
+/// \return
+/// A "Module" JSON object with that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateModule(lldb::SBModule );
+
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
 /// \param[in] event_name
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,41 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateModule(lldb::SBModule ) {
+  llvm::json::Object object;
+  if (!module.IsValid())
+return llvm::json::Value(std::move(object));
+  object.try_emplace("id", std::string(module.GetUUIDString()));
+  object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
+  char module_path_arr[PATH_MAX];
+  module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
+  std::string module_path(module_path_arr);
+  object.try_emplace("path", module_path);
+  if 

[Lldb-commits] [PATCH] D83359: [SymbolFileDWARF] Lock the module when parsing call site info

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 276280.
vsk added a comment.

Move locking up into lldb::Function, and leave a comment in SymbolFileDWARF 
explaining why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83359

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp


Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -290,6 +290,8 @@
 }
 
 llvm::ArrayRef> Function::GetCallEdges() {
+  std::lock_guard guard(m_call_edges_lock);
+
   if (m_call_edges_resolved)
 return m_call_edges;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3844,6 +3844,11 @@
 
 std::vector>
 SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
+  // ParseCallEdgesInFunction must be called at the behest of an exclusively
+  // locked lldb::Function instance. Storage for parsed call edges is owned by
+  // the lldb::Function instance: locking at the SymbolFile level would be too
+  // late, because the act of storing results from ParseCallEdgesInFunction
+  // would be racy.
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
 return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
Index: lldb/include/lldb/Symbol/Function.h
===
--- lldb/include/lldb/Symbol/Function.h
+++ lldb/include/lldb/Symbol/Function.h
@@ -17,6 +17,8 @@
 #include "lldb/Utility/UserID.h"
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace lldb_private {
 
 class ExecutionContext;
@@ -655,6 +657,9 @@
   uint32_t
   m_prologue_byte_size; ///< Compute the prologue size once and cache it
 
+  std::mutex
+  m_call_edges_lock; ///< Exclusive lock that controls read/write
+ ///  access to m_call_edges and m_call_edges_resolved.
   bool m_call_edges_resolved = false; ///< Whether call site info has been
   ///  parsed.
   std::vector> m_call_edges; ///< Outgoing call 
edges.


Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -290,6 +290,8 @@
 }
 
 llvm::ArrayRef> Function::GetCallEdges() {
+  std::lock_guard guard(m_call_edges_lock);
+
   if (m_call_edges_resolved)
 return m_call_edges;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3844,6 +3844,11 @@
 
 std::vector>
 SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
+  // ParseCallEdgesInFunction must be called at the behest of an exclusively
+  // locked lldb::Function instance. Storage for parsed call edges is owned by
+  // the lldb::Function instance: locking at the SymbolFile level would be too
+  // late, because the act of storing results from ParseCallEdgesInFunction
+  // would be racy.
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
 return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
Index: lldb/include/lldb/Symbol/Function.h
===
--- lldb/include/lldb/Symbol/Function.h
+++ lldb/include/lldb/Symbol/Function.h
@@ -17,6 +17,8 @@
 #include "lldb/Utility/UserID.h"
 #include "llvm/ADT/ArrayRef.h"
 
+#include 
+
 namespace lldb_private {
 
 class ExecutionContext;
@@ -655,6 +657,9 @@
   uint32_t
   m_prologue_byte_size; ///< Compute the prologue size once and cache it
 
+  std::mutex
+  m_call_edges_lock; ///< Exclusive lock that controls read/write
+ ///  access to m_call_edges and m_call_edges_resolved.
   bool m_call_edges_resolved = false; ///< Whether call site info has been
   ///  parsed.
   std::vector> m_call_edges; ///< Outgoing call edges.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Greg Clayton via Phabricator via lldb-commits]

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

Just test for paths and this will be good to go!




Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:35
+self.assertEqual(program_basename, program_module['name'])
+self.assertIn('path', program_module, 'make sure path is in module')
+self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')

add a check for the path:
```
self.assertEqual(program, program_module['path'])
```



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:42
+program_module = active_modules[program_basename]
+self.assertEqual(program_basename, program_module['name'])
+self.assertEqual('Symbols loaded.', program_module['symbolStatus'])

check 'path' again here.



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:47
+self.assertIn('addressRange', program_module)
\ No newline at end of file


add a newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477



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

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 276274.
aelitashen added a comment.

Fix messy diff stack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/module/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/VSCode.cpp
  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
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
@@ -434,6 +435,30 @@
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
+  } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
+event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
+event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+  for (int i = 0; i < num_modules; i++) {
+auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+auto module_event = CreateEventObject("module");
+llvm::json::Value module_value = CreateModule(module);
+llvm::json::Object body;
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
+  body.try_emplace("reason", "new");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+  body.try_emplace("reason", "removed");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  body.try_emplace("reason", "changed");
+}
+body.try_emplace("module", module_value);
+module_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
+  }
+}
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
 if (event_mask & eBroadcastBitStopEventThread) {
   done = true;
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -358,6 +358,11 @@
 lldb::SBTarget::eBroadcastBitBreakpointChanged);
 listener.StartListeningForEvents(this->broadcaster,
  eBroadcastBitStopEventThread);
+listener.StartListeningForEvents(
+  this->target.GetBroadcaster(),
+  lldb::SBTarget::eBroadcastBitModulesLoaded |
+  lldb::SBTarget::eBroadcastBitModulesUnloaded |
+  lldb::SBTarget::eBroadcastBitSymbolsLoaded);
   }
 }
 
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "VSCodeForward.h"
+#include "lldb/API/SBModule.h"
 
 namespace lldb_vscode {
 
@@ -237,6 +238,16 @@
  llvm::Optional request_path = llvm::None,
  llvm::Optional request_line = llvm::None);
 
+/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
+///
+/// \param[in] module
+/// A LLDB module object to convert into a JSON value
+///
+/// \return
+/// A "Module" JSON object with that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateModule(lldb::SBModule );
+
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
 /// \param[in] event_name
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,41 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateModule(lldb::SBModule ) {
+  llvm::json::Object object;
+  if (!module.IsValid())
+return llvm::json::Value(std::move(object));
+  object.try_emplace("id", std::string(module.GetUUIDString()));
+  object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
+  char module_path_arr[PATH_MAX];
+  module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
+  std::string module_path(module_path_arr);
+  object.try_emplace("path", module_path);
+  if (module.GetNumCompileUnits() > 0) {

[Lldb-commits] [PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

[David Blaikie via Phabricator via lldb-commits]

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

Could you include some comparative data in the description/commit message 
demonstrating this generally ends up emitting all the same types for a clang 
build before/after (& explanations for a sample of any differences)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147



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

[Lldb-commits] [PATCH] D83359: [SymbolFileDWARF] Lock the module when parsing call site info

[Vedant Kumar via Phabricator via lldb-commits]

vsk planned changes to this revision.
vsk added a comment.

Hm, this doesn't totally fix the race. If the mutex is contested, the Function 
instance may overwrite its m_call_edges vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83359



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

[Lldb-commits] [PATCH] D83359: [SymbolFileDWARF] Lock the module when parsing call site info

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: jasonmolenda, friss, jingham.
Herald added a subscriber: aprantl.
Herald added a project: LLDB.

DWARF-parsing methods in SymbolFileDWARF which update module state
should take the module lock. Have ParseCallEdgesInFunction do this.

This could explain some as-of-yet unreproducible crashes which occur in
Function::GetTailCallingEdges(), in which the `m_call_edges` vector is
both non-empty and contains a nullptr, which shouldn't be possible. (If
this vector is non-empty, it _must_ contain a non-null unique_ptr.)

This may address rdar://55622443, rdar://65119458.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83359

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3844,6 +3844,7 @@
 
 std::vector>
 SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
+  std::lock_guard guard(GetModuleMutex());
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
 return CollectCallEdges(GetObjectFile()->GetModule(), func_die);


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3844,6 +3844,7 @@
 
 std::vector>
 SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
+  std::lock_guard guard(GetModuleMutex());
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
 return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79147: [WIP] Merge -debug-info-kind=constructor into -debug-info-kind=limited

[Amy Huang via Phabricator via lldb-commits]

akhuang updated this revision to Diff 276252.
akhuang added a comment.
Herald added a subscriber: fedor.sergeev.

Instead of merging =constructor and =limited, change the default to use 
=constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-g-opts.c
  clang/test/Driver/cuda-dwarf-2.cu
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  clang/test/Driver/integrated-as.s
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/split-debug.c
  lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Index: lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
===
--- lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
+++ lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
@@ -106,6 +106,7 @@
 int main() {
   MemberTest::Base B1;
   B1.Get();
+  MemberTest::Class C1;
   MemberTest::Class::StaticMemberFunc(1, 10, 2);
   return 0;
 }
Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -68,18 +68,18 @@
 // RUN: FileCheck -check-prefix=CHECK-NOINLINE-WITHOUT-SPLIT < %t %s
 //
 // CHECK-NOINLINE-WITHOUT-SPLIT: "-fno-split-dwarf-inlining"
-// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=limited"
+// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=constructor"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-NOINL < %t %s
 //
-// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-NOINL: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gmlt -fsplit-dwarf-inlining -S -### %s 2> %t
@@ -92,7 +92,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-GMLT < %t %s
 //
-// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-output"
 
@@ -117,6 +117,6 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
-// CHECK-SPLIT-OVER-G0: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-G0: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-output"
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ clang/test/Driver/myriad-toolchain.c
@@ -83,7 +83,7 @@
 // NOSTDLIB-NOT: "-lc"
 
 // RUN: %clang -### -c -g %s -target sparc-myriad 2>&1 | FileCheck -check-prefix=G_SPARC %s
-// G_SPARC: "-debug-info-kind=limited" "-dwarf-version=2"
+// G_SPARC: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -c %s -target sparc-myriad-rtems -fuse-init-array 2>&1 \
 // RUN: | FileCheck -check-prefix=USE-INIT-ARRAY %s
Index: clang/test/Driver/integrated-as.s
===
--- clang/test/Driver/integrated-as.s
+++ clang/test/Driver/integrated-as.s
@@ -27,19 +27,19 @@
 // XA_INCLUDE2: "-Ifoo_dir"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2 %s
-// DWARF2: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-3 2>&1 | FileCheck --check-prefix=DWARF3 %s
-// DWARF3: "-debug-info-kind=limited" "-dwarf-version=3"
+// DWARF3: "-debug-info-kind=constructor" "-dwarf-version=3"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 2>&1 | FileCheck --check-prefix=DWARF4 %s
-// DWARF4: "-debug-info-kind=limited" "-dwarf-version=4"
+// DWARF4: "-debug-info-kind=constructor" "-dwarf-version=4"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -Xassembler -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2XASSEMBLER %s
-// DWARF2XASSEMBLER: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2XASSEMBLER: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### 

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
davide marked an inline comment as done.
Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if 
--out-of-tree-debugserver is passed. (authored by davide).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82804

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 276229.
shafik marked 5 inline comments as done.
shafik added a comment.

- Removing spurious local variables in test
- Simplifying the test


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

https://reviews.llvm.org/D83008

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  lldb/test/Shell/Expr/Inputs/layout.cpp
  lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test


Index: lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
@@ -0,0 +1,7 @@
+# RUN: %clangxx_host %p/Inputs/layout.cpp -g -o %t
+
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $0 = 8
Index: lldb/test/Shell/Expr/Inputs/layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/layout.cpp
@@ -0,0 +1,17 @@
+#include 
+
+struct B1 {
+  char f1;
+};
+
+struct alignas(8) B2 {
+  char f2;
+};
+
+struct D : B1, B2 {
+};
+
+D d3g;
+
+int main() {
+}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1187,11 +1187,10 @@
   // Query the external layout to see if it provides an offset.
   bool HasExternalLayout = false;
   if (UseExternalLayout) {
-// FIXME: This appears to be reversed.
 if (Base->IsVirtual)
-  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, 
Offset);
-else
   HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
+else
+  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, 
Offset);
   }
 
   // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.


Index: lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
@@ -0,0 +1,7 @@
+# RUN: %clangxx_host %p/Inputs/layout.cpp -g -o %t
+
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $0 = 8
Index: lldb/test/Shell/Expr/Inputs/layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/layout.cpp
@@ -0,0 +1,17 @@
+#include 
+
+struct B1 {
+  char f1;
+};
+
+struct alignas(8) B2 {
+  char f2;
+};
+
+struct D : B1, B2 {
+};
+
+D d3g;
+
+int main() {
+}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1187,11 +1187,10 @@
   // Query the external layout to see if it provides an offset.
   bool HasExternalLayout = false;
   if (UseExternalLayout) {
-// FIXME: This appears to be reversed.
 if (Base->IsVirtual)
-  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
-else
   HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
+else
+  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
   }
 
   // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4dba3f4 - [dotest] Log a warning when --server and --out-of-tree-debugserver are set

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-07-07T15:03:08-07:00
New Revision: 4dba3f4e030a906f9ec7e940dfc4a57b94374154

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

LOG: [dotest] Log a warning when --server and --out-of-tree-debugserver are set

Suggested by Vedant.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f13f445aaa9f..f9975b27c475 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,6 +363,9 @@ def parseOptionsAndInitTestdirs():
 args.executable)
 sys.exit(-1)
 
+if args.server and args.out_of_tree_debugserver:
+logging.warning('Both --server and --out-of-tree-debugserver are set')
+
 if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 



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

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
davide marked an inline comment as done.
Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if 
--out-of-tree-debugserver is passed. (authored by davide).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82804

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5832473 - Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-07-07T15:01:21-07:00
New Revision: 5832473dcf4e5b22c9eb6381fb291be92f431208

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

LOG: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

This gets rid of some surprising interplay between the flags.
Mainly needed because of Rosetta debugserver & Apple Silicon.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 2b8185a18b77..f13f445aaa9f 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@ def parseOptionsAndInitTestdirs():
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:



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

[Lldb-commits] [PATCH] D83343: [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0d7401cf9d5c: [lldb/api] Add checks for 
StackFrame::GetRegisterContext calls (NFC) (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83343

Files:
  lldb/source/API/SBFrame.cpp


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -354,15 +354,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  ret_val = reg_ctx_sp->SetPC(new_pc);
+}
   }
 }
   }
@@ -377,15 +377,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetSP();
+}
   }
 }
   }
@@ -400,15 +400,16 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetFP();
+}
+  }
 }
   }
 


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -354,15 +354,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  ret_val = reg_ctx_sp->SetPC(new_pc);
+}
   }
 }
   }
@@ -377,15 +377,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetSP();
+}
   }
 }
   }
@@ -400,15 +400,16 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetFP();
+}
+  }
 }
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0d7401c - [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2020-07-07T23:30:24+02:00
New Revision: 0d7401cf9d5cc0db3de9a8ddb8ea2362d5412d2f

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

LOG: [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

This patch fixes a crash that is happening because of a null pointer
dereference in SBFrame.

StackFrame::GetRegisterContext says explicitly that you might not get
a valid RegisterContext back but the pointer wasn't tested before,
resulting in crashes. This should solve the issue.

rdar://54462095

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/API/SBFrame.cpp

Removed: 




diff  --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index 5ecf1c537536..81782dbf838f 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -354,15 +354,15 @@ bool SBFrame::SetPC(addr_t new_pc) {
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  ret_val = reg_ctx_sp->SetPC(new_pc);
+}
   }
 }
   }
@@ -377,15 +377,15 @@ addr_t SBFrame::GetSP() const {
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetSP();
+}
   }
 }
   }
@@ -400,15 +400,16 @@ addr_t SBFrame::GetFP() const {
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetFP();
+}
+  }
 }
   }
 



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

[Lldb-commits] [PATCH] D83343: [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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

A test would be great but since we can't repro this is strictly better than the 
current state. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83343



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

[Lldb-commits] [PATCH] D83343: [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 276209.
mib added a comment.

Changed style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83343

Files:
  lldb/source/API/SBFrame.cpp


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -354,15 +354,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  ret_val = reg_ctx_sp->SetPC(new_pc);
+}
   }
 }
   }
@@ -377,15 +377,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetSP();
+}
   }
 }
   }
@@ -400,15 +400,16 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetFP();
+}
+  }
 }
   }
 


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -354,15 +354,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  ret_val = reg_ctx_sp->SetPC(new_pc);
+}
   }
 }
   }
@@ -377,15 +377,15 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetSP();
+}
   }
 }
   }
@@ -400,15 +400,16 @@
   std::unique_lock lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-  StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
   if (target && process) {
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
-  frame = exe_ctx.GetFramePtr();
-  if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
+if (RegisterContextSP reg_ctx_sp = frame->GetRegisterContext()) {
+  addr = reg_ctx_sp->GetFP();
+}
+  }
 }
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77045: Minor fixups to LLDB AArch64 register infos macros for SVE register infos

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bf6c50c7fe2: Minor fixups to LLDB AArch64 register infos 
macros for SVE register infos (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

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
@@ -456,37 +456,26 @@
 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};
 
-// Generates register kinds array for 64-bit general purpose registers
-#define GPR64_KIND(reg, generic_kind)  
\
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind) 
\
   {
\
 arm64_ehframe::reg, arm64_dwarf::reg, generic_kind, LLDB_INVALID_REGNUM,   
\
-gpr_##reg  
\
+type##_##reg   
\
   }
 
-// Generates register kinds array for registers with lldb kind
-#define MISC_KIND(lldb_kind)   
\
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)   
\
   {
\
 LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, 
\
 LLDB_INVALID_REGNUM, lldb_kind 
\
   }
 
 // Generates register kinds array for vector registers
-#define VREG_KIND(reg) 
\
-  {
\
-LLDB_INVALID_REGNUM, arm64_dwarf::reg, LLDB_INVALID_REGNUM,
\
-LLDB_INVALID_REGNUM, fpu_##reg 
\
-  }
-
-// Generates register kinds array for cpsr
-#define CPSR_KIND(lldb_kind)   
\
-  {
\
-arm64_ehframe::cpsr, arm64_dwarf::cpsr, LLDB_REGNUM_GENERIC_FLAGS, 
\
-LLDB_INVALID_REGNUM, lldb_kind 
\
-  }
-
-#define MISC_GPR_KIND(lldb_kind) CPSR_KIND(lldb_kind)
-#define MISC_FPU_KIND(lldb_kind) MISC_KIND(lldb_kind)
-#define MISC_EXC_KIND(lldb_kind) MISC_KIND(lldb_kind)
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+#define VREG_KIND(reg) MISC_KIND(reg, fpu, LLDB_INVALID_REGNUM)
+#define MISC_GPR_KIND(lldb_kind) MISC_KIND(cpsr, gpr, 
LLDB_REGNUM_GENERIC_FLAGS)
+#define MISC_FPU_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+#define MISC_EXC_KIND(lldb_kind) LLDB_KIND(lldb_kind)
 
 // Defines a 64-bit general purpose register
 #define DEFINE_GPR64(reg, generic_kind)
\
@@ -509,7 +498,7 @@
   {
\
 #wreg, nullptr, 4, 
\
 GPR_OFFSET(gpr_##xreg) + GPR_W_PSEUDO_REG_ENDIAN_OFFSET,   
\
-lldb::eEncodingUint, lldb::eFormatHex, MISC_KIND(gpr_##wreg),  
\
+lldb::eEncodingUint, lldb::eFormatHex, LLDB_KIND(gpr_##wreg),  
\
 g_contained_##xreg, g_##wreg##_invalidates, nullptr, 0 
\
   }
 
@@ -525,7 +514,7 @@
 #define DEFINE_FPU_PSEUDO(reg, size, offset, vreg) 
\
   {
\
 #reg, nullptr, size, FPU_OFFSET(fpu_##vreg - fpu_v0) + offset, 
\
-lldb::eEncodingIEEE754, lldb::eFormatFloat, MISC_KIND(fpu_##reg),  
\
+lldb::eEncodingIEEE754, lldb::eFormatFloat, LLDB_KIND(fpu_##reg),  
\
 g_contained_##vreg, g_##reg##_invalidates, nullptr, 0  
\
   }
 


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
@@ -456,37 +456,26 @@
 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};
 
-// 

[Lldb-commits] [PATCH] D83343: [lldb/api] Add checks for StackFrame::GetRegisterContext calls (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: teemperor, JDevlieghere.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch fixes a crash that is happening because of a null pointer
dereference in SBFrame.

StackFrame::GetRegisterContext says explicitly that you might not get
a valid RegisterContext back but the pointer wasn't tested before,
resulting in crashes. This should solve the issue.

rdar://54462095

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83343

Files:
  lldb/source/API/SBFrame.cpp


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -361,9 +361,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
-  }
+  RegisterContextSP reg_ctx_sp = nullptr;
+  if (frame)
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+ret_val = reg_ctx_sp->SetPC(new_pc);
 }
   }
 
@@ -384,9 +386,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
-  }
+  RegisterContextSP reg_ctx_sp = nullptr;
+  if (frame)
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+addr = reg_ctx_sp->GetSP();
 }
   }
 
@@ -407,8 +411,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
+  RegisterContextSP reg_ctx_sp = nullptr;
   if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+addr = reg_ctx_sp->GetFP();
 }
   }
 


Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -361,9 +361,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
-  if (frame) {
-ret_val = frame->GetRegisterContext()->SetPC(new_pc);
-  }
+  RegisterContextSP reg_ctx_sp = nullptr;
+  if (frame)
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+ret_val = reg_ctx_sp->SetPC(new_pc);
 }
   }
 
@@ -384,9 +386,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
-  if (frame) {
-addr = frame->GetRegisterContext()->GetSP();
-  }
+  RegisterContextSP reg_ctx_sp = nullptr;
+  if (frame)
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+addr = reg_ctx_sp->GetSP();
 }
   }
 
@@ -407,8 +411,11 @@
 Process::StopLocker stop_locker;
 if (stop_locker.TryLock(>GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
+  RegisterContextSP reg_ctx_sp = nullptr;
   if (frame)
-addr = frame->GetRegisterContext()->GetFP();
+reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp)
+addr = reg_ctx_sp->GetFP();
 }
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77045: Minor fixups to LLDB AArch64 register infos macros for SVE register infos

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bf6c50c7fe2: Minor fixups to LLDB AArch64 register infos 
macros for SVE register infos (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

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
@@ -456,37 +456,26 @@
 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};
 
-// Generates register kinds array for 64-bit general purpose registers
-#define GPR64_KIND(reg, generic_kind)  
\
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind) 
\
   {
\
 arm64_ehframe::reg, arm64_dwarf::reg, generic_kind, LLDB_INVALID_REGNUM,   
\
-gpr_##reg  
\
+type##_##reg   
\
   }
 
-// Generates register kinds array for registers with lldb kind
-#define MISC_KIND(lldb_kind)   
\
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)   
\
   {
\
 LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, 
\
 LLDB_INVALID_REGNUM, lldb_kind 
\
   }
 
 // Generates register kinds array for vector registers
-#define VREG_KIND(reg) 
\
-  {
\
-LLDB_INVALID_REGNUM, arm64_dwarf::reg, LLDB_INVALID_REGNUM,
\
-LLDB_INVALID_REGNUM, fpu_##reg 
\
-  }
-
-// Generates register kinds array for cpsr
-#define CPSR_KIND(lldb_kind)   
\
-  {
\
-arm64_ehframe::cpsr, arm64_dwarf::cpsr, LLDB_REGNUM_GENERIC_FLAGS, 
\
-LLDB_INVALID_REGNUM, lldb_kind 
\
-  }
-
-#define MISC_GPR_KIND(lldb_kind) CPSR_KIND(lldb_kind)
-#define MISC_FPU_KIND(lldb_kind) MISC_KIND(lldb_kind)
-#define MISC_EXC_KIND(lldb_kind) MISC_KIND(lldb_kind)
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+#define VREG_KIND(reg) MISC_KIND(reg, fpu, LLDB_INVALID_REGNUM)
+#define MISC_GPR_KIND(lldb_kind) MISC_KIND(cpsr, gpr, 
LLDB_REGNUM_GENERIC_FLAGS)
+#define MISC_FPU_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+#define MISC_EXC_KIND(lldb_kind) LLDB_KIND(lldb_kind)
 
 // Defines a 64-bit general purpose register
 #define DEFINE_GPR64(reg, generic_kind)
\
@@ -509,7 +498,7 @@
   {
\
 #wreg, nullptr, 4, 
\
 GPR_OFFSET(gpr_##xreg) + GPR_W_PSEUDO_REG_ENDIAN_OFFSET,   
\
-lldb::eEncodingUint, lldb::eFormatHex, MISC_KIND(gpr_##wreg),  
\
+lldb::eEncodingUint, lldb::eFormatHex, LLDB_KIND(gpr_##wreg),  
\
 g_contained_##xreg, g_##wreg##_invalidates, nullptr, 0 
\
   }
 
@@ -525,7 +514,7 @@
 #define DEFINE_FPU_PSEUDO(reg, size, offset, vreg) 
\
   {
\
 #reg, nullptr, size, FPU_OFFSET(fpu_##vreg - fpu_v0) + offset, 
\
-lldb::eEncodingIEEE754, lldb::eFormatFloat, MISC_KIND(fpu_##reg),  
\
+lldb::eEncodingIEEE754, lldb::eFormatFloat, LLDB_KIND(fpu_##reg),  
\
 g_contained_##vreg, g_##reg##_invalidates, nullptr, 0  
\
   }
 


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
@@ -456,37 +456,26 @@
 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};
 
-// 

[Lldb-commits] [lldb] 2bf6c50 - Minor fixups to LLDB AArch64 register infos macros for SVE register infos

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-07-08T01:07:17+05:00
New Revision: 2bf6c50c7fe2a68b0cf61568bc31c9966bbf1c3e

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

LOG: Minor fixups to LLDB AArch64 register infos macros for SVE register infos

Summary:
This patch adds some cosmetic changes to LLDB AArch64 register infos macros in 
order to use them in SVE register infos struct in follow up patches.
This patch initially added invalidate lists to register infos struct but that 
is no longer needed and problem disappeared after updating qemu testing 
environment.

old headline comments for reference:
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.

Reviewers: labath, rengolin

Reviewed By: labath

Subscribers: tschuett, kristof.beyls, danielkiss, lldb-commits

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
index 0a0979c92071..4aee55e7afba 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -456,37 +456,26 @@ static uint32_t g_d29_invalidates[] = {fpu_v29, fpu_s29, 
LLDB_INVALID_REGNUM};
 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};
 
-// Generates register kinds array for 64-bit general purpose registers
-#define GPR64_KIND(reg, generic_kind)  
\
+// Generates register kinds array with DWARF, EH frame and generic kind
+#define MISC_KIND(reg, type, generic_kind) 
\
   {
\
 arm64_ehframe::reg, arm64_dwarf::reg, generic_kind, LLDB_INVALID_REGNUM,   
\
-gpr_##reg  
\
+type##_##reg   
\
   }
 
-// Generates register kinds array for registers with lldb kind
-#define MISC_KIND(lldb_kind)   
\
+// Generates register kinds array for registers with only lldb kind
+#define LLDB_KIND(lldb_kind)   
\
   {
\
 LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, 
\
 LLDB_INVALID_REGNUM, lldb_kind 
\
   }
 
 // Generates register kinds array for vector registers
-#define VREG_KIND(reg) 
\
-  {
\
-LLDB_INVALID_REGNUM, arm64_dwarf::reg, LLDB_INVALID_REGNUM,
\
-LLDB_INVALID_REGNUM, fpu_##reg 
\
-  }
-
-// Generates register kinds array for cpsr
-#define CPSR_KIND(lldb_kind)   
\
-  {
\
-arm64_ehframe::cpsr, arm64_dwarf::cpsr, LLDB_REGNUM_GENERIC_FLAGS, 
\
-LLDB_INVALID_REGNUM, lldb_kind 
\
-  }
-
-#define MISC_GPR_KIND(lldb_kind) CPSR_KIND(lldb_kind)
-#define MISC_FPU_KIND(lldb_kind) MISC_KIND(lldb_kind)
-#define MISC_EXC_KIND(lldb_kind) MISC_KIND(lldb_kind)
+#define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
+#define VREG_KIND(reg) MISC_KIND(reg, fpu, LLDB_INVALID_REGNUM)
+#define MISC_GPR_KIND(lldb_kind) MISC_KIND(cpsr, gpr, 
LLDB_REGNUM_GENERIC_FLAGS)
+#define MISC_FPU_KIND(lldb_kind) LLDB_KIND(lldb_kind)
+#define MISC_EXC_KIND(lldb_kind) LLDB_KIND(lldb_kind)
 
 // Defines a 64-bit general purpose register
 #define DEFINE_GPR64(reg, generic_kind)
\
@@ -509,7 +498,7 @@ static uint32_t g_d31_invalidates[] = {fpu_v31, fpu_s31, 
LLDB_INVALID_REGNUM};
   {
\
 #wreg, nullptr, 4, 
\
 GPR_OFFSET(gpr_##xreg) + GPR_W_PSEUDO_REG_ENDIAN_OFFSET,   
\
-

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Greg Clayton via Phabricator via lldb-commits]

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

So looks like you didn't use "git commit --amend -a" again here. These changes 
are incremental changes on your patch which hasn't been submitted yet?




Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:21
+program_basename = "a.out.stripped"
+program= self.getBuildArtifact(program_basename)
 self.build_and_launch(program)

add a space after "program"



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:33-34
+program_module = active_modules[program_basename]
+self.assertIn('name', program_module, 'make sure name is in module')
+self.assertEqual(program_basename, program_module['name'])
+self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')

Do a similar check for ='path'
```
self.assertIn('name', program_module, 'make sure "name" is in module')
self.assertEqual(program_basename, program_module['name'])
self.assertIn('path', program_module, 'make sure "path" is in module')
self.assertEqual(program, program_module['path'])
```
In general, make sure you test every key that you have added support for.



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:35
+self.assertEqual(program_basename, program_module['name'])
+self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')
+symbol_path = self.getBuildArtifact("a.out")

Check for the symbol status here:
```
self.assertEqual('Symbols not found.', program_module['symbolStatus'])
```



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:40
+program_module = active_modules[program_basename]
+self.assertEqual(program_basename, program_module['name'])
+self.assertEqual('Symbols loaded.', program_module['symbolStatus'])

verify 'path' again



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:346
+object.try_emplace("symbolFilePath", symbol_path);
   }
   std::string loaded_addr = std::to_string(

Add an else clause here:

```
} else {
  object.try_emplace("symbolStatus", "Symbols not found.");
}
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:352-357
+  uint32_t num_versions = module.GetVersion(version_nums, 
sizeof(version_nums)/sizeof(uint32_t));
+  for (uint32_t i=0; ihttps://reviews.llvm.org/D82477/new/

https://reviews.llvm.org/D82477



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

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Yifan Shen via Phabricator via lldb-commits]

aelitashen updated this revision to Diff 276172.
aelitashen added a comment.

Add test for loading symbols, other module info and Add version to module info


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/module/main.cpp
  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
@@ -1171,31 +1171,6 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
-void request_getCompileUnits(const llvm::json::Object ) {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  lldb::SBProcess process = g_vsc.target.GetProcess();
-  llvm::json::Object body;
-  llvm::json::Array units;
-  auto arguments = request.getObject("arguments");
-  std::string module_id = std::string(GetString(arguments, "moduleId"));
-  int num_modules = g_vsc.target.GetNumModules();
-  for (int i = 0; i < num_modules; i++) {
-auto curr_module = g_vsc.target.GetModuleAtIndex(i);
-if (module_id == curr_module.GetUUIDString()) {
-  int num_units = curr_module.GetNumCompileUnits();
-  for (int j = 0; j < num_units; j++) {
-auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
-units.emplace_back(CreateCompileUnit(curr_unit));
-  }
-  body.try_emplace("compileUnits", std::move(units));
-  break;
-}
-  }
-  response.try_emplace("body", std::move(body));
-  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
-}
-
 // "InitializeRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 // "type": "object",
@@ -2779,7 +2754,6 @@
   REQUEST_CALLBACK(disconnect),
   REQUEST_CALLBACK(evaluate),
   REQUEST_CALLBACK(exceptionInfo),
-  REQUEST_CALLBACK(getCompileUnits),
   REQUEST_CALLBACK(initialize),
   REQUEST_CALLBACK(launch),
   REQUEST_CALLBACK(next),
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -441,8 +441,6 @@
 llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
  int64_t varID, bool format_hex);
 
-llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
-
 } // namespace lldb_vscode
 
 #endif
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -342,25 +342,22 @@
 char symbol_path_arr[PATH_MAX];
 module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
-if (symbol_path != module_path) {
-  object.try_emplace("symbolFilePath", symbol_path);
-}
+object.try_emplace("symbolFilePath", symbol_path);
   }
   std::string loaded_addr = std::to_string(
   module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target));
   object.try_emplace("addressRange", loaded_addr);
-  // uint32_t version_nums[5];
-  // const uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums));
-  // std::string version_str = "dummy";
-  // for (uint32_t i = 0; i < num_versions; ++i) {
-  //   if (!version_str.empty()) {
-  // version_str += ".";
-  //   }
-  //   version_str += std::to_string(version_nums[i]);
-  // }
-  // if (!version_str.empty()) {
-  //   object.try_emplace("version", version_str);
-  // }
+  std::string version_str;
+  uint32_t version_nums[3];
+  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
+  for (uint32_t i=0; i___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib closed this revision.
mib added a comment.

Patch landed in 7177e63fb554 
 !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [lldb] 7177e63 - [lldb/Core] Fix crash in ValueObject::CreateChildAtIndex

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2020-07-07T20:37:13+02:00
New Revision: 7177e63fb554cfac3c252327e344fb5a17d6bd65

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

LOG: [lldb/Core] Fix crash in ValueObject::CreateChildAtIndex

The patch fixes a crash in ValueObject::CreateChildAtIndex caused by a
null pointer dereferencing. This is a corner case that is happening when
trying to dereference a variable with an incomplete type, and this same
variable doesn't have a synthetic value to get the child ValueObject.

If this happens, lldb will now return a null pointer that will results
in an error message.

rdar://65181171

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/test/API/functionalities/target_var/main.c

Modified: 
lldb/source/Core/ValueObject.cpp
lldb/test/API/functionalities/target_var/Makefile
lldb/test/API/functionalities/target_var/TestTargetVar.py

Removed: 
lldb/test/API/functionalities/target_var/globals.c
lldb/test/API/functionalities/target_var/globals.ll



diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index dfadb3c5233f..8600469580e8 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -687,10 +687,15 @@ ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
 language_flags);
   }
 
-  if (!valobj && synthetic_array_member)
-valobj = GetSyntheticValue()
- ->GetChildAtIndex(synthetic_index, synthetic_array_member)
- .get();
+  // In case of an incomplete type, LLDB will try to use the ValueObject's
+  // synthetic value to create the child ValueObject.
+  if (!valobj && synthetic_array_member) {
+if (ValueObjectSP synth_valobj_sp = GetSyntheticValue()) {
+  valobj = synth_valobj_sp
+   ->GetChildAtIndex(synthetic_index, synthetic_array_member)
+   .get();
+}
+  }
 
   return valobj;
 }

diff  --git a/lldb/test/API/functionalities/target_var/Makefile 
b/lldb/test/API/functionalities/target_var/Makefile
index e51de3a02a46..10495940055b 100644
--- a/lldb/test/API/functionalities/target_var/Makefile
+++ b/lldb/test/API/functionalities/target_var/Makefile
@@ -1,5 +1,3 @@
-include Makefile.rules
+C_SOURCES := main.c
 
-a.out: globals.ll
-   $(CC) $(CFLAGS) -g -c $^ -o globals.o
-   $(LD) $(LDFLAGS) -g globals.o -o $@
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/target_var/TestTargetVar.py 
b/lldb/test/API/functionalities/target_var/TestTargetVar.py
index f8c2a6901472..4eee0e61816e 100644
--- a/lldb/test/API/functionalities/target_var/TestTargetVar.py
+++ b/lldb/test/API/functionalities/target_var/TestTargetVar.py
@@ -20,3 +20,5 @@ def testTargetVarExpr(self):
 self.build()
 lldbutil.run_to_name_breakpoint(self, 'main')
 self.expect("target variable i", substrs=['i', '42'])
+self.expect("target variable var", patterns=['\(incomplete \*\) var = 
0[xX](0)*dead'])
+self.expect("target variable var[0]", error=True, substrs=["can't find 
global variable 'var[0]'"])

diff  --git a/lldb/test/API/functionalities/target_var/globals.c 
b/lldb/test/API/functionalities/target_var/globals.c
deleted file mode 100644
index 266192849641..
--- a/lldb/test/API/functionalities/target_var/globals.c
+++ /dev/null
@@ -1,6 +0,0 @@
-int i = 42;
-int *p = 
-
-int main() {
-  return *p;
-}

diff  --git a/lldb/test/API/functionalities/target_var/globals.ll 
b/lldb/test/API/functionalities/target_var/globals.ll
deleted file mode 100644
index 192d4e126981..
--- a/lldb/test/API/functionalities/target_var/globals.ll
+++ /dev/null
@@ -1,42 +0,0 @@
-source_filename = "globals.c"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.14.0"
-
-@i = global i32 42, align 4
-@p = global i32* @i, align 8, !dbg !0, !dbg !6
-
-; Function Attrs: noinline nounwind optnone ssp uwtable
-define i32 @main() #0 !dbg !15 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval, align 4
-  %0 = load i32*, i32** @p, align 8, !dbg !18
-  %1 = load i32, i32* %0, align 4, !dbg !18
-  ret i32 %1, !dbg !18
-}
-
-attributes #0 = { noinline nounwind optnone ssp uwtable }
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!10, !11, !12, !13}
-!llvm.ident = !{!14}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
-!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: 
!9, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: 
FullDebug, globals: !5)
-!3 = !DIFile(filename: "globals.c", directory: "/")
-!4 = !{}
-!5 = !{!0, !6}
-!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
-!7 

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 276157.
mib added a comment.

- Addressed Jim's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/target_var/Makefile
  lldb/test/API/functionalities/target_var/TestTargetVar.py
  lldb/test/API/functionalities/target_var/globals.c
  lldb/test/API/functionalities/target_var/globals.ll
  lldb/test/API/functionalities/target_var/main.c

Index: lldb/test/API/functionalities/target_var/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/target_var/main.c
@@ -0,0 +1,7 @@
+int i = 42;
+int *p = 
+
+struct incomplete;
+struct incomplete *var = (struct incomplete *)0xdead;
+
+int main() { return *p; }
Index: lldb/test/API/functionalities/target_var/globals.ll
===
--- lldb/test/API/functionalities/target_var/globals.ll
+++ /dev/null
@@ -1,42 +0,0 @@
-source_filename = "globals.c"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.14.0"
-
-@i = global i32 42, align 4
-@p = global i32* @i, align 8, !dbg !0, !dbg !6
-
-; Function Attrs: noinline nounwind optnone ssp uwtable
-define i32 @main() #0 !dbg !15 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval, align 4
-  %0 = load i32*, i32** @p, align 8, !dbg !18
-  %1 = load i32, i32* %0, align 4, !dbg !18
-  ret i32 %1, !dbg !18
-}
-
-attributes #0 = { noinline nounwind optnone ssp uwtable }
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!10, !11, !12, !13}
-!llvm.ident = !{!14}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
-!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: FullDebug, globals: !5)
-!3 = !DIFile(filename: "globals.c", directory: "/")
-!4 = !{}
-!5 = !{!0, !6}
-!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
-!7 = distinct !DIGlobalVariable(name: "p", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
-!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
-!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!10 = !{i32 2, !"Dwarf Version", i32 4}
-!11 = !{i32 2, !"Debug Info Version", i32 3}
-!12 = !{i32 1, !"wchar_size", i32 4}
-!13 = !{i32 7, !"PIC Level", i32 2}
-!14 = !{!"clang version 8.0.0 (trunk 340838) (llvm/trunk 340843)"}
-!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !2, retainedNodes: !4)
-!16 = !DISubroutineType(types: !17)
-!17 = !{!9}
-!18 = !DILocation(line: 5, scope: !15)
Index: lldb/test/API/functionalities/target_var/globals.c
===
--- lldb/test/API/functionalities/target_var/globals.c
+++ /dev/null
@@ -1,6 +0,0 @@
-int i = 42;
-int *p = 
-
-int main() {
-  return *p;
-}
Index: lldb/test/API/functionalities/target_var/TestTargetVar.py
===
--- lldb/test/API/functionalities/target_var/TestTargetVar.py
+++ lldb/test/API/functionalities/target_var/TestTargetVar.py
@@ -20,3 +20,5 @@
 self.build()
 lldbutil.run_to_name_breakpoint(self, 'main')
 self.expect("target variable i", substrs=['i', '42'])
+self.expect("target variable var", patterns=['\(incomplete \*\) var = 0[xX](0)*dead'])
+self.expect("target variable var[0]", error=True, substrs=["can't find global variable 'var[0]'"])
Index: lldb/test/API/functionalities/target_var/Makefile
===
--- lldb/test/API/functionalities/target_var/Makefile
+++ lldb/test/API/functionalities/target_var/Makefile
@@ -1,5 +1,3 @@
-include Makefile.rules
+C_SOURCES := main.c
 
-a.out: globals.ll
-	$(CC) $(CFLAGS) -g -c $^ -o globals.o
-	$(LD) $(LDFLAGS) -g globals.o -o $@
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -687,10 +687,15 @@
 language_flags);
   }
 
-  if (!valobj && synthetic_array_member)
-valobj = GetSyntheticValue()
- ->GetChildAtIndex(synthetic_index, synthetic_array_member)
- .get();
+  // In case of an incomplete type, LLDB will try to use the ValueObject's
+  // synthetic value to create the child ValueObject.
+  if (!valobj && synthetic_array_member) {
+if (ValueObjectSP synth_valobj_sp = GetSyntheticValue()) {
+  valobj = synth_valobj_sp
+   

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Jim Ingham via Phabricator via lldb-commits]

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

Other than a quibble about using _sp suffix, this is fine.




Comment at: lldb/source/Core/ValueObject.cpp:693
+  if (!valobj && synthetic_array_member) {
+if (ValueObjectSP synth_valobj = GetSyntheticValue()) {
+  valobj =

We usually use _sp suffix for shared pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D83327#2136863 , @davide wrote:

> In D83327#2136842 , @jingham wrote:
>
> > In D83327#2136814 , @davide wrote:
> >
> > > Aside from cosmetics, I'm not entirely sure this is the correct fix. Why 
> > > are we calling this code _at all_ if the type is incomplete?
> >
> >
> > Doing so allows one to write a synthetic child provider that provides the 
> > fields for an incomplete type.  This is useful if you don't have debug info 
> > for a given type but know its layouts by some other means.
>
>
> Interesting, thanks. Do we have an example of when this triggers?


This change was first introduced by D79554 , 
with the `CFDictionaryRef` formatter. They reused the same logic as 
NSCFDictionary but needed this to generate the ValueObject's children.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 276152.
mib added a comment.

- Addressed style comment.
- Added a comment to give more context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/target_var/Makefile
  lldb/test/API/functionalities/target_var/TestTargetVar.py
  lldb/test/API/functionalities/target_var/globals.c
  lldb/test/API/functionalities/target_var/globals.ll
  lldb/test/API/functionalities/target_var/main.c

Index: lldb/test/API/functionalities/target_var/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/target_var/main.c
@@ -0,0 +1,7 @@
+int i = 42;
+int *p = 
+
+struct incomplete;
+struct incomplete *var = (struct incomplete *)0xdead;
+
+int main() { return *p; }
Index: lldb/test/API/functionalities/target_var/globals.ll
===
--- lldb/test/API/functionalities/target_var/globals.ll
+++ /dev/null
@@ -1,42 +0,0 @@
-source_filename = "globals.c"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.14.0"
-
-@i = global i32 42, align 4
-@p = global i32* @i, align 8, !dbg !0, !dbg !6
-
-; Function Attrs: noinline nounwind optnone ssp uwtable
-define i32 @main() #0 !dbg !15 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval, align 4
-  %0 = load i32*, i32** @p, align 8, !dbg !18
-  %1 = load i32, i32* %0, align 4, !dbg !18
-  ret i32 %1, !dbg !18
-}
-
-attributes #0 = { noinline nounwind optnone ssp uwtable }
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!10, !11, !12, !13}
-!llvm.ident = !{!14}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
-!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: FullDebug, globals: !5)
-!3 = !DIFile(filename: "globals.c", directory: "/")
-!4 = !{}
-!5 = !{!0, !6}
-!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
-!7 = distinct !DIGlobalVariable(name: "p", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
-!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
-!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!10 = !{i32 2, !"Dwarf Version", i32 4}
-!11 = !{i32 2, !"Debug Info Version", i32 3}
-!12 = !{i32 1, !"wchar_size", i32 4}
-!13 = !{i32 7, !"PIC Level", i32 2}
-!14 = !{!"clang version 8.0.0 (trunk 340838) (llvm/trunk 340843)"}
-!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !2, retainedNodes: !4)
-!16 = !DISubroutineType(types: !17)
-!17 = !{!9}
-!18 = !DILocation(line: 5, scope: !15)
Index: lldb/test/API/functionalities/target_var/globals.c
===
--- lldb/test/API/functionalities/target_var/globals.c
+++ /dev/null
@@ -1,6 +0,0 @@
-int i = 42;
-int *p = 
-
-int main() {
-  return *p;
-}
Index: lldb/test/API/functionalities/target_var/TestTargetVar.py
===
--- lldb/test/API/functionalities/target_var/TestTargetVar.py
+++ lldb/test/API/functionalities/target_var/TestTargetVar.py
@@ -20,3 +20,5 @@
 self.build()
 lldbutil.run_to_name_breakpoint(self, 'main')
 self.expect("target variable i", substrs=['i', '42'])
+self.expect("target variable var", patterns=['\(incomplete \*\) var = 0[xX](0)*dead'])
+self.expect("target variable var[0]", error=True, substrs=["can't find global variable 'var[0]'"])
Index: lldb/test/API/functionalities/target_var/Makefile
===
--- lldb/test/API/functionalities/target_var/Makefile
+++ lldb/test/API/functionalities/target_var/Makefile
@@ -1,5 +1,3 @@
-include Makefile.rules
+C_SOURCES := main.c
 
-a.out: globals.ll
-	$(CC) $(CFLAGS) -g -c $^ -o globals.o
-	$(LD) $(LDFLAGS) -g globals.o -o $@
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -687,10 +687,15 @@
 language_flags);
   }
 
-  if (!valobj && synthetic_array_member)
-valobj = GetSyntheticValue()
- ->GetChildAtIndex(synthetic_index, synthetic_array_member)
- .get();
+  // In case of an incomplete type, LLDB will try to use the ValueObject's
+  // synthetic value to create the child ValueObject.
+  if (!valobj && synthetic_array_member) {
+if (ValueObjectSP synth_valobj = GetSyntheticValue()) {
+  valobj =
+  

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D83327#2136842 , @jingham wrote:

> In D83327#2136814 , @davide wrote:
>
> > Aside from cosmetics, I'm not entirely sure this is the correct fix. Why 
> > are we calling this code _at all_ if the type is incomplete?
>
>
> Doing so allows one to write a synthetic child provider that provides the 
> fields for an incomplete type.  This is useful if you don't have debug info 
> for a given type but know its layouts by some other means.


Interesting, thanks. Do we have an example of when this triggers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

It sounds like that would be worth adding as a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D83327#2136814 , @davide wrote:

> Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are 
> we calling this code _at all_ if the type is incomplete?


The code is called because some incomplete types can actually be typedefs. In 
this case, we use the pointee type to create the ValueObject child (in 
ValueObject::CreateChildAtIndex)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D83327#2136814 , @davide wrote:

> Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are 
> we calling this code _at all_ if the type is incomplete?


Doing so allows one to write a synthetic child provider that provides the 
fields for an incomplete type.  This is useful if you don't have debug info for 
a given type but know its layouts by some other means.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we 
calling this code _at all_ if the type is incomplete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:691
+  if (!valobj && synthetic_array_member) {
+auto synth_valobj = GetSyntheticValue();
+if (!synth_valobj)

Style-nit: I think few people love early returns a much as I do, but here I 
think the llvm-way of checking a pointer is would be preferable over a 
very-late-early-exit: 
```
if (auto* synth_valobj = GetSyntheticValue()) {
  valobj =
synth_valobj->GetChildAtIndex(synthetic_index, synthetic_array_member)
.get();
}
```
Additionally, I'm not sure if this should be `auto` according to the LLVM 
coding rules. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

@labath Thanks for reporting that crash. D83327 
 should fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: jingham, labath.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.

The patch fixes a crash in ValueObject::CreateChildAtIndex caused by a
null pointer dereferencing. This is a corner case that is happening when
trying to dereference a variable with an incomplete type, and this same
variable doesn't have a synthetic value to get the child ValueObject.

If this happens, lldb will now return a null pointer that will results
in an error message.

rdar://65181171

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83327

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/target_var/Makefile
  lldb/test/API/functionalities/target_var/TestTargetVar.py
  lldb/test/API/functionalities/target_var/globals.c
  lldb/test/API/functionalities/target_var/globals.ll
  lldb/test/API/functionalities/target_var/main.c

Index: lldb/test/API/functionalities/target_var/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/target_var/main.c
@@ -0,0 +1,7 @@
+int i = 42;
+int *p = 
+
+struct incomplete;
+struct incomplete *var = (struct incomplete *)0xdead;
+
+int main() { return *p; }
Index: lldb/test/API/functionalities/target_var/globals.ll
===
--- lldb/test/API/functionalities/target_var/globals.ll
+++ /dev/null
@@ -1,42 +0,0 @@
-source_filename = "globals.c"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.14.0"
-
-@i = global i32 42, align 4
-@p = global i32* @i, align 8, !dbg !0, !dbg !6
-
-; Function Attrs: noinline nounwind optnone ssp uwtable
-define i32 @main() #0 !dbg !15 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval, align 4
-  %0 = load i32*, i32** @p, align 8, !dbg !18
-  %1 = load i32, i32* %0, align 4, !dbg !18
-  ret i32 %1, !dbg !18
-}
-
-attributes #0 = { noinline nounwind optnone ssp uwtable }
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!10, !11, !12, !13}
-!llvm.ident = !{!14}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
-!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: FullDebug, globals: !5)
-!3 = !DIFile(filename: "globals.c", directory: "/")
-!4 = !{}
-!5 = !{!0, !6}
-!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
-!7 = distinct !DIGlobalVariable(name: "p", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
-!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
-!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!10 = !{i32 2, !"Dwarf Version", i32 4}
-!11 = !{i32 2, !"Debug Info Version", i32 3}
-!12 = !{i32 1, !"wchar_size", i32 4}
-!13 = !{i32 7, !"PIC Level", i32 2}
-!14 = !{!"clang version 8.0.0 (trunk 340838) (llvm/trunk 340843)"}
-!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !2, retainedNodes: !4)
-!16 = !DISubroutineType(types: !17)
-!17 = !{!9}
-!18 = !DILocation(line: 5, scope: !15)
Index: lldb/test/API/functionalities/target_var/globals.c
===
--- lldb/test/API/functionalities/target_var/globals.c
+++ /dev/null
@@ -1,6 +0,0 @@
-int i = 42;
-int *p = 
-
-int main() {
-  return *p;
-}
Index: lldb/test/API/functionalities/target_var/TestTargetVar.py
===
--- lldb/test/API/functionalities/target_var/TestTargetVar.py
+++ lldb/test/API/functionalities/target_var/TestTargetVar.py
@@ -20,3 +20,5 @@
 self.build()
 lldbutil.run_to_name_breakpoint(self, 'main')
 self.expect("target variable i", substrs=['i', '42'])
+self.expect("target variable var", patterns=['\(incomplete \*\) var = 0[xX](0)*dead'])
+self.expect("target variable var[0]", error=True, substrs=["can't find global variable 'var[0]'"])
Index: lldb/test/API/functionalities/target_var/Makefile
===
--- lldb/test/API/functionalities/target_var/Makefile
+++ lldb/test/API/functionalities/target_var/Makefile
@@ -1,5 +1,3 @@
-include Makefile.rules
+C_SOURCES := main.c
 
-a.out: globals.ll
-	$(CC) $(CFLAGS) -g -c $^ -o globals.o
-	$(LD) $(LDFLAGS) -g globals.o -o $@
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -687,10 +687,15 @@
 language_flags);
   }
 
-  if (!valobj && synthetic_array_member)
-valobj = GetSyntheticValue()
-  

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e9b16b67f5b: [lldb] Fix unaligned load in DataExtractor 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83256

Files:
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -299,3 +299,105 @@
   EXPECT_EQ(expected, BE.GetULEB128());
   EXPECT_EQ(9U, offset);
 }
+
+TEST(DataExtractorTest, GetFloat) {
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetFloatUnaligned) {
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDouble) {
+  if (sizeof(double) != 8)
+return;
+
+  double expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDoubleUnaligned) {
+  if (sizeof(double) != 8)
+return;
+
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+}
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -15,7 +15,6 @@
 
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -624,41 +623,11 @@
 }
 
 float DataExtractor::GetFloat(offset_t *offset_ptr) const {
-  typedef float float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0f);
 }
 
 double DataExtractor::GetDouble(offset_t *offset_ptr) const {
-  typedef double float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-

[Lldb-commits] [lldb] 5e9b16b - [lldb] Fix unaligned load in DataExtractor

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-07-07T10:13:41-07:00
New Revision: 5e9b16b67f5b9543e8d22b5b0f22cc0980c310bd

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

LOG: [lldb] Fix unaligned load in DataExtractor

Somehow UBSan would only report the unaligned load in TestLinuxCore.py
when running the tests with reproducers. This patch fixes the issue by
using a memcpy in the GetDouble and the GetFloat method.

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

Added: 


Modified: 
lldb/include/lldb/Utility/DataExtractor.h
lldb/source/Utility/DataExtractor.cpp
lldb/unittests/Utility/DataExtractorTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/DataExtractor.h 
b/lldb/include/lldb/Utility/DataExtractor.h
index 8f82ae5af42d..0210af5cf6d1 100644
--- a/lldb/include/lldb/Utility/DataExtractor.h
+++ b/lldb/include/lldb/Utility/DataExtractor.h
@@ -9,12 +9,14 @@
 #ifndef LLDB_UTILITY_DATAEXTRACTOR_H
 #define LLDB_UTILITY_DATAEXTRACTOR_H
 
+#include "lldb/Utility/Endian.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/SwapByteOrder.h"
 
 #include 
 #include 
@@ -979,6 +981,21 @@ class DataExtractor {
   }
 
 protected:
+  template  T Get(lldb::offset_t *offset_ptr, T fail_value) const {
+constexpr size_t src_size = sizeof(T);
+T val = fail_value;
+
+const T *src = static_cast(GetData(offset_ptr, src_size));
+if (!src)
+  return val;
+
+memcpy(, src, src_size);
+if (m_byte_order != endian::InlHostByteOrder())
+  llvm::sys::swapByteOrder(val);
+
+return val;
+  }
+
   // Member variables
   const uint8_t *m_start; ///< A pointer to the first byte of data.
   const uint8_t

diff  --git a/lldb/source/Utility/DataExtractor.cpp 
b/lldb/source/Utility/DataExtractor.cpp
index ac3662a8e3c8..64b878d7dee7 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -15,7 +15,6 @@
 
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -624,41 +623,11 @@ int64_t DataExtractor::GetMaxS64Bitfield(offset_t 
*offset_ptr, size_t size,
 }
 
 float DataExtractor::GetFloat(offset_t *offset_ptr) const {
-  typedef float float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0f);
 }
 
 double DataExtractor::GetDouble(offset_t *offset_ptr) const {
-  typedef double float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0);
 }
 
 long double DataExtractor::GetLongDouble(offset_t *offset_ptr) const {

diff  --git a/lldb/unittests/Utility/DataExtractorTest.cpp 
b/lldb/unittests/Utility/DataExtractorTest.cpp
index 109c15297b16..536e69755d17 100644
--- a/lldb/unittests/Utility/DataExtractorTest.cpp
+++ b/lldb/unittests/Utility/DataExtractorTest.cpp
@@ -299,3 +299,105 @@ TEST(DataExtractorTest, GetULEB128_bit63) {
   EXPECT_EQ(expected, BE.GetULEB128());
   EXPECT_EQ(9U, offset);
 }
+
+TEST(DataExtractorTest, GetFloat) {
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+}
+
+TEST(DataExtractorTest, 

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Host/Editline.h:381
   void *m_completion_callback_baton = nullptr;
-
+  std::string m_current_autosuggestion = "";
+  bool m_use_autosuggestion = false;

Strings are empty by default, so you can omit the assignment: `= ""`. 



Comment at: lldb/include/lldb/Host/Editline.h:382
+  std::string m_current_autosuggestion = "";
+  bool m_use_autosuggestion = false;
   std::mutex m_output_mutex;

Why do we need this? Can we check that the `m_suggestion_callback` is null 
instead? 



Comment at: lldb/include/lldb/Host/Editline.h:320
 
+  unsigned char ApplyCompleteCommand(int ch);
+

teemperor wrote:
> This function and the one below should be documented like the rest.
It's not really clear to me what this method does based on the name or the 
description above. It seems similar to `TabCommand`, so maybe 
`AutosuggestCommand` would be a better name and more consistent? 



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:355
+  /// command line.
+  llvm::Optional
+  GetAutoSuggestionForCommand(llvm::StringRef line);

Although I believe the current implementation is correct (because the returned 
string is backed by `m_command_history`) I think this interface looks rather 
suspicious by returning a `StringRef`. Do we envision this returning //new// 
string in the future? If so we might consider having it return a 
`llvm::Optional`. 



Comment at: lldb/source/Core/CoreProperties.td:137
+DefaultTrue,
+Desc<"Whether to show autosuggestions or not.">;
 }

teemperor wrote:
> Usually these settings start like `If true, LLDB will show suggestions on 
> possible commands the user might want to type.` or something like that.
> If true, LLDB will show suggestions to complete the command the user typed.



Comment at: lldb/source/Core/IOHandler.cpp:271
 m_editline_up->SetAutoCompleteCallback(AutoCompleteCallback, this);
+m_editline_up->SetShowAutosuggestion(debugger.GetUseAutosuggestion());
 // See if the delegate supports fixing indentation

Why should we set the callback if the Autosuggestions are off? Why not do:

```
if(debugger.GetUseAutosuggestion()))
  m_editline_up->SetSuggestionCallback(SuggestionCallback, this);
```

Both have the same downside we already discussed, that the setting cannot be 
changed for the current EditLine instance. 


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D83008#2136303 , @teemperor wrote:

> I think we are talking about different things. My question was why is this a 
> 'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and 
> not a test in the `API` directory (using Python). An 'API' test could use the 
> proper expression testing tools. And it could actually run when doing 
> on-device testing (which is to my knowledge not supported for Shell tests) 
> which seems important for a test concerning a bug that only triggers when 
> doing on-device testing (beside that one ubuntu ARM bot).


I did not realize the shell tests were not tested on device.

> Also when looking over the ARM-specific test, I think there might be two bugs 
> that were involved in triggering it. One is the bug fixed here which triggers 
> that Clang will produce its own layout for those classes. Now I also wonder 
> why the layout the expression parser Clang generates doesn't match the one 
> from the test (which appears to be a second bug). The ARM-specific test 
> doesn't have any information in its AST that isn't also available in the 
> expression AST, so why would they produce different layouts? Not sure what 
> exactly is behind the "differences in how it deals with tail padding" 
> description but I assume this is related to tail padding reuse? If yes, then 
> maybe the second bug is that the records in our AST are (not) PODs in the 
> expression AST (see the inline comment for where the tail padding is 
> enabled/disabling based on whether the RD is POD).

So we are hitting this code for non-arm64 case:

  case TargetCXXABI::UseTailPaddingUnlessPOD03:
//...
return RD->isPOD();

and we return `false` which is correct for the original test case since it has 
a base class and base therefore not an aggregate (until C++17) see it here 

 and struct that had the difference I was looking at. I did not check for the 
test cases in the PR though.

In the arm64 case from the original problem we hit the following:

  case TargetCXXABI::UseTailPaddingUnlessPOD11:
 // ...
 return RD->isTrivial() && RD->isCXX11StandardLayout();

which returns `true` for the original case.


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

https://reviews.llvm.org/D83008



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

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

This is fine.  If there were a universal convention (e.g. : separated lists) 
for environment variables, it would be reasonable for "append" behavior to 
extend rather than replace extant variables, but in the absence of the 
universality of such a convention, the best we can do is overwrite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83306



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

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 276100.
JDevlieghere marked 3 inline comments as done.

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

https://reviews.llvm.org/D83256

Files:
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -299,3 +299,105 @@
   EXPECT_EQ(expected, BE.GetULEB128());
   EXPECT_EQ(9U, offset);
 }
+
+TEST(DataExtractorTest, GetFloat) {
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetFloatUnaligned) {
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDouble) {
+  if (sizeof(double) != 8)
+return;
+
+  double expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDoubleUnaligned) {
+  if (sizeof(double) != 8)
+return;
+
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+}
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -15,7 +15,6 @@
 
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -624,41 +623,11 @@
 }
 
 float DataExtractor::GetFloat(offset_t *offset_ptr) const {
-  typedef float float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0f);
 }
 
 double DataExtractor::GetDouble(offset_t *offset_ptr) const {
-  typedef double float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D81697#2131004 , @bbli wrote:

> > Ideally this error should include the actual command line
>
> So to clarify, you want to also print out the exact clang command/what flags 
> were passed to clang whenever compile errors happen?


Yes, basically the entire output of the `make` command, as if I had run it in a 
shell (that's what I normally do when I can't decipher the test suite error). I 
think it might be possible to get make to print _only_ the command that failed, 
but I think that's: a) not worth it; b) the content of the other commands can 
be useful at times.

> On another note, would it be more appropriate to be communicating through 
> email rather than on this pull request?

I don't care either way. Feel free to drop me an email if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81697



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7fa7b81bcbd0: Combine multiple defs of arm64 register sets 
(authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D80105?vs=274382=275668#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80105

Files:
  lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
  
lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.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/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoAndSetInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -82,7 +82,6 @@
 case llvm::Triple::FreeBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -111,7 +110,6 @@
 case llvm::Triple::NetBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
@@ -128,7 +126,6 @@
 reg_interface = new RegisterInfoPOSIX_arm(arch);
 break;
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::mipsel:
   case llvm::Triple::mips:
@@ -159,7 +156,6 @@
 case llvm::Triple::OpenBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -180,7 +176,7 @@
   break;
 }
 
-if (!reg_interface) {
+if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64) {
   LLDB_LOGF(log, "elf-core::%s:: Architecture(%d) or OS(%d) not supported",
 __FUNCTION__, arch.GetMachine(), arch.GetTriple().getOS());
   assert(false && "Architecture or OS not supported");
@@ -189,7 +185,8 @@
 switch (arch.GetMachine()) {
 case llvm::Triple::aarch64:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, std::make_unique(arch),
+  m_gpregset_data, m_notes);
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm64(
   lldb_private::Thread ,
-  lldb_private::RegisterInfoInterface *register_info,
+  std::unique_ptr register_info,
   const lldb_private::DataExtractor ,
   llvm::ArrayRef notes);
 
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -17,16 +17,16 @@
 using namespace lldb_private;
 
 RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
-Thread , RegisterInfoInterface *register_info,
+Thread , std::unique_ptr register_info,
 const DataExtractor , llvm::ArrayRef notes)
-: RegisterContextPOSIX_arm64(thread, 0, register_info) {
+: RegisterContextPOSIX_arm64(thread, std::move(register_info)) {
   m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think we are talking about different things. My question was why is this a 
'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and 
not a test in the `API` directory (using Python). An 'API' test could use the 
proper expression testing tools. And it could actually run when doing on-device 
testing (which is to my knowledge not supported for Shell tests) which seems 
important for a test concerning a bug that only triggers when doing on-device 
testing (beside that one ubuntu ARM bot).

Also when looking over the ARM-specific test, I think there might be two bugs 
that were involved in triggering it. One is the bug fixed here which triggers 
that Clang will produce its own layout for those classes. Now I also wonder why 
the layout the expression parser Clang generates doesn't match the one from the 
test (which appears to be a second bug). The ARM-specific test doesn't have any 
information in its AST that isn't also available in the expression AST, so why 
would they produce different layouts? Not sure what exactly is behind the 
"differences in how it deals with tail padding" description but I assume this 
is related to tail padding reuse? If yes, then maybe the second bug is that the 
records in our AST are (not) PODs in the expression AST (see the inline comment 
for where the tail padding is enabled/disabling based on whether the RD is POD).




Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2197
 // least as many of them as possible).
 return RD->isTrivial() && RD->isCXX11StandardLayout();
   }

See here for the POD check that we might get wrong.



Comment at: lldb/test/Shell/Expr/Inputs/layout.cpp:22
+
+class D2 : public B2, public Mixin {};
+

I think there should be some comment that explains why this test is structured 
like this (maybe point out where the tail padding change is happening).



Comment at: lldb/test/Shell/Expr/Inputs/layout.cpp:40
+  D2 d2;
+  D3 d3;
+

Do we actually need these locals in addition to the globals?


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

https://reviews.llvm.org/D83008



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7fa7b81bcbd0: Combine multiple defs of arm64 register sets 
(authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80105

Files:
  lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
  
lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.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/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoAndSetInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -82,7 +82,6 @@
 case llvm::Triple::FreeBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -111,7 +110,6 @@
 case llvm::Triple::NetBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
@@ -128,7 +126,6 @@
 reg_interface = new RegisterInfoPOSIX_arm(arch);
 break;
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::mipsel:
   case llvm::Triple::mips:
@@ -159,7 +156,6 @@
 case llvm::Triple::OpenBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -180,7 +176,7 @@
   break;
 }
 
-if (!reg_interface) {
+if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64) {
   LLDB_LOGF(log, "elf-core::%s:: Architecture(%d) or OS(%d) not supported",
 __FUNCTION__, arch.GetMachine(), arch.GetTriple().getOS());
   assert(false && "Architecture or OS not supported");
@@ -189,7 +185,8 @@
 switch (arch.GetMachine()) {
 case llvm::Triple::aarch64:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, std::make_unique(arch),
+  m_gpregset_data, m_notes);
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm64(
   lldb_private::Thread ,
-  lldb_private::RegisterInfoInterface *register_info,
+  std::unique_ptr register_info,
   const lldb_private::DataExtractor ,
   llvm::ArrayRef notes);
 
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -17,16 +17,16 @@
 using namespace lldb_private;
 
 RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
-Thread , RegisterInfoInterface *register_info,
+Thread , std::unique_ptr register_info,
 const DataExtractor , llvm::ArrayRef notes)
-: RegisterContextPOSIX_arm64(thread, 0, register_info) {
+: RegisterContextPOSIX_arm64(thread, std::move(register_info)) {
   m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
   m_gpr.SetByteOrder(gpregset.GetByteOrder());
 
   

[Lldb-commits] [lldb] 7fa7b81 - Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-07-07T20:25:02+05:00
New Revision: 7fa7b81bcbd040d036f9a4155486ccebc3c13be7

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

LOG: Combine multiple defs of arm64 register sets

Summary:
This patch aims to combine similar arm64 register set definitions defined in 
NativeRegisterContextLinux_arm64 and RegisterContextPOSIX_arm64.
I have implemented a register set interface out of RegisterInfoInterface class 
and moved arm64 register sets into RegisterInfosPOSIX_arm64 which is similar to 
Utility/RegisterContextLinux_* implemented by various other targets. This will 
help in managing register sets of new ARM64 architecture features in one place.

Built and tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabihf 
targets.

Reviewers: labath

Reviewed By: labath

Subscribers: mhorne, emaste, kristof.beyls, atanasyan, danielkiss, lldb-commits

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

Added: 
lldb/source/Plugins/Process/Utility/RegisterInfoAndSetInterface.h

Modified: 
lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp

lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp

lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.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/Utility/NativeRegisterContextRegisterInfo.h
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp 
b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
index 32c6a440db59..48dbddb86cca 100644
--- a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
@@ -164,7 +164,6 @@ lldb::RegisterContextSP FreeBSDThread::GetRegisterContext() 
{
 assert(target_arch.GetTriple().getOS() == llvm::Triple::FreeBSD);
 switch (target_arch.GetMachine()) {
 case llvm::Triple::aarch64:
-  reg_interface = new RegisterInfoPOSIX_arm64(target_arch);
   break;
 case llvm::Triple::arm:
   reg_interface = new RegisterInfoPOSIX_arm(target_arch);
@@ -193,7 +192,8 @@ lldb::RegisterContextSP FreeBSDThread::GetRegisterContext() 
{
 switch (target_arch.GetMachine()) {
 case llvm::Triple::aarch64: {
   RegisterContextPOSIXProcessMonitor_arm64 *reg_ctx =
-  new RegisterContextPOSIXProcessMonitor_arm64(*this, 0, 
reg_interface);
+  new RegisterContextPOSIXProcessMonitor_arm64(
+  *this, std::make_unique(target_arch));
   m_posix_thread = reg_ctx;
   m_reg_context_sp.reset(reg_ctx);
   break;

diff  --git 
a/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
 
b/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
index 6403d98a74cb..d3eafae1e6de 100644
--- 
a/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
@@ -22,9 +22,9 @@ using namespace lldb_private;
 
 RegisterContextPOSIXProcessMonitor_arm64::
 RegisterContextPOSIXProcessMonitor_arm64(
-lldb_private::Thread , uint32_t concrete_frame_idx,
-lldb_private::RegisterInfoInterface *register_info)
-: RegisterContextPOSIX_arm64(thread, concrete_frame_idx, register_info) {}
+lldb_private::Thread ,
+std::unique_ptr register_info)
+: RegisterContextPOSIX_arm64(thread, std::move(register_info)) {}
 
 ProcessMonitor _arm64::GetMonitor() {
   lldb::ProcessSP base = CalculateProcess();

diff  --git 
a/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.h
 
b/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.h
index d54d34e89cad..f100d905e28f 100644
--- 
a/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.h
+++ 
b/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.h
@@ -17,8 +17,8 @@ class RegisterContextPOSIXProcessMonitor_arm64
   public POSIXBreakpointProtocol {
 public:
   RegisterContextPOSIXProcessMonitor_arm64(
-   

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

[walter erquinigo via Phabricator via lldb-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83306



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

[Lldb-commits] [lldb] 52495b9 - [lldb/Utility] Fix float->integral conversions in Scalar APInt getters

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-07T16:59:06+02:00
New Revision: 52495b98eecefcbaea2e30edec6816e43653d175

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

LOG: [lldb/Utility] Fix float->integral conversions in Scalar APInt getters

These functions were doing a bitcast on the float value, which is not
consistent with the other getters, which were doing a numeric conversion
(47.0 -> 47). Change these to do numeric conversions too.

Added: 


Modified: 
lldb/source/Utility/Scalar.cpp
lldb/unittests/Utility/ScalarTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 7397744fb51c..6e2715130c70 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -557,6 +557,14 @@ bool Scalar::MakeUnsigned() {
   return success;
 }
 
+static llvm::APInt ToAPInt(const llvm::APFloat , unsigned bits,
+   bool is_unsigned) {
+  llvm::APSInt result(bits, is_unsigned);
+  bool isExact;
+  f.convertToInteger(result, llvm::APFloat::rmTowardZero, );
+  return std::move(result);
+}
+
 template  T Scalar::GetAs(T fail_value) const {
   switch (GetCategory(m_type)) {
   case Category::Void:
@@ -565,12 +573,9 @@ template  T Scalar::GetAs(T fail_value) const {
 if (IsSigned(m_type))
   return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
 return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue();
-  case Category::Float: {
-llvm::APSInt result(sizeof(T) * 8, std::is_unsigned::value);
-bool isExact;
-m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, );
-return result.getSExtValue();
-  }
+  case Category::Float:
+return ToAPInt(m_float, sizeof(T) * 8, std::is_unsigned::value)
+.getSExtValue();
   }
   return fail_value;
 }
@@ -612,51 +617,25 @@ unsigned long long Scalar::ULongLong(unsigned long long 
fail_value) const {
 }
 
 llvm::APInt Scalar::SInt128(const llvm::APInt _value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
 break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
 return m_integer;
-  case e_float:
-  case e_double:
-  case e_long_double:
-return m_float.bitcastToAPInt();
+  case Category::Float:
+return ToAPInt(m_float, 128, /*is_unsigned=*/false);
   }
   return fail_value;
 }
 
 llvm::APInt Scalar::UInt128(const llvm::APInt _value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
 break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
 return m_integer;
-  case e_float:
-  case e_double:
-  case e_long_double:
-return m_float.bitcastToAPInt();
+  case Category::Float:
+return ToAPInt(m_float, 128, /*is_unsigned=*/true);
   }
   return fail_value;
 }

diff  --git a/lldb/unittests/Utility/ScalarTest.cpp 
b/lldb/unittests/Utility/ScalarTest.cpp
index 910ff173bc92..42a2f2aaebf2 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -66,16 +66,6 @@ TEST(ScalarTest, ComparisonFloat) {
   ASSERT_TRUE(s2 >= s1);
 }
 
-template 
-static T2 ConvertHost(T1 val, T2 (Scalar::*)(T2) const) {
-  return T2(val);
-}
-
-template 
-static T2 ConvertScalar(T1 val, T2 (Scalar::*conv)(T2) const) {
-  return (Scalar(val).*conv)(T2());
-}
-
 template  static void CheckConversion(T val) {
   SCOPED_TRACE("val = " + std::to_string(val));
   EXPECT_EQ((signed char)val, Scalar(val).SChar());
@@ -102,6 +92,19 @@ TEST(ScalarTest, Getters) {
   CheckConversion(0x8765432112345678ull);
   CheckConversion(42.25f);
   CheckConversion(42.25);
+
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0f, 
70.0f)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, -1, true) << 70,
+Scalar(-std::pow(2.0f, 70.0f)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, 1) << 70,
+Scalar(std::pow(2.0f, 70.0f)).UInt128(APInt()));
+  EXPECT_EQ(APInt(128, 0), Scalar(-std::pow(2.0f, 70.0f)).UInt128(APInt()));
+
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0, 70.0)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, -1, true) << 70,
+Scalar(-std::pow(2.0, 70.0)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0, 70.0)).UInt128(APInt()));
+  EXPECT_EQ(APInt(128, 0), Scalar(-std::pow(2.0, 70.0)).UInt128(APInt()));
 }
 
 TEST(ScalarTest, RightShiftOperator) {

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

this 

 is the line that motivated this patch -- if one has a LD_LIBRARY_PATH 
environment variable already set (as I happen to do), then that line will not 
overwrite it with the value needed for the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83306



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

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, wallace, jingham.
Herald added a project: LLDB.

This function was documented to overwrite entries with D76111 
, which was
adding a couple of similar functions. However, this function (unlike the
functions added in that patch) was/is not actually overwriting variables

- any pre-existing variables would get ignored.

This behavior does not seem to be intentional. In fact, before the refactor in
D41359 , this function could introduce 
duplicate entries, which could
have very surprising effects both inside lldb and on other applications
(some applications would take the first value, some the second one; in
lldb, attempting to unset a variable could make the second variable
become active, etc.).

Overwriting seems to be the most reasonable behavior here, so change the
code to match documentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83306

Files:
  lldb/source/API/SBLaunchInfo.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py


Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -53,6 +53,11 @@
 launch_info.SetEnvironment(env, append=True)
 self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 
env_count + 1)
 
+env.Set("FOO", "baz", overwrite=True)
+launch_info.SetEnvironment(env, append=True)
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 
env_count + 1)
+self.assertEqual(launch_info.GetEnvironment().Get("FOO"), "baz")
+
 # Make sure we can replace the launchInfo's environment
 env.Clear()
 env.Set("BAR", "foo", overwrite=True)
@@ -120,6 +125,11 @@
 env.SetEntries(entries, append=False)
 self.assertEqualEntries(env, ["X=x", "Y=y"])
 
+entries.Clear()
+entries.AppendList(["X=y", "Y=x"], 2)
+env.SetEntries(entries, append=True)
+self.assertEqualEntries(env, ["X=y", "Y=x"])
+
 # Test clear
 env.Clear()
 self.assertEqualEntries(env, [])
Index: lldb/source/API/SBLaunchInfo.cpp
===
--- lldb/source/API/SBLaunchInfo.cpp
+++ lldb/source/API/SBLaunchInfo.cpp
@@ -190,9 +190,10 @@
   LLDB_RECORD_METHOD(void, SBLaunchInfo, SetEnvironment,
  (const lldb::SBEnvironment &, bool), env, append);
   Environment  = env.ref();
-  if (append)
-m_opaque_sp->GetEnvironment().insert(refEnv.begin(), refEnv.end());
-  else
+  if (append) {
+for (auto  : refEnv)
+  m_opaque_sp->GetEnvironment().insert_or_assign(KV.first(), KV.second);
+  } else
 m_opaque_sp->GetEnvironment() = refEnv;
   m_opaque_sp->RegenerateEnvp();
 }


Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -53,6 +53,11 @@
 launch_info.SetEnvironment(env, append=True)
 self.assertEqual(launch_info.GetEnvironment().GetNumValues(), env_count + 1)
 
+env.Set("FOO", "baz", overwrite=True)
+launch_info.SetEnvironment(env, append=True)
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), env_count + 1)
+self.assertEqual(launch_info.GetEnvironment().Get("FOO"), "baz")
+
 # Make sure we can replace the launchInfo's environment
 env.Clear()
 env.Set("BAR", "foo", overwrite=True)
@@ -120,6 +125,11 @@
 env.SetEntries(entries, append=False)
 self.assertEqualEntries(env, ["X=x", "Y=y"])
 
+entries.Clear()
+entries.AppendList(["X=y", "Y=x"], 2)
+env.SetEntries(entries, append=True)
+self.assertEqualEntries(env, ["X=y", "Y=x"])
+
 # Test clear
 env.Clear()
 self.assertEqualEntries(env, [])
Index: lldb/source/API/SBLaunchInfo.cpp
===
--- lldb/source/API/SBLaunchInfo.cpp
+++ lldb/source/API/SBLaunchInfo.cpp
@@ -190,9 +190,10 @@
   LLDB_RECORD_METHOD(void, SBLaunchInfo, SetEnvironment,
  (const lldb::SBEnvironment &, bool), env, append);
   Environment  = env.ref();
-  if (append)
-m_opaque_sp->GetEnvironment().insert(refEnv.begin(), refEnv.end());
-  else
+  if (append) {
+for (auto  : refEnv)
+  m_opaque_sp->GetEnvironment().insert_or_assign(KV.first(), KV.second);
+  } else
 m_opaque_sp->GetEnvironment() = refEnv;
   m_opaque_sp->RegenerateEnvp();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] 72ae700 - [lldb/test] Fix lldbutil.run_to_***_breakpoint for shared libraries

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-07T16:07:35+02:00
New Revision: 72ae70032ca3fa3e3bd9a3524bf245d5978c0467

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

LOG: [lldb/test] Fix lldbutil.run_to_***_breakpoint for shared libraries

Even non-remote targets may need to set the launch environment
((DY)LD_LIBRARY_PATH, specifically) to run successfully.

Also, add an assertion to better detect the case when launching a target
fails and the breakpoint is never hit.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbutil.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index ef0df90c416e..25fcd5f29ee2 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -775,7 +775,7 @@ def run_to_breakpoint_do_run(test, target, bkpt, 
launch_info = None,
 launch_info = target.GetLaunchInfo()
 launch_info.SetWorkingDirectory(test.get_process_working_directory())
 
-if extra_images and lldb.remote_platform:
+if extra_images:
 environ = test.registerSharedLibrariesWithTarget(target, extra_images)
 launch_info.SetEnvironmentEntries(environ, True)
 
@@ -788,6 +788,8 @@ def run_to_breakpoint_do_run(test, target, bkpt, 
launch_info = None,
 test.assertFalse(error.Fail(),
  "Process launch failed: %s" % (error.GetCString()))
 
+test.assertEqual(process.GetState(), lldb.eStateStopped)
+
 # Frame #0 should be at our breakpoint.
 threads = get_threads_stopped_at_breakpoint(
 process, bkpt)



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

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, aprantl, JDevlieghere.
Herald added a reviewer: shafik.
Herald added a project: LLDB.

This effectively reverts r188124, which added code to handle
(DW_AT_)declarations of structures with some kinds of children as
definitions. The commit message claims this is a workaround for some
kind of debug info produced by gcc. However, it does not go into
specifics, so it's hard to reproduce or verify that this is indeed still a
problem.

Having this code is definitely a problem though, because it mistakenly
declares incomplete dwarf declarations to be complete. Both clang (with
-flimit-debug-info) and gcc (by default) generate DW_AT_declarations of
structs with children. This happens when full debug info for a class is
not emitted in a given compile unit (e.g. because of vtable homing), but
the class has inline methods which are used in the given compile unit.
In that case, the compilers emit a DW_AT_declaration of a class, but
add a DW_TAG_subprogram child to it to describe the inlined instance of
the method.

Even though the class tag has some children, it definitely does not
contain enough information to construct a full class definition (most
notably, it lacks any members). Keeping the class as incomplete allows
us to search for a real definition in other modules, helping the
-flimit-debug-info flow. And in case the definition is not found we can
display a error message saying that, instead of just showing an empty
struct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83302

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
  lldb/test/API/functionalities/limit-debug-info/main.cpp
  lldb/test/API/functionalities/limit-debug-info/one.cpp
  lldb/test/API/functionalities/limit-debug-info/onetwo.h
  lldb/test/API/functionalities/limit-debug-info/two.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
@@ -0,0 +1,160 @@
+# Test that a forward-declared (DW_AT_declaration) structure is treated as a
+# forward-declaration even if it has children. These types can be produced due
+# to vtable-based type homing, or other -flimit-debug-info optimizations.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj > %t
+# RUN: %lldb %t -o "expr a" -o exit 2>&1 | FileCheck %s --check-prefix=EXPR
+# RUN: %lldb %t -o "target var a" -o exit 2>&1 | FileCheck %s --check-prefix=VAR
+
+# EXPR: incomplete type 'A' where a complete type is required
+
+# FIXME: This should also produce some kind of an error.
+# VAR: (A) a = {}
+
+.text
+_ZN1AC2Ev:
+retq
+.LZN1AC2Ev_end:
+
+.data
+a:
+.quad   $_ZTV1A+16
+.quad   $0xdeadbeef
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   3   # Abbreviation Code
+.byte   19  # DW_TAG_structure_type
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   60  # DW_AT_declaration
+.byte   25  # DW_FORM_flag_present
+.byte   0  

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:690-694
+  if (!valobj && synthetic_array_member)
+valobj = GetSyntheticValue()
+ ->GetChildAtIndex(synthetic_index, synthetic_array_member)
+ .get();
+

I'm getting a crash here when attempting to dereference incomplete c(++) types. 
The simplest (albeit contrived) repro is:
```
$ cat /tmp/a.c 
struct incomplete;

struct incomplete *var = (struct incomplete *)0xdead;

int main() {}
$ clang /tmp/a.c -o /tmp/a.out -g
$ lldb /tmp/a.out -o "target variable var[0]"
(lldb) target create "/tmp/a.out"
Current executable set to '/tmp/a.out' (x86_64).
(lldb) target variable var[0]
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
Stack dump:
0.  Program arguments: lldb /tmp/a.out -o target variable var[0] 
 #0 0x561dcd331cea llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(lldb+0x29cea)
 #1 0x561dcd32ffc4 llvm::sys::RunSignalHandlers() (lldb+0x27fc4)
 #2 0x561dcd330108 SignalHandler(int) (lldb+0x28108)
 #3 0x7f62b35175b0 __restore_rt (/lib64/libpthread.so.0+0x135b0)
 #4 0x7f62ae9235fc lldb_private::ValueObject::CreateChildAtIndex(unsigned 
long, bool, int) (../lib/liblldb.so.11git+0xb985fc)
 #5 0x7f62ae923e2b 
lldb_private::ValueObject::GetSyntheticArrayMember(unsigned long, bool) 
(../lib/liblldb.so.11git+0xb98e2b)
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Pavel Labath via Phabricator via lldb-commits]

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

ship it.


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

https://reviews.llvm.org/D80105



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 276015.
omjavaid added a comment.

This revision fixed issues highlighted in last review.

LGTM?


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

https://reviews.llvm.org/D80105

Files:
  lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
  
lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_arm64.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/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoAndSetInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -82,7 +82,6 @@
 case llvm::Triple::FreeBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -111,7 +110,6 @@
 case llvm::Triple::NetBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
@@ -128,7 +126,6 @@
 reg_interface = new RegisterInfoPOSIX_arm(arch);
 break;
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::mipsel:
   case llvm::Triple::mips:
@@ -159,7 +156,6 @@
 case llvm::Triple::OpenBSD: {
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
-reg_interface = new RegisterInfoPOSIX_arm64(arch);
 break;
   case llvm::Triple::arm:
 reg_interface = new RegisterInfoPOSIX_arm(arch);
@@ -180,7 +176,7 @@
   break;
 }
 
-if (!reg_interface) {
+if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64) {
   LLDB_LOGF(log, "elf-core::%s:: Architecture(%d) or OS(%d) not supported",
 __FUNCTION__, arch.GetMachine(), arch.GetTriple().getOS());
   assert(false && "Architecture or OS not supported");
@@ -189,7 +185,8 @@
 switch (arch.GetMachine()) {
 case llvm::Triple::aarch64:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, std::make_unique(arch),
+  m_gpregset_data, m_notes);
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm64(
   lldb_private::Thread ,
-  lldb_private::RegisterInfoInterface *register_info,
+  std::unique_ptr register_info,
   const lldb_private::DataExtractor ,
   llvm::ArrayRef notes);
 
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -17,16 +17,16 @@
 using namespace lldb_private;
 
 RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
-Thread , RegisterInfoInterface *register_info,
+Thread , std::unique_ptr register_info,
 const DataExtractor , llvm::ArrayRef notes)
-: RegisterContextPOSIX_arm64(thread, 0, register_info) {
+: RegisterContextPOSIX_arm64(thread, std::move(register_info)) {
   m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
   m_gpr.SetByteOrder(gpregset.GetByteOrder());
 
   m_fpregset = getRegset(
-  notes, register_info->GetTargetArchitecture().GetTriple(), FPR_Desc);
+  notes, 

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid marked 3 inline comments as done.
omjavaid added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp:46
+: lldb_private::RegisterContext(thread, 0) {
+  m_register_info_up = std::move(register_info);
 

labath wrote:
> move this to the initializer list
ACK.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:85
+gpr_w22, gpr_w23,  gpr_w24, gpr_w25, gpr_w26, gpr_w27, gpr_w28,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};

labath wrote:
> I'd probably just delete this comment (or merge it with the leading comment 
> above the array definition), and then let clang-format lay this out 
> normally...
Let me check what clang-format emits.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:19
 public:
+  enum { ARM64GPR = 0, ARM64FPR };
+

labath wrote:
> Why these names? I think [GF]PRegSet would be better for two reasons:
> - the same names with the same purpose already exist in 
> `NativeRegisterContextNetBSD_x86_64.h`
> - it seems like a better way to differentiate from the [GF]PR structs below 
> than adding a redundant ARM64 prefix.
Indeed ARM64 is redundant now that we have these enums in 
RegisterInfosPOSIX_arm64 . I will fix this in updated revision.


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

https://reviews.llvm.org/D80105



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

[Lldb-commits] [lldb] de0175d - [lldb] Make TestIOHandlerResizeNoEditline pass with Python 2

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-07T13:54:14+02:00
New Revision: de0175d04bc3679c7bd8dc64520e790bf38f30b0

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

LOG: [lldb] Make TestIOHandlerResizeNoEditline pass with Python 2

io.BytesIO seems to produce a stream in Python 2 which isn't recognized
as a file object in the SWIG API, so this test fails for Python 2 (and I assume
also an old SWIG version needs to be involved).

Instead just open an empty input file which is a file object in all Python
versions to make this test pass everywhere.

Added: 
lldb/test/API/iohandler/resize/input_file

Modified: 
lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py

Removed: 




diff  --git a/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py 
b/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
index b0ef259fd85f..323e511496cd 100644
--- a/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
+++ b/lldb/test/API/iohandler/resize/TestIOHandlerResizeNoEditline.py
@@ -14,7 +14,7 @@ def test_resize_no_editline(self):
 dbg = lldb.SBDebugger.Create(False)
 # Set the input handle to some stream so that we don't start the
 # editline interface.
-dbg.SetInputFileHandle(io.BytesIO(b""), True)
+dbg.SetInputFileHandle(open("input_file"), True)
 opts = lldb.SBCommandInterpreterRunOptions()
 # Launch the command interpreter now.
 dbg.RunCommandInterpreter(True, True, opts, 0, False, False)

diff  --git a/lldb/test/API/iohandler/resize/input_file 
b/lldb/test/API/iohandler/resize/input_file
new file mode 100644
index ..e69de29bb2d1



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

[Lldb-commits] [lldb] 2cdf108 - [lldb/DWARF] Add a utility function for (forceful) completion of types

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-07T11:37:55+02:00
New Revision: 2cdf108d329bda280948ad634aa0a070337a5f88

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

LOG: [lldb/DWARF] Add a utility function for (forceful) completion of types

Summary:
Unify the code for requiring a complete type and move it into a single
place. The only functional change is that the "cannot start a definition
of an incomplete type" is upgrated from a runtime error/warning to an
lldbassert. An plain assert might also be fine, since (AFAICT) this can
only happen in case of a programmer error.

Reviewers: teemperor, aprantl, shafik

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 0bd2d0c05c1b..7de88274ccf6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1263,32 +1263,7 @@ TypeSP DWARFASTParserClang::ParseArrayType(const 
DWARFDIE ,
   if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
 attrs.byte_stride = element_type->GetByteSize().getValueOr(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
-
-  if (TypeSystemClang::IsCXXClassType(array_element_type) &&
-  !array_element_type.GetCompleteType()) {
-ModuleSP module_sp = die.GetModule();
-
-// Mark the class as complete, but we make a note of the fact that
-// this class is not _really_ complete so we can later search for a
-// definition in a 
diff erent module.
-// Since we provide layout assistance, all ivars in this class and other
-// classes will be fine even if we are not able to find the definition
-// elsewhere.
-if (TypeSystemClang::StartTagDeclarationDefinition(array_element_type)) {
-  TypeSystemClang::CompleteTagDeclarationDefinition(array_element_type);
-  const auto *td =
-  TypeSystemClang::GetQualType(array_element_type.GetOpaqueQualType())
-  .getTypePtr()
-  ->getAsTagDecl();
-  m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-} else {
-  module_sp->ReportError("DWARF DIE at 0x%8.8x was not able to "
- "start its definition.\nPlease file a "
- "bug and attach the file at the start "
- "of this error message",
- type_die.GetOffset());
-}
-  }
+  CompleteType(array_element_type);
 
   uint64_t array_element_bit_stride =
   attrs.byte_stride * 8 + attrs.bit_stride;
@@ -1343,6 +1318,28 @@ TypeSP DWARFASTParserClang::ParsePointerToMemberType(
   return nullptr;
 }
 
+void DWARFASTParserClang::CompleteType(CompilerType type) {
+  // Technically, enums can be incomplete too, but we don't handle those as 
they
+  // are emitted even under -flimit-debug-info.
+  if (!TypeSystemClang::IsCXXClassType(type))
+return;
+
+  if (type.GetCompleteType())
+return;
+
+  // No complete definition in this module.  Mark the class as complete to
+  // satisfy local ast invariants, but make a note of the fact that
+  // it is not _really_ complete so we can later search for a definition in a
+  // 
diff erent module.
+  // Since we provide layout assistance, layouts of types containing this class
+  // will be correct even if we  are not able to find the definition elsewhere.
+  bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+  lldbassert(started && "Unable to start a class type definition.");
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+  m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
 TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
 const SymbolContext , const DWARFDIE , TypeSP type_sp) {
   if (!type_sp)
@@ -2045,26 +2042,8 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE ,
   for (const auto _class : bases) {
 clang::TypeSourceInfo *type_source_info =
 base_class->getTypeSourceInfo();
-if (type_source_info) {
-  CompilerType base_class_type =
-  m_ast.GetType(type_source_info->getType());
-  if (!base_class_type.GetCompleteType()) {
-// We mark the class as complete to allow the TransferBaseClasses
-// call to succeed. But we make a note of the fact that this class
-// is not _really_ complete so we can later search for a definition

[Lldb-commits] [PATCH] D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types

[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 rG2cdf108d329b: [lldb/DWARF] Add a utility function for 
(forceful) completion of types (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D83199?vs=275612=275960#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83199

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -219,6 +219,12 @@
   ParsedDWARFTypeAttributes );
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE ,
 const ParsedDWARFTypeAttributes );
+
+  /// Complete a type from debug info, or mark it as forcefully completed if
+  /// there is no of the type in the current Module. Call this function in
+  /// contexts where the usual C++ rules require a type to be complete (base
+  /// class, member, etc.).
+  void CompleteType(lldb_private::CompilerType type);
 };
 
 /// Parsed form of all attributes that are relevant for type reconstruction.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1263,32 +1263,7 @@
   if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
 attrs.byte_stride = element_type->GetByteSize().getValueOr(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
-
-  if (TypeSystemClang::IsCXXClassType(array_element_type) &&
-  !array_element_type.GetCompleteType()) {
-ModuleSP module_sp = die.GetModule();
-
-// Mark the class as complete, but we make a note of the fact that
-// this class is not _really_ complete so we can later search for a
-// definition in a different module.
-// Since we provide layout assistance, all ivars in this class and other
-// classes will be fine even if we are not able to find the definition
-// elsewhere.
-if (TypeSystemClang::StartTagDeclarationDefinition(array_element_type)) {
-  TypeSystemClang::CompleteTagDeclarationDefinition(array_element_type);
-  const auto *td =
-  TypeSystemClang::GetQualType(array_element_type.GetOpaqueQualType())
-  .getTypePtr()
-  ->getAsTagDecl();
-  m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-} else {
-  module_sp->ReportError("DWARF DIE at 0x%8.8x was not able to "
- "start its definition.\nPlease file a "
- "bug and attach the file at the start "
- "of this error message",
- type_die.GetOffset());
-}
-  }
+  CompleteType(array_element_type);
 
   uint64_t array_element_bit_stride =
   attrs.byte_stride * 8 + attrs.bit_stride;
@@ -1343,6 +1318,28 @@
   return nullptr;
 }
 
+void DWARFASTParserClang::CompleteType(CompilerType type) {
+  // Technically, enums can be incomplete too, but we don't handle those as they
+  // are emitted even under -flimit-debug-info.
+  if (!TypeSystemClang::IsCXXClassType(type))
+return;
+
+  if (type.GetCompleteType())
+return;
+
+  // No complete definition in this module.  Mark the class as complete to
+  // satisfy local ast invariants, but make a note of the fact that
+  // it is not _really_ complete so we can later search for a definition in a
+  // different module.
+  // Since we provide layout assistance, layouts of types containing this class
+  // will be correct even if we  are not able to find the definition elsewhere.
+  bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+  lldbassert(started && "Unable to start a class type definition.");
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+  m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
 TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
 const SymbolContext , const DWARFDIE , TypeSP type_sp) {
   if (!type_sp)
@@ -2045,26 +2042,8 @@
   for (const auto _class : bases) {
 clang::TypeSourceInfo *type_source_info =
 base_class->getTypeSourceInfo();
-if (type_source_info) {
-  CompilerType base_class_type =
-  m_ast.GetType(type_source_info->getType());
-  if (!base_class_type.GetCompleteType()) {
-// We mark the class as complete to allow the TransferBaseClasses
-// call 

[Lldb-commits] [PATCH] D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1339
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const auto *td = TypeSystemClang::GetQualType(type.GetOpaqueQualType())
+   .getTypePtr()

teemperor wrote:
> shafik wrote:
> > This has to be a `TagDecl` so why use `auto`?
> `const TagDecl *td = ClangUtil::GetAsTagDecl(type);`
I think this is a borderline case of "use auto when the type is obvious" policy 
-- the type name is embedded in the function name, but there's technically no 
guarantee that it really returns that (it could return an Expected 
among other things). OTOH, the name is short enough to just spell it out.
The ClangUtil thingy is cool though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83199



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

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

This looks great. I just have a quick question about the GPR vs GPRegSet 
thingy...




Comment at: lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp:167
 assert(target_arch.GetTriple().getOS() == llvm::Triple::FreeBSD);
 switch (target_arch.GetMachine()) {
 case llvm::Triple::aarch64:

omjavaid wrote:
> labath wrote:
> > It would be good to merge these two switches. Then the reg(set)_interface 
> > variables would be local to each case label and it would not be so weird 
> > that we sometimes use one and sometimes the other.
> I have tried a couple of options but the no of different combinations 
> involved I feel current implementation should stay untill we incrementally 
> move remaining architectures to user RegisterInfosAndSet interface.
I like this solution.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp:46
+: lldb_private::RegisterContext(thread, 0) {
+  m_register_info_up = std::move(register_info);
 

move this to the initializer list



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:85
+gpr_w22, gpr_w23,  gpr_w24, gpr_w25, gpr_w26, gpr_w27, gpr_w28,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};

I'd probably just delete this comment (or merge it with the leading comment 
above the array definition), and then let clang-format lay this out normally...



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:19
 public:
+  enum { ARM64GPR = 0, ARM64FPR };
+

Why these names? I think [GF]PRegSet would be better for two reasons:
- the same names with the same purpose already exist in 
`NativeRegisterContextNetBSD_x86_64.h`
- it seems like a better way to differentiate from the [GF]PR structs below 
than adding a redundant ARM64 prefix.


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

https://reviews.llvm.org/D80105



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

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The approach is right, but can be simplified.

I'd also remove the sizeof checks in the tests. We have a lot of code depending 
on the fact that floats and doubles are consistent across targets (e.g. the 
swapByteOrder function I mention). If that ever becomes untrue it would be 
probably nice to get an early warning about it.

long doubles are a different story, as they behave differently on pretty much 
every platform. That's why `GetLongDouble` is full of ifdefs, and is still 
wrong, because normally we want to read the targets notion of a "long double" 
not that of a host. That's why I would ideally want to replace these float 
accessors with a single unified accessor like `llvm::APFloat GetFloat(uint64_t 
*, llvm::fltSemantics), but that's a story for another day.




Comment at: lldb/include/lldb/Utility/DataExtractor.h:991-1001
+if (m_byte_order != endian::InlHostByteOrder()) {
+  const uint8_t *src_data = reinterpret_cast(src);
+  uint8_t *dst_data = reinterpret_cast();
+  for (size_t i = 0; i < src_size; ++i)
+dst_data[src_size - 1 - i] = src_data[i];
+  return val;
+} else {

```
memcpy(, src, src_size);
if (m_byte_order != endian::InlHostByteOrder())
  llvm::sys::swapByteOrder(val);
return val;
```
will only work float and double (and all integers..) but it doesn't like you 
need anything else anyway. Long double is broken enough to not care about it.



Comment at: lldb/source/Utility/DataExtractor.cpp:630
 double DataExtractor::GetDouble(offset_t *offset_ptr) const {
-  typedef double float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0f);
 }

In this case the `f` suffix is actually not appropriate.
`f` -> `float` literal
no suffix -> `double` literal
`L` -> `long double` literal



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:380
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,

clayborg wrote:
> You might want to make check if the size of a double is 8 bytes here and 
> return without testing anything if it isn't or this will fail. Windows used 
> 10 byte floats for doubles I believe?
> 
> 
Windows doesn't use 10 bytes for doubles (definitely not on x86, but I would be 
very surprised if it was true elsewhere too). You're probably confusing this 
with x86 "extended" floats, which some ABIs map to "long double". These contain 
10 bytes of useful data, but due to alignment considerations their `sizeof` is 
usually 12 or 16.


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

https://reviews.llvm.org/D83256



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

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 275915.

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

https://reviews.llvm.org/D83256

Files:
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -299,3 +299,111 @@
   EXPECT_EQ(expected, BE.GetULEB128());
   EXPECT_EQ(9U, offset);
 }
+
+TEST(DataExtractorTest, GetFloat) {
+  if (sizeof(float) != 4)
+return;
+
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(4U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetFloatUnaligned) {
+  if (sizeof(float) != 4)
+return;
+
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x80, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x80, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetFloat());
+EXPECT_EQ(5U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDouble) {
+  if (sizeof(double) != 8)
+return;
+
+  double expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 0;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(8U, offset);
+  }
+}
+
+TEST(DataExtractorTest, GetDoubleUnaligned) {
+  if (sizeof(double) != 8)
+return;
+
+  float expected = 4.0f;
+  lldb::offset_t offset;
+
+  {
+uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
+DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, LE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+
+  {
+uint8_t buffer[] = {0x00, 0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+offset = 1;
+EXPECT_DOUBLE_EQ(expected, BE.GetDouble());
+EXPECT_EQ(9U, offset);
+  }
+}
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -15,7 +15,6 @@
 
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -624,41 +623,11 @@
 }
 
 float DataExtractor::GetFloat(offset_t *offset_ptr) const {
-  typedef float float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - i] = src_data[i];
-} else {
-  val = *src;
-}
-  }
-  return val;
+  return Get(offset_ptr, 0.0f);
 }
 
 double DataExtractor::GetDouble(offset_t *offset_ptr) const {
-  typedef double float_type;
-  float_type val = 0.0;
-  const size_t src_size = sizeof(float_type);
-  const float_type *src =
-  static_cast(GetData(offset_ptr, src_size));
-  if (src) {
-if (m_byte_order != endian::InlHostByteOrder()) {
-  const uint8_t *src_data = reinterpret_cast(src);
-  uint8_t *dst_data = reinterpret_cast();
-  for (size_t i = 0; i < sizeof(float_type); ++i)
-dst_data[sizeof(float_type) - 1 - 

[Lldb-commits] [PATCH] D83256: [lldb] Fix unaligned load in DataExtractor

[Greg Clayton via Phabricator via lldb-commits]

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

sizeof(double) instead of sizeof(float) for the double tests.




Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:358
+TEST(DataExtractorTest, GetDouble) {
+  if (sizeof(float) != 8)
+return;

sizeof(double)



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:385
+TEST(DataExtractorTest, GetDoubleUnaligned) {
+  if (sizeof(float) != 8)
+return;

sizeof(double)


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

https://reviews.llvm.org/D83256



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