[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well... I think that will make the test fail on mac, as debugserver hasn't been 
updated.

This test comes from D12592 , and is actually 
serves a very similar purpose to the test you just wrote. Normally, I would say 
we don't need both of them, but since we still have debugserver with the old 
behavior, and your new test is consistent with how we've (you've) been writing 
other register tests, maybe we could just slap `@skipUnlessDarwin` on 
TestRegisters.py, and put some comments (on both tests) that explain the 
situation and cross-reference each other (?)


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

I ran manual tests for this, but I did so by introducing an intentional crash 
in a place that obviously can't be checked in. Does LLDB have any kind of 
intentional-crash-for-test mechanism that could be used for a test?

Also, I picked `SBDebugger` for this as it seems like a very global option, but 
I'm happy to put it somewhere else. Not sure where though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91835

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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
rupprecht added reviewers: teemperor, JDevlieghere, labath, vsk.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
rupprecht requested review of this revision.

As noticed in D87637 , when LLDB crashes, we 
only print stack traces if LLDB is directly executed, not when used via Python 
bindings. Enabling this by default may be undesirable (libraries shouldn't be 
messing with signal handlers), so make this an explicit opt-in.

When adding an `abort()` to `CommandInterpreter::HandleCommand()`, this prints:

  $ ninja check-lldb-api-commands-apropos-basic
  FAIL: lldb-api :: commands/apropos/basic/TestApropos.py (1 of 1)
   TEST 'lldb-api :: commands/apropos/basic/TestApropos.py' 
FAILED 
  ...
  Command Output (stderr):
  --
  python3.8: 
/home/rupprecht/src/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1655:
 bool lldb_private::CommandInterpreter::HandleCommand(const char *, 
lldb_private::LazyBool, lldb_private::CommandReturnObject &, 
lldb_private::ExecutionContext *, bool, bool): Assertion `false && "crash!"' 
failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.  HandleCommand(command = "settings clear -all")
   #0 0x7f1e9a0ff4ea llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:563:11
   #1 0x7f1e9a0ff6bb PrintStackTraceSignalHandler(void*) 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:630:1
   #2 0x7f1e9a0fdcdb llvm::sys::RunSignalHandlers() 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Signals.cpp:70:5
   #3 0x7f1e9a0ffded SignalHandler(int) 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:405:1
  ...
   #9 0x7f1e997e6228 
/home/rupprecht/src/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1655:3


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91835

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/API/SBDebugger.cpp


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -56,6 +56,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -166,6 +168,14 @@
   return LLDB_RECORD_RESULT(error);
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_RECORD_STATIC_METHOD_NO_ARGS(void, SBDebugger, PrintStackTraceOnError);
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_RECORD_STATIC_METHOD_NO_ARGS(void, SBDebugger, Terminate);
 
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,9 @@
 lldbconfig.INITIALIZE = False
 import lldb
 
+# Print stack traces from coming from inside lldb.
+lldb.SBDebugger.PrintStackTraceOnError()
+
 if configuration.capture_path:
 lldb.SBReproducer.Capture(configuration.capture_path)
 lldb.SBReproducer.SetAutoGenerate(True)
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -47,6 +47,8 @@
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -124,6 +124,8 @@
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -56,6 +56,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -166,6 +168,14 @@
   return LLDB_RECORD_RESULT(error);
 }
 
+void 

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/test/Shell/Register/x86-fp-write.test:2
 # XFAIL: system-windows
-# REQUIRES: native && target-x86
+# REQUIRES: native && (target-x86 || target-x86_64)
 # RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t

I removed `target-x86_64` in a fix this is failing on the green dragon build 
bot: 
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/24969/#showFailuresLink

and I trying to get back green again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91497

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

[Lldb-commits] [lldb] 0fd0433 - [LLDB] Fixing lldb/test/Shell/Register/x86-fp-write.test

[via lldb-commits]

Author: shafik
Date: 2020-11-19T16:29:28-08:00
New Revision: 0fd04337a17138174adf9e6d408cf9c885dea086

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

LOG: [LLDB] Fixing lldb/test/Shell/Register/x86-fp-write.test

D91497 changed lldb/test/Shell/Register/x86-fp-write.test and added 
target-x86_64 to the REQUIRES clause.
It looks this test does not pass on this platform so removing it since it one 
of tests failing on the
green dragon build bot.

Added: 


Modified: 
lldb/test/Shell/Register/x86-fp-write.test

Removed: 




diff  --git a/lldb/test/Shell/Register/x86-fp-write.test 
b/lldb/test/Shell/Register/x86-fp-write.test
index 38ffe16bab02..f944ec6767bd 100644
--- a/lldb/test/Shell/Register/x86-fp-write.test
+++ b/lldb/test/Shell/Register/x86-fp-write.test
@@ -1,5 +1,5 @@
 # XFAIL: system-windows
-# REQUIRES: native && (target-x86 || target-x86_64)
+# REQUIRES: native && target-x86
 # RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch



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

[Lldb-commits] [PATCH] D91810: [lldb] [Process/FreeBSD] Fix 'process connect' plugin choice

[Ed Maste via Phabricator via lldb-commits]

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

Looks ok to me


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

https://reviews.llvm.org/D91810

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

[Lldb-commits] [PATCH] D91810: [lldb] [Process/FreeBSD] Fix 'process connect' plugin choice

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Fix ProcessFreeBSD::CanDebug() to indicate whether the legacy plugin
is actually being used.  This is necessary to prevent LLDB from wrongly
selecting the legacy plugin when searching for a process plugin for
'process connect' command.


https://reviews.llvm.org/D91810

Files:
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
  lldb/test/API/commands/process/attach-resume/TestAttachResume.py
  lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
  lldb/test/Shell/Commands/command-process-connect.test

Index: lldb/test/Shell/Commands/command-process-connect.test
===
--- lldb/test/Shell/Commands/command-process-connect.test
+++ lldb/test/Shell/Commands/command-process-connect.test
@@ -1,5 +1,4 @@
 # UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
 
 # Synchronous
 # RUN: %lldb -o 'platform select remote-gdb-server' -o 'process connect connect://localhost:4321' 2>&1 | FileCheck %s
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -36,7 +36,6 @@
 self.dbg.GetSelectedPlatform().DisconnectRemote()
 
 @skipIfWindows
-@expectedFailureAll(oslist=["freebsd"])
 def test_process_connect_sync(self):
 """Test the gdb-remote command in synchronous mode"""
 try:
@@ -48,7 +47,6 @@
 self.dbg.GetSelectedPlatform().DisconnectRemote()
 
 @skipIfWindows
-@expectedFailureAll(oslist=["freebsd"])
 @skipIfReproducer # Reproducer don't support async.
 def test_process_connect_async(self):
 """Test the gdb-remote command in asynchronous mode"""
Index: lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -16,7 +16,7 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows # cannot delete a running executable
-@expectedFailureAll(oslist=["freebsd", "linux"],
+@expectedFailureAll(oslist=["linux"],
 triple=no_match('aarch64-.*-android'))
 # determining the architecture of the process fails
 @skipIfReproducer # File synchronization is not supported during replay.
Index: lldb/test/API/commands/process/attach-resume/TestAttachResume.py
===
--- lldb/test/API/commands/process/attach-resume/TestAttachResume.py
+++ lldb/test/API/commands/process/attach-resume/TestAttachResume.py
@@ -18,7 +18,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfRemote
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr19310')
 @expectedFailureNetBSD
 @skipIfWindows # llvm.org/pr24778, llvm.org/pr21753
 @skipIfReproducer # FIXME: Unexpected packet during (active) replay
Index: lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
@@ -28,6 +28,7 @@
 #include "lldb/Utility/State.h"
 
 #include "FreeBSDThread.h"
+#include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "Plugins/Process/Utility/FreeBSDSignals.h"
 #include "Plugins/Process/Utility/InferiorCallPOSIX.h"
@@ -273,6 +274,9 @@
 
 bool ProcessFreeBSD::CanDebug(lldb::TargetSP target_sp,
   bool plugin_specified_by_name) {
+  if (!platform_freebsd::PlatformFreeBSD::UseLegacyPlugin())
+return false;
+
   // For now we are just making sure the file exists for a given module
   ModuleSP exe_module_sp(target_sp->GetExecutableModule());
   if (exe_module_sp.get())
Index: lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
===
--- lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
+++ lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
@@ -42,6 +42,8 @@
 
   bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec ) override;
 
+  static bool UseLegacyPlugin();
+
   bool CanDebugProcess() override;
 
   size_t GetSoftwareBreakpointTrapOpcode(Target ,
Index: lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp

[Lldb-commits] [PATCH] D87173: Ignores functions that have a range starting outside of a code section

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

When I landed this I got test errors on the windows machine (the same tests 
pass on the other machine). I tried to repro this on 2 different machines I was 
able to get access but to no avail. 
These are the failing tests: http://lab.llvm.org:8011/#/builders/83/builds/644 
but when I ran lldb-test on the find-basic-function the output was what I would 
expect so not really sure how I can figure this out. Any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87173

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

[Lldb-commits] [lldb] c77aefb - [lldb] Fix another Python2/3 string<->bytes type error in patch-crashlog.py

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-19T19:24:40+01:00
New Revision: c77aefb0ff36277c97f52e22cec3ffcc5db43064

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

LOG: [lldb] Fix another Python2/3 string<->bytes type error in patch-crashlog.py

Added: 


Modified: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py

Removed: 




diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
index 9616591c1f74..28396c530355 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/patch-crashlog.py
@@ -32,7 +32,7 @@ def patch_uuid(self):
 def patch_addresses(self):
 if not self.offsets:
 return
-output = subprocess.check_output(['nm', self.binary])
+output = subprocess.check_output(['nm', self.binary]).decode("utf-8")
 for line in output.splitlines():
 m = self.SYMBOL_REGEX.match(line)
 if m:



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

[Lldb-commits] [PATCH] D91801: [lldb] Fix incorrect error handling in GDBRemoteCommunicationClient::SendGetSupportedTraceType

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

From what understand this is Linux exclusive feature at the moment, and the 
command test itself is testing the command but missing the edge case of testing 
this against a running process. Not sure if it's worth starting a process just 
to test that the command still just prints the "unsupported feature" error. 
I'll leave a dedicated test up to @wallace .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91801

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

[Lldb-commits] [PATCH] D91801: [lldb] Fix incorrect error handling in GDBRemoteCommunicationClient::SendGetSupportedTraceType

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG47b7138b484b: [lldb] Fix incorrect error handling in  
GDBRemoteCommunicationClient… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91801

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


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3465,8 +3465,11 @@
   if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
true) ==
   GDBRemoteCommunication::PacketResult::Success) {
-if (!response.IsNormalResponse())
+if (response.IsErrorResponse())
   return response.GetStatus().ToError();
+if (response.IsUnsupportedResponse())
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "jLLDBTraceSupportedType is unsupported");
 
 if (llvm::Expected type =
 llvm::json::parse(response.Peek()))


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3465,8 +3465,11 @@
   if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
true) ==
   GDBRemoteCommunication::PacketResult::Success) {
-if (!response.IsNormalResponse())
+if (response.IsErrorResponse())
   return response.GetStatus().ToError();
+if (response.IsUnsupportedResponse())
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "jLLDBTraceSupportedType is unsupported");
 
 if (llvm::Expected type =
 llvm::json::parse(response.Peek()))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 47b7138 - [lldb] Fix incorrect error handling in GDBRemoteCommunicationClient::SendGetSupportedTraceType

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-19T19:14:04+01:00
New Revision: 47b7138b484b8fc94633ac4750a11acad797473e

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

LOG: [lldb] Fix incorrect error handling in  
GDBRemoteCommunicationClient::SendGetSupportedTraceType

GDBRemoteCommunicationClient::SendGetSupportedTraceType is checking whether the
response is `!response.IsNormalResponse()` and infers from that that it is an 
error response.
However, it could be either "unsupported" or "error". If we get an unsupported 
response,
the code then tries to generate an llvm::Expected from the non-error response 
which then asserts.

Debugserver doesn't implement `jLLDBTraceSupportedType`, so we get an 
unsupported response
whenever this function is called on macOS.

This fixes the TestAproposWithProcess on macOS (where the `apropos` command 
will query
the CommandObjectTraceStart which then sends the trace type query package).

Reviewed By: wallace, shafik

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d661423f1107..b1552a3a43ad 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3465,8 +3465,11 @@ 
GDBRemoteCommunicationClient::SendGetSupportedTraceType() {
   if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
true) ==
   GDBRemoteCommunication::PacketResult::Success) {
-if (!response.IsNormalResponse())
+if (response.IsErrorResponse())
   return response.GetStatus().ToError();
+if (response.IsUnsupportedResponse())
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "jLLDBTraceSupportedType is unsupported");
 
 if (llvm::Expected type =
 llvm::json::parse(response.Peek()))



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

[Lldb-commits] [PATCH] D90857: [lldb] add a missing dependency on intrinsics_gen

[Richard Howell via Phabricator via lldb-commits]

rmaz added a comment.

> I'd still like to hear an explanation on how is this patch relevant (in this 
> repository).

Apologies, missed the last notification.

> Judging by the error message, this dependency is not correct even in the 
> swift fork. lldbSymbol does not have a direct dependency on llvmIR. It has a 
> dependency on swift (through SwiftASTContext), and swift (through 
> AST/Builtins.h) has a dependency on the generated headers.

Correct.

> So, the right fix should be to add a lldbSymbol->Swift dependency (if one 
> isn't there already), and then a swift->LLVMIR dep.

This sounds reasonable, the Swift build script seems to enforce a similar 
compilation order of:

1. LLVM & Clang
2. Swift
3. LLDB

There are some existing dependencies on Swift libs in standalone mode, let me 
take a look at this approach instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90857

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

[Lldb-commits] [lldb] b7a09de - [lldb][NFC] Add a FIXME for ClangASTSource::FindExternalLexicalDecls's unused 'decls' parameter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-19T17:15:14+01:00
New Revision: b7a09de10ffc59dec78a7d4b39bc78d07d3110eb

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

LOG: [lldb][NFC] Add a FIXME for ClangASTSource::FindExternalLexicalDecls's 
unused 'decls' parameter

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 6fe85a1298fc..783a5f178803 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -482,6 +482,15 @@ void ClangASTSource::FindExternalLexicalDecls(
   if (!copied_decl)
 continue;
 
+  // FIXME: We should add the copied decl to the 'decls' list. This would
+  // add the copied Decl into the DeclContext and make sure that we
+  // correctly propagate that we added some Decls back to Clang.
+  // By leaving 'decls' empty we incorrectly return false from
+  // DeclContext::LoadLexicalDeclsFromExternalStorage which might cause
+  // lookup issues later on.
+  // We can't just add them for now as the ASTImporter already added the
+  // decl into the DeclContext and this would add it twice.
+
   if (FieldDecl *copied_field = dyn_cast(copied_decl)) {
 QualType copied_field_type = copied_field->getType();
 



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

[Lldb-commits] [lldb] a703998 - [lldb] Remove legacy casts from TestStackFromStdModule

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-19T17:07:12+01:00
New Revision: a703998e66f7500442e8f55bfcddf4a5820ca023

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

LOG: [lldb] Remove legacy casts from TestStackFromStdModule

We can handle all the types in the expression evaluator now without casting.
On Linux, we have a system header declaration that is still causing issues, so
I'm skipping the test there until I get around to fix this.

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
index fe19a4e5f1b3..d9a7e186fa6f 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
@@ -12,6 +12,7 @@ class TestStack(TestBase):
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIfLinux # Declaration in some Linux headers causes LLDB to crash.
 def test(self):
 self.build()
 
@@ -21,25 +22,50 @@ def test(self):
 self.runCmd("settings set target.import-std-module true")
 
 # Test std::stack functionality with a std::deque.
+stack_type = "std::stack > >"
+size_type = stack_type + "::size_type"
+
+self.expect_expr("s_deque", result_type=stack_type)
 self.expect("expr s_deque.pop()")
 self.expect("expr s_deque.push({4})")
-self.expect("expr (size_t)s_deque.size()", substrs=['(size_t) $0 = 3'])
-self.expect("expr (int)s_deque.top().i", substrs=['(int) $1 = 4'])
+self.expect_expr("s_deque.size()",
+ result_type=size_type,
+ result_value="3")
+self.expect_expr("s_deque.top().i",
+ result_type="int",
+ result_value="4")
 self.expect("expr s_deque.emplace(5)")
-self.expect("expr (int)s_deque.top().i", substrs=['(int) $2 = 5'])
+self.expect_expr("s_deque.top().i",
+ result_type="int",
+ result_value="5")
 
 # Test std::stack functionality with a std::vector.
+stack_type = "std::stack > >"
+size_type = stack_type + "::size_type"
+
+self.expect_expr("s_vector", result_type=stack_type)
 self.expect("expr s_vector.pop()")
 self.expect("expr s_vector.push({4})")
-self.expect("expr (size_t)s_vector.size()", substrs=['(size_t) $3 = 
3'])
-self.expect("expr (int)s_vector.top().i", substrs=['(int) $4 = 4'])
+self.expect_expr("s_vector.size()",
+ result_type=size_type,
+ result_value="3")
+self.expect_expr("s_vector.top().i",
+ result_type="int",
+ result_value="4")
 self.expect("expr s_vector.emplace(5)")
-self.expect("expr (int)s_vector.top().i", substrs=['(int) $5 = 5'])
+self.expect_expr("s_vector.top().i",
+ result_type="int",
+ result_value="5")
 
 # Test std::stack functionality with a std::list.
+stack_type = "std::stack > >"
+size_type = stack_type + "::size_type"
+self.expect_expr("s_list", result_type=stack_type)
 self.expect("expr s_list.pop()")
 self.expect("expr s_list.push({4})")
-self.expect("expr (size_t)s_list.size()", substrs=['(size_t) $6 = 3'])
-self.expect("expr (int)s_list.top().i", substrs=['(int) $7 = 4'])
+self.expect_expr("s_list.size()",
+ result_type=size_type,
+ result_value="3")
+self.expect_expr("s_list.top().i", result_type="int", result_value="4")
 self.expect("expr s_list.emplace(5)")
-self.expect("expr (int)s_list.top().i", substrs=['(int) $8 = 5'])
+self.expect_expr("s_list.top().i", result_type="int", result_value="5")



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

[Lldb-commits] [PATCH] D87442: [lldb][AArch64/Linux] Show memory tagged memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 306406.
DavidSpickett added a comment.

- Use assertIn
- Cleanup Status/Error handling
- Refactor test so it runs once and we skip based on whether it returns a value 
or hits a breakpoint.
- Seperated exit codes for non MTE toolchain and non MTE target, for more 
specific skip reasons.

On the test, hopefully the comment in main.c
makes sense. Here's a cut down demo of what I'm
explaining:
https://godbolt.org/z/rex96b

This means we can always set an exact breakpoint,
we just won't hit it unless you have MTE. Which is
what we want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/docs/use/qemu-testing.rst
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/scripts/lldb-test-qemu/run-qemu.sh
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Target/MemoryRegionInfo.cpp
  lldb/test/API/linux/aarch64/mte_memory_region/Makefile
  
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
  lldb/test/API/linux/aarch64/mte_memory_region/main.c
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -378,15 +378,15 @@
   parser->BuildMemoryRegions(),
   testing::Pair(testing::ElementsAre(
 MemoryRegionInfo({0x0, 0x1}, no, no, no, no,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
+ ConstString(), unknown, 0, unknown),
 MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
- ConstString(), unknown, 0)),
+ ConstString(), unknown, 0, unknown)),
 true));
 }
 
@@ -409,12 +409,13 @@
 
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
-  testing::Pair(testing::ElementsAre(
-MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0),
-MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0)),
-false));
+  testing::Pair(
+  testing::ElementsAre(
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown),
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+   ConstString(), unknown, 0, unknown)),
+  false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemory64List) {
@@ -424,12 +425,13 @@
   // we don't have a MemoryInfoListStream.
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
-  testing::Pair(testing::ElementsAre(
-MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0),
-MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
- yes, ConstString(), unknown, 0)),
-false));
+  testing::Pair(
+  testing::ElementsAre(
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

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

Does this change to TestRegister.py look good? It makes the test pass for me 
(and seems logically correct).


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306403.
mgorny added a comment.

Update TestRegisters.py.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D91504#2405504 , @goncharov wrote:

> Hi @mgorny! http://lab.llvm.org:8011/#/builders/68/builds/2298 build failed. 
> Could you please fix the test?

I'm sorry about that. I'll look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [lldb] 193a9b3 - Revert "[lldb] Use translated full ftag values"

[Mikhail Goncharov via lldb-commits]

Author: Mikhail Goncharov
Date: 2020-11-19T15:24:59+01:00
New Revision: 193a9b374e24d31b30095f2789f1994bc0c7b663

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

LOG: Revert "[lldb] Use translated full ftag values"

This reverts commit c43abf043692babf9ad4f8bded2fdf6ab9c354b0.

Test commands/register/register/register_command/TestRegisters.py fails.
Buildbot http://lab.llvm.org:8011/#/changes/4149

Added: 


Modified: 

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
lldb/test/Shell/Register/x86-64-fp-read.test
lldb/test/Shell/Register/x86-64-fp-write.test
lldb/test/Shell/Register/x86-fp-read.test
lldb/test/Shell/Register/x86-fp-write.test
lldb/unittests/Process/Utility/CMakeLists.txt

Removed: 
lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
lldb/unittests/Process/Utility/RegisterContextTest.cpp



diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index ea5400c55713..ea2494dabf27 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -451,16 +451,10 @@ NativeRegisterContextFreeBSD_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet: {
-void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
-FXSAVE *fpr = reinterpret_cast(m_fpr.data());
-if (data == >ftag) // ftag
-  reg_value.SetUInt16(
-  AbridgedToFullTagWord(fpr->ftag, fpr->fstat, fpr->stmm));
-else
-  reg_value.SetBytes(data, reg_info->byte_size, 
endian::InlHostByteOrder());
+  case DBRegSet:
+reg_value.SetBytes(GetOffsetRegSetData(set, reg_info->byte_offset),
+   reg_info->byte_size, endian::InlHostByteOrder());
 break;
-  }
   case YMMRegSet: {
 llvm::Optional ymm_reg = GetYMMSplitReg(reg);
 if (!ymm_reg) {
@@ -517,15 +511,10 @@ Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet: {
-void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
-FXSAVE *fpr = reinterpret_cast(m_fpr.data());
-if (data == >ftag) // ftag
-  fpr->ftag = FullToAbridgedTagWord(reg_value.GetAsUInt16());
-else
-  ::memcpy(data, reg_value.GetBytes(), reg_value.GetByteSize());
+  case DBRegSet:
+::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
+ reg_value.GetBytes(), reg_value.GetByteSize());
 break;
-  }
   case YMMRegSet: {
 llvm::Optional ymm_reg = GetYMMSplitReg(reg);
 if (!ymm_reg) {

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index 6462441249c0..20cd5e3f62ff 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -530,13 +530,6 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
   assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(FPR));
   uint8_t *src = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
  m_fctrl_offset_in_userarea;
-
-  if (src == reinterpret_cast(_xstate->fxsave.ftag)) {
-reg_value.SetUInt16(AbridgedToFullTagWord(
-m_xstate->fxsave.ftag, m_xstate->fxsave.fstat, m_xstate->fxsave.stmm));
-return error;
-  }
-
   switch (reg_info->byte_size) {
   case 1:
 reg_value.SetUInt8(*(uint8_t *)src);
@@ -646,28 +639,23 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
  sizeof(FPR));
   uint8_t *dst = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
  m_fctrl_offset_in_userarea;
-
-  if (dst == reinterpret_cast(_xstate->fxsave.ftag))
-m_xstate->fxsave.ftag = FullToAbridgedTagWord(reg_value.GetAsUInt16());
-  else {
-switch (reg_info->byte_size) {
-case 1:
-  *(uint8_t *)dst = reg_value.GetAsUInt8();
-  break;
-case 2:
-  *(uint16_t *)dst = reg_value.GetAsUInt16();
-  break;
-case 4:
-  *(uint32_t *)dst = reg_value.GetAsUInt32();
-  break;
-case 8:
-  *(uint64_t *)dst = 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Mikhail Goncharov via Phabricator via lldb-commits]

goncharov added a comment.
Herald added a subscriber: JDevlieghere.

Hi @mgorny! http://lab.llvm.org:8011/#/builders/68/builds/2298 build failed. 
Could you please fix the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D90857: [lldb] add a missing dependency on intrinsics_gen

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Ping!

I'd still like to hear an explanation on how is this patch relevant (in this 
repository).

Per our code review  policy , the 
responsibility for a patch does not end the moment the patch is committed :

  Post-commit review is encouraged, and can be accomplished using any of the 
tools detailed below. There is a strong expectation that authors respond 
promptly to post-commit feedback and address it. Failure to do so is cause for 
the patch to be reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90857

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

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8ff269f6776: [lldb] Add explicit 64-bit fip/fdp registers 
on x86_64 (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D91497?vs=306160=306369#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91497

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -82,11 +82,15 @@
   EXPECT_OFF(ftag_x86_64, 0x04, 2);
   EXPECT_OFF(fop_x86_64, 0x06, 2);
   // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
-  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  // are reserved.  However, LLDB defines them to be 32-bit long for backwards
+  // compatibility, as they were used to reconstruct FIP/FDP before explicit
+  // register entries for them were added.  Also, this is still how GDB does it.
   EXPECT_OFF(fioff_x86_64, 0x08, 4);
   EXPECT_OFF(fiseg_x86_64, 0x0C, 4);
+  EXPECT_OFF(fip_x86_64, 0x08, 8);
   EXPECT_OFF(fooff_x86_64, 0x10, 4);
   EXPECT_OFF(foseg_x86_64, 0x14, 4);
+  EXPECT_OFF(fdp_x86_64, 0x10, 8);
   EXPECT_OFF(mxcsr_x86_64, 0x18, 4);
   EXPECT_OFF(mxcsrmask_x86_64, 0x1C, 4);
   EXPECT_OFF(st0_x86_64, 0x20, 10);
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -1,5 +1,5 @@
 # XFAIL: system-windows
-# REQUIRES: native && target-x86
+# REQUIRES: native && (target-x86 || target-x86_64)
 # RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch
@@ -31,8 +31,11 @@
 # CHECK-DAG: fstat = 0x8884
 # CHECK-DAG: ftag = 0xa961
 # CHECK-DAG: fop = 0x0033
-# CHECK-DAG: fip = 0x89abcdef
-# CHECK-DAG: fdp = 0x76543210
+
+# This test is run both on 32-bit and 64-bit systems, in order to test
+# that fioff/fooff setting works as well as fip/fdp.
+# CHECK-DAG: fip = 0x{{()?}}89abcdef
+# CHECK-DAG: fdp = 0x{{()?}}76543210
 
 # CHECK-DAG: st0 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40 }
 # CHECK-DAG: st1 = { 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00 }
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- lldb/test/Shell/Register/x86-64-fp-write.test
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -14,10 +14,8 @@
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: fxrstor64 apparently truncates this to 48 bits, and sign extends
 #   the highest bits, so let's keep the value safely below
-register write fiseg 0x0567
-register write fioff 0x89abcdef
-register write foseg 0x0a98
-register write fooff 0x76543210
+register write fip 0x056789abcdef
+register write fdp 0x0a9876543210
 
 register write st0 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}"
 register write st1 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}"
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -1,10 +1,16 @@
 # XFAIL: system-windows
-# REQUIRES: native && (target-x86 || target-x86_64)
+# REQUIRES: native && target-x86
 # RUN: %clangxx_host -g %p/Inputs/x86-fp-read.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch
 # CHECK: Process {{.*}} stopped
 
+# fdiv (%rbx) gets encoded into 2 bytes, int3 into 1 byte
+print (void*)($pc-3)
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+print 
+# CHECK: (uint32_t *) $1 = [[ZERO:0x[0-9a-f]*]]
+
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
@@ -12,6 +18,8 @@
 # FXSAVE/XSAVE is interpreted
 # CHECK-DAG: ftag = 0x007f
 # CHECK-DAG: fop = 0x0033
+# CHECK-DAG: fioff = [[FDIV]]
+# CHECK-DAG: fooff = [[ZERO]]
 
 # 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc43abf043692: [lldb] Use translated full ftag values 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ 

[Lldb-commits] [lldb] c43abf0 - [lldb] Use translated full ftag values

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-19T13:23:12+01:00
New Revision: c43abf043692babf9ad4f8bded2fdf6ab9c354b0

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

LOG: [lldb] Use translated full ftag values

Translate between abridged and full ftag values in order to expose
the latter in the gdb-remote protocol while the former are used by
FXSAVE/XSAVE...  This matches the gdb behavior.

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

Added: 
lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
lldb/unittests/Process/Utility/RegisterContextTest.cpp

Modified: 

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
lldb/test/Shell/Register/x86-64-fp-read.test
lldb/test/Shell/Register/x86-64-fp-write.test
lldb/test/Shell/Register/x86-fp-read.test
lldb/test/Shell/Register/x86-fp-write.test
lldb/unittests/Process/Utility/CMakeLists.txt

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index ea2494dabf27..ea5400c55713 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -451,10 +451,16 @@ NativeRegisterContextFreeBSD_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet:
-reg_value.SetBytes(GetOffsetRegSetData(set, reg_info->byte_offset),
-   reg_info->byte_size, endian::InlHostByteOrder());
+  case DBRegSet: {
+void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
+FXSAVE *fpr = reinterpret_cast(m_fpr.data());
+if (data == >ftag) // ftag
+  reg_value.SetUInt16(
+  AbridgedToFullTagWord(fpr->ftag, fpr->fstat, fpr->stmm));
+else
+  reg_value.SetBytes(data, reg_info->byte_size, 
endian::InlHostByteOrder());
 break;
+  }
   case YMMRegSet: {
 llvm::Optional ymm_reg = GetYMMSplitReg(reg);
 if (!ymm_reg) {
@@ -511,10 +517,15 @@ Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet:
-::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
- reg_value.GetBytes(), reg_value.GetByteSize());
+  case DBRegSet: {
+void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
+FXSAVE *fpr = reinterpret_cast(m_fpr.data());
+if (data == >ftag) // ftag
+  fpr->ftag = FullToAbridgedTagWord(reg_value.GetAsUInt16());
+else
+  ::memcpy(data, reg_value.GetBytes(), reg_value.GetByteSize());
 break;
+  }
   case YMMRegSet: {
 llvm::Optional ymm_reg = GetYMMSplitReg(reg);
 if (!ymm_reg) {

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index 20cd5e3f62ff..6462441249c0 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -530,6 +530,13 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
   assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(FPR));
   uint8_t *src = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
  m_fctrl_offset_in_userarea;
+
+  if (src == reinterpret_cast(_xstate->fxsave.ftag)) {
+reg_value.SetUInt16(AbridgedToFullTagWord(
+m_xstate->fxsave.ftag, m_xstate->fxsave.fstat, m_xstate->fxsave.stmm));
+return error;
+  }
+
   switch (reg_info->byte_size) {
   case 1:
 reg_value.SetUInt8(*(uint8_t *)src);
@@ -639,23 +646,28 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
  sizeof(FPR));
   uint8_t *dst = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
  m_fctrl_offset_in_userarea;
-  switch (reg_info->byte_size) {
-  case 1:
-*(uint8_t *)dst = reg_value.GetAsUInt8();
-break;
-  case 2:
-*(uint16_t *)dst = reg_value.GetAsUInt16();
-break;
-  case 4:
-*(uint32_t *)dst = reg_value.GetAsUInt32();
-break;
-  case 8:
-*(uint64_t *)dst = reg_value.GetAsUInt64();
-break;
-  default:
-assert(false && "Unhandled data size.");
-return Status("unhandled register data size 

[Lldb-commits] [lldb] d8ff269 - [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-19T13:23:12+01:00
New Revision: d8ff269f677621a773d75c79d50bb3f59a6b052e

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

LOG: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

The FXSAVE/XSAVE data can have two different layouts on x86_64.  When
called as FXSAVE/XSAVE..., the Instruction Pointer and Address Pointer
registers are reported using a 16-bit segment identifier and a 32-bit
offset.  When called as FXSAVE64/XSAVE64..., they are reported using
a complete 64-bit offsets instead.

LLDB has historically followed GDB and unconditionally used to assume
the 32-bit layout, with the slight modification of possibly
using a 32-bit segment register (i.e. extending the register into
the reserved 16 upper bits).  When the underlying operating system used
FXSAVE64/XSAVE64..., the pointer was split into two halves,
with the upper half repored as the segment registers.  While
reconstructing the full address was possible on the user end (and e.g.
the FPU register tests did that), it certainly was not the most
convenient option.

Introduce a two additional 'fip' and 'fdp' registers that overlap
with 'fiseg'/'fioff' and 'foseg'/'foff' respectively, and report
the complete 64-bit address.

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

Added: 
lldb/test/Shell/Register/x86-64-fp-read.test

Modified: 

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h
lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
lldb/test/Shell/Register/x86-64-fp-write.test
lldb/test/Shell/Register/x86-fp-read.test
lldb/test/Shell/Register/x86-fp-write.test
lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index 1682a13977d9..ea2494dabf27 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -80,20 +80,21 @@ static_assert((sizeof(g_gpr_regnums_x86_64) / 
sizeof(g_gpr_regnums_x86_64[0])) -
 
 // x86 64-bit floating point registers.
 static const uint32_t g_fpu_regnums_x86_64[] = {
-lldb_fctrl_x86_64, lldb_fstat_x86_64, lldb_ftag_x86_64,
-lldb_fop_x86_64,   lldb_fiseg_x86_64, lldb_fioff_x86_64,
-lldb_foseg_x86_64, lldb_fooff_x86_64, lldb_mxcsr_x86_64,
-lldb_mxcsrmask_x86_64, lldb_st0_x86_64,   lldb_st1_x86_64,
-lldb_st2_x86_64,   lldb_st3_x86_64,   lldb_st4_x86_64,
-lldb_st5_x86_64,   lldb_st6_x86_64,   lldb_st7_x86_64,
-lldb_mm0_x86_64,   lldb_mm1_x86_64,   lldb_mm2_x86_64,
-lldb_mm3_x86_64,   lldb_mm4_x86_64,   lldb_mm5_x86_64,
-lldb_mm6_x86_64,   lldb_mm7_x86_64,   lldb_xmm0_x86_64,
-lldb_xmm1_x86_64,  lldb_xmm2_x86_64,  lldb_xmm3_x86_64,
-lldb_xmm4_x86_64,  lldb_xmm5_x86_64,  lldb_xmm6_x86_64,
-lldb_xmm7_x86_64,  lldb_xmm8_x86_64,  lldb_xmm9_x86_64,
-lldb_xmm10_x86_64, lldb_xmm11_x86_64, lldb_xmm12_x86_64,
-lldb_xmm13_x86_64, lldb_xmm14_x86_64, lldb_xmm15_x86_64,
+lldb_fctrl_x86_64,  lldb_fstat_x86_64, lldb_ftag_x86_64,
+lldb_fop_x86_64,lldb_fiseg_x86_64, lldb_fioff_x86_64,
+lldb_fip_x86_64,lldb_foseg_x86_64, lldb_fooff_x86_64,
+lldb_fdp_x86_64,lldb_mxcsr_x86_64, lldb_mxcsrmask_x86_64,
+lldb_st0_x86_64,lldb_st1_x86_64,   lldb_st2_x86_64,
+lldb_st3_x86_64,lldb_st4_x86_64,   lldb_st5_x86_64,
+lldb_st6_x86_64,lldb_st7_x86_64,   lldb_mm0_x86_64,
+lldb_mm1_x86_64,lldb_mm2_x86_64,   lldb_mm3_x86_64,
+lldb_mm4_x86_64,lldb_mm5_x86_64,   lldb_mm6_x86_64,
+lldb_mm7_x86_64,lldb_xmm0_x86_64,  lldb_xmm1_x86_64,
+lldb_xmm2_x86_64,   lldb_xmm3_x86_64,  lldb_xmm4_x86_64,
+lldb_xmm5_x86_64,   lldb_xmm6_x86_64,  lldb_xmm7_x86_64,
+lldb_xmm8_x86_64,   lldb_xmm9_x86_64,  lldb_xmm10_x86_64,
+lldb_xmm11_x86_64,  lldb_xmm12_x86_64, lldb_xmm13_x86_64,
+lldb_xmm14_x86_64,  lldb_xmm15_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_fpu_regnums_x86_64) / sizeof(g_fpu_regnums_x86_64[0])) 
-
@@ -280,7 +281,7 @@ 
NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[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/D91504/new/

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91728: [lldb] [Process/Utility] Declare register overlaps between ST and MM [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306363.
mgorny added a comment.

Updated to cover i386, reformatted the macro.


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

https://reviews.llvm.org/D91728

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_i386.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
@@ -88,15 +88,14 @@
  nullptr, nullptr, nullptr, 0  \
   }
 
-#define DEFINE_FP_MM(reg, i)   \
+#define DEFINE_FP_MM(reg, i, streg)\
   {\
-#reg #i, nullptr, sizeof(uint64_t),\
-  LLVM_EXTENSION FPR_OFFSET(   \
-  stmm[i]), eEncodingUint, eFormatHex, \
-  {dwarf_mm##i##_x86_64, dwarf_mm##i##_x86_64, \
-   LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,   \
-   lldb_mm##i##_x86_64 },  \
-   nullptr, nullptr, nullptr, 0\
+#reg #i, nullptr, sizeof(uint64_t), LLVM_EXTENSION FPR_OFFSET(stmm[i]),\
+eEncodingUint, eFormatHex, \
+{dwarf_mm##i##_x86_64, dwarf_mm##i##_x86_64, LLDB_INVALID_REGNUM,  \
+ LLDB_INVALID_REGNUM, lldb_mm##i##_x86_64 },   \
+RegisterContextPOSIX_x86::g_contained_##streg##_64,\
+RegisterContextPOSIX_x86::g_invalidate_##streg##_64, nullptr, 0\
   }
 
 #define DEFINE_XMM(reg, i) \
@@ -277,10 +276,12 @@
 // FP registers.
 DEFINE_FP_ST(st, 0), DEFINE_FP_ST(st, 1), DEFINE_FP_ST(st, 2),
 DEFINE_FP_ST(st, 3), DEFINE_FP_ST(st, 4), DEFINE_FP_ST(st, 5),
-DEFINE_FP_ST(st, 6), DEFINE_FP_ST(st, 7), DEFINE_FP_MM(mm, 0),
-DEFINE_FP_MM(mm, 1), DEFINE_FP_MM(mm, 2), DEFINE_FP_MM(mm, 3),
-DEFINE_FP_MM(mm, 4), DEFINE_FP_MM(mm, 5), DEFINE_FP_MM(mm, 6),
-DEFINE_FP_MM(mm, 7),
+DEFINE_FP_ST(st, 6), DEFINE_FP_ST(st, 7),
+
+DEFINE_FP_MM(mm, 0, st0), DEFINE_FP_MM(mm, 1, st1),
+DEFINE_FP_MM(mm, 2, st2), DEFINE_FP_MM(mm, 3, st3),
+DEFINE_FP_MM(mm, 4, st4), DEFINE_FP_MM(mm, 5, st5),
+DEFINE_FP_MM(mm, 6, st6), DEFINE_FP_MM(mm, 7, st7),
 
 // XMM registers
 DEFINE_XMM(xmm, 0), DEFINE_XMM(xmm, 1), DEFINE_XMM(xmm, 2),
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_i386.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_i386.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_i386.h
@@ -87,15 +87,14 @@
  nullptr, nullptr, nullptr, 0  \
   }
 
-#define DEFINE_FP_MM(reg, i)   \
+#define DEFINE_FP_MM(reg, i, streg)\
   {\
-#reg #i, nullptr, sizeof(uint64_t),\
-  LLVM_EXTENSION FPR_OFFSET(   \
-  stmm[i]), eEncodingUint, eFormatHex, \
-  {ehframe_mm##i##_i386, dwarf_mm##i##_i386,   \
-   LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,   \
-   lldb_mm##i##_i386 },\
-   nullptr, nullptr, nullptr, 0\
+#reg #i, nullptr, sizeof(uint64_t), LLVM_EXTENSION FPR_OFFSET(stmm[i]),\
+eEncodingUint, eFormatHex, \
+{dwarf_mm##i##_i386, dwarf_mm##i##_i386, LLDB_INVALID_REGNUM,  \
+ LLDB_INVALID_REGNUM, lldb_mm##i##_i386 }, \
+RegisterContextPOSIX_x86::g_contained_##streg##_32,\
+RegisterContextPOSIX_x86::g_invalidate_##streg##_32, nullptr, 0\
   }
 
 #define DEFINE_XMM(reg, i) \
@@ -251,10 +250,12 @@
 // FP registers.
 DEFINE_FP_ST(st, 0), DEFINE_FP_ST(st, 1), DEFINE_FP_ST(st, 2),
 DEFINE_FP_ST(st, 3), DEFINE_FP_ST(st, 4), DEFINE_FP_ST(st, 5),
-DEFINE_FP_ST(st, 6), 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306360.
mgorny added a comment.

Declare `MMSRegComp` type explicitly to avoid warnings:

  In file included from 
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp:11:
  In file included from 
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h:12:
  
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h:247:5:
 warning: anonymous types declared in an anonymous union are an extension 
[-Wnested-anon-types]
  struct {
  ^
  1 warning generated.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306356.
mgorny marked an inline comment as done.
mgorny added a comment.

Simplified FreeBSD & Linux code as @labath suggested. Also added the scope 
thingy to tests.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read 

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D91497#2404813 , @labath wrote:

> Looks good to me. @omjavaid, could you take a quick look at the 
> invalidate/contained register lists?

These list look good. Also I like the approach of using single invalidate list 
for fiseg and fioff unlike AArch64 where we use separate list for S, D and V 
regs. Its a lot cleaner this way and serves the purpose.


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

https://reviews.llvm.org/D91497

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

[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D91241#2402491 , @labath wrote:

> Yes, this is pretty much what I hoped for. Thanks.
>
> Besides relaxing existing tests, it would be nice to have a positive test 
> that checks for the behavior that we want. Maybe a gdb-client test which 
> checks that the register values are extracted from the appropriate place from 
> within a g packet (send custom target.xml, and a `g` response, and ensure the 
> register values come out what we expect)?
>
> In D91241#2398995 , @omjavaid wrote:
>
>> I have made some updates which are sufficient for current AArch64 scenario 
>> where offset field is not present. For ordering registers based on register 
>> numbers I think we may skip doing that for now though i intend to come back 
>> to it after getting SVE through.
>
> That seems fair.

Let me take a look into writing a test case for this change.

Moreover, I have tried what you suggested about initializing all register 
offset with LLDB_INVALID_INDEX32 unless an offset is explicitly specified. The 
problem is that once we make that change in ProcessGDBRemote.cpp more changes 
are required elsewhere and many tests start failing. I kept plugging those 
tests with adjustments untill I decided on current solution which required less 
no of changes.

DynamicRegisterInfo::AddRegister also calculated the maximum offset if we take 
maximum offset calculation then it impacts DynamicRegisterInfo::SetRegisterInfo 
which is using Finalize but calculating offsets itself. Also oid 
GDBRemoteDynamicRegisterInfo::HardcodeARMRegisters also use 
DynamicRegisterInfo::AddRegister but calculates offsets by itself.


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

https://reviews.llvm.org/D91241

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

[Lldb-commits] [PATCH] D87442: [lldb][AArch64/Linux] Show memory tagged memory regions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Hopefully last round of cosmetic fixes, and then this should be good.




Comment at: lldb/include/lldb/Target/MemoryRegionInfo.h:15
 #include "lldb/Utility/RangeMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatProviders.h"

I guess this is not used anymore..



Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py:737
 "name",
-"error"])
+"error"], "Unexpected key \"%s\"" % key)
 self.assertIsNotNone(val)

I guess you could change this to `self.assertIn(needle, haystack)`



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1309-1313
+llvm::handleAllErrors(Info.takeError(),
+  [](const llvm::StringError ) {
+Result.SetErrorToGenericError();
+Result.SetErrorString(e.getMessage());
+  });

Status has an llvm::Error constructor. Some variation on `Result = 
Info.takeError()` should work.



Comment at: lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp:196
+callback(*region);
+  }
+}

[[ 
http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
 | no braces for simple statements]]



Comment at: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp:270-272
+ llvm::handleAllErrors(
+ region.takeError(),
+ [](const llvm::StringError ) {});

`llvm::consumeError(region.takeError())`, though maybe it would be better to 
actually log it (LLDB_LOG_ERROR)



Comment at: 
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py:31
+# 47 is our magic status meaning MTE isn't available
+if "exited with status = 47" in self.res.GetOutput():
+self.skipTest("MTE must be available in toolchain and on target")

`if self.process().GetState() == lldb.eStateExited and 
self.process().GetExitStatus() == 47` would be nicer



Comment at: 
lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py:34-38
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)

I think you could just run this once, and then choose to skip the test or not 
depending on whether the process exited, or hit a breakpoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:537
+uint16_t sw = m_xstate->fxsave.fstat;
+llvm::ArrayRef st_regs{m_xstate->fxsave.stmm, 8};
+reg_value.SetUInt16(AbridgedToFullTagWord(abridged_tw, sw, st_regs));

labath wrote:
> Is this really needed? I would have hoped that just passing 
> `m_xstate->fxsave.stmm` would be enough and that the c array constructor of 
> ArrayRef would do the right thing...
Indeed. I have missed that `ArrayRef` can figure the size of a C-style array 
out.


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91728: [lldb] [Process/Utility] Declare register overlaps between ST and MM [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h:93
   {
\
 #reg #i, nullptr, sizeof(uint64_t),
\
   LLVM_EXTENSION FPR_OFFSET(   
\

labath wrote:
> mgorny wrote:
> > To be honest, I completely don't get the indentation below.
> Yeah... You could try explicitly re-indenting the block to see whether it 
> produces something better. If not, it's likely a clang-format bug (though I 
> can't really blame it for getting this mess wrong)
Actually, clang-format produces something like this, for the whole file:

```
#define DEFINE_FP_MM(reg, i, streg)\
  {\
#reg #i, nullptr, sizeof(uint64_t), LLVM_EXTENSION FPR_OFFSET(stmm[i]),\
eEncodingUint, eFormatHex, \
{dwarf_mm##i##_x86_64, dwarf_mm##i##_x86_64, LLDB_INVALID_REGNUM,  \
 LLDB_INVALID_REGNUM, lldb_mm##i##_x86_64 },   \
 RegisterContextPOSIX_x86::g_contained_##streg,\
 RegisterContextPOSIX_x86::g_invalidate_##streg, nullptr, 0\
  }
```

Note that the `#reg` line gets `\` wrong — if I add 4 spaces to the beginning, 
it starts looking sane-ish, so it probably gets that wrong.


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

https://reviews.llvm.org/D91728

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Continuing the discussion from the inline comment:

> Lua can be compiled to use try/catch instead of longjmp, but that is an 
> exception (no pun intended). Since we rely on the host's library, we need to 
> assume that it's using longjmp.

Right. In fact even if it had exceptions enabled, that still wouldn't help as 
llvm itself is also compiled without exceptions (it can be, but it's an 
exception :P).

> Any C++ code that runs in a lua_pcall seems to face this issue. I checked a 
> very complete C++ Lua wrapper called sol2 and they seem to have the same 
> issue. I don't think there's a way out of it except code discipline.

I'm not sure what you mean by that. Can you elaborate?

The way I see it, the problem is not about which environment is the c++ code 
running in, as it can't throw an exception (in llvm). The problem is what 
happens when we call lua from c++, and the lua code throws. And this is a 
problem regardless of the environment the C(++) code is in -- the difference is 
"only" in whether lua will panic or jump over the C code, but something bad 
will happen anyway.

I think that could be solved by ensuring we always call lua in a protected 
environment. That way lua will never unwind through the C code (regardless of 
what environment it is in) because it would always stop right at the boundary.

That would essentially mean, lua_call is banned (except maybe as an 
optimization, if you can be sure that the code you're about to call will not 
throw), and all C->lua transitions should go through lua_pcall. And we can 
provide a wrapper that handles catches those errors and converts them to 
llvm::Error/Expected, similar to how the python things do it.

> Lua provides lua_atpanic as a way to recover from unprotected throws. Perhaps 
> we can leverage this to throw an exception, guaranteeing stack unwinding and 
> avoiding a call to abort(). We would then let the callback run unprotected 
> and delegate any calls to lua_pcall to the callback.

I'm afraid that won't work, because exceptions are not allowed in llvm. But I 
think something similar to what I propose above should do the trick.




Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:60-78
+static int runCallback(lua_State *L) {
+  LuaCallback *callback = static_cast(lua_touserdata(L, -1));
+  return (*callback)(L);
+}
+
+llvm::Error Lua::Run(LuaCallback ) {
+  lua_pushcfunction(m_lua_state, runCallback);

tammela wrote:
> labath wrote:
> > tammela wrote:
> > > labath wrote:
> > > > I'm confused. Why use lua to call a c callback, when you could just do 
> > > > `calllback()`?
> > > Some Lua API functions throw errors, if there's any it will `abort()` the 
> > > program if no panic handler or protected call is provided.
> > > To guarantee that the callback always runs in a protected environment and 
> > > it has error handling, we do the above.
> > > Delegating this to the callback itself makes it cumbersome to write.
> > Aha, I see.
> > 
> > So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but 
> > that was years ago..), whenever there's a lua exception inside this (c++) 
> > callback, lua will longjmp(3) back to the lua_pcall call on line 68, 
> > skipping any destructors that should normally be invoked. Is that so?
> > 
> > If that's the case, then I think this is a dangerous api, that should at 
> > the very least get a big fat warning, but that ideally shouldn't exist at 
> > all.
> > 
> > What's the part that makes delegating this to the callback "cumersome to 
> > write"? And why can't that be overcome by a suitable utility function which 
> > wraps `lua_pcall` or whatever else is needed?
> > 
> > The approach that we've chosen in python is to have very little code 
> > interacting with the python C API directly. Instead, code generally works 
> > with our C++ wrappers defined in `PythonDataObject.h`. These functions try 
> > to hide the python exception magic as much as possible, and present a c++-y 
> > version of the interface.
> > 
> > Now, since lua is stack-based, something like LuaDataObjects.h would 
> > probably not work. However, that doesn't meant we should give up on the c++ 
> > wrapper  idea altogether. During the intitial reviews, my intention was for 
> > the `Lua` class to serve this purpose. I still think this can be achieved 
> > if we make the callback functions take `Lua&` instead of `lua_State*` as an 
> > argument, and then ensure the class contains whatever is needed to make the 
> > callbacks not cumerbsome to write.
> > So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but 
> > that was years ago..), whenever there's a lua exception inside this (c++) 
> > callback, lua will longjmp(3) back to the lua_pcall call on line 68, 
> > skipping any destructors that should normally be invoked. Is that so?
> >
> > If that's the case, then I think this is a dangerous api, that should at 
> > the very least get 

[Lldb-commits] [PATCH] D91728: [lldb] [Process/Utility] Declare register overlaps between ST and MM [WIP]

[Pavel Labath via Phabricator via lldb-commits]

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

I don't know omitting this had any practical effects (my experiments with rax 
were... inconclusive) , but it definitely sounds like the right to do.




Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h:93
   {
\
 #reg #i, nullptr, sizeof(uint64_t),
\
   LLVM_EXTENSION FPR_OFFSET(   
\

mgorny wrote:
> To be honest, I completely don't get the indentation below.
Yeah... You could try explicitly re-indenting the block to see whether it 
produces something better. If not, it's likely a clang-format bug (though I 
can't really blame it for getting this mess wrong)


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

https://reviews.llvm.org/D91728

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

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good to me. @omjavaid, could you take a quick look at the 
invalidate/contained register lists?




Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:84-97
+lldb_fop_x86_64,lldb_fiseg_x86_64, lldb_fioff_x86_64,
+lldb_fip_x86_64,lldb_foseg_x86_64, lldb_fooff_x86_64,
+lldb_fdp_x86_64,lldb_mxcsr_x86_64, lldb_mxcsrmask_x86_64,
+lldb_st0_x86_64,lldb_st1_x86_64,   lldb_st2_x86_64,
+lldb_st3_x86_64,lldb_st4_x86_64,   lldb_st5_x86_64,
+lldb_st6_x86_64,lldb_st7_x86_64,   lldb_mm0_x86_64,
+lldb_mm1_x86_64,lldb_mm2_x86_64,   lldb_mm3_x86_64,

lol@diff



Comment at: lldb/test/Shell/Register/x86-64-fp-read.test:11-12
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+print (void*)($fiseg*0x1 + $fioff)
+# CHECK: (void *) $1 = [[FDIV]]
+print 

mgorny wrote:
> labath wrote:
> > Move this part after the `fip` check and add a comment to indicate its 
> > legacy/compat status?
> I suppose it also makes sense to remove the fiseg/fioff logic from the 32-bit 
> `x86-fp-read.test`, as it is only relevant to the FXSAVE64 approach.
Yep, sounds good.


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

https://reviews.llvm.org/D91497

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks good. Some minor questions inline.




Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:523
+  case DBRegSet: {
+uint8_t *data = GetOffsetRegSetData(set, reg_info->byte_offset);
+FXSAVE *fpr = reinterpret_cast(m_fpr.data());

if you make data a `void*`, then you can remove the `reinterpret_cast` two 
lines below.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:528
+else
+  ::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
+   reg_value.GetBytes(), reg_value.GetByteSize());

reuse data from above



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:537
+uint16_t sw = m_xstate->fxsave.fstat;
+llvm::ArrayRef st_regs{m_xstate->fxsave.stmm, 8};
+reg_value.SetUInt16(AbridgedToFullTagWord(abridged_tw, sw, st_regs));

Is this really needed? I would have hoped that just passing 
`m_xstate->fxsave.stmm` would be enough and that the c array constructor of 
ArrayRef would do the right thing...



Comment at: lldb/unittests/Process/Utility/RegisterContextTest.cpp:55
+  for (const TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)

add `SCOPED_TRACE` macro to make it clear which test case failed, when it 
fails. You can use llvm::enumerate to produce sequence numbers for the test 
cases...


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

https://reviews.llvm.org/D91504

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