[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
omjavaid updated this revision to Diff 75103.
omjavaid added a comment.
Sorry I was on Holiday so couldnt get back to this earlier.
I ll get back to your comments about GDB packet sequence in a separate thread
after collecting further information.
I have made the suggested corrections in above patch.
https://reviews.llvm.org/D25057
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -538,42 +538,33 @@
control_value |= ((1 << size) - 1) << 5;
control_value |= (2 << 1) | 1;
- // Iterate over stored watchpoints
- // Find a free wp_index or update reference count if duplicate.
+ // Iterate over stored watchpoints and find a free wp_index
wp_index = LLDB_INVALID_INDEX32;
for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
if ((m_hwp_regs[i].control & 1) == 0) {
wp_index = i; // Mark last free slot
-} else if (m_hwp_regs[i].address == addr &&
- m_hwp_regs[i].control == control_value) {
- wp_index = i; // Mark duplicate index
- break;// Stop searching here
+} else if (m_hwp_regs[i].address == addr) {
+ return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
}
}
if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Add new or update existing watchpoint
- if ((m_hwp_regs[wp_index].control & 1) == 0) {
-// Update watchpoint in local cache
-m_hwp_regs[wp_index].real_addr = real_addr;
-m_hwp_regs[wp_index].address = addr;
-m_hwp_regs[wp_index].control = control_value;
-m_hwp_regs[wp_index].refcount = 1;
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].real_addr = real_addr;
+ m_hwp_regs[wp_index].address = addr;
+ m_hwp_regs[wp_index].control = control_value;
-// PTRACE call to set corresponding watchpoint register.
-error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
-if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
+ if (error.Fail()) {
+m_hwp_regs[wp_index].address = 0;
+m_hwp_regs[wp_index].control &= ~1;
- return LLDB_INVALID_INDEX32;
-}
- } else
-m_hwp_regs[wp_index].refcount++;
+return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -596,36 +587,25 @@
if (wp_index >= m_max_hwp_supported)
return false;
- // Update reference count if multiple references.
- if (m_hwp_regs[wp_index].refcount > 1) {
-m_hwp_regs[wp_index].refcount--;
-return true;
- } else if (m_hwp_regs[wp_index].refcount == 1) {
-// Create a backup we can revert to in case of failure.
-lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
-uint32_t tempControl = m_hwp_regs[wp_index].control;
-uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
-
-// Update watchpoint in local cache
-m_hwp_regs[wp_index].control &= ~1;
-m_hwp_regs[wp_index].address = 0;
-m_hwp_regs[wp_index].refcount = 0;
-
-// Ptrace call to update hardware debug registers
-error = WriteHardwareDebugRegs(eDREGTypeWATCH);
-
-if (error.Fail()) {
- m_hwp_regs[wp_index].control = tempControl;
- m_hwp_regs[wp_index].address = tempAddr;
- m_hwp_regs[wp_index].refcount = tempRefCount;
-
- return false;
-}
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control &= ~1;
+ m_hwp_regs[wp_index].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+ if (error.Fail()) {
+m_hwp_regs[wp_index].control = tempControl;
+m_hwp_regs[wp_index].address = tempAddr;
-return true;
+return false;
}
- return false;
+ return true;
}
Error NativeRegisterContextLinux_arm64::ClearAllHardwareWatchpoints() {
@@ -650,20 +630,17 @@
// Create a backup we can revert to in case of failure.
tempAddr = m_hwp_regs[i].address;
tempControl = m_hwp_regs[i].control;
- tempRefCount = m_hwp_regs[i].refcount;
// Clear watchpoints in local c
[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284706: Fix ARM/AArch64 Step-Over watchpoint issue remove
provision for duplicate… (authored by omjavaid).
Changed prior to commit:
https://reviews.llvm.org/D25057?vs=75103&id=75272#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25057
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
@@ -0,0 +1,97 @@
+"""
+Test watchpoint slots we should not be able to install multiple watchpoints
+within same word boundary. We should be able to install individual watchpoints
+on any of the bytes, half-word, or word. This is only for ARM/AArch64 targets.
+"""
+
+from __future__ import print_function
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class WatchpointSlotsTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Source filename.
+self.source = 'main.c'
+
+# Output filename.
+self.exe_name = 'a.out'
+self.d = {'C_SOURCES': self.source, 'EXE': self.exe_name}
+
+# Watchpoints not supported
+@expectedFailureAndroid(archs=['arm', 'aarch64'])
+# This is a arm and aarch64 specific test case. No other architectures tested.
+@skipIf(archs=no_match(['arm', 'aarch64']))
+def test_multiple_watchpoints_on_same_word(self):
+
+self.build(dictionary=self.d)
+self.setTearDownCleanup(dictionary=self.d)
+
+exe = os.path.join(os.getcwd(), self.exe_name)
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Detect line number after which we are going to increment arrayName.
+loc_line = line_number('main.c', '// About to write byteArray')
+
+# Set a breakpoint on the line detected above.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", loc_line, num_expected_locations=1, loc_exact=True)
+
+# Run the program.
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped', 'stop reason = breakpoint'])
+
+# Delete breakpoint we just hit.
+self.expect("breakpoint delete 1", substrs=['1 breakpoints deleted'])
+
+# Set a watchpoint at byteArray[0]
+self.expect("watchpoint set variable byteArray[0]", WATCHPOINT_CREATED,
+substrs=['Watchpoint created','size = 1'])
+
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should be 0 initially.
+self.expect("watchpoint list -v 1", substrs=['hit_count = 0'])
+
+# Try setting a watchpoint at byteArray[1]
+self.expect("watchpoint set variable byteArray[1]", error=True,
+substrs=['Watchpoint creation failed'])
+
+self.runCmd("process continue")
+
+# We should be stopped due to the watchpoint.
+# The stop reason of the thread should be watchpoint.
+self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+substrs=['stopped', 'stop reason = watchpoint 1'])
+
+# Delete the watchpoint we hit above successfully.
+self.expect("watchpoint delete 1", substrs=['1 watchpoints deleted'])
+
+# Set a watchpoint at byteArray[3]
+self.expect("watchpoint set variable byteArray[3]", WATCHPOINT_CREATED,
+substrs=['Watchpoint created','size = 1'])
+
+# Resume inferior.
+self.runCmd("process continue")
+
+# We should be stopped due to the watchpoint.
+# The stop reason of the thread should be watchpoint.
+self.expect("thread list -v", STOPPED_DUE_TO_WATCHPOINT,
+substrs=['stopped', 'stop reason = watchpoint 3'])
+
+# Resume inferior.
+self.runCmd("process continue")
Index:
[Lldb-commits] [PATCH] D27124: [LLDB][MIPS] Fix TestWatchpointIter failure
omjavaid added a comment. Although this patch fixes the test case in question but in theory EphemeralMode watchpoint enable/disable cycles should be independent of step-over watchpoint enable disable cycle. On gdb-remote type targets we only update hardware_watch_id when a watchpoint is hit so when we clear hw_watch_id in Watchpoint::SetEnabled (source/Breakpoint/Watchpoint.cpp line 238) then information about last hardware watchpoint id that was hit also gets removed. To update hardware watchpoint id on gdb-remote targets we ll have to send/recv an extra query packet asking about hardware watchpoint id corresponding to particular watchpoint we just enabled. One possible solution could be to preserver the hardware watchpint id like way we are preserving stop info in this case. But there no harm in even not clearing watchpoint hardware id on disable rather leave that job for the target to decide if it can update hardware id upon every enable disable or just when watchpoint is hit. https://reviews.llvm.org/D27124 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid abandoned this revision. omjavaid added a comment. There is not exact solution that satisfies all corner cases. Abandoning for now until I come up with a solution that covers us from all corners. https://reviews.llvm.org/D24610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12522: AArch64 Watchpoints: Make sure we are only setting supported no of debug registers.
omjavaid updated this revision to Diff 37748.
omjavaid added a comment.
This patch fixes unexpected behaviour of watchpoint code on Nexus 9 (AArch64).
http://reviews.llvm.org/D12522
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -807,11 +807,12 @@
memset (&dreg_state, 0, sizeof (dreg_state));
ioVec.iov_base = &dreg_state;
-ioVec.iov_len = sizeof (dreg_state);
if (hwbType == eDREGTypeWATCH)
{
hwbType = NT_ARM_HW_WATCH;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hwp_supported);
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
@@ -822,6 +823,8 @@
else
{
hwbType = NT_ARM_HW_BREAK;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hbp_supported);
for (uint32_t i = 0; i < m_max_hbp_supported; i++)
{
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -807,11 +807,12 @@
memset (&dreg_state, 0, sizeof (dreg_state));
ioVec.iov_base = &dreg_state;
-ioVec.iov_len = sizeof (dreg_state);
if (hwbType == eDREGTypeWATCH)
{
hwbType = NT_ARM_HW_WATCH;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hwp_supported);
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
@@ -822,6 +823,8 @@
else
{
hwbType = NT_ARM_HW_BREAK;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hbp_supported);
for (uint32_t i = 0; i < m_max_hbp_supported; i++)
{
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D12522: AArch64 Watchpoints: Make sure we are only setting supported no of debug registers.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL250700: Fix for random watchpoint testsuite failures on
AArch64 targets. (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D12522?vs=37748&id=37758#toc
Repository:
rL LLVM
http://reviews.llvm.org/D12522
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -807,11 +807,12 @@
memset (&dreg_state, 0, sizeof (dreg_state));
ioVec.iov_base = &dreg_state;
-ioVec.iov_len = sizeof (dreg_state);
if (hwbType == eDREGTypeWATCH)
{
hwbType = NT_ARM_HW_WATCH;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hwp_supported);
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
@@ -822,6 +823,8 @@
else
{
hwbType = NT_ARM_HW_BREAK;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hbp_supported);
for (uint32_t i = 0; i < m_max_hbp_supported; i++)
{
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -807,11 +807,12 @@
memset (&dreg_state, 0, sizeof (dreg_state));
ioVec.iov_base = &dreg_state;
-ioVec.iov_len = sizeof (dreg_state);
if (hwbType == eDREGTypeWATCH)
{
hwbType = NT_ARM_HW_WATCH;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hwp_supported);
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
@@ -822,6 +823,8 @@
else
{
hwbType = NT_ARM_HW_BREAK;
+ioVec.iov_len = sizeof (dreg_state.dbg_info) + sizeof (dreg_state.pad)
++ (sizeof (dreg_state.dbg_regs [0]) * m_max_hbp_supported);
for (uint32_t i = 0; i < m_max_hbp_supported; i++)
{
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D14051: Fix for Arm watchpoint cache corruption in case of ptrace failure
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
Arm watchpoint handlers incorrectly updates watchpoint cache incase we have a
ptrace failures.
This patch fixes this issue by reverting the cache to its previous state in
case of a ptrace failure.
http://reviews.llvm.org/D14051
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -451,7 +451,13 @@
error = WriteHardwareDebugRegs(eDREGTypeBREAK, bp_index);
if (error.Fail())
+{
+m_hbr_regs[bp_index].address = 0;
+m_hbr_regs[bp_index].control &= ~1;
+m_hbr_regs[bp_index].refcount = 0;
+
return LLDB_INVALID_INDEX32;
+}
}
else
m_hbr_regs[bp_index].refcount++;
@@ -486,15 +492,26 @@
}
else if (m_hbr_regs[hw_idx].refcount == 1)
{
+// Create a backup we can revert to in case of failure.
+lldb::addr_t tempAddr = m_hbr_regs[hw_idx].address;
+uint32_t tempControl = m_hbr_regs[hw_idx].control;
+uint32_t tempRefCount = m_hbr_regs[hw_idx].refcount;
+
m_hbr_regs[hw_idx].control &= ~1;
m_hbr_regs[hw_idx].address = 0;
m_hbr_regs[hw_idx].refcount = 0;
// PTRACE call to clear corresponding hardware breakpoint register.
WriteHardwareDebugRegs(eDREGTypeBREAK, hw_idx);
if (error.Fail())
-return LLDB_INVALID_INDEX32;
+{
+m_hbr_regs[hw_idx].control = tempControl;
+m_hbr_regs[hw_idx].address = tempAddr;
+m_hbr_regs[hw_idx].refcount = tempRefCount;
+
+return false;
+}
return true;
}
@@ -614,7 +631,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
if (error.Fail())
+{
+m_hwp_regs[wp_index].address = 0;
+m_hwp_regs[wp_index].control &= ~1;
+m_hwp_regs[wp_index].refcount = 0;
+
return LLDB_INVALID_INDEX32;
+}
}
else
m_hwp_regs[wp_index].refcount++;
@@ -649,6 +672,11 @@
}
else if (m_hwp_regs[wp_index].refcount == 1)
{
+// Create a backup we can revert to in case of failure.
+lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+uint32_t tempControl = m_hwp_regs[wp_index].control;
+uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
+
// Update watchpoint in local cache
m_hwp_regs[wp_index].control &= ~1;
m_hwp_regs[wp_index].address = 0;
@@ -658,7 +686,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
if (error.Fail())
+{
+m_hwp_regs[wp_index].control = tempControl;
+m_hwp_regs[wp_index].address = tempAddr;
+m_hwp_regs[wp_index].refcount = tempRefCount;
+
return false;
+}
return true;
}
@@ -682,10 +716,18 @@
if (error.Fail())
return error;
+lldb::addr_t tempAddr = 0;
+uint32_t tempControl = 0, tempRefCount = 0;
+
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
if (m_hwp_regs[i].control & 0x01)
{
+// Create a backup we can revert to in case of failure.
+tempAddr = m_hwp_regs[i].address;
+tempControl = m_hwp_regs[i].control;
+tempRefCount = m_hwp_regs[i].refcount;
+
// Clear watchpoints in local cache
m_hwp_regs[i].control &= ~1;
m_hwp_regs[i].address = 0;
@@ -695,7 +737,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, i);
if (error.Fail())
+{
+m_hwp_regs[i].control = tempControl;
+m_hwp_regs[i].address = tempAddr;
+m_hwp_regs[i].refcount = tempRefCount;
+
return error;
+}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14051: Fix for Arm watchpoint cache corruption in case of ptrace failure
This revision was automatically updated to reflect the committed changes.
Closed by commit rL251386: Fix for Arm watchpoint cache corruption in case of
ptrace failure (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D14051?vs=38364&id=38502#toc
Repository:
rL LLVM
http://reviews.llvm.org/D14051
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -451,7 +451,13 @@
error = WriteHardwareDebugRegs(eDREGTypeBREAK, bp_index);
if (error.Fail())
+{
+m_hbr_regs[bp_index].address = 0;
+m_hbr_regs[bp_index].control &= ~1;
+m_hbr_regs[bp_index].refcount = 0;
+
return LLDB_INVALID_INDEX32;
+}
}
else
m_hbr_regs[bp_index].refcount++;
@@ -486,15 +492,26 @@
}
else if (m_hbr_regs[hw_idx].refcount == 1)
{
+// Create a backup we can revert to in case of failure.
+lldb::addr_t tempAddr = m_hbr_regs[hw_idx].address;
+uint32_t tempControl = m_hbr_regs[hw_idx].control;
+uint32_t tempRefCount = m_hbr_regs[hw_idx].refcount;
+
m_hbr_regs[hw_idx].control &= ~1;
m_hbr_regs[hw_idx].address = 0;
m_hbr_regs[hw_idx].refcount = 0;
// PTRACE call to clear corresponding hardware breakpoint register.
WriteHardwareDebugRegs(eDREGTypeBREAK, hw_idx);
if (error.Fail())
+{
+m_hbr_regs[hw_idx].control = tempControl;
+m_hbr_regs[hw_idx].address = tempAddr;
+m_hbr_regs[hw_idx].refcount = tempRefCount;
+
return false;
+}
return true;
}
@@ -614,7 +631,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
if (error.Fail())
+{
+m_hwp_regs[wp_index].address = 0;
+m_hwp_regs[wp_index].control &= ~1;
+m_hwp_regs[wp_index].refcount = 0;
+
return LLDB_INVALID_INDEX32;
+}
}
else
m_hwp_regs[wp_index].refcount++;
@@ -649,6 +672,11 @@
}
else if (m_hwp_regs[wp_index].refcount == 1)
{
+// Create a backup we can revert to in case of failure.
+lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+uint32_t tempControl = m_hwp_regs[wp_index].control;
+uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
+
// Update watchpoint in local cache
m_hwp_regs[wp_index].control &= ~1;
m_hwp_regs[wp_index].address = 0;
@@ -658,7 +686,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
if (error.Fail())
+{
+m_hwp_regs[wp_index].control = tempControl;
+m_hwp_regs[wp_index].address = tempAddr;
+m_hwp_regs[wp_index].refcount = tempRefCount;
+
return false;
+}
return true;
}
@@ -682,10 +716,18 @@
if (error.Fail())
return error;
+lldb::addr_t tempAddr = 0;
+uint32_t tempControl = 0, tempRefCount = 0;
+
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
if (m_hwp_regs[i].control & 0x01)
{
+// Create a backup we can revert to in case of failure.
+tempAddr = m_hwp_regs[i].address;
+tempControl = m_hwp_regs[i].control;
+tempRefCount = m_hwp_regs[i].refcount;
+
// Clear watchpoints in local cache
m_hwp_regs[i].control &= ~1;
m_hwp_regs[i].address = 0;
@@ -695,7 +737,13 @@
error = WriteHardwareDebugRegs(eDREGTypeWATCH, i);
if (error.Fail())
+{
+m_hwp_regs[i].control = tempControl;
+m_hwp_regs[i].address = tempAddr;
+m_hwp_regs[i].refcount = tempRefCount;
+
return error;
+}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D14823: Disable forcing -marm (A32 instruction set) while running testsuite on arm targets.
omjavaid created this revision. omjavaid added reviewers: tberghammer, clayborg. omjavaid added a subscriber: lldb-commits. Herald added subscribers: rengolin, aemerson. This patch disables forcing -marm (A32 instruction set) while running lldb testsuite on arm targets. gcc uses -marm and -mthumb flag to choose between A32 and T32 instruction sets. For most processors A32 (arm mode) code is generated by default but many modern cpus also use T32 code generation as the default or a mix of both. If we do not provide a flag then compiler decides the best possible code generation for the target which is most commonly used configuration. Thats why i have removed this flag from out testing so that we can have a clarity on what is working when compiler selects both A32/T32 and T16 instructions interchangeably. http://reviews.llvm.org/D14823 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -164,6 +164,10 @@ override ARCH := override ARCHFLAG := endif + ifeq "$(ARCH)" "arm" + override ARCH := + override ARCHFLAG := + endif ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -164,6 +164,10 @@ override ARCH := override ARCHFLAG := endif + ifeq "$(ARCH)" "arm" + override ARCH := + override ARCHFLAG := + endif ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14816: Use thumb instruction set for ldb-server on android arm
omjavaid added a subscriber: omjavaid. omjavaid added a comment. -mthumb will force T32 instruction set while -marm will force A32. Best is not to use any of these flags to let the compiler decide best possible instruction set combination. http://reviews.llvm.org/D14816 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14823: Disable forcing -marm (A32 instruction set) while running testsuite on arm targets.
omjavaid added a comment. -mthumb and -marm are compiler flags so we can put these into our CXX or CFLAGS if we need to run those configurations. http://reviews.llvm.org/D14823 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14823: Disable forcing -marm (A32 instruction set) while running testsuite on arm targets.
omjavaid added a comment. I agree with you on having multiple configuration options to help identify failures in different configurations. What I mean is that we should keep clarity on architecture that is to use arm or aarch32 for 32bit and aarch64 for 64bit. With that we should run tests in default configurations which compiler is offering that means not to use any -m(arch) flag. This will help us Xfail based on architecture in default configuration. Additional config option that we ll add later can take up values to test some additional flags specifying ABI and ISA features like thumb, nothumb, interwork, vfp, neon, soft float, hard float etc. We can then xfail if something fails based on these additional feature. http://reviews.llvm.org/D14823 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14823: Disable forcing -marm (A32 instruction set) while running testsuite on arm targets.
This revision was automatically updated to reflect the committed changes. Closed by commit rL253973: Disable forcing -marm (A32 instruction set) while running testsuite on arm… (authored by omjavaid). Changed prior to commit: http://reviews.llvm.org/D14823?vs=40652&id=41017#toc Repository: rL LLVM http://reviews.llvm.org/D14823 Files: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -164,6 +164,10 @@ override ARCH := override ARCHFLAG := endif + ifeq "$(ARCH)" "arm" + override ARCH := + override ARCHFLAG := + endif ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -164,6 +164,10 @@ override ARCH := override ARCHFLAG := endif + ifeq "$(ARCH)" "arm" + override ARCH := + override ARCHFLAG := + endif ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14985: Add 64/128 bit arm neon register definitions on linux
omjavaid added a comment.
Some minor typos inline comments.
I havnt applied nd tested this patch. Are you able to read these registers, i
guess you need to generate neon or vfp code to test all registers properly.
There no arm register test to my knowledge currently in testsuite we can put
this as a future todo to verify we are writing and reading properly.
Comment at: source/Plugins/Process/Utility/RegisterInfos_arm.h:316
@@ +315,3 @@
+static uint32_t g_q5_contains[] = { fpu_d10, fpu_d11, fpu_s20, fpu_s21,
fpu_s22, fpu_s23, LLDB_INVALID_REGNUM };
+static uint32_t g_q6_contains[] = { fpu_d12, fpu_d13, fpu_s24, fpu_s25,
fpu_s24, fpu_s27, LLDB_INVALID_REGNUM };
+static uint32_t g_q7_contains[] = { fpu_d14, fpu_d15, fpu_s28, fpu_s29,
fpu_s28, fpu_s31, LLDB_INVALID_REGNUM };
here fpu_s24 is being repeated twice instead of fpu_s26.
Comment at: source/Plugins/Process/Utility/RegisterInfos_arm.h:317
@@ +316,3 @@
+static uint32_t g_q6_contains[] = { fpu_d12, fpu_d13, fpu_s24, fpu_s25,
fpu_s24, fpu_s27, LLDB_INVALID_REGNUM };
+static uint32_t g_q7_contains[] = { fpu_d14, fpu_d15, fpu_s28, fpu_s29,
fpu_s28, fpu_s31, LLDB_INVALID_REGNUM };
+static uint32_t g_q8_contains[] = { fpu_d16, fpu_d17, LLDB_INVALID_REGNUM };
here fpu_s28 is being repeated twice instead of fpu_s30.
http://reviews.llvm.org/D14985
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14985: Add 64/128 bit arm neon register definitions on linux
omjavaid accepted this revision. omjavaid added a comment. LGTM http://reviews.llvm.org/D14985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D15061: Correction in TestFrames.py test for arm targets in thumb mode
omjavaid created this revision.
omjavaid added a reviewer: tberghammer.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch adds required correction in TestFrames.py for arm targets in thumb
mode.
On arm hardware thumb mode is identified by setting the least significant bit.
In such case when we read PC we ll end up getting an unaligned address which
has last bit set and will cause a mismatch between address read from dwarf
frame information. I have corrected this by adding an appropriate check in
TestFrames.py.
http://reviews.llvm.org/D15061
Files:
packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
Index: packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
===
--- packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
+++ packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
@@ -80,9 +80,12 @@
gpr_reg_set = lldbutil.get_GPRs(frame)
pc_value = gpr_reg_set.GetChildMemberWithName("pc")
self.assertTrue (pc_value, "We should have a valid PC.")
-pc_value_str = pc_value.GetValue()
-self.assertTrue (pc_value_str, "We should have a valid PC
string.")
-self.assertTrue (int(pc_value_str, 0) == frame.GetPC(), "PC
gotten as a value should equal frame's GetPC")
+pc_value_int = int(pc_value.GetValue(), 0)
+# Make sure on arm targets we dont mismatch PC value on the
basis of thumb bit.
+# Frame PC will not have thumb bit set in case of a thumb
instruction as PC.
+if self.getArchitecture() in ['arm']:
+pc_value_int &= ~1
+self.assertTrue (pc_value_int == frame.GetPC(), "PC gotten as
a value should equal frame's GetPC")
sp_value = gpr_reg_set.GetChildMemberWithName("sp")
self.assertTrue (sp_value, "We should have a valid Stack
Pointer.")
self.assertTrue (int(sp_value.GetValue(), 0) == frame.GetSP(),
"SP gotten as a value should equal frame's GetSP")
Index: packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
===
--- packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
+++ packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
@@ -80,9 +80,12 @@
gpr_reg_set = lldbutil.get_GPRs(frame)
pc_value = gpr_reg_set.GetChildMemberWithName("pc")
self.assertTrue (pc_value, "We should have a valid PC.")
-pc_value_str = pc_value.GetValue()
-self.assertTrue (pc_value_str, "We should have a valid PC string.")
-self.assertTrue (int(pc_value_str, 0) == frame.GetPC(), "PC gotten as a value should equal frame's GetPC")
+pc_value_int = int(pc_value.GetValue(), 0)
+# Make sure on arm targets we dont mismatch PC value on the basis of thumb bit.
+# Frame PC will not have thumb bit set in case of a thumb instruction as PC.
+if self.getArchitecture() in ['arm']:
+pc_value_int &= ~1
+self.assertTrue (pc_value_int == frame.GetPC(), "PC gotten as a value should equal frame's GetPC")
sp_value = gpr_reg_set.GetChildMemberWithName("sp")
self.assertTrue (sp_value, "We should have a valid Stack Pointer.")
self.assertTrue (int(sp_value.GetValue(), 0) == frame.GetSP(), "SP gotten as a value should equal frame's GetSP")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D15355: Add failure paths to a few JSONNumber members
omjavaid created this revision.
omjavaid added a reviewer: tberghammer.
omjavaid added a subscriber: lldb-commits.
This patch updates GetAsUnsigned(), GetAsSigned(), and GetAsDouble() JSONNumber
functions to add failure check.
The previous code was generating compiler warnings for not being able to
provide a return path for all possibilities.
http://reviews.llvm.org/D15355
Files:
include/lldb/Utility/JSON.h
source/Utility/JSON.cpp
Index: source/Utility/JSON.cpp
===
--- source/Utility/JSON.cpp
+++ source/Utility/JSON.cpp
@@ -60,46 +60,64 @@
s.Printf("\"%s\"", json_string_quote_metachars(m_data).c_str());
}
-uint64_t
-JSONNumber::GetAsUnsigned() const
+bool
+JSONNumber::GetAsUnsigned(uint64_t &value) const
{
switch (m_data_type)
{
-case DataType::Unsigned:
-return m_data.m_unsigned;
case DataType::Signed:
-return (uint64_t)m_data.m_signed;
+ value = (uint64_t)m_data.m_signed;
+ break;
case DataType::Double:
-return (uint64_t)m_data.m_double;
+ value = (uint64_t)m_data.m_double;
+ break;
+case DataType::Unsigned:
+ value = m_data.m_unsigned;
+ break;
+default:
+ return false;
}
+return true;
}
-uint64_t
-JSONNumber::GetAsSigned() const
+bool
+JSONNumber::GetAsSigned(int64_t &value) const
{
switch (m_data_type)
{
case DataType::Unsigned:
-return (int64_t)m_data.m_unsigned;
-case DataType::Signed:
-return m_data.m_signed;
+ value = (int64_t)m_data.m_unsigned;
+ break;
case DataType::Double:
-return (int64_t)m_data.m_double;
+ value = (int64_t)m_data.m_double;
+ break;
+case DataType::Signed:
+ value = m_data.m_signed;
+ break;
+default:
+ return false;
}
+return true;
}
-double
-JSONNumber::GetAsDouble() const
+bool
+JSONNumber::GetAsDouble(double &value) const
{
switch (m_data_type)
{
case DataType::Unsigned:
-return (double)m_data.m_unsigned;
+ value = (double)m_data.m_unsigned;
+ break;
case DataType::Signed:
-return (double)m_data.m_signed;
+ value = (double)m_data.m_signed;
+ break;
case DataType::Double:
-return m_data.m_double;
+ value = m_data.m_double;
+ break;
+default:
+ return false;
}
+return true;
}
void
Index: include/lldb/Utility/JSON.h
===
--- include/lldb/Utility/JSON.h
+++ include/lldb/Utility/JSON.h
@@ -142,14 +142,14 @@
void
Write(Stream& s) override;
-uint64_t
-GetAsUnsigned() const;
+bool
+GetAsUnsigned(uint64_t &value) const;
-uint64_t
-GetAsSigned() const;
+bool
+GetAsSigned(int64_t &value) const;
-double
-GetAsDouble() const;
+bool
+GetAsDouble(double &value) const;
static bool classof(const JSONValue *V)
{
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D15357: Update code to silent some ARM/ARM64 specific compiler warnings
omjavaid created this revision.
omjavaid added a reviewer: tberghammer.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
Following changes remove conditions where an unsigned integer is being tested
to be greater or equal to zero.
These conditions will always remain true and will result in compiler warning
though most compilers will optimize this out.
http://reviews.llvm.org/D15357
Files:
source/Plugins/Instruction/ARM/EmulationStateARM.cpp
source/Utility/ARM64_DWARF_Registers.cpp
Index: source/Utility/ARM64_DWARF_Registers.cpp
===
--- source/Utility/ARM64_DWARF_Registers.cpp
+++ source/Utility/ARM64_DWARF_Registers.cpp
@@ -109,7 +109,7 @@
::memset (®_info, 0, sizeof(RegisterInfo));
::memset (reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
-if (reg_num >= x0 && reg_num <= pc)
+if (reg_num <= pc)
{
reg_info.byte_size = 8;
reg_info.format = eFormatHex;
Index: source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -77,7 +77,7 @@
bool
EmulationStateARM::StorePseudoRegisterValue (uint32_t reg_num, uint64_t value)
{
-if ((dwarf_r0 <= reg_num) && (reg_num <= dwarf_cpsr))
+if (reg_num <= dwarf_cpsr)
m_gpr[reg_num - dwarf_r0] = (uint32_t) value;
else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31))
{
@@ -105,7 +105,7 @@
uint64_t value = 0;
success = true;
-if ((dwarf_r0 <= reg_num) && (reg_num <= dwarf_cpsr))
+if (reg_num <= dwarf_cpsr)
value = m_gpr[reg_num - dwarf_r0];
else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31))
{
Index: source/Utility/ARM64_DWARF_Registers.cpp
===
--- source/Utility/ARM64_DWARF_Registers.cpp
+++ source/Utility/ARM64_DWARF_Registers.cpp
@@ -109,7 +109,7 @@
::memset (®_info, 0, sizeof(RegisterInfo));
::memset (reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
-if (reg_num >= x0 && reg_num <= pc)
+if (reg_num <= pc)
{
reg_info.byte_size = 8;
reg_info.format = eFormatHex;
Index: source/Plugins/Instruction/ARM/EmulationStateARM.cpp
===
--- source/Plugins/Instruction/ARM/EmulationStateARM.cpp
+++ source/Plugins/Instruction/ARM/EmulationStateARM.cpp
@@ -77,7 +77,7 @@
bool
EmulationStateARM::StorePseudoRegisterValue (uint32_t reg_num, uint64_t value)
{
-if ((dwarf_r0 <= reg_num) && (reg_num <= dwarf_cpsr))
+if (reg_num <= dwarf_cpsr)
m_gpr[reg_num - dwarf_r0] = (uint32_t) value;
else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31))
{
@@ -105,7 +105,7 @@
uint64_t value = 0;
success = true;
-if ((dwarf_r0 <= reg_num) && (reg_num <= dwarf_cpsr))
+if (reg_num <= dwarf_cpsr)
value = m_gpr[reg_num - dwarf_r0];
else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31))
{
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.
omjavaid requested changes to this revision.
omjavaid added a comment.
This revision now requires changes to proceed.
Please see inline comments.
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:149
@@ +148,3 @@
+{
+spec.SetTriple("armv7-pc-windows");
+specs.Append(ModuleSpec(file, spec));
compnerd wrote:
> This may be a bit tricky. `armv7-windows` is unsupported in LLVM/clang (and
> we silently rewrite that in the clang frontend), and you need
> `thumbv7-windows` (ARM NT). Though, it is possible that LLDB is unable to
> handle that distinction right now.
>
> That said, the `pc` vendor is silly, and `unknown` sounds better to me, but
> this shouldn't matter too much.
>
> Finally, the Windows environment defaults to `msvc` here, which has a slight
> issue that it can sometimes fail to generate an assembly listing (the code
> generation is correct, its just a serialization issue caused by not having
> invested sufficiently in generating MASM style assembly listing).
>
> The safest triple would be `thumbv7-unknown-windows-itanium`. But, if lldb
> is going to ensure that the code is handled as thumb, using `armv7` should be
> fine.
Can we recognize armv7 and armv8 independent of each other?
For 32 bit arm normally we use arm-linux-gnueabi independent of instruction
modes (thumb or arm) and architecture versions (v5, v5 or v7).
And we right now do not specifically distinguish between thumb and arm triples.
Better select arm-unknown-windows.
Please also handle arm v8 case where header is MachineArm64 and triple should
be like aarch64-unknown-windows.
http://reviews.llvm.org/D19604
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.
omjavaid added a comment. @sas Ideally it should be thumb-* if an environment is thumb only but lldb right now has some areas where we need to handle this case. Either you put arm-* and leave it for someone else to correct the problems or put thumb-* and fix the issues that come up after that. I think better to take the first option with a note to come back and fix this once other areas in the code are able to handle this. http://reviews.llvm.org/D19604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.
omjavaid added a comment. In case of ELF .arm attributes contains tags providing information on underlying CPU specification used. Thats only for the inferior being debugged but actually knowing which target we are running on, like for example if we want to figure out if we are running on a armv7 or armv8 hardware thats not decoded by LLDB right now. For user space applications dont want to know that either. So triple is just architecture-vendor-platform-* but for detailed information we have to populate arch specification structure after decoding the binary which is something we need to do in future. Right now we can distingusih between hard and soft float based on ABI information in elf. But cant really tell if hard float is legacy VFP or neon. http://reviews.llvm.org/D19604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid created this revision. omjavaid added reviewers: labath, tberghammer. omjavaid added a subscriber: lldb-commits. Herald added subscribers: danalbert, tberghammer, rengolin, aemerson. This patch makes sure that we are using correct target specific ar and objcopy executables. I have added logic to extract ar and objcopy from CC compiler name in case of arm and aarch64 targets. In case of android this is overidden by android specific logic implemented in makefile.rules. http://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -131,6 +131,7 @@ ARFLAGS := -static -o else AR := $(CROSS_COMPILE)ar + OBJCOPY := $(CROSS_COMPILE)objcopy # On non-Apple platforms, -arch becomes -m ARCHFLAG := -m @@ -161,10 +162,26 @@ override ARCH := $(subst powerpc64,64,$(ARCH)) endif ifeq "$(ARCH)" "aarch64" + OBJCOPY := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,objcopy,$(CC)), \ + $(if $(findstring clang,$(CC)), \ + $(subst clang,objcopy,$(CC + AR := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,ar,$(CC)), \ + $(if $(findstring clang,$(CC)), \ + $(subst clang,ar,$(CC override ARCH := override ARCHFLAG := endif ifeq "$(ARCH)" "arm" + OBJCOPY := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,objcopy,$(CC)), \ + $(if $(findstring clang,$(CC)), \ + $(subst clang,objcopy,$(CC + AR := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,ar,$(CC)), \ + $(if $(findstring clang,$(CC)), \ + $(subst clang,ar,$(CC override ARCH := override ARCHFLAG := endif @@ -260,8 +277,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -131,6 +131,7 @@ ARFLAGS := -static -o else AR := $(CROSS_COMPILE)ar + OBJCOPY := $(CROSS_COMPILE)objcopy # On non-Apple platforms, -arch becomes -m ARCHFLAG := -m @@ -161,10 +162,26 @@ override ARCH := $(subst powerpc64,64,$(ARCH)) endif ifeq "$(ARCH)" "aarch64" + OBJCOPY := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,objcopy,$(CC)), \ + $(if $(findstring clang,$(CC)), \ +$(subst clang,objcopy,$(CC + AR := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,ar,$(CC)), \ + $(if $(findstring clang,$(CC)), \ +$(subst clang,ar,$(CC override ARCH := override ARCHFLAG := endif ifeq "$(ARCH)" "arm" + OBJCOPY := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,objcopy,$(CC)), \ + $(if $(findstring clang,$(CC)), \ +$(subst clang,objcopy,$(CC + AR := $(if $(findstring gcc,$(CC)), \ + $(subst gcc,ar,$(CC)), \ + $(if $(findstring clang,$(CC)), \ +$(subst clang,ar,$(CC override ARCH := override ARCHFLAG := endif @@ -260,8 +277,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20368: Remove Platform usages from NativeProcessLinux
omjavaid accepted this revision. omjavaid added a comment. Seems to be causing no regressions on arm-linux. http://reviews.llvm.org/D20368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D21164: Improve watchpoint error reporting specially for arm/aarch64 targets
omjavaid created this revision.
omjavaid added reviewers: tberghammer, labath.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch takes a few corrective measures to make sure we display meaningful
reason against a watchpoint creation failure.
This is particularly important for the targets where we get to know dynamically
about the availability of hardware watchpoint resources.
We are trying to check whether we have sufficient hardware watchpoint slots
available before we go ahead and create a new watchpoint.
http://reviews.llvm.org/D21164
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Target/Target.cpp
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -712,9 +712,20 @@
if (rc.Success())
{
uint32_t num_current_watchpoints =
target->GetWatchpointList().GetSize();
-if (num_current_watchpoints >= num_supported_hardware_watchpoints)
-error.SetErrorStringWithFormat("number of supported hardware
watchpoints (%u) has been reached",
+if (num_supported_hardware_watchpoints == 0)
+{
+error.SetErrorStringWithFormat ("Target supports (%u) hardware
watchpoint slots.\n",
+num_supported_hardware_watchpoints);
+return false;
+}
+else if (num_current_watchpoints >= num_supported_hardware_watchpoints)
+{
+error.SetErrorStringWithFormat("All (%u) hardware watchpoint slots
already in use.\n",
num_supported_hardware_watchpoints);
+return false;
+}
+else
+return true;
}
return false;
}
@@ -750,6 +761,9 @@
error.SetErrorStringWithFormat ("invalid watchpoint type: %d", kind);
}
+if (!CheckIfWatchpointsExhausted (this, error))
+return wp_sp;
+
// Currently we only support one watchpoint per address, with total number
// of watchpoints limited by the hardware which the inferior is running on.
@@ -798,11 +812,9 @@
// Remove the said watchpoint from the list maintained by the target
instance.
m_watchpoint_list.Remove (wp_sp->GetID(), true);
// See if we could provide more helpful error message.
-if (!CheckIfWatchpointsExhausted(this, error))
-{
-if (!OptionGroupWatchpoint::IsWatchSizeSupported(size))
-error.SetErrorStringWithFormat("watch size of %" PRIu64 " is
not supported", (uint64_t)size);
-}
+if (!OptionGroupWatchpoint::IsWatchSizeSupported(size))
+error.SetErrorStringWithFormat("watch size of %" PRIu64 " is not
supported", (uint64_t)size);
+
wp_sp.reset();
}
else
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -544,7 +544,7 @@
error = ReadHardwareDebugInfo ();
if (error.Fail())
-return LLDB_INVALID_INDEX32;
+return 0;
return m_max_hwp_supported;
}
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -592,7 +592,7 @@
error = ReadHardwareDebugInfo ();
if (error.Fail())
-return LLDB_INVALID_INDEX32;
+return 0;
return m_max_hwp_supported;
}
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -712,9 +712,20 @@
if (rc.Success())
{
uint32_t num_current_watchpoints = target->GetWatchpointList().GetSize();
-if (num_current_watchpoints >= num_supported_hardware_watchpoints)
-error.SetErrorStringWithFormat("number of supported hardware watchpoints (%u) has been reached",
+if (num_supported_hardware_watchpoints == 0)
+{
+error.SetErrorStringWithFormat ("Target supports (%u) hardware watchpoint slots.\n",
+num_supported_hardware_watchpoints);
+return false;
+}
+else if (num_current_watchpoints >= num_supported_hardware_watchpoints)
+{
+error.SetErrorStringWithFormat("All (%u) hardware watchpoint slots already in use.\n",
num_supported_hardware_watchpoints);
+return false;
+}
+else
+return true;
}
return false;
}
@@ -750,6 +761,9 @
[Lldb-commits] [PATCH] D21280: Allow installing watchpoints at less than 8-byte alligned addresses for AArch64 targets
omjavaid created this revision.
omjavaid added reviewers: labath, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: danalbert, tberghammer, rengolin, aemerson.
This patch adds logic to make sure we can install watchpoints at 1,2 and 4 byte
alligned addresses.
ptrace interface allows watchpoints at 8-byte alligned addresses. Therefor for
all lower allignment levels we have to watch full 8-bytes.
We will ignore all irrelevant watchpoint trigger exceptions and will continue
the target after stepping over the watchpoint instruction.
In worst case while watching a 8-byte array like byteArr[8] we may have to
ignore 7 false watchpoint hits if we install watchpoint at the last byte in the
array.
However overall advantage of this solution overwhelms this disadvantage. We now
have all watchpoint tests passing on AArch64 Linux (HiKey board) and Arm64
Android (Nexus9).
http://reviews.llvm.org/D21280
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2059,7 +2059,8 @@
{
WatchpointSP wp_sp;
ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
-if (core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last)
+if ((core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last) ||
+(core >= ArchSpec::eCore_arm_arm64 && core <= ArchSpec::eCore_arm_aarch64))
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
if (!wp_sp)
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -74,6 +74,9 @@
GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
lldb::addr_t
+GetWatchpointHitAddress (uint32_t wp_index) override;
+
+lldb::addr_t
GetWatchpointAddress (uint32_t wp_index) override;
uint32_t
@@ -161,6 +164,8 @@
struct DREG
{
lldb::addr_t address; // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
uint32_t control; // Breakpoint/watchpoint control value.
uint32_t refcount; // Serves as enable/disable and refernce counter.
};
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -566,6 +566,7 @@
return LLDB_INVALID_INDEX32;
uint32_t control_value = 0, wp_index = 0;
+lldb::addr_t real_addr = addr;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match AArch64 write-read bit configuration.
@@ -588,9 +589,23 @@
return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
-// TODO: Add support for watching un-aligned addresses
+// Below is a hack to recalculate address and size in order to
+// make sure we can watch non 8-byte alligned addresses as well.
if (addr & 0x07)
-return LLDB_INVALID_INDEX32;
+{
+uint8_t watch_mask = (addr & 0x07) + size;
+
+if (watch_mask > 0x08)
+return LLDB_INVALID_INDEX32;
+else if (watch_mask <= 0x02)
+size = 2;
+else if (watch_mask <= 0x04)
+size = 4;
+else
+size = 8;
+
+addr = addr & (~0x07);
+}
// Setup control value
control_value = watch_flags << 3;
@@ -620,6 +635,7 @@
if ((m_hwp_regs[wp_index].control & 1) == 0)
{
// Update watchpoint in local cache
+m_hwp_regs[wp_index].real_addr = real_addr;
m_hwp_regs[wp_index].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
@@ -801,6 +817,7 @@
if (m_hwp_regs[wp_index].refcount >= 1 &
Re: [Lldb-commits] [PATCH] D21280: Allow installing watchpoints at less than 8-byte alligned addresses for AArch64 targets
omjavaid added a comment. In http://reviews.llvm.org/D21280#457196, @labath wrote: > The overall change looks good, but please also add a test which specifically > tests for watchpoints at unaligned addresses. Last time I checked, we all > watchpoint tests were passing (at least on android) even without this patch, > so it looks like our tests coverage is not sufficient here. Patch causes no regressions and following tests fail without this patch on Nexus 9 and arm-linux hikey board: No test currently fully tests the functionality supported by this patch. So I ll add a new test that tests different watchpoint sizes. UNEXPECTED SUCCESS: test_watchpoint_command_can_disable_a_watchpoint_dwarf (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py) UNEXPECTED SUCCESS: test_watchpoint_command_can_disable_a_watchpoint_dwo (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py) UNEXPECTED SUCCESS: test_watchpoint_command_dwarf (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py) UNEXPECTED SUCCESS: test_watchpoint_command_dwarf (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py) UNEXPECTED SUCCESS: test_watchpoint_command_dwo (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py) UNEXPECTED SUCCESS: test_watchpoint_command_dwo (functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py) UNEXPECTED SUCCESS: test_watchpoint_cond_api_dwarf (python_api/watchpoint/condition/TestWatchpointConditionAPI.py) UNEXPECTED SUCCESS: test_watchpoint_cond_api_dwo (python_api/watchpoint/condition/TestWatchpointConditionAPI.py) UNEXPECTED SUCCESS: test_watchpoint_cond_dwarf (functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py) UNEXPECTED SUCCESS: test_watchpoint_cond_dwo (functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py) UNEXPECTED SUCCESS: test_with_python_api_dwarf (functionalities/watchpoint/watchpoint_events/TestWatchpointEvents.py) UNEXPECTED SUCCESS: test_with_python_api_dwo (functionalities/watchpoint/watchpoint_events/TestWatchpointEvents.py) In http://reviews.llvm.org/D21280#456670, @clayborg wrote: > Does this patch handle being able to share an 8 byte watchpoint between two > watchpoints? Lets say you have an 8 byte array named "a" and watch to watch > a[0] and a[3] and a[7]. You should be able to allow the watchpoints to share > the 1 watchpoint. This patch is fine as is, just something to think about for > a future patch. This functionality may not work with current patch and may require more work. I will follow up with another patch after adding support for this kind of scenario. http://reviews.llvm.org/D21280 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D21280: Allow installing watchpoints at less than 8-byte alligned addresses for AArch64 targets
omjavaid updated this revision to Diff 60942.
omjavaid added a comment.
Herald added a subscriber: srhines.
I have added a test cases that tests all possibilities supported by current
configuration.
Tests pass on Nexus 9 and aarch64-linux-gnu (hikey board).
LGTM?
http://reviews.llvm.org/D21280
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2059,7 +2059,8 @@
{
WatchpointSP wp_sp;
ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
-if (core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last)
+if ((core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last) ||
+(core >= ArchSpec::eCore_arm_arm64 && core <= ArchSpec::eCore_arm_aarch64))
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
if (!wp_sp)
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -74,6 +74,9 @@
GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
lldb::addr_t
+GetWatchpointHitAddress (uint32_t wp_index) override;
+
+lldb::addr_t
GetWatchpointAddress (uint32_t wp_index) override;
uint32_t
@@ -161,6 +164,8 @@
struct DREG
{
lldb::addr_t address; // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
uint32_t control; // Breakpoint/watchpoint control value.
uint32_t refcount; // Serves as enable/disable and refernce counter.
};
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -566,6 +566,7 @@
return LLDB_INVALID_INDEX32;
uint32_t control_value = 0, wp_index = 0;
+lldb::addr_t real_addr = addr;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match AArch64 write-read bit configuration.
@@ -588,9 +589,23 @@
return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
-// TODO: Add support for watching un-aligned addresses
+// Below is a hack to recalculate address and size in order to
+// make sure we can watch non 8-byte alligned addresses as well.
if (addr & 0x07)
-return LLDB_INVALID_INDEX32;
+{
+uint8_t watch_mask = (addr & 0x07) + size;
+
+if (watch_mask > 0x08)
+return LLDB_INVALID_INDEX32;
+else if (watch_mask <= 0x02)
+size = 2;
+else if (watch_mask <= 0x04)
+size = 4;
+else
+size = 8;
+
+addr = addr & (~0x07);
+}
// Setup control value
control_value = watch_flags << 3;
@@ -620,6 +635,7 @@
if ((m_hwp_regs[wp_index].control & 1) == 0)
{
// Update watchpoint in local cache
+m_hwp_regs[wp_index].real_addr = real_addr;
m_hwp_regs[wp_index].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
@@ -801,6 +817,7 @@
if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index)
&& trap_addr >= watch_addr && trap_addr < watch_addr + watch_size)
{
+m_hwp_regs[wp_index].hit_addr = trap_addr;
return Error();
}
}
@@ -821,7 +838,24 @@
return LLDB_INVALID_ADDRESS;
if (WatchpointIsEnabled(wp_index))
-return m_hwp_regs[wp_index].address
Re: [Lldb-commits] [PATCH] D21280: Allow installing watchpoints at less than 8-byte alligned addresses for AArch64 targets
omjavaid marked 3 inline comments as done.
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:104
@@ +103,3 @@
+self.expect("watchpoint list -v",
+substrs = ['hit_count = 2'])
+
labath wrote:
> I guess the intention here is to test both read and write watchpoints. If so,
> could you state that somewhere (in a comment at least, if it's possible to
> somehow verify the watchpoint type in code, even better).
We cant possibly figure out the watchpoint type read or write based on output
string emitted when watchpoint is hit.
I have added a comment here which tells the user about why hit count should be
updated here.
http://reviews.llvm.org/D21280
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D21280: Allow installing watchpoints at less than 8-byte alligned addresses for AArch64 targets
This revision was automatically updated to reflect the committed changes.
Closed by commit rL272916: Allow installing watchpoints at less than 8-byte
alligned addresses for… (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D21280?vs=60942&id=60987#toc
Repository:
rL LLVM
http://reviews.llvm.org/D21280
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/main.c
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2059,7 +2059,8 @@
{
WatchpointSP wp_sp;
ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
-if (core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last)
+if ((core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last) ||
+(core >= ArchSpec::eCore_arm_arm64 && core <= ArchSpec::eCore_arm_aarch64))
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
if (!wp_sp)
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -74,6 +74,9 @@
GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
lldb::addr_t
+GetWatchpointHitAddress (uint32_t wp_index) override;
+
+lldb::addr_t
GetWatchpointAddress (uint32_t wp_index) override;
uint32_t
@@ -161,6 +164,8 @@
struct DREG
{
lldb::addr_t address; // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
uint32_t control; // Breakpoint/watchpoint control value.
uint32_t refcount; // Serves as enable/disable and refernce counter.
};
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -566,6 +566,7 @@
return LLDB_INVALID_INDEX32;
uint32_t control_value = 0, wp_index = 0;
+lldb::addr_t real_addr = addr;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match AArch64 write-read bit configuration.
@@ -588,9 +589,23 @@
return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
-// TODO: Add support for watching un-aligned addresses
+// Below is a hack to recalculate address and size in order to
+// make sure we can watch non 8-byte alligned addresses as well.
if (addr & 0x07)
-return LLDB_INVALID_INDEX32;
+{
+uint8_t watch_mask = (addr & 0x07) + size;
+
+if (watch_mask > 0x08)
+return LLDB_INVALID_INDEX32;
+else if (watch_mask <= 0x02)
+size = 2;
+else if (watch_mask <= 0x04)
+size = 4;
+else
+size = 8;
+
+addr = addr & (~0x07);
+}
// Setup control value
control_value = watch_flags << 3;
@@ -620,6 +635,7 @@
if ((m_hwp_regs[wp_index].control & 1) == 0)
{
// Update watchpoint in local cache
+m_hwp_regs[wp_index].real_addr = real_addr;
m_hwp_regs[wp_index].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
@@ -801,6 +817,7 @@
if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index)
&& trap_addr >= watch_addr && trap_addr < watch_addr + watch_size)
{
+m_hwp_regs[wp_index].hit_addr = trap
Re: [Lldb-commits] [PATCH] D21164: Improve watchpoint error reporting specially for arm/aarch64 targets
omjavaid added a comment. @clayborg Any comments about this change? Thanks! http://reviews.llvm.org/D21164 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D21516: Allow unaligned byte/word selection watchpoints for arm- linux/android targets.
omjavaid created this revision.
omjavaid added reviewers: labath, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: danalbert, tberghammer, rengolin, aemerson.
We recently committed a patch for AArch64 targets that allows us to watch any
byte address individually in a 8 byte aligned region.
This patch implements the same for arm targets with region being watched is a 4
byte region.
We cannot watch 8 consecutive bytes using a single watchpoint resource on Arm
targets so maximum we can do is 4 bytes.
As a results TestWatchpointSize tests pass on arm targets which were previously
failing.
http://reviews.llvm.org/D21516
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2060,7 +2060,7 @@
WatchpointSP wp_sp;
ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
if ((core >= ArchSpec::kCore_mips_first && core <= ArchSpec::kCore_mips_last) ||
-(core >= ArchSpec::eCore_arm_arm64 && core <= ArchSpec::eCore_arm_aarch64))
+(core >= ArchSpec::eCore_arm_generic && core <= ArchSpec::eCore_arm_aarch64))
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
if (!wp_sp)
wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
@@ -74,6 +74,9 @@
GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
lldb::addr_t
+GetWatchpointHitAddress (uint32_t wp_index) override;
+
+lldb::addr_t
GetWatchpointAddress (uint32_t wp_index) override;
uint32_t
@@ -162,6 +165,8 @@
struct DREG
{
lldb::addr_t address; // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
uint32_t control; // Breakpoint/watchpoint control value.
uint32_t refcount; // Serves as enable/disable and refernce counter.
};
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -614,6 +614,7 @@
return LLDB_INVALID_INDEX32;
uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+lldb::addr_t real_addr = addr;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match Arm write-read bit configuration.
@@ -637,7 +638,24 @@
if (size == 0 || size > 4)
return LLDB_INVALID_INDEX32;
-// We can only watch up to four bytes that follow a 4 byte aligned address
+// Check 4-byte alignment for hardware watchpoint target address.
+// Below is a hack to recalculate address and size in order to
+// make sure we can watch non 4-byte alligned addresses as well.
+if (addr & 0x03)
+{
+uint8_t watch_mask = (addr & 0x03) + size;
+
+if (watch_mask > 0x04)
+return LLDB_INVALID_INDEX32;
+else if (watch_mask <= 0x02)
+size = 2;
+else if (watch_mask <= 0x04)
+size = 4;
+
+addr = addr & (~0x03);
+}
+
+ // We can only watch up to four bytes that follow a 4 byte aligned address
// per watchpoint register pair, so make sure we can properly encode this.
addr_word_offset = addr % 4;
byte_mask = ((1u << size) - 1u) << addr_word_offset;
@@ -682,6 +700,7 @@
if ((m_hwp_regs[wp_index].control & 1) == 0)
{
// Update watchpoint in local cache
+m_hwp_regs[wp_index].real_addr = real_addr;
m_hwp_regs[wp_index].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
@@ -864,6 +883,7 @@
if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index)
&& trap_addr >= watch_addr && trap_addr < watch_addr + watch_size)
{
+
Re: [Lldb-commits] [PATCH] D21516: Allow unaligned byte/word selection watchpoints for arm- linux/android targets.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL273863: Allow unaligned byte/word selection watchpoints for
arm- linux/android targets. (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D21516?vs=61234&id=61954#toc
Repository:
rL LLVM
http://reviews.llvm.org/D21516
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
@@ -74,6 +74,9 @@
GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
lldb::addr_t
+GetWatchpointHitAddress (uint32_t wp_index) override;
+
+lldb::addr_t
GetWatchpointAddress (uint32_t wp_index) override;
uint32_t
@@ -162,6 +165,8 @@
struct DREG
{
lldb::addr_t address; // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
uint32_t control; // Breakpoint/watchpoint control value.
uint32_t refcount; // Serves as enable/disable and refernce counter.
};
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -614,6 +614,7 @@
return LLDB_INVALID_INDEX32;
uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+lldb::addr_t real_addr = addr;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match Arm write-read bit configuration.
@@ -637,7 +638,24 @@
if (size == 0 || size > 4)
return LLDB_INVALID_INDEX32;
-// We can only watch up to four bytes that follow a 4 byte aligned address
+// Check 4-byte alignment for hardware watchpoint target address.
+// Below is a hack to recalculate address and size in order to
+// make sure we can watch non 4-byte alligned addresses as well.
+if (addr & 0x03)
+{
+uint8_t watch_mask = (addr & 0x03) + size;
+
+if (watch_mask > 0x04)
+return LLDB_INVALID_INDEX32;
+else if (watch_mask <= 0x02)
+size = 2;
+else if (watch_mask <= 0x04)
+size = 4;
+
+addr = addr & (~0x03);
+}
+
+ // We can only watch up to four bytes that follow a 4 byte aligned address
// per watchpoint register pair, so make sure we can properly encode this.
addr_word_offset = addr % 4;
byte_mask = ((1u << size) - 1u) << addr_word_offset;
@@ -682,6 +700,7 @@
if ((m_hwp_regs[wp_index].control & 1) == 0)
{
// Update watchpoint in local cache
+m_hwp_regs[wp_index].real_addr = real_addr;
m_hwp_regs[wp_index].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
@@ -864,6 +883,7 @@
if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index)
&& trap_addr >= watch_addr && trap_addr < watch_addr + watch_size)
{
+m_hwp_regs[wp_index].hit_addr = trap_addr;
return Error();
}
}
@@ -884,7 +904,24 @@
return LLDB_INVALID_ADDRESS;
if (WatchpointIsEnabled(wp_index))
-return m_hwp_regs[wp_index].address;
+return m_hwp_regs[wp_index].real_addr;
+else
+return LLDB_INVALID_ADDRESS;
+}
+
+lldb::addr_t
+NativeRegisterContextLinux_arm::GetWatchpointHitAddress (uint32_t wp_index)
+{
+Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_WATCHPOINTS));
+
+if (log)
+log->Printf ("NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
+
+if (wp_index >= m_max_hwp_supported)
+return LLDB_INVALID_ADDRESS;
+
+if (WatchpointIsEnabled(wp_index))
+return m_hwp_regs[wp_index].hit_addr;
else
return LLDB_INVALID_ADDRESS;
}
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2060,7 +2060,7 @@
WatchpointSP wp_sp;
ArchSpec::
Re: [Lldb-commits] [PATCH] D21164: Improve watchpoint error reporting specially for arm/aarch64 targets
omjavaid added inline comments.
Comment at: source/Target/Target.cpp:714
@@ -713,2 +713,3 @@
{
uint32_t num_current_watchpoints =
target->GetWatchpointList().GetSize();
+if (num_supported_hardware_watchpoints == 0)
clayborg wrote:
> This logic isn't necessarily correct. If we have the ability to watch N bytes
> at a time with a single hardware watchpoint, we might have 1 hardware
> watchpoint that is able to watch multiple things. So for code like:
>
> ```
> char buffer[8] = ...;
> ```
>
> then we watch "buffer[1]" and "buffer[7]", we could actually have 2
> watchpoints but only use 1 hardware watchpoint. We really should be allowing
> each process plug-in to try and set the watchpoint and return the correct
> error instead of generically trying to catch _anything_ at the target level.
> So it seems like this code should be removed and moved into RegisterContext
> and allow each register context plug-in to respond correctly as only the it
> will know what can be done.
>
I am seeing this a little different. GetWatchpointSupportInfo has to provide
information on number of watchpoint resources a target supports.
We should only fail a watchpoint on the basis of GetWatchpointSupportInfo if it
returns zero that is for a particular hardware we have no watchpoint support.
For the case where we want to utilize same hardware resource to watch multiple
watchpoints, we should try to manage that in register context where we can keep
track of watchpoints previously installed without getting the target backend
involved. First we can attempt to intall a watchpoint using an existing
resource and if that fails we can use a new hardware resource. Return failure
if watchpoint resources are used up. This way you can put multiple watchpoint
on same address without having to use up hardware watchpoint resources. I will
post this solution of arm and aarch64 soon.
Comment at: source/Target/Target.cpp:721
@@ +720,3 @@
+}
+else if (num_current_watchpoints >= num_supported_hardware_watchpoints)
+{
I am going to remove this case from final commit as we may have more
watchpoints utilizing one or more hardware watchpoint resources.
http://reviews.llvm.org/D21164
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D21164: Improve watchpoint error reporting specially for arm/aarch64 targets
This revision was automatically updated to reflect the committed changes.
Closed by commit rL273869: Improve watchpoint error reporting specially for
arm/aarch64 targets (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D21164?vs=60127&id=61959#toc
Repository:
rL LLVM
http://reviews.llvm.org/D21164
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/trunk/source/Target/Target.cpp
Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -709,14 +709,13 @@
{
uint32_t num_supported_hardware_watchpoints;
Error rc =
target->GetProcessSP()->GetWatchpointSupportInfo(num_supported_hardware_watchpoints);
-if (rc.Success())
+if (num_supported_hardware_watchpoints == 0)
{
-uint32_t num_current_watchpoints =
target->GetWatchpointList().GetSize();
-if (num_current_watchpoints >= num_supported_hardware_watchpoints)
-error.SetErrorStringWithFormat("number of supported hardware
watchpoints (%u) has been reached",
- num_supported_hardware_watchpoints);
+error.SetErrorStringWithFormat ("Target supports (%u) hardware
watchpoint slots.\n",
+num_supported_hardware_watchpoints);
+return false;
}
-return false;
+return true;
}
// See also Watchpoint::SetWatchpointType(uint32_t type) and
@@ -750,6 +749,9 @@
error.SetErrorStringWithFormat ("invalid watchpoint type: %d", kind);
}
+if (!CheckIfWatchpointsExhausted (this, error))
+return wp_sp;
+
// Currently we only support one watchpoint per address, with total number
// of watchpoints limited by the hardware which the inferior is running on.
@@ -798,11 +800,9 @@
// Remove the said watchpoint from the list maintained by the target
instance.
m_watchpoint_list.Remove (wp_sp->GetID(), true);
// See if we could provide more helpful error message.
-if (!CheckIfWatchpointsExhausted(this, error))
-{
-if (!OptionGroupWatchpoint::IsWatchSizeSupported(size))
-error.SetErrorStringWithFormat("watch size of %" PRIu64 " is
not supported", (uint64_t)size);
-}
+if (!OptionGroupWatchpoint::IsWatchSizeSupported(size))
+error.SetErrorStringWithFormat("watch size of %" PRIu64 " is not
supported", (uint64_t)size);
+
wp_sp.reset();
}
else
Index:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -544,7 +544,7 @@
error = ReadHardwareDebugInfo ();
if (error.Fail())
-return LLDB_INVALID_INDEX32;
+return 0;
return m_max_hwp_supported;
}
Index:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -592,7 +592,7 @@
error = ReadHardwareDebugInfo ();
if (error.Fail())
-return LLDB_INVALID_INDEX32;
+return 0;
return m_max_hwp_supported;
}
Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -709,14 +709,13 @@
{
uint32_t num_supported_hardware_watchpoints;
Error rc = target->GetProcessSP()->GetWatchpointSupportInfo(num_supported_hardware_watchpoints);
-if (rc.Success())
+if (num_supported_hardware_watchpoints == 0)
{
-uint32_t num_current_watchpoints = target->GetWatchpointList().GetSize();
-if (num_current_watchpoints >= num_supported_hardware_watchpoints)
-error.SetErrorStringWithFormat("number of supported hardware watchpoints (%u) has been reached",
- num_supported_hardware_watchpoints);
+error.SetErrorStringWithFormat ("Target supports (%u) hardware watchpoint slots.\n",
+num_supported_hardware_watchpoints);
+return false;
}
-return false;
+return true;
}
// See also Watchpoint::SetWatchpointType(uint32_t type) and
@@ -750,6 +749,9 @@
error.SetErrorStringWithFormat ("invalid watchpoint type: %d", kind);
}
+if (!CheckIfWatchpointsExhausted (this, error))
+return wp_sp;
+
// Currently we only support one watchpoint per address, with
[Lldb-commits] [PATCH] D22771: Fix LLDBConfig.cmake to enable python enabled build for all 64 bit lldb targets
omjavaid created this revision.
omjavaid added reviewers: labath, Eugene.Zelenko.
omjavaid added a subscriber: lldb-commits.
Herald added a subscriber: aemerson.
This patch allows correct selection of CMAKE_LIBRARY_ARCHITECTURE instead of
previously hard-coded value.
All cross builds for 64bit targets with python support enabled will fail in
absence of this patch
I am trying to get LLDB host build working on AArch64 native host running
ubuntu Xenial 64bit.
My build system is ubuntu 16.04 amd64.
I am able to build LLDB successfully on Ubuntu Trusty 14.04 and 16.04 after
applying this patch.
Please review if there are any possible build breakups due to this patch.
https://reviews.llvm.org/D22771
Files:
cmake/modules/LLDBConfig.cmake
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -167,12 +167,6 @@
endfunction(find_python_libs_windows)
if (NOT LLDB_DISABLE_PYTHON)
- if(UNIX)
-# This is necessary for crosscompile on Ubuntu 14.04 64bit. Need a proper
fix.
-if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- set(CMAKE_LIBRARY_ARCHITECTURE "x86_64-linux-gnu")
-endif()
- endif()
if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
find_python_libs_windows()
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -167,12 +167,6 @@
endfunction(find_python_libs_windows)
if (NOT LLDB_DISABLE_PYTHON)
- if(UNIX)
-# This is necessary for crosscompile on Ubuntu 14.04 64bit. Need a proper fix.
-if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- set(CMAKE_LIBRARY_ARCHITECTURE "x86_64-linux-gnu")
-endif()
- endif()
if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
find_python_libs_windows()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D22771: Fix LLDBConfig.cmake to enable python enabled build for all 64 bit lldb targets
omjavaid added a comment. I am going ahead and committing this patch. If it breaks any build please revert it. Repository: rL LLVM https://reviews.llvm.org/D22771 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D22771: Fix LLDBConfig.cmake to enable python enabled build for all 64 bit lldb targets
This revision was automatically updated to reflect the committed changes.
Closed by commit rL276814: Fix LLDBConfig.cmake to enable python enabled build
for all 64 bit lldb targets (authored by omjavaid).
Changed prior to commit:
https://reviews.llvm.org/D22771?vs=65397&id=65612#toc
Repository:
rL LLVM
https://reviews.llvm.org/D22771
Files:
lldb/trunk/cmake/modules/LLDBConfig.cmake
Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -167,12 +167,6 @@
endfunction(find_python_libs_windows)
if (NOT LLDB_DISABLE_PYTHON)
- if(UNIX)
-# This is necessary for crosscompile on Ubuntu 14.04 64bit. Need a proper
fix.
-if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- set(CMAKE_LIBRARY_ARCHITECTURE "x86_64-linux-gnu")
-endif()
- endif()
if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
find_python_libs_windows()
Index: lldb/trunk/cmake/modules/LLDBConfig.cmake
===
--- lldb/trunk/cmake/modules/LLDBConfig.cmake
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake
@@ -167,12 +167,6 @@
endfunction(find_python_libs_windows)
if (NOT LLDB_DISABLE_PYTHON)
- if(UNIX)
-# This is necessary for crosscompile on Ubuntu 14.04 64bit. Need a proper fix.
-if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- set(CMAKE_LIBRARY_ARCHITECTURE "x86_64-linux-gnu")
-endif()
- endif()
if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
find_python_libs_windows()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid updated this revision to Diff 66258. omjavaid added a comment. Herald added a subscriber: samparker. Sorry about the delay updating this. I lost track of this rev earlier. Have updated diff to use macro already present within Android specific block for all cases. Tested with no regressions on arm/aarch64 linux and android targets. This possibly should address similar issues arising where there is architecture difference between host and target. https://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,21 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +replace_with = $(if $(findstring clang,$(1)), \ +$(subst clang,$(2),$(1)), \ +$(if $(findstring gcc,$(1)), \ + $(subst gcc,$(2),$(1)), \ + $(subst cc,$(2),$(1 +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(CC),$(1)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) +endif +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,21 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +replace_with = $(if $(findstring clang,$(1)), \ +$(subst clang,$(2),$(1)), \ +$(if $(findstring gcc,$(1)), \ + $(subst gcc,$(2),$(1)), \ + $(subst cc,$(2),$(1 +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(CC),$(1)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) +endif +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
This revision was automatically updated to reflect the committed changes. Closed by commit rL277429: Correct makefile.rules to use toolchain specific AR and OBJCOPY (authored by omjavaid). Changed prior to commit: https://reviews.llvm.org/D20386?vs=66258&id=66438#toc Repository: rL LLVM https://reviews.llvm.org/D20386 Files: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -287,24 +285,25 @@ #-- # Android specific options #-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) + +ifdef PIE +LDFLAGS += -pie +endif + +replace_with = $(if $(findstring clang,$(1)), \ +$(subst clang,$(2),$(1)), \ +$(if $(findstring gcc,$(1)), \ + $(subst gcc,$(2),$(1)), \ + $(subst cc,$(2),$(1 +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(CC),$(1)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) endif +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -287,24 +285,25 @@ #-- # Android specific options #-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) + +ifdef PIE +LDFLAGS += -pie +endif + +replace_with = $(if $(findstring clang,$(1)), \ +$(subst clang,$(2),$(1)), \ +$(if $(findstring gcc,$(1)), \ + $(subst gcc,$(2),$(1)), \ + $(subst cc,$(2),$(1 +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(CC),$(1)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) endif +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- ___
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid removed rL LLVM as the repository for this revision. omjavaid updated this revision to Diff 67228. omjavaid added a comment. I have updated previous patch which handles compiler binaries which have version string appended at the end like gcc-4.9 or clang-3.5. Kindly give it a review and let me know if its breaking any buildbots. https://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,33 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(if $(findstring ar,$(1)), \ +$(if $(findstring gcc,$(3)), \ + $(subst gcc,gcc-ar,$(2)), \ + $(subst $(3),$(1),$(2))), \ +$(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2)) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,33 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(if $(findstring ar,$(1)), \ +$(if $(findstring gcc,$(3)), \ + $(subst gcc,gcc-ar,$(2)), \ +
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid reopened this revision. omjavaid added a comment. reopening this for new review. https://reviews.llvm.org/D20386 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid updated this revision to Diff 67229. omjavaid added a comment. Adding context. https://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,33 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(if $(findstring ar,$(1)), \ +$(if $(findstring gcc,$(3)), \ + $(subst gcc,gcc-ar,$(2)), \ + $(subst $(3),$(1),$(2))), \ +$(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2)) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) + #-- # C++ standard library options #-- Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,8 +265,6 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy - #-- # Windows specific options #-- @@ -291,20 +289,33 @@ ifdef PIE LDFLAGS += -pie endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) endif +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(if $(findstring ar,$(1)), \ +$(if $(findstring gcc,$(3)), \ + $(subst gcc,gcc-ar,$(2)), \ + $(subst $(3),$(1),$(2))), \ +$(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2)) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid added inline comments. Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:304 @@ +303,3 @@ + $(subst $(3),$(1),$(2)), \ + $(if $(findstring ar,$(1)), \ +$(if $(findstring gcc,$(3)), \ labath wrote: > I think like we should replace ar with gcc-ar always, and not just when we > don't have a version suffix present. When I have a gcc installed to a custom > prefix, I get `$prefix/gcc-ar`, but not `$prefix/ar`. (Previously we would > just use `ar` unconditionally, which was probably a bug.) > > Next, when you specify a clang compiler as `clang-3.5`, you will produce > `ar-3.5`, which almost certainly does not exist. I think that in case of > clang we should just strip the version suffix (produce `XXX-ar`) and hope for > the best (should work for all our current use cases). > > Finally, I don't think objcopy is ever versioned with gcc (correct me if I am > wrong), so I think that in case of objcopy we should strip the version suffix > unconditionally `XXX-objcopy`. > > So, to summarize, these are the transformations I think are wrong: > `foo/gcc` -> `foo/ar` (should be `foo/gcc-ar`) > `clang-3.5` -> `ar-3.5` (should be `ar`) > `gcc-4.8` -> `objcopy-4.8` (should be `objcopy`) > > Let me know if these are compatible with your requirements. If we can't find > a set of rules that work everywhere, we will have to abandon the magic (or > maybe just leave a simple one), and require the user to specify the paths > manually... I checked with gcc people in my team and here is what they have to say. omjavaid, gcc-ar is a version of ar with LTO support omjavaid, ar is just ar omjavaid, likewise with gcc-nm and gcc-ranlib omjavaid, ar comes from binutils, gcc-ar comes from gcc I agree with all other points you raised except for using gcc-ar in all cases. We should decide between ar (binutils) or gcc-ar (LTO support gcc packaged). Also present code doesnt generate obcopy-4.8 for me when i specify gcc-4.8. If it does for you let me know. https://reviews.llvm.org/D20386 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D23395: Make sure LldbGdbServerTestCase is built in arm mode to avoid failures due thumb instructions
omjavaid created this revision.
omjavaid added a reviewer: labath.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: samparker, rengolin, aemerson.
LldbGdbServerTestCase.test_software_breakpoint_set* are failing when breakpoint
target address is an thumb16 instruction.
Test right now doesnt handle thumb mode which means that it should only be
built in arm mode using -marm flag in gcc.
This patch makes sure that we are building this test in arm mode.
https://reviews.llvm.org/D23395
Files:
packages/Python/lldbsuite/test/tools/lldb-server/Makefile
packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
Index: packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -1233,15 +1233,23 @@
@debugserver_test
def test_software_breakpoint_set_and_remove_work_debugserver(self):
self.init_debugserver_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
@llgs_test
@expectedFlakeyLinux("llvm.org/pr25652")
def test_software_breakpoint_set_and_remove_work_llgs(self):
self.init_llgs_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
Index: packages/Python/lldbsuite/test/tools/lldb-server/Makefile
===
--- packages/Python/lldbsuite/test/tools/lldb-server/Makefile
+++ packages/Python/lldbsuite/test/tools/lldb-server/Makefile
@@ -1,6 +1,6 @@
LEVEL = ../../make
-CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
+override CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
ENABLE_THREADS := YES
CXX_SOURCES := main.cpp
MAKE_DSYM :=NO
Index: packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -1233,15 +1233,23 @@
@debugserver_test
def test_software_breakpoint_set_and_remove_work_debugserver(self):
self.init_debugserver_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
@llgs_test
@expectedFlakeyLinux("llvm.org/pr25652")
def test_software_breakpoint_set_and_remove_work_llgs(self):
self.init_llgs_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
Index: packages/Python/lldbsuite/test/tools/lldb-server/Makefile
===
--- packages/Python/lldbsuite/test/tools/lldb-server/Makefile
+++ packages/Python/lldbsuite/test/tools/lldb-server/Makefile
@@ -1,6 +1,6 @@
LEVEL = ../../make
-CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
+override CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
ENABLE_THREADS := YES
CXX_SOURCES := main.cpp
MAKE_DSYM :=NO
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid updated this revision to Diff 67669. omjavaid added a comment. Herald added a subscriber: srhines. Updated with suggestion incorporated. Here's what I am getting on different inputs: Testing make clean CC=gcc ar objcopy Testing make clean CC=clang ar objcopy Testing make clean CC=gcc-5 ar objcopy Testing make clean CC=clang-3.8 ar objcopy Testing make clean CC=arm-linux-gnueabihf-gcc ARCH=arm arm-linux-gnueabihf-ar arm-linux-gnueabihf-objcopy Testing make clean CC=arm-linux-gnueabihf-gcc-5 ARCH=arm arm-linux-gnueabihf-ar arm-linux-gnueabihf-objcopy Testing make clean CC=/home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-gcc ARCH=arm /home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-ar /home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-objcopy Testing make clean CC=/home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-gcc-4.9 ARCH=arm /home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-ar /home/omair/work/toolchains/arm32-android-toolchain/bin/arm-linux-androideabi-objcopy https://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,32 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY = $(call replace_cc_with,objcopy) +AR = $(call replace_cc_with,ar) +endif + +ifdef PIE +LDFLAGS += -pie +endif #-- # Windows specific options @@ -285,27 +310,6 @@ endif #-- -# Android specific options -#-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) -endif - -#-- # C++ standard library options #-- ifeq (1,$(USE_LIBSTDCPP)) Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,32 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +
Re: [Lldb-commits] [PATCH] D23395: Make sure LldbGdbServerTestCase is built in arm mode to avoid failures due thumb instructions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278326: Make sure LldbGdbServerTestCase is built in arm mode
to avoid failures due… (authored by omjavaid).
Changed prior to commit:
https://reviews.llvm.org/D23395?vs=67663&id=67670#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23395
Files:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
@@ -1,6 +1,6 @@
LEVEL = ../../make
-CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
+override CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
ENABLE_THREADS := YES
CXX_SOURCES := main.cpp
MAKE_DSYM :=NO
Index:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -1233,15 +1233,23 @@
@debugserver_test
def test_software_breakpoint_set_and_remove_work_debugserver(self):
self.init_debugserver_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
@llgs_test
@expectedFlakeyLinux("llvm.org/pr25652")
def test_software_breakpoint_set_and_remove_work_llgs(self):
self.init_llgs_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/Makefile
@@ -1,6 +1,6 @@
LEVEL = ../../make
-CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
+override CFLAGS_EXTRAS += -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
ENABLE_THREADS := YES
CXX_SOURCES := main.cpp
MAKE_DSYM :=NO
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -1233,15 +1233,23 @@
@debugserver_test
def test_software_breakpoint_set_and_remove_work_debugserver(self):
self.init_debugserver_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
@llgs_test
@expectedFlakeyLinux("llvm.org/pr25652")
def test_software_breakpoint_set_and_remove_work_llgs(self):
self.init_llgs_test()
-self.build()
+if self.getArchitecture() == "arm":
+# TODO: Handle case when setting breakpoint in thumb code
+self.build(dictionary={'CFLAGS_EXTRAS': '-marm'})
+else:
+self.build()
self.set_inferior_startup_launch()
self.software_breakpoint_set_and_remove_work()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid added a comment. I like your suggestions and I dont think we have any other way but to use preset environment variable to detect what kind of TOOLCHAIN we want to use apart from some standard cases where we have the ability to detect through proposed hack logic. I agree we should have the ability to override this hack as well just for the fact that it allows us to use various other versions of AR, OBJCOPY etc. https://reviews.llvm.org/D20386 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
omjavaid updated this revision to Diff 68339. omjavaid added a comment. so I have used ?= now with following new changes OBJCOPY ?= $(call replace_cc_with,objcopy) ARCHIVER ?= $(call replace_cc_with,ar) override AR = $(ARCHIVER) https://reviews.llvm.org/D20386 Files: packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,33 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY ?= $(call replace_cc_with,objcopy) +ARCHIVER ?= $(call replace_cc_with,ar) +override AR = $(ARCHIVER) +endif + +ifdef PIE +LDFLAGS += -pie +endif #-- # Windows specific options @@ -285,27 +311,6 @@ endif #-- -# Android specific options -#-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) -endif - -#-- # C++ standard library options #-- ifeq (1,$(USE_LIBSTDCPP)) Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,33 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY ?= $(call replace_cc_with,objcopy) +ARCHIVER ?= $(call replace_cc_with,ar) +override AR = $(ARCHIVER) +endif + +ifdef PIE +LDFLAGS += -pie +endif #-- # Windows specific options @@ -285,27 +311,6 @@ endif #-- -# Android specific options -#-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -
Re: [Lldb-commits] [PATCH] D20386: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY
This revision was automatically updated to reflect the committed changes. Closed by commit rL278947: Correct makefile.rules to use arm/aarch64 target specific AR and OBJCOPY (authored by omjavaid). Changed prior to commit: https://reviews.llvm.org/D20386?vs=68339&id=68374#toc Repository: rL LLVM https://reviews.llvm.org/D20386 Files: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,33 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY ?= $(call replace_cc_with,objcopy) +ARCHIVER ?= $(call replace_cc_with,ar) +override AR = $(ARCHIVER) +endif + +ifdef PIE +LDFLAGS += -pie +endif #-- # Windows specific options @@ -285,27 +311,6 @@ endif #-- -# Android specific options -#-- -ifeq "$(OS)" "Android" -ifdef PIE -LDFLAGS += -pie -endif -replace_with = $(if $(findstring clang,$(1)), \ -$(subst clang,$(2),$(1)), \ -$(if $(findstring gcc,$(1)), \ - $(subst gcc,$(2),$(1)), \ - $(subst cc,$(2),$(1 -ifeq "$(notdir $(CC))" "$(CC)" -replace_cc_with = $(call replace_with,$(CC),$(1)) -else -replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(notdir $(CC)),$(1))) -endif -OBJCOPY = $(call replace_cc_with,objcopy) -AR = $(call replace_cc_with,ar) -endif - -#-- # C++ standard library options #-- ifeq (1,$(USE_LIBSTDCPP)) Index: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules === --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules @@ -265,7 +265,33 @@ $(subst cc,c++,$(1)) cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1))) -OBJCOPY := $(CROSS_COMPILE)objcopy +ifneq "$(OS)" "Darwin" +CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \ + $(findstring clang,$(CC)), \ + $(if $(findstring gcc,$(CC)), \ + $(findstring gcc,$(CC)), \ + cc))) + +CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC + +replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \ + $(subst $(3),$(1),$(2)), \ + $(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2) + +ifeq "$(notdir $(CC))" "$(CC)" +replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC)) +else +replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC))) +endif + +OBJCOPY ?= $(call replace_cc_with,objcopy) +ARCHIVER ?= $(call replace_cc_with,ar) +override AR = $(ARCHIVER) +endif + +ifdef PIE +LDFLAGS += -pie +endif #-- # Windows specific options @@ -285,27 +311,6 @@ endif #-- -# Android specific options -#-- -ifeq "$(OS)" "Android" -ifdef PIE -
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid added a comment.
comments inline.
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:43
@@ -42,2 +42,3 @@
"""Test to selectively watch different bytes in a 8-byte array."""
-self.run_watchpoint_size_test('byteArray', 8, '1')
+if self.getArchitecture() in ['arm']:
+self.run_watchpoint_size_test('byteArray', 8, '1', 1)
labath wrote:
> It's not clear to me why you need to modify the existing test for this
> change. You are adding functionality, so all existing tests should pass as-is
> (which will also validate that your change did not introduce regressions).
As we keep on adding these tests its increasing our overall testing time. I
just thought using the same test with some changes will cover the cases we want
to test.
Anyways we can write separate tests as well.
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:556
@@ +555,3 @@
+uint32_t current_size = GetWatchpointSize(wp_index);
+if ((current_size == 4) || (current_size == 2 && watch_mask <= 2) ||
+(current_size == 1 && watch_mask == 1))
labath wrote:
> The logic here is extremely convoluted. Doesn't this code basically boil down
> to:
> ```
> current_size = m_hwp_regs[wp_index].control & 1 ? GetWatchpointSize(wp_index)
> : 0;
> new_size = llvm::NextPowerOf2(std::max(current_size, watch_mask));
> // update the control value, write the debug registers...
> ```
>
> Also `watch_mask` should probably be renamed to `watch_size`, as it doesn't
> appear to be a mask.
Seems legit. I ll update this in next patch.
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:602
@@ -612,3 +601,3 @@
bool NativeRegisterContextLinux_arm::ClearHardwareWatchpoint(
uint32_t wp_index) {
labath wrote:
> This looks a bit worrying.
> What will happen after the following sequence of events:
> - client tells us to set a watchpoint at 0x1000
> - we set the watchpoint
> - client tells us to set a watchpoint at 0x1001
> - we extend the previous watchpoint to watch this address as well
> - client tells us to delete the watchpoint at 0x1000
> - ???
>
> Will we remain watching the address 0x1001? I don't see how will you be able
> to do that without maintaining a some info about the original watchpoints the
> client requested (and I have not seen that code). Please add a test for this.
I just realized that I missed a crucial change in my last patch that we need to
make all this work.
Let me get back with correction and desired test cases.
https://reviews.llvm.org/D24610
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid updated this revision to Diff 71633.
omjavaid added a comment.
Herald added subscribers: srhines, danalbert, tberghammer.
I have added a new test case that tests suggested scnario without changing any
previous test cases.
Also I have made sure we re validate all watchpoint installed on thread resume
to make sure we have the latest values assigned to hardware watchpoint
registers.
This passes on ARM (RaspberryPi3, Samsung Chromebook). I have not yet tested on
android.
This will fail on targets which dont support multiple watchpoint slots.
Also this should fail on AArch64 which I am currently working on.
https://reviews.llvm.org/D24610
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeThreadLinux.cpp
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -198,6 +198,9 @@
m_stop_info.reason = StopReason::eStopReasonNone;
m_stop_description.clear();
+ // Invalidate watchpoint index map to re-sync watchpoint registers.
+ m_watchpoint_index_map.clear();
+
// If watchpoints have been set, but none on this thread,
// then this is a new thread. So set all existing watchpoints.
if (m_watchpoint_index_map.empty()) {
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -410,15 +410,13 @@
if ((m_hbr_regs[bp_index].control & 1) == 0) {
m_hbr_regs[bp_index].address = addr;
m_hbr_regs[bp_index].control = control_value;
-m_hbr_regs[bp_index].refcount = 1;
// PTRACE call to set corresponding hardware breakpoint register.
error = WriteHardwareDebugRegs(eDREGTypeBREAK, bp_index);
if (error.Fail()) {
m_hbr_regs[bp_index].address = 0;
m_hbr_regs[bp_index].control &= ~1;
- m_hbr_regs[bp_index].refcount = 0;
return LLDB_INVALID_INDEX32;
}
@@ -508,8 +506,19 @@
if (error.Fail())
return LLDB_INVALID_INDEX32;
- uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+ uint32_t control_value = 0;
lldb::addr_t real_addr = addr;
+ uint32_t wp_index = LLDB_INVALID_INDEX32;
+
+ // Find out how many bytes we need to watch after 4-byte alignment boundary.
+ uint8_t watch_size = (addr & 0x03) + size;
+
+ // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary.
+ if (size == 0 || watch_size > 4)
+return LLDB_INVALID_INDEX32;
+
+ // Strip away last two bits of address for byte/half-word/word selection.
+ addr &= ~((lldb::addr_t)3);
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match Arm write-read bit configuration.
@@ -526,86 +535,58 @@
return LLDB_INVALID_INDEX32;
}
- // Can't watch zero bytes
- // Can't watch more than 4 bytes per WVR/WCR pair
-
- if (size == 0 || size > 4)
-return LLDB_INVALID_INDEX32;
-
- // Check 4-byte alignment for hardware watchpoint target address.
- // Below is a hack to recalculate address and size in order to
- // make sure we can watch non 4-byte alligned addresses as well.
- if (addr & 0x03) {
-uint8_t watch_mask = (addr & 0x03) + size;
-
-if (watch_mask > 0x04)
- return LLDB_INVALID_INDEX32;
-else if (watch_mask <= 0x02)
- size = 2;
-else if (watch_mask <= 0x04)
- size = 4;
-
-addr = addr & (~0x03);
+ // Iterate over stored watchpoints and find a free or duplicate wp_index
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+if ((m_hwp_regs[i].control & 1) == 0) {
+ wp_index = i; // Mark last free slot
+} else if (m_hwp_regs[i].address == addr) {
+ wp_index = i; // Mark duplicate index
+ break;// Stop searching here
+}
}
- // We can only watch up to four bytes that follow a 4 byte aligned address
- // per watchpoint register pair, so make sure we can properly encode this.
- addr_word_offset = addr % 4;
- byte_mask = ((1u << size) - 1u) << addr_word_offset;
-
- // Check if we need multiple watchpoint register
- if (byte_mask > 0xfu)
+ // No vaccant slot available and no duplicate slot found.
+ if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
+ uint8_t current_watch_size, new_watch_size;
+ // Calculate overall size width to be watched by current hardware watchpoint slot.
+ current_watch_
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid added a comment.
Answers to comments. I will upload a updated patch after corrections and
updates.
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c:23
@@ +22,3 @@
+{
+printf("About to write byteArray[%d] ...\n", i); // About to write
byteArray
+
labath wrote:
> zturner wrote:
> > What's up with all the double spaced source code? Is this intentional?
> Indeed. The spaces seem superfluous.
Agreed. Will be removed in next iteration.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:513-521
@@ -513,1 +512,11 @@
+
+ // Find out how many bytes we need to watch after 4-byte alignment boundary.
+ uint8_t watch_size = (addr & 0x03) + size;
+
+ // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary.
+ if (size == 0 || watch_size > 4)
+return LLDB_INVALID_INDEX32;
+
+ // Strip away last two bits of address for byte/half-word/word selection.
+ addr &= ~((lldb::addr_t)3);
labath wrote:
> zturner wrote:
> > This block of code is a bit confusing to me. Is this equivalent to:
> >
> > ```
> > lldb::addr_t start = llvm::alignDown(addr, 4);
> > lldb::addr_t end = addr + size;
> > if (start == end || (end-start)>4)
> > return LLDB_INVALID_INDEX32;
> > ```
> I am not sure this is much clearer, especially, as we will later need a
> separate varaible for `end-start` anyway.
>
> +1 for `llvm::alignDown` though.
There is significant performance difference when we choose addr = addr &
(~0x03); over llvm::alignDown
It will eventually effect responsiveness if we keep increasing code size like
this.
Even if we use -Os then alignDown is squeezed down to 8-9 instructions. I havnt
tried clang though.
Instructions needed for addr = addr & (~0x03):
4008bd: 48 8b 45 e8 mov-0x18(%rbp),%rax
4008c1: 48 83 e0 fc and$0xfffc,%rax
4008c5: 48 89 45 e8 mov%rax,-0x18(%rbp)
Call penalty for llvm::alignDown
400918: ba 00 00 00 00 mov$0x0,%edx
40091d: 48 89 cemov%rcx,%rsi
400920: 48 89 c7mov%rax,%rdi
400923: e8 ae 00 00 00 callq 4009d6 <_Z9alignDownmmm>
400928: 49 89 c4mov%rax,%r12
40092b: 48 8b 5d d8 mov-0x28(%rbp),%rbx
Disassembly for llvm::alignDown
004009d6 <_Z9alignDownmmm>:
4009d6: 55 push %rbp
4009d7: 48 89 e5mov%rsp,%rbp
4009da: 48 89 7d f8 mov%rdi,-0x8(%rbp)
4009de: 48 89 75 f0 mov%rsi,-0x10(%rbp)
4009e2: 48 89 55 e8 mov%rdx,-0x18(%rbp)
4009e6: 48 8b 45 e8 mov-0x18(%rbp),%rax
4009ea: ba 00 00 00 00 mov$0x0,%edx
4009ef: 48 f7 75 f0 divq -0x10(%rbp)
4009f3: 48 89 55 e8 mov%rdx,-0x18(%rbp)
4009f7: 48 8b 45 f8 mov-0x8(%rbp),%rax
4009fb: 48 2b 45 e8 sub-0x18(%rbp),%rax
4009ff: ba 00 00 00 00 mov$0x0,%edx
400a04: 48 f7 75 f0 divq -0x10(%rbp)
400a08: 48 0f af 45 f0 imul -0x10(%rbp),%rax
400a0d: 48 89 c2mov%rax,%rdx
400a10: 48 8b 45 e8 mov-0x18(%rbp),%rax
400a14: 48 01 d0add%rdx,%rax
400a17: 5d pop%rbp
400a18: c3 retq
400a19: 0f 1f 80 00 00 00 00nopl 0x0(%rax)
Number of instructions generated for alignDown with gcc -Os
400892: 48 8b 6c 24 08 mov0x8(%rsp),%rbp
400897: 48 8b 4c 24 10 mov0x10(%rsp),%rcx
40089c: 31 d2 xor%edx,%edx
40089e: be c2 0a 40 00 mov$0x400ac2,%esi
4008a3: bf a0 11 60 00 mov$0x6011a0,%edi
4008a8: 48 89 e8mov%rbp,%rax
4008ab: 48 f7 f1div%rcx
4008ae: 48 0f af c1 imul %rcx,%rax
4008b2: 48 89 c3mov%rax,%rbx
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:552
@@ -559,1 +551,3 @@
+ uint8_t current_watch_size, new_watch_size;
+ // Calculate overall size width to be watched by current hardware watchpoint
slot.
labath wrote:
> Looks much better. Any reason for not using `NextPowerOf2` ? Among other
> things, it is self-documenting, so you do not need the comment above that.
so llvm::NextPowerOf2 doesnt serve the intended behaviour.
llvm::NextPowerOf2 returns 2 for 1, 4 for 2 or 3, and 8 for 4.
We just want to make sure that our new_size is a power of 2 if its already not
which will only be the case if size turns out to be 3.
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid updated this revision to Diff 72589.
omjavaid added a comment.
This is a new version of what seems to me fully implementing functionality we
intend to have here.
On a second thought nuking ClearHardwareWatchpoint function seems to be the
wrong approach here. I spent some time taking different approaches and it turns
out that if we do not ClearHardwareWatchpoint when back-end asks us to remove
it then we wont be able to step over watchpoints. On ARM targets we have to
first clear and then reinstall watchpoints to step over the watchpoint
instruction.
On the other hand if we call
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint then that watchpoint
stands removed if call is just to delete watch on one of the bytes. And if we
follow up with creating a new watchpoint on a different word the slot being
used may appear vaccant which is actually inconsistent behavior.
So I have a new approach that does clear watchpoint registers if
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint is called but we still
track reference counts by re-introducing refcount that I removed in my last
patch. This will mean that a follow up create may fail just because there are
still references to disabled watchpoint and watchpoint slots are still not
vaccant. I have made changes to the test to reflect this behaviour.
Please comment if you have any reservation about this approach.
https://reviews.llvm.org/D24610
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeThreadLinux.cpp
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -198,8 +198,10 @@
m_stop_info.reason = StopReason::eStopReasonNone;
m_stop_description.clear();
- // If watchpoints have been set, but none on this thread,
- // then this is a new thread. So set all existing watchpoints.
+ // Invalidate watchpoint index map for a re-sync
+ m_watchpoint_index_map.clear();
+
+ // Re-sync all available watchpoints.
if (m_watchpoint_index_map.empty()) {
NativeProcessLinux &process = GetProcess();
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -508,8 +508,19 @@
if (error.Fail())
return LLDB_INVALID_INDEX32;
- uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+ uint32_t control_value = 0;
lldb::addr_t real_addr = addr;
+ uint32_t wp_index = LLDB_INVALID_INDEX32;
+
+ // Find out how many bytes we need to watch after 4-byte alignment boundary.
+ uint8_t watch_size = (addr & 0x03) + size;
+
+ // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary.
+ if (size == 0 || watch_size > 4)
+return LLDB_INVALID_INDEX32;
+
+ // Strip away last two bits of address for byte/half-word/word selection.
+ addr &= ~((lldb::addr_t)3);
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match Arm write-read bit configuration.
@@ -526,86 +537,63 @@
return LLDB_INVALID_INDEX32;
}
- // Can't watch zero bytes
- // Can't watch more than 4 bytes per WVR/WCR pair
+ // Iterate over stored watchpoints and find a free or duplicate wp_index
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+if ((m_hwp_regs[i].control & 1) == 0 && (m_hwp_regs[i].refcount <= 0)) {
+ wp_index = i; // Mark last free slot
+} else if (m_hwp_regs[i].address == addr) {
+ wp_index = i; // Mark duplicate index
+ break;// Stop searching here
+}
+ }
- if (size == 0 || size > 4)
+ // No vaccant slot available and no duplicate slot found.
+ if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Check 4-byte alignment for hardware watchpoint target address.
- // Below is a hack to recalculate address and size in order to
- // make sure we can watch non 4-byte alligned addresses as well.
- if (addr & 0x03) {
-uint8_t watch_mask = (addr & 0x03) + size;
-
-if (watch_mask > 0x04)
- return LLDB_INVALID_INDEX32;
-else if (watch_mask <= 0x02)
- size = 2;
-else if (watch_mask <= 0x04)
- size = 4;
-
-addr = addr & (~0x03);
+ uint8_t current_watch_size, new_watch_size;
+ // Calculate overall size width to be watched by current hardware watchpoint slot.
+ current_watch_size =
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid updated this revision to Diff 72723.
omjavaid added a comment.
Give this approach a rethink I dont see a lot of problems with this final
implementation unless it fails on other architectures.
We are already hacking our way to have these byte selection watchpoints working
in existing code. New code seems to be improving the hack in my opinion.
Let me explain what I am doing and may be you can provide your suggestion and
feedback.
Watchpoint Install => Register watchpoint into hardware watchpoint register
cache
Watchpoint Enable => Enable in cache and write registers using ptrace interface.
Ideally we should be able to install/uninstall watchpoints in cache and then
enable them all on a resume.
In case of arm If a watchpoint is hit we should be able to disable that
watchpoint.
Step over watchpoint instruction and then re-enable the watchpoint.
Our existing implementation will require a lot of changes to do that so thats
why here is what i am doing.
SetHardwareWatchpoint => Performs Install and Enable
- If a new watchpoint slot is going to be used we Install and enable.
- For new watchpoint we should be able to complete both Install and or we
report an error.
- If a duplicate slot is going to be used we Install and enable if required.
- Install means updating size if needed plus updating ref count.
- Enable means updating registers if size was updated.
ClearHardwareWatchpoint
- Disable and uinstall watchpoint means
- Decrement ref count and clear hardware watchpoint regsiters.
- Advantage of keeping ref counts is:
- If refcount is greater than zero then SetHardwareWatchpoint cannot use this
slot for a new watchpoint (new address).
- But SetHardwareWatchpoint can be use this slot to install duplicate
watchpoints (same address but different byte or word)
ClearAllHardwareWatchpoint -- Just clear the whole watchpoint cache and call
SetHardwareWatchpoint for all available watchpoints.
NativeThreadLinux:
On Watchpoint Remove -> Invalidate watchpoint cache
On Resume - > Re-validate watchpoints by creating a new cache and re-enabling
all watchpoints
So this fixes our step-over issue and also preserves watchpoint slot if it is
being used by multiple watchpoints.
Can you think of any scenarios which might fail for this approach?
https://reviews.llvm.org/D24610
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeThreadLinux.cpp
source/Plugins/Process/Linux/NativeThreadLinux.h
Index: source/Plugins/Process/Linux/NativeThreadLinux.h
===
--- source/Plugins/Process/Linux/NativeThreadLinux.h
+++ source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -108,6 +108,7 @@
using WatchpointIndexMap = std::map;
WatchpointIndexMap m_watchpoint_index_map;
cpu_set_t m_original_cpu_set; // For single-step workaround.
+ bool m_invalidate_watchpoints;
};
typedef std::shared_ptr NativeThreadLinuxSP;
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -87,7 +87,8 @@
NativeThreadLinux::NativeThreadLinux(NativeProcessLinux *process,
lldb::tid_t tid)
: NativeThreadProtocol(process, tid), m_state(StateType::eStateInvalid),
- m_stop_info(), m_reg_context_sp(), m_stop_description() {}
+ m_stop_info(), m_reg_context_sp(), m_stop_description(),
+ m_invalidate_watchpoints(false) {}
std::string NativeThreadLinux::GetName() {
NativeProcessProtocolSP process_sp = m_process_wp.lock();
@@ -185,8 +186,10 @@
return Error();
uint32_t wp_index = wp->second;
m_watchpoint_index_map.erase(wp);
- if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index))
+ if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index)) {
+m_invalidate_watchpoints = true;
return Error();
+ }
return Error("Clearing hardware watchpoint failed.");
}
@@ -198,8 +201,13 @@
m_stop_info.reason = StopReason::eStopReasonNone;
m_stop_description.clear();
- // If watchpoints have been set, but none on this thread,
- // then this is a new thread. So set all existing watchpoints.
+ // Invalidate watchpoint index map for a re-sync
+ if (m_invalidate_watchpoints) {
+m_invalidate_watchpoints = false;
+m_watchpoint_index_map.clear();
+ }
+
+ // Re-sync all available watchpoints.
if (m_watchpoint_index_map.empty()) {
NativeProcessLinux &process = GetProcess();
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_ar
[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
omjavaid created this revision.
omjavaid added a reviewer: labath.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: samparker, srhines, danalbert, tberghammer, rengolin,
aemerson.
On ARM Linux targets watchpoints are reported by PTrace before the instruction
causing watchpoint to trigger is executed.
This means we cannot step-over watchpoint causing instruction as long as there
is watchpoint installed on that location.
For this reason we cannot have multiple watchpoint slots watching same region
word/byte/halfword etc as we have to disable watchpoint for stepping over and
only watchpoint index reported by lldb-server is disabled.
Similarly we cannot keep a single hardware watchpoint slot referring to
multiple watchpoints in LLDB. This actually means that hware watchpoint indexes
are exclusively assigned to a watchpoint and cannot be shared by other
watchpoint. Also no other watchpoint should watch same address already watched
by any other index.
More discussion on this can be found here:
https://reviews.llvm.org/D24610
This patch fixes issue with stepping over watchpoint by removing the provision
to enable multiple watchpoints referring to same hardware watchpoint slot (same
HW index). Also we restrict one watchpoint per word aligned address so
duplication of address will not be allowed to avoid any step-over failures.
I have also attached a test-case that tests this scenario.
https://reviews.llvm.org/D25057
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -538,42 +538,34 @@
control_value |= ((1 << size) - 1) << 5;
control_value |= (2 << 1) | 1;
- // Iterate over stored watchpoints
- // Find a free wp_index or update reference count if duplicate.
+ // Iterate over stored watchpoints and find a free wp_index
wp_index = LLDB_INVALID_INDEX32;
for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
if ((m_hwp_regs[i].control & 1) == 0) {
wp_index = i; // Mark last free slot
-} else if (m_hwp_regs[i].address == addr &&
- m_hwp_regs[i].control == control_value) {
- wp_index = i; // Mark duplicate index
- break;// Stop searching here
+}
+else if (m_hwp_regs[i].address == addr) {
+ return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
}
}
if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Add new or update existing watchpoint
- if ((m_hwp_regs[wp_index].control & 1) == 0) {
-// Update watchpoint in local cache
-m_hwp_regs[wp_index].real_addr = real_addr;
-m_hwp_regs[wp_index].address = addr;
-m_hwp_regs[wp_index].control = control_value;
-m_hwp_regs[wp_index].refcount = 1;
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].real_addr = real_addr;
+ m_hwp_regs[wp_index].address = addr;
+ m_hwp_regs[wp_index].control = control_value;
-// PTRACE call to set corresponding watchpoint register.
-error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
-if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
+ if (error.Fail()) {
+m_hwp_regs[wp_index].address = 0;
+m_hwp_regs[wp_index].control &= ~1;
- return LLDB_INVALID_INDEX32;
-}
- } else
-m_hwp_regs[wp_index].refcount++;
+return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -596,36 +588,25 @@
if (wp_index >= m_max_hwp_supported)
return false;
- // Update reference count if multiple references.
- if (m_hwp_regs[wp_index].refcount > 1) {
-m_hwp_regs[wp_index].refcount--;
-return true;
- } else if (m_hwp_regs[wp_index].refcount == 1) {
-// Create a backup we can revert to in case of failure.
-lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
-uint32_t tempControl = m_hwp_regs[wp_index].control;
-uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control &= ~1;
+ m_hwp_regs[wp_index
[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
omjavaid added a comment. @labath Referring to your email on the mailing list. Thanks for helping out with this work. I think we should push this fix, as you suggested this does not fix everything in a holistic way but it corrects the functionality that is currently available right now with limitations ofcourse. So here is functionality we are currently lacking: - Ability to put multiple watchpoints with same address range. This is more concerning because we cannot put a watch on say byte 0 and byte 7 in case of aarch64. - Ability to use single slot for multiple watchpoints. This is more like a nice to have and I think if we fix the above functionality this may well be fixed automatically. This is what I think LLDB client or server has to do: - Keep a cache of watchpoint registers available and modify registers in cache when a set/remove request is made. - As pre-req for set/remove is to have the target in stopped state this will mean that when we set/remove we make changes to actual registers before we resume for continue or stepping. - I dont think keeping the cache and then updating on resume actually means we are lying to client. Cache will also remain limited and will behave like an actual write to the registers. It will serve us well to support the functionality we intend to support. In case of GDB this functionality is handled by gdbserver and gdb client is not aware of the fact that watchpoint registers are not actually written until we resume. To implement this in LLDB client or server is a design decision and I just see that it should be easier to achieve on LLDB server side though it may require changes to the way we approach watchpoint for all targets but it will then remain restricted to the concerning target specific area. I am OOO till 16th If you are OK with this change I will push it whenever it gets a LGTM. https://reviews.llvm.org/D25057 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15355: Add failure paths to a few JSONNumber members
omjavaid abandoned this revision. omjavaid added a comment. Correction made upstream by @tberghammer. http://reviews.llvm.org/D15355 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15357: Update code to silent some ARM/ARM64 specific compiler warnings
omjavaid abandoned this revision. omjavaid added a comment. Let the warnings stay for now. http://reviews.llvm.org/D15357 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15355: Add failure paths to a few JSONNumber members
omjavaid updated this revision to Diff 42706.
omjavaid added a comment.
Sorry @tberghammer there was corruption in my setup showing changes which i
just made internally.
I have updated this diff as per suggestions.
LGTM?
http://reviews.llvm.org/D15355
Files:
include/lldb/Utility/JSON.h
source/Utility/JSON.cpp
Index: source/Utility/JSON.cpp
===
--- source/Utility/JSON.cpp
+++ source/Utility/JSON.cpp
@@ -12,6 +12,7 @@
#include
#include "lldb/Core/StreamString.h"
#include "lldb/Host/StringConvert.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace lldb_private;
@@ -72,9 +73,10 @@
case DataType::Double:
return (uint64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
-uint64_t
+int64_t
JSONNumber::GetAsSigned() const
{
switch (m_data_type)
@@ -86,6 +88,7 @@
case DataType::Double:
return (int64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
double
@@ -100,6 +103,7 @@
case DataType::Double:
return m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
void
Index: include/lldb/Utility/JSON.h
===
--- include/lldb/Utility/JSON.h
+++ include/lldb/Utility/JSON.h
@@ -145,7 +145,7 @@
uint64_t
GetAsUnsigned() const;
-uint64_t
+int64_t
GetAsSigned() const;
double
Index: source/Utility/JSON.cpp
===
--- source/Utility/JSON.cpp
+++ source/Utility/JSON.cpp
@@ -12,6 +12,7 @@
#include
#include "lldb/Core/StreamString.h"
#include "lldb/Host/StringConvert.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace lldb_private;
@@ -72,9 +73,10 @@
case DataType::Double:
return (uint64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
-uint64_t
+int64_t
JSONNumber::GetAsSigned() const
{
switch (m_data_type)
@@ -86,6 +88,7 @@
case DataType::Double:
return (int64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
double
@@ -100,6 +103,7 @@
case DataType::Double:
return m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
void
Index: include/lldb/Utility/JSON.h
===
--- include/lldb/Utility/JSON.h
+++ include/lldb/Utility/JSON.h
@@ -145,7 +145,7 @@
uint64_t
GetAsUnsigned() const;
-uint64_t
+int64_t
GetAsSigned() const;
double
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15355: Add failure paths to a few JSONNumber members
This revision was automatically updated to reflect the committed changes.
Closed by commit rL255499: Add failure paths to a few JSONNumber members
(authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D15355?vs=42706&id=42712#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15355
Files:
lldb/trunk/include/lldb/Utility/JSON.h
lldb/trunk/source/Utility/JSON.cpp
Index: lldb/trunk/include/lldb/Utility/JSON.h
===
--- lldb/trunk/include/lldb/Utility/JSON.h
+++ lldb/trunk/include/lldb/Utility/JSON.h
@@ -145,7 +145,7 @@
uint64_t
GetAsUnsigned() const;
-uint64_t
+int64_t
GetAsSigned() const;
double
Index: lldb/trunk/source/Utility/JSON.cpp
===
--- lldb/trunk/source/Utility/JSON.cpp
+++ lldb/trunk/source/Utility/JSON.cpp
@@ -12,6 +12,7 @@
#include
#include "lldb/Core/StreamString.h"
#include "lldb/Host/StringConvert.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace lldb_private;
@@ -72,9 +73,10 @@
case DataType::Double:
return (uint64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
-uint64_t
+int64_t
JSONNumber::GetAsSigned() const
{
switch (m_data_type)
@@ -86,6 +88,7 @@
case DataType::Double:
return (int64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
double
@@ -100,6 +103,7 @@
case DataType::Double:
return m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
void
Index: lldb/trunk/include/lldb/Utility/JSON.h
===
--- lldb/trunk/include/lldb/Utility/JSON.h
+++ lldb/trunk/include/lldb/Utility/JSON.h
@@ -145,7 +145,7 @@
uint64_t
GetAsUnsigned() const;
-uint64_t
+int64_t
GetAsSigned() const;
double
Index: lldb/trunk/source/Utility/JSON.cpp
===
--- lldb/trunk/source/Utility/JSON.cpp
+++ lldb/trunk/source/Utility/JSON.cpp
@@ -12,6 +12,7 @@
#include
#include "lldb/Core/StreamString.h"
#include "lldb/Host/StringConvert.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace lldb_private;
@@ -72,9 +73,10 @@
case DataType::Double:
return (uint64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
-uint64_t
+int64_t
JSONNumber::GetAsSigned() const
{
switch (m_data_type)
@@ -86,6 +88,7 @@
case DataType::Double:
return (int64_t)m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
double
@@ -100,6 +103,7 @@
case DataType::Double:
return m_data.m_double;
}
+llvm_unreachable("Unhandled data type");
}
void
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15061: Correction in TestFrames.py test for arm targets in thumb mode
This revision was automatically updated to reflect the committed changes.
Closed by commit rL255547: Correction in TestFrames.py test for arm targets in
thumb mode (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D15061?vs=41359&id=42765#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15061
Files:
lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
Index: lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
@@ -80,9 +80,12 @@
gpr_reg_set = lldbutil.get_GPRs(frame)
pc_value = gpr_reg_set.GetChildMemberWithName("pc")
self.assertTrue (pc_value, "We should have a valid PC.")
-pc_value_str = pc_value.GetValue()
-self.assertTrue (pc_value_str, "We should have a valid PC
string.")
-self.assertTrue (int(pc_value_str, 0) == frame.GetPC(), "PC
gotten as a value should equal frame's GetPC")
+pc_value_int = int(pc_value.GetValue(), 0)
+# Make sure on arm targets we dont mismatch PC value on the
basis of thumb bit.
+# Frame PC will not have thumb bit set in case of a thumb
instruction as PC.
+if self.getArchitecture() in ['arm']:
+pc_value_int &= ~1
+self.assertTrue (pc_value_int == frame.GetPC(), "PC gotten as
a value should equal frame's GetPC")
sp_value = gpr_reg_set.GetChildMemberWithName("sp")
self.assertTrue (sp_value, "We should have a valid Stack
Pointer.")
self.assertTrue (int(sp_value.GetValue(), 0) == frame.GetSP(),
"SP gotten as a value should equal frame's GetSP")
Index: lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/frame/TestFrames.py
@@ -80,9 +80,12 @@
gpr_reg_set = lldbutil.get_GPRs(frame)
pc_value = gpr_reg_set.GetChildMemberWithName("pc")
self.assertTrue (pc_value, "We should have a valid PC.")
-pc_value_str = pc_value.GetValue()
-self.assertTrue (pc_value_str, "We should have a valid PC string.")
-self.assertTrue (int(pc_value_str, 0) == frame.GetPC(), "PC gotten as a value should equal frame's GetPC")
+pc_value_int = int(pc_value.GetValue(), 0)
+# Make sure on arm targets we dont mismatch PC value on the basis of thumb bit.
+# Frame PC will not have thumb bit set in case of a thumb instruction as PC.
+if self.getArchitecture() in ['arm']:
+pc_value_int &= ~1
+self.assertTrue (pc_value_int == frame.GetPC(), "PC gotten as a value should equal frame's GetPC")
sp_value = gpr_reg_set.GetChildMemberWithName("sp")
self.assertTrue (sp_value, "We should have a valid Stack Pointer.")
self.assertTrue (int(sp_value.GetValue(), 0) == frame.GetSP(), "SP gotten as a value should equal frame's GetSP")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15533: Make the aarch64 lldb-server capable of debugging arm32 applications
omjavaid added a comment. I am wondering whats the reason of replacing arm ptrace calls with aarch64 specific calls that use iovec parameters. If arm calls can work then may be dont use aarch64 specific calls at all. If they dont work kindly make relevant changes to NativeRegisterContextLinux_arm::WriteHardwareDebugRegs and NativeRegisterContextLinux_arm::ReadHardwareDebugInfo(). I guess these functions will throw same errors in case calls propagated using aarch64 lib interface and not backward compaitible with arm specific interface. http://reviews.llvm.org/D15533 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D15877: Fix for undefined behavior while updating PC value on arm-linux
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch provides fix for the cases where an expression or jump tries to
update the value of PC in thumb mode.
As precaution for an undefined behavior encountered while setting PC we will
clear thumb bit of new PC if we are already in thumb mode; that is CPSR thumb
mode bit is set.
Also if intended behavior of setting new PC value is to change to ARM/Thumb
mode then user must switch to arm or thumb mode by explicitly setting mode bit
in CPSR. This update to CPSR should be done before writing new PC value.
http://reviews.llvm.org/D15877
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -973,7 +973,24 @@
if (error.Fail())
return error;
-m_gpr_arm[offset / sizeof(uint32_t)] = value.GetAsUInt32();
+uint32_t reg_value = value.GetAsUInt32();
+// As precaution for an undefined behavior encountered while setting PC we
+// will clear thumb bit of new PC if we are already in thumb mode; that is
+// CPSR thumb mode bit is set.
+if (offset / sizeof(uint32_t) == gpr_pc_arm)
+{
+// Check if we are already in thumb mode and
+ // thumb bit of current PC is read out to be zero and
+ // thumb bit of next PC is read out to be one.
+ if ((m_gpr_arm[gpr_cpsr_arm] & 0x20) &&
+ !(m_gpr_arm[gpr_pc_arm] & 0x01) &&
+ (value.GetAsUInt32() & 0x01))
+ {
+ reg_value &= (~1ull);
+ }
+}
+
+m_gpr_arm[offset / sizeof(uint32_t)] = reg_value;
return DoWriteGPR(m_gpr_arm, sizeof(m_gpr_arm));
}
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -973,7 +973,24 @@
if (error.Fail())
return error;
-m_gpr_arm[offset / sizeof(uint32_t)] = value.GetAsUInt32();
+uint32_t reg_value = value.GetAsUInt32();
+// As precaution for an undefined behavior encountered while setting PC we
+// will clear thumb bit of new PC if we are already in thumb mode; that is
+// CPSR thumb mode bit is set.
+if (offset / sizeof(uint32_t) == gpr_pc_arm)
+{
+// Check if we are already in thumb mode and
+ // thumb bit of current PC is read out to be zero and
+ // thumb bit of next PC is read out to be one.
+ if ((m_gpr_arm[gpr_cpsr_arm] & 0x20) &&
+ !(m_gpr_arm[gpr_pc_arm] & 0x01) &&
+ (value.GetAsUInt32() & 0x01))
+ {
+ reg_value &= (~1ull);
+ }
+}
+
+m_gpr_arm[offset / sizeof(uint32_t)] = reg_value;
return DoWriteGPR(m_gpr_arm, sizeof(m_gpr_arm));
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15877: Fix for undefined behavior while updating PC value on arm-linux
This revision was automatically updated to reflect the committed changes.
Closed by commit rL256847: Fix for undefined behavior while updating PC value
on arm-linux (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D15877?vs=43941&id=44015#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15877
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
Index:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -973,7 +973,24 @@
if (error.Fail())
return error;
-m_gpr_arm[offset / sizeof(uint32_t)] = value.GetAsUInt32();
+uint32_t reg_value = value.GetAsUInt32();
+// As precaution for an undefined behavior encountered while setting PC we
+// will clear thumb bit of new PC if we are already in thumb mode; that is
+// CPSR thumb mode bit is set.
+if (offset / sizeof(uint32_t) == gpr_pc_arm)
+{
+// Check if we are already in thumb mode and
+// thumb bit of current PC is read out to be zero and
+// thumb bit of next PC is read out to be one.
+if ((m_gpr_arm[gpr_cpsr_arm] & 0x20) &&
+!(m_gpr_arm[gpr_pc_arm] & 0x01) &&
+(value.GetAsUInt32() & 0x01))
+{
+reg_value &= (~1ull);
+}
+}
+
+m_gpr_arm[offset / sizeof(uint32_t)] = reg_value;
return DoWriteGPR(m_gpr_arm, sizeof(m_gpr_arm));
}
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -973,7 +973,24 @@
if (error.Fail())
return error;
-m_gpr_arm[offset / sizeof(uint32_t)] = value.GetAsUInt32();
+uint32_t reg_value = value.GetAsUInt32();
+// As precaution for an undefined behavior encountered while setting PC we
+// will clear thumb bit of new PC if we are already in thumb mode; that is
+// CPSR thumb mode bit is set.
+if (offset / sizeof(uint32_t) == gpr_pc_arm)
+{
+// Check if we are already in thumb mode and
+// thumb bit of current PC is read out to be zero and
+// thumb bit of next PC is read out to be one.
+if ((m_gpr_arm[gpr_cpsr_arm] & 0x20) &&
+!(m_gpr_arm[gpr_pc_arm] & 0x01) &&
+(value.GetAsUInt32() & 0x01))
+{
+reg_value &= (~1ull);
+}
+}
+
+m_gpr_arm[offset / sizeof(uint32_t)] = reg_value;
return DoWriteGPR(m_gpr_arm, sizeof(m_gpr_arm));
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D15893: Adds expectedFailureArmLinux test decorator
omjavaid created this revision.
omjavaid added a reviewer: clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch adds a new expectedFailureArmLinux which marks arm-linux tests as
xfails.
Also marked a couple of failing tests to use expectedFailureArmLinux decorator.
http://reviews.llvm.org/D15893
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
packages/Python/lldbsuite/test/lldbtest.py
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -703,6 +703,12 @@
(debug_info is None or self.debug_info in debug_info))
return expectedFailure(fn, bugnumber)
+def expectedFailureArmLinux(bugnumber=None, compilers=None, debug_info=None):
+def fn(self):
+return (self.getPlatform() in ['linux'] and
+self.getArchitecture() in ['arm'])
+return expectedFailure(fn, bugnumber)
+
def expectedFailureHostOS(oslist, bugnumber=None, compilers=None):
def fn(self):
return (getHostPlatform() in oslist and
Index:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
---
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -27,6 +27,7 @@
# Build dictionary to have unique executable names for each test
method.
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureArmLinux("llvm.org/pr26031")
@expectedFailureWindows("llvm.org/pr24446") # WINDOWS XFAIL TRIAGE -
Watchpoints not supported on Windows
def test_watchlocation_using_watchpoint_set(self):
"""Test watching a location with 'watchpoint set expression -w write
-s size' option."""
Index:
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
===
---
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
+++
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -17,6 +17,7 @@
return ['basic_process']
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureArmLinux("llvm.org/pr26031")
@expectedFailureWindows("llvm.org/pr24446")
def test(self):
"""Test stepping over watchpoints."""
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -703,6 +703,12 @@
(debug_info is None or self.debug_info in debug_info))
return expectedFailure(fn, bugnumber)
+def expectedFailureArmLinux(bugnumber=None, compilers=None, debug_info=None):
+def fn(self):
+return (self.getPlatform() in ['linux'] and
+self.getArchitecture() in ['arm'])
+return expectedFailure(fn, bugnumber)
+
def expectedFailureHostOS(oslist, bugnumber=None, compilers=None):
def fn(self):
return (getHostPlatform() in oslist and
Index: packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
--- packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++ packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -27,6 +27,7 @@
# Build dictionary to have unique executable names for each test method.
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not supported
+@expectedFailureArmLinux("llvm.org/pr26031")
@expectedFailureWindows("llvm.org/pr24446") # WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows
def test_watchlocation_using_watchpoint_set(self):
"""Test watching a location with 'watchpoint set expression -w write -s size' option."""
Index: packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
===
--- packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
+++ packages/Python/lldbsuite/test/funct
Re: [Lldb-commits] [PATCH] D15533: Make the aarch64 lldb-server capable of debugging arm32 applications
omjavaid added a subscriber: omjavaid. omjavaid added a comment. LGTM. I think we should submit this patch as tberghammer explained. http://reviews.llvm.org/D15533 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D15893: Adds expectedFailureArmLinux test decorator
omjavaid updated this revision to Diff 44550.
omjavaid added a comment.
Removed expectedFailureArmLinux and updated expectedFailureLinux decorator to
reflect architecture if needed.
Marked some triaged failures as xfails on arm with updated expectedFailureLinux
decorator.
LGTM?
http://reviews.llvm.org/D15893
Files:
packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
packages/Python/lldbsuite/test/lldbtest.py
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -696,10 +696,11 @@
def expectedFailurex86_64(bugnumber=None):
return expectedFailureArch('x86_64', bugnumber)
-def expectedFailureOS(oslist, bugnumber=None, compilers=None, debug_info=None):
+def expectedFailureOS(oslist, bugnumber=None, compilers=None, debug_info=None,
archs=None):
def fn(self):
return (self.getPlatform() in oslist and
self.expectedCompiler(compilers) and
+(archs is None or self.getArchitecture() in archs) and
(debug_info is None or self.debug_info in debug_info))
return expectedFailure(fn, bugnumber)
@@ -716,8 +717,8 @@
def expectedFailureFreeBSD(bugnumber=None, compilers=None, debug_info=None):
return expectedFailureOS(['freebsd'], bugnumber, compilers,
debug_info=debug_info)
-def expectedFailureLinux(bugnumber=None, compilers=None, debug_info=None):
-return expectedFailureOS(['linux'], bugnumber, compilers,
debug_info=debug_info)
+def expectedFailureLinux(bugnumber=None, compilers=None, debug_info=None,
archs=None):
+return expectedFailureOS(['linux'], bugnumber, compilers,
debug_info=debug_info, archs=archs)
def expectedFailureNetBSD(bugnumber=None, compilers=None, debug_info=None):
return expectedFailureOS(['netbsd'], bugnumber, compilers,
debug_info=debug_info)
Index:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
---
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -27,6 +27,7 @@
# Build dictionary to have unique executable names for each test
method.
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureLinux(bugnumber="llvm.org/pr26031", archs=['arm'])
@expectedFailureWindows("llvm.org/pr24446") # WINDOWS XFAIL TRIAGE -
Watchpoints not supported on Windows
def test_watchlocation_using_watchpoint_set(self):
"""Test watching a location with 'watchpoint set expression -w write
-s size' option."""
Index:
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
===
---
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
+++
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -17,6 +17,7 @@
return ['basic_process']
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureLinux(bugnumber="llvm.org/pr26031", archs=['arm'])
@expectedFailureWindows("llvm.org/pr24446")
def test(self):
"""Test stepping over watchpoints."""
Index:
packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
===
---
packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
+++
packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
@@ -16,6 +16,7 @@
@expectedFailureWindows("llvm.org/pr21793: need to implement support for
detecting assertion / abort on Windows")
@expectedFailurei386("llvm.org/pr25338")
+@expectedFailureLinux("llvm.org/pr25338", archs=['arm', 'i386'])
def test_inferior_asserting(self):
"""Test that lldb reliably catches the inferior asserting (command)."""
self.build()
@@ -30,6 +31,7 @@
@expectedFailureWindows("llvm.org/pr21793: need to implement support for
detecting assertion / abort on Windows")
@expectedFailurei386("llvm.org/pr25338")
+@expectedFailureLinux("llvm.org/pr25338", archs=['arm', 'i386'])
def test_inferior_asserting_disassemble(self):
"""Test that lldb reliably disassembles frames a
Re: [Lldb-commits] [PATCH] D15893: Adds expectedFailureArmLinux test decorator
This revision was automatically updated to reflect the committed changes.
Closed by commit rL257405: Xfail some Arm-Linux specific failures (authored by
omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D15893?vs=44550&id=44563#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15893
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -696,10 +696,11 @@
def expectedFailurex86_64(bugnumber=None):
return expectedFailureArch('x86_64', bugnumber)
-def expectedFailureOS(oslist, bugnumber=None, compilers=None, debug_info=None):
+def expectedFailureOS(oslist, bugnumber=None, compilers=None, debug_info=None,
archs=None):
def fn(self):
return (self.getPlatform() in oslist and
self.expectedCompiler(compilers) and
+(archs is None or self.getArchitecture() in archs) and
(debug_info is None or self.debug_info in debug_info))
return expectedFailure(fn, bugnumber)
@@ -716,8 +717,8 @@
def expectedFailureFreeBSD(bugnumber=None, compilers=None, debug_info=None):
return expectedFailureOS(['freebsd'], bugnumber, compilers,
debug_info=debug_info)
-def expectedFailureLinux(bugnumber=None, compilers=None, debug_info=None):
-return expectedFailureOS(['linux'], bugnumber, compilers,
debug_info=debug_info)
+def expectedFailureLinux(bugnumber=None, compilers=None, debug_info=None,
archs=None):
+return expectedFailureOS(['linux'], bugnumber, compilers,
debug_info=debug_info, archs=archs)
def expectedFailureNetBSD(bugnumber=None, compilers=None, debug_info=None):
return expectedFailureOS(['netbsd'], bugnumber, compilers,
debug_info=debug_info)
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py
@@ -27,6 +27,7 @@
# Build dictionary to have unique executable names for each test
method.
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureLinux(bugnumber="llvm.org/pr26031", archs=['arm'])
@expectedFailureWindows("llvm.org/pr24446") # WINDOWS XFAIL TRIAGE -
Watchpoints not supported on Windows
def test_watchlocation_using_watchpoint_set(self):
"""Test watching a location with 'watchpoint set expression -w write
-s size' option."""
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -17,6 +17,7 @@
return ['basic_process']
@expectedFailureAndroid(archs=['arm', 'aarch64']) # Watchpoints not
supported
+@expectedFailureLinux(bugnumber="llvm.org/pr26031", archs=['arm'])
@expectedFailureWindows("llvm.org/pr24446")
def test(self):
"""Test stepping over watchpoints."""
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
@@ -16,6 +16,7 @@
@expectedFailureWindows("llvm.org/pr21793: need to implement support for
detecting assertion / abort on Windows")
@expectedFailurei386("llvm.org/pr25338")
+@expectedFailureLinux("llvm.org/pr25338", archs=['arm', 'i386'])
def test_inferior_asserting(self):
"""Test that lldb reliably catches the inferior asserting (command)."""
self.build()
@@ -30,6 +31,7 @@
@expectedFailureWindows("llvm.org/pr21793: need to implement support for
detecting assertion / abort on Windows")
@expectedFailurei386("llvm.org/pr25338")
+@expectedFailu
[Lldb-commits] [PATCH] D16627: Add support to detect arm hard float ABI based binaries for ABISysV_arm
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch adds logic to detect if underlying binary is using arm hard float
abi and use that information while handling return values in ABISysV_arm.
This patch only handles float and double passed using floating point registers.
More return types and argument passing will be handled in a follow up patches.
http://reviews.llvm.org/D16627
Files:
include/lldb/Core/ArchSpec.h
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
source/Plugins/ABI/SysV-arm/ABISysV_arm.h
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1562,6 +1562,15 @@
}
}
+if (arch_spec.GetMachine() == llvm::Triple::arm ||
+arch_spec.GetMachine() == llvm::Triple::thumb)
+{
+if (header.e_flags & llvm::ELF::EF_ARM_SOFT_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_soft_float);
+else if (header.e_flags & llvm::ELF::EF_ARM_VFP_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_hard_float);
+}
+
// If there are no section headers we are done.
if (header.e_shnum == 0)
return 0;
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.h
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.h
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.h
@@ -79,6 +79,9 @@
const lldb_private::RegisterInfo *
GetRegisterInfoArray (uint32_t &count) override;
+bool
+IsArmHardFloat (lldb_private::Thread *thread) const;
+
//--
// Static Functions
//--
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -414,6 +414,25 @@
return true;
}
+bool
+ABISysV_arm::IsArmHardFloat (Thread *thread) const
+{
+bool is_armhf = false;
+if (thread)
+{
+ProcessSP process_sp (thread->GetProcess());
+if (process_sp)
+{
+const ArchSpec &arch (process_sp->GetTarget().GetArchitecture());
+if (arch.GetFlags() == ArchSpec::eARM_abi_hard_float)
+{
+is_armhf = true;
+}
+}
+}
+return is_armhf;
+}
+
ValueObjectSP
ABISysV_arm::GetReturnValueObjectImpl (Thread &thread,
lldb_private::CompilerType &compiler_type) const
@@ -516,19 +535,44 @@
case 64:
{
static_assert(sizeof(double) == sizeof(uint64_t), "");
-const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
-uint64_t raw_value;
-raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (!IsArmHardFloat(&thread))
+{
+uint64_t raw_value;
+const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
+raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
+raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
+value.GetScalar() = *reinterpret_cast(&raw_value);
+}
+else
+{
+RegisterValue reg_value;
+const RegisterInfo *d0_reg_info = reg_ctx->GetRegisterInfoByName("d0", 0);
+reg_ctx->ReadRegister(d0_reg_info, reg_value);
+value.GetScalar() = reg_value.GetAsDouble();
+}
+
break;
}
case 16: // Half precision returned after a conversion to single precision
case 32:
{
static_assert(sizeof(float) == sizeof(uint32_t), "");
-uint32_t raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (!IsArmHardFloat(&thread))
+{
+uint32_t raw_value;
+
Re: [Lldb-commits] [PATCH] D16772: Fix single stepping over the IT instruction
omjavaid accepted this revision. omjavaid added a comment. Looks good. Was there a test failing in testsuite due to this? http://reviews.llvm.org/D16772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D16627: Add support to detect arm hard float ABI based binaries for ABISysV_arm
omjavaid updated this revision to Diff 46620.
omjavaid marked 4 inline comments as done.
omjavaid added a comment.
Updated after addressing concerns.
LGTM?
http://reviews.llvm.org/D16627
Files:
include/lldb/Core/ArchSpec.h
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
source/Plugins/ABI/SysV-arm/ABISysV_arm.h
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1562,6 +1562,15 @@
}
}
+if (arch_spec.GetMachine() == llvm::Triple::arm ||
+arch_spec.GetMachine() == llvm::Triple::thumb)
+{
+if (header.e_flags & llvm::ELF::EF_ARM_SOFT_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_soft_float);
+else if (header.e_flags & llvm::ELF::EF_ARM_VFP_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_hard_float);
+}
+
// If there are no section headers we are done.
if (header.e_shnum == 0)
return 0;
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.h
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.h
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.h
@@ -79,6 +79,9 @@
const lldb_private::RegisterInfo *
GetRegisterInfoArray (uint32_t &count) override;
+bool
+IsArmHardFloat (lldb_private::Thread &thread) const;
+
//--
// Static Functions
//--
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -414,6 +414,20 @@
return true;
}
+bool
+ABISysV_arm::IsArmHardFloat (Thread &thread) const
+{
+ProcessSP process_sp (thread.GetProcess());
+if (process_sp)
+{
+const ArchSpec &arch (process_sp->GetTarget().GetArchitecture());
+
+return (arch.GetFlags() & ArchSpec::eARM_abi_hard_float) != 0;
+}
+
+return false;
+}
+
ValueObjectSP
ABISysV_arm::GetReturnValueObjectImpl (Thread &thread,
lldb_private::CompilerType &compiler_type) const
@@ -516,19 +530,42 @@
case 64:
{
static_assert(sizeof(double) == sizeof(uint64_t), "");
-const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
-uint64_t raw_value;
-raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (IsArmHardFloat(thread))
+{
+RegisterValue reg_value;
+const RegisterInfo *d0_reg_info = reg_ctx->GetRegisterInfoByName("d0", 0);
+reg_ctx->ReadRegister(d0_reg_info, reg_value);
+value.GetScalar() = reg_value.GetAsDouble();
+}
+else
+{
+uint64_t raw_value;
+const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
+raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
+raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
+value.GetScalar() = *reinterpret_cast(&raw_value);
+}
break;
}
case 16: // Half precision returned after a conversion to single precision
case 32:
{
static_assert(sizeof(float) == sizeof(uint32_t), "");
-uint32_t raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (IsArmHardFloat(thread))
+{
+RegisterValue reg_value;
+const RegisterInfo *s0_reg_info = reg_ctx->GetRegisterInfoByName("s0", 0);
+reg_ctx->ReadRegister(s0_reg_info, reg_value);
+value.GetScalar() = reg_value.GetAsFloat();
+}
+else
+{
+uint32_t raw_value;
+raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
+
Re: [Lldb-commits] [PATCH] D16627: Add support to detect arm hard float ABI based binaries for ABISysV_arm
This revision was automatically updated to reflect the committed changes.
Closed by commit rL259885: Add support to detect arm hard float ABI based
binaries for ABISysV_arm (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D16627?vs=46620&id=47014#toc
Repository:
rL LLVM
http://reviews.llvm.org/D16627
Files:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.h
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1562,6 +1562,15 @@
}
}
+if (arch_spec.GetMachine() == llvm::Triple::arm ||
+arch_spec.GetMachine() == llvm::Triple::thumb)
+{
+if (header.e_flags & llvm::ELF::EF_ARM_SOFT_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_soft_float);
+else if (header.e_flags & llvm::ELF::EF_ARM_VFP_FLOAT)
+arch_spec.SetFlags (ArchSpec::eARM_abi_hard_float);
+}
+
// If there are no section headers we are done.
if (header.e_shnum == 0)
return 0;
Index: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.h
===
--- lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.h
+++ lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.h
@@ -79,6 +79,9 @@
const lldb_private::RegisterInfo *
GetRegisterInfoArray (uint32_t &count) override;
+bool
+IsArmHardFloat (lldb_private::Thread &thread) const;
+
//--
// Static Functions
//--
Index: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -414,6 +414,20 @@
return true;
}
+bool
+ABISysV_arm::IsArmHardFloat (Thread &thread) const
+{
+ProcessSP process_sp (thread.GetProcess());
+if (process_sp)
+{
+const ArchSpec &arch (process_sp->GetTarget().GetArchitecture());
+
+return (arch.GetFlags() & ArchSpec::eARM_abi_hard_float) != 0;
+}
+
+return false;
+}
+
ValueObjectSP
ABISysV_arm::GetReturnValueObjectImpl (Thread &thread,
lldb_private::CompilerType &compiler_type) const
@@ -516,19 +530,42 @@
case 64:
{
static_assert(sizeof(double) == sizeof(uint64_t), "");
-const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
-uint64_t raw_value;
-raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (IsArmHardFloat(thread))
+{
+RegisterValue reg_value;
+const RegisterInfo *d0_reg_info = reg_ctx->GetRegisterInfoByName("d0", 0);
+reg_ctx->ReadRegister(d0_reg_info, reg_value);
+value.GetScalar() = reg_value.GetAsDouble();
+}
+else
+{
+uint64_t raw_value;
+const RegisterInfo *r1_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG2);
+raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
+raw_value |= ((uint64_t)(reg_ctx->ReadRegisterAsUnsigned(r1_reg_info, 0) & UINT32_MAX)) << 32;
+value.GetScalar() = *reinterpret_cast(&raw_value);
+}
break;
}
case 16: // Half precision returned after a conversion to single precision
case 32:
{
static_assert(sizeof(float) == sizeof(uint32_t), "");
-uint32_t raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info, 0) & UINT32_MAX;
-value.GetScalar() = *reinterpret_cast(&raw_value);
+
+if (IsArmHardFloat(thread))
+{
+RegisterValue reg_value;
+const RegisterInfo *s0_reg_info = reg_ctx->GetRegisterInfoByName("s0", 0);
+reg_ctx->ReadRegister(s0_reg_info, reg_value);
+
[Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
Arm hard float ABI can use floating point registers for returning structures
containing all 4 or 8 byte floating point elements.
Arm ABI documentation call such structs a Homogeneous Aggregate, which is a
Composite Type where all of the Fundamental Data Types that compose the type
are the same.
With Arm Hard float ABI a Homogeneous Aggregate with a Base Type of a single-
or double-precision floating-point type with one to four Elements can be
returned using register s0-s3 and d0-d3.
This patch updates ABISysV_arm::GetReturnValueObjectImpl to handle these cases.
ReturnValueTestCase passes on armhf based linux target after applying this
patch.
http://reviews.llvm.org/D16975
Files:
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -587,6 +587,74 @@
}
else
{
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count =
compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (!is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new
DataBufferHeap(byte_size, 0));
+const size_t base_byte_size =
base_type.GetByteSize(nullptr);
+uint32_t data_offset = 0;
+
+for (uint32_t reg_index = 0; reg_index <
homogeneous_count; reg_index++)
+{
+char reg_name[8];
+
+if (base_byte_size == 4)
+::snprintf (reg_name, sizeof(reg_name),
"s%u", reg_index);
+else if (base_byte_size == 8)
+::snprintf (reg_name, sizeof(reg_name),
"d%u", reg_index);
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfoByName(reg_name, 0);
+if (reg_info == NULL)
+break;
+
+RegisterValue reg_value;
+if (!reg_ctx->ReadRegister(reg_info,
reg_value))
+break;
+
+// Make sure we have enough room in "data_sp"
+if ((data_offset + base_byte_size) <=
data_sp->GetByteSize())
+{
+Error error;
+const size_t bytes_copied =
reg_value.GetAsMemoryData (reg_info,
+
data_sp->GetBytes() + data_offset,
+
base_byte_size,
+
byte_order,
+
error);
+if (bytes_copied != base_byte_size)
+break;
+
+data_offset += bytes_copied;
+}
+}
+
+if (data_offset == byte_size)
+{
+DataExtractor data;
+data.SetByteOrder(byte_order);
+
data.SetAddressByteSize(process_sp->GetAddressByteSize());
+data.SetData(data_sp);
+
+return ValueObjectConstResult::Create
(&thread, compiler_type, ConstString(""), data);
+}
+
+}
+}
+}
+}
+
if (!GetReturnValuePassedInMemory(thread, reg_ctx, byte_size,
value))
return return_valobj_sp;
}
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid added inline comments.
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:581-591
@@ -580,10 +580,13 @@
{
size_t byte_size = compiler_type.GetByteSize(&thread);
if (byte_size <= 4)
{
RegisterValue r0_reg_value;
uint32_t raw_value = reg_ctx->ReadRegisterAsUnsigned(r0_reg_info,
0) & UINT32_MAX;
value.SetBytes(&raw_value, byte_size);
}
else
{
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
tberghammer wrote:
> I think if we are returning an aggregate containing 1 32 bit float (byte_size
> == 4) then it will be returned in s0 while your current implementation expect
> it to be returned in r0. I am not sure about it but please take a look.
>
> If this is the case I would suggest to order the conditions the following way
> to decrees the nesting level of the ifs:
>
> ```
> if (IsArmHardFloat(thread))
> {
> ...
> }
> else if (byte_size <= 4)
> {
>
> }
>
> ```
Yes, you are right. I ll rearrange the code accordingly.
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:597
@@ +596,3 @@
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
tberghammer wrote:
> What is float_count means here? Do we have to check its value to decide if we
> can print the return value (e.g. what happens when float_count == 2)?
float_count is just used to fullfil argument requirements. It was already
declared in the function so didnt have to define it here.
It returns 1 if type is a standard builtin type (float or double), returns 2
for complex and number of elements for vector types.
We are using homogeneous_count to tell the number of elements in our aggregate
type.
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:612-619
@@ +611,10 @@
+
+if (base_byte_size == 4)
+::snprintf (reg_name, sizeof(reg_name),
"s%u", reg_index);
+else if (base_byte_size == 8)
+::snprintf (reg_name, sizeof(reg_name),
"d%u", reg_index);
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfoByName(reg_name, 0);
+if (reg_info == NULL)
tberghammer wrote:
> I would suggest to use the dwarf register numbers instead of the register
> names for the lookup:
>
> ```
> uint32_t regnum = 0;
> if (byte_size == 4)
> regnum = dwarf_s0 + reg_index;
> else if (base_byte_size == 8)
> regnum = dwarf_d0 + reg_index;
> else
> break;
> const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoBy(eRegisterKindDWARF,
> regnum);
> ```
Alright I ll make the appropriate change.
http://reviews.llvm.org/D16975
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid updated this revision to Diff 47191.
omjavaid added a comment.
updated diff after incorporating suggested corrections.
http://reviews.llvm.org/D16975
Files:
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,75 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count =
compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (!is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size,
0));
+const size_t base_byte_size =
base_type.GetByteSize(nullptr);
+uint32_t data_offset = 0;
+
+for (uint32_t reg_index = 0; reg_index <
homogeneous_count; reg_index++)
+{
+uint32_t regnum = 0;
+
+if (base_byte_size == 4)
+regnum = dwarf_s0 + reg_index;
+else if (base_byte_size == 8)
+regnum = dwarf_d0 + reg_index;
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfo (eRegisterKindDWARF, regnum);
+if (reg_info == NULL)
+break;
+
+RegisterValue reg_value;
+if (!reg_ctx->ReadRegister(reg_info, reg_value))
+break;
+
+// Make sure we have enough room in "data_sp"
+if ((data_offset + base_byte_size) <=
data_sp->GetByteSize())
+{
+Error error;
+const size_t bytes_copied =
reg_value.GetAsMemoryData (reg_info,
+
data_sp->GetBytes() + data_offset,
+
base_byte_size,
+
byte_order,
+
error);
+if (bytes_copied != base_byte_size)
+break;
+
+data_offset += bytes_copied;
+}
+}
+
+if (data_offset == byte_size)
+{
+DataExtractor data;
+data.SetByteOrder(byte_order);
+
data.SetAddressByteSize(process_sp->GetAddressByteSize());
+data.SetData(data_sp);
+
+return ValueObjectConstResult::Create (&thread,
compiler_type, ConstString(""), data);
+}
+
+}
+}
+}
+}
+
if (byte_size <= 4)
{
RegisterValue r0_reg_value;
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,75 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count = compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (!is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size, 0));
+const size_t base_byte_size = base_type.GetByteSize(nullptr);
+
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid updated this revision to Diff 47198.
omjavaid added a comment.
Updated adding float_count check.
http://reviews.llvm.org/D16975
Files:
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,74 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count =
compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (float_count == 1 && !is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size,
0));
+const size_t base_byte_size =
base_type.GetByteSize(nullptr);
+uint32_t data_offset = 0;
+
+for (uint32_t reg_index = 0; reg_index <
homogeneous_count; reg_index++)
+{
+uint32_t regnum = 0;
+
+if (base_byte_size == 4)
+regnum = dwarf_s0 + reg_index;
+else if (base_byte_size == 8)
+regnum = dwarf_d0 + reg_index;
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfo (eRegisterKindDWARF, regnum);
+if (reg_info == NULL)
+break;
+
+RegisterValue reg_value;
+if (!reg_ctx->ReadRegister(reg_info, reg_value))
+break;
+
+// Make sure we have enough room in "data_sp"
+if ((data_offset + base_byte_size) <=
data_sp->GetByteSize())
+{
+Error error;
+const size_t bytes_copied =
reg_value.GetAsMemoryData (reg_info,
+
data_sp->GetBytes() + data_offset,
+
base_byte_size,
+
byte_order,
+
error);
+if (bytes_copied != base_byte_size)
+break;
+
+data_offset += bytes_copied;
+}
+}
+
+if (data_offset == byte_size)
+{
+DataExtractor data;
+data.SetByteOrder(byte_order);
+
data.SetAddressByteSize(process_sp->GetAddressByteSize());
+data.SetData(data_sp);
+
+return ValueObjectConstResult::Create (&thread,
compiler_type, ConstString(""), data);
+}
+
+}
+}
+}
+}
+
if (byte_size <= 4)
{
RegisterValue r0_reg_value;
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,74 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count = compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (float_count == 1 && !is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size, 0));
+const size_t base_byte_size = base_type.GetByteSize(nullptr);
+
Re: [Lldb-commits] [PATCH] D16853: Use BKPT instead of UDF for arm/thumb breakpoints
omjavaid added a comment. GDB doesnt use bkpt instruction for user debugging for the reason that it interferes with jtag debug probes. I am not sure if LLDB will be ever used with a jtag probe in near future but still a jtag prob might be connected to the same hardware which is using LLDB to debug a user space application. heres a discussion which happened some years back on similar issues: https://sourceware.org/ml/gdb-patches/2010-01/msg00624.html http://www.spinics.net/lists/arm-kernel/msg80476.html I am fine with using bkpt as long as it helps us debug IT blocks and doesnt interfere with any of our currently functionality. http://reviews.llvm.org/D16853 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid added a comment. Is this good to go ? http://reviews.llvm.org/D16975 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid updated this revision to Diff 47625.
omjavaid added a comment.
I have added an else case for unhandled or error cases.
I have added to Todo for handling complex and vector types. ABI document doesnt
say much about how complex will be returned so I am doing a bit investigation
with how they are being managed by compiler. I will handle them both in next
patch.
http://reviews.llvm.org/D16975
Files:
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,82 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count =
compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (float_count == 1 && !is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size,
0));
+const size_t base_byte_size =
base_type.GetByteSize(nullptr);
+uint32_t data_offset = 0;
+
+for (uint32_t reg_index = 0; reg_index <
homogeneous_count; reg_index++)
+{
+uint32_t regnum = 0;
+
+if (base_byte_size == 4)
+regnum = dwarf_s0 + reg_index;
+else if (base_byte_size == 8)
+regnum = dwarf_d0 + reg_index;
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfo (eRegisterKindDWARF, regnum);
+if (reg_info == NULL)
+break;
+
+RegisterValue reg_value;
+if (!reg_ctx->ReadRegister(reg_info, reg_value))
+break;
+
+// Make sure we have enough room in "data_sp"
+if ((data_offset + base_byte_size) <=
data_sp->GetByteSize())
+{
+Error error;
+const size_t bytes_copied =
reg_value.GetAsMemoryData (reg_info,
+
data_sp->GetBytes() + data_offset,
+
base_byte_size,
+
byte_order,
+
error);
+if (bytes_copied != base_byte_size)
+break;
+
+data_offset += bytes_copied;
+}
+}
+
+if (data_offset == byte_size)
+{
+DataExtractor data;
+data.SetByteOrder(byte_order);
+
data.SetAddressByteSize(process_sp->GetAddressByteSize());
+data.SetData(data_sp);
+
+return ValueObjectConstResult::Create (&thread,
compiler_type, ConstString(""), data);
+}
+else
+{ // Some error occurred while getting values from
registers
+return return_valobj_sp;
+}
+
+}
+else
+{ // TODO: Add code to handle complex and vector types.
+return return_valobj_sp;
+}
+}
+}
+}
+
if (byte_size <= 4)
{
RegisterValue r0_reg_value;
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,82 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homo
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
omjavaid added inline comments.
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:658
@@ -582,2 +657,3 @@
+
if (byte_size <= 4)
{
Homogenous types with elements more than 4 will fall through and will be
handled by this even in case of hardfloat ABI so we cannot make it an elseif
here.
http://reviews.llvm.org/D16975
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D16975: Handle floating-point type homogeneous aggregate return values in ABISysV_arm
This revision was automatically updated to reflect the committed changes.
Closed by commit rL260512: Handle floating-point type homogeneous aggregate
return values in ABISysV_arm (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D16975?vs=47625&id=47626#toc
Repository:
rL LLVM
http://reviews.llvm.org/D16975
Files:
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,82 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uint32_t homogeneous_count =
compiler_type.IsHomogeneousAggregate (&base_type);
+
+if (homogeneous_count > 0 && homogeneous_count <= 4)
+{
+if (base_type.IsFloatingPointType(float_count, is_complex))
+{
+if (float_count == 1 && !is_complex)
+{
+ProcessSP process_sp (thread.GetProcess());
+ByteOrder byte_order = process_sp->GetByteOrder();
+
+DataBufferSP data_sp (new DataBufferHeap(byte_size,
0));
+const size_t base_byte_size =
base_type.GetByteSize(nullptr);
+uint32_t data_offset = 0;
+
+for (uint32_t reg_index = 0; reg_index <
homogeneous_count; reg_index++)
+{
+uint32_t regnum = 0;
+
+if (base_byte_size == 4)
+regnum = dwarf_s0 + reg_index;
+else if (base_byte_size == 8)
+regnum = dwarf_d0 + reg_index;
+else
+break;
+
+const RegisterInfo *reg_info =
reg_ctx->GetRegisterInfo (eRegisterKindDWARF, regnum);
+if (reg_info == NULL)
+break;
+
+RegisterValue reg_value;
+if (!reg_ctx->ReadRegister(reg_info, reg_value))
+break;
+
+// Make sure we have enough room in "data_sp"
+if ((data_offset + base_byte_size) <=
data_sp->GetByteSize())
+{
+Error error;
+const size_t bytes_copied =
reg_value.GetAsMemoryData (reg_info,
+
data_sp->GetBytes() + data_offset,
+
base_byte_size,
+
byte_order,
+
error);
+if (bytes_copied != base_byte_size)
+break;
+
+data_offset += bytes_copied;
+}
+}
+
+if (data_offset == byte_size)
+{
+DataExtractor data;
+data.SetByteOrder(byte_order);
+
data.SetAddressByteSize(process_sp->GetAddressByteSize());
+data.SetData(data_sp);
+
+return ValueObjectConstResult::Create (&thread,
compiler_type, ConstString(""), data);
+}
+else
+{ // Some error occurred while getting values from
registers
+return return_valobj_sp;
+}
+
+}
+else
+{ // TODO: Add code to handle complex and vector types.
+return return_valobj_sp;
+}
+}
+}
+}
+
if (byte_size <= 4)
{
RegisterValue r0_reg_value;
Index: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -579,6 +579,82 @@
else if (compiler_type.IsAggregateType())
{
size_t byte_size = compiler_type.GetByteSize(&thread);
+if (IsArmHardFloat(thread))
+{
+CompilerType base_type;
+const uin
[Lldb-commits] [PATCH] D17501: Fix test for homogeneity in case of aggregate consisting of containerized vector types
omjavaid created this revision.
omjavaid added a reviewer: clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added a subscriber: aemerson.
This patch ClangASTContext::IsHomogeneousAggregate test for homogeneity in
light of Arm procedure call standard definition of a homogeneous aggregate
given below:
A Homogeneous Aggregate is a Composite Type where all of the Fundamental Data
Types that compose the type
are the same. The test for homogeneity is applied after data layout is
completed and without regard to access
control or other source language restrictions.
An aggregate consisting of containerized vector types is treated as homogeneous
if all the members are of the
same size, even if the internal format of the containerized members are
different. For example, a structure
containing a vector of 8 bytes and a vector of 4 half-words satisfies the
requirements for a homogeneous
aggregate.
A Homogenous Aggregate has a Base Type, which is the Fundamental Data Type of
each Element. The overall
size is the size of the Base Type multiplied by the number of Elements; its
alignment will be the alignment of the
Base Type.
ClangASTContext::IsHomogeneousAggregate functions is currently only used by ARM
ABI and I have not seen a conflicting definition of a Homogenous Aggregate
elsewhere so seems safe to use ARM's definition as is.
LGTM?
http://reviews.llvm.org/D17501
Files:
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3100,9 +3100,11 @@
bool is_hva = false;
bool is_hfa = false;
clang::QualType base_qual_type;
+uint64_t base_bitwidth = 0;
for (field_pos = record_decl->field_begin(); field_pos
!= field_end; ++field_pos)
{
clang::QualType field_qual_type =
field_pos->getType();
+uint64_t field_bitwidth =
getASTContext()->getTypeSize (qual_type);
if (field_qual_type->isFloatingType())
{
if (field_qual_type->isComplexType())
@@ -3123,22 +3125,21 @@
}
else if (field_qual_type->isVectorType() ||
field_qual_type->isExtVectorType())
{
-const clang::VectorType *array =
field_qual_type.getTypePtr()->getAs();
-if (array && array->getNumElements() <= 4)
+if (num_fields == 0)
{
-if (num_fields == 0)
-base_qual_type =
array->getElementType();
-else
-{
-if (is_hfa)
-return 0;
-is_hva = true;
-if (field_qual_type.getTypePtr() !=
base_qual_type.getTypePtr())
-return 0;
-}
+base_qual_type = field_qual_type;
+base_bitwidth = field_bitwidth;
}
else
-return 0;
+{
+if (is_hfa)
+return 0;
+is_hva = true;
+if (base_bitwidth != field_bitwidth)
+return 0;
+if (field_qual_type.getTypePtr() !=
base_qual_type.getTypePtr())
+return 0;
+}
}
else
return 0;
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3100,9 +3100,11 @@
bool is_hva = false;
bool is_hfa = false;
clang::QualType base_qual_type;
+uint64_t base_bitwidth = 0;
for (field_pos = record_decl->field_begin(); field_pos != field_end; ++field_pos)
{
clang::QualType field_qual_type = field_pos->getType();
+uint64_t field_bitwidth = getASTContext()->getTypeSize (qual_type);
Re: [Lldb-commits] [PATCH] D17501: Fix test for homogeneity in case of aggregate consisting of containerized vector types
omjavaid added a comment. Just to mention this simplifies vector return type handling for ARM. A patch implementing that is following up if this gets approves. http://reviews.llvm.org/D17501 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D17501: Fix test for homogeneity in case of aggregate consisting of containerized vector types
This revision was automatically updated to reflect the committed changes.
Closed by commit rL261734: Fix test for homogeneity in case of aggregate
consisting of containerized… (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D17501?vs=48663&id=48909#toc
Repository:
rL LLVM
http://reviews.llvm.org/D17501
Files:
lldb/trunk/source/Symbol/ClangASTContext.cpp
Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -3100,9 +3100,11 @@
bool is_hva = false;
bool is_hfa = false;
clang::QualType base_qual_type;
+uint64_t base_bitwidth = 0;
for (field_pos = record_decl->field_begin(); field_pos
!= field_end; ++field_pos)
{
clang::QualType field_qual_type =
field_pos->getType();
+uint64_t field_bitwidth =
getASTContext()->getTypeSize (qual_type);
if (field_qual_type->isFloatingType())
{
if (field_qual_type->isComplexType())
@@ -3123,22 +3125,21 @@
}
else if (field_qual_type->isVectorType() ||
field_qual_type->isExtVectorType())
{
-const clang::VectorType *array =
field_qual_type.getTypePtr()->getAs();
-if (array && array->getNumElements() <= 4)
+if (num_fields == 0)
{
-if (num_fields == 0)
-base_qual_type =
array->getElementType();
-else
-{
-if (is_hfa)
-return 0;
-is_hva = true;
-if (field_qual_type.getTypePtr() !=
base_qual_type.getTypePtr())
-return 0;
-}
+base_qual_type = field_qual_type;
+base_bitwidth = field_bitwidth;
}
else
-return 0;
+{
+if (is_hfa)
+return 0;
+is_hva = true;
+if (base_bitwidth != field_bitwidth)
+return 0;
+if (field_qual_type.getTypePtr() !=
base_qual_type.getTypePtr())
+return 0;
+}
}
else
return 0;
Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -3100,9 +3100,11 @@
bool is_hva = false;
bool is_hfa = false;
clang::QualType base_qual_type;
+uint64_t base_bitwidth = 0;
for (field_pos = record_decl->field_begin(); field_pos != field_end; ++field_pos)
{
clang::QualType field_qual_type = field_pos->getType();
+uint64_t field_bitwidth = getASTContext()->getTypeSize (qual_type);
if (field_qual_type->isFloatingType())
{
if (field_qual_type->isComplexType())
@@ -3123,22 +3125,21 @@
}
else if (field_qual_type->isVectorType() || field_qual_type->isExtVectorType())
{
-const clang::VectorType *array = field_qual_type.getTypePtr()->getAs();
-if (array && array->getNumElements() <= 4)
+if (num_fields == 0)
{
-if (num_fields == 0)
-base_qual_type = array->getElementType();
-else
-{
-if (is_hfa)
-return 0;
-
[Lldb-commits] [PATCH] D17708: Add/Improve complex, vector, aggregate types handling for SysV ARM (hard/soft) ABI.
omjavaid created this revision.
omjavaid added a reviewer: tberghammer.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch adds code to SysV ARM ABI for handling complex and aggregates
containing complex return types.
It also improves support for handling vectors and aggregates containing vector
return types for both ARM hard and soft ABIs.
This patch further enables vector value tests for both gcc and clang.
There is some work in progress to add more comprehensive return value tests and
will be followed up in couple of days.
http://reviews.llvm.org/D17708
Files:
packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -452,12 +452,16 @@
bool is_signed;
bool is_complex;
uint32_t float_count;
+bool is_vfp_candidate = false;
+uint8_t vfp_count = 0;
+uint8_t vfp_byte_size = 0;
// Get the pointer to the first stack argument so we have a place to start
// when reading data
const RegisterInfo *r0_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG1);
size_t bit_width = compiler_type.GetBitSize(&thread);
+size_t byte_size = compiler_type.GetByteSize(&thread);
if (compiler_type.IsIntegerType (is_signed))
{
@@ -504,8 +508,13 @@
}
else if (compiler_type.IsVectorType(nullptr, nullptr))
{
-size_t byte_size = compiler_type.GetByteSize(&thread);
-if (byte_size <= 16)
+if (IsArmHardFloat(thread) && (byte_size == 8 || byte_size == 16))
+{
+is_vfp_candidate = true;
+vfp_byte_size = 8;
+vfp_count = (byte_size == 8?1:2);
+}
+else if (byte_size <= 16)
{
DataBufferHeap buffer(16, 0);
uint32_t* buffer_ptr = (uint32_t*)buffer.GetBytes();
@@ -576,84 +585,84 @@
}
else
{
+if (is_complex && float_count == 2)
+{
+if (IsArmHardFloat(thread))
+{
+is_vfp_candidate = true;
+vfp_byte_size = byte_size / 2;
+vfp_count = 2;
+}
+else if (!GetReturnValuePassedInMemory(thread, reg_ctx, bit_width / 8, value))
+return return_valobj_sp;
+}
+else
// not handled yet
return return_valobj_sp;
}
}
else if (compiler_type.IsAggregateType())
{
-size_t byte_size = compiler_type.GetByteSize(&thread);
if (IsArmHardFloat(thread))
{
CompilerType base_type;
const uint32_t homogeneous_count = compiler_type.IsHomogeneousAggregate (&base_type);
if (homogeneous_count > 0 && homogeneous_count <= 4)
{
-if (base_type.IsFloatingPointType(float_count, is_complex))
+if (base_type.IsVectorType(nullptr, nullptr))
+{
+uint64_t base_byte_size = base_type.GetByteSize(nullptr);
+if (base_byte_size == 8 || base_byte_size == 16)
+{
+is_vfp_candidate = true;
+vfp_byte_size = 8;
+vfp_count = (base_type.GetByteSize(nullptr) == 8 ? homogeneous_count : homogeneous_count * 2);
+}
+}
+else if (base_type.IsFloatingPointType(float_count, is_complex))
{
if (float_count == 1 && !is_complex)
{
-ProcessSP process_sp (thread.GetProcess());
-ByteOrder byte_order = process_sp->GetByteOrder();
+is_vfp_candidate = true;
+vfp_byte_size = base_type.GetByteSize(nullptr);
+vfp_count = homogeneous_count;
+}
+}
+}
+else if (homogeneous_count == 0)
+{
+const uint32_t num_children = compiler_type.GetNumFields ();
-DataBufferSP data_sp (new DataBufferHeap(byte_size, 0));
-const size_t base_byte_size = base_type.GetByteSize(nullptr);
-uint32_t data_offset = 0;
+if (num_children > 0 && num_children <=2)
+{
+uint32_t index = 0;
+for (index = 0; index < num_children; index++)
+{
+std::string name;
+base_type = compiler_type.GetFieldAtIndex (index, name, NULL, NULL, NULL);
-
Re: [Lldb-commits] [PATCH] D17708: Add/Improve complex, vector, aggregate types handling for SysV ARM (hard/soft) ABI.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL262218: Add/Improve complex, vector, aggregate types
handling for SysV ARM… (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D17708?vs=49353&id=49364#toc
Repository:
rL LLVM
http://reviews.llvm.org/D17708
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
Index: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -452,12 +452,16 @@
bool is_signed;
bool is_complex;
uint32_t float_count;
+bool is_vfp_candidate = false;
+uint8_t vfp_count = 0;
+uint8_t vfp_byte_size = 0;
// Get the pointer to the first stack argument so we have a place to start
// when reading data
const RegisterInfo *r0_reg_info = reg_ctx->GetRegisterInfo(eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG1);
size_t bit_width = compiler_type.GetBitSize(&thread);
+size_t byte_size = compiler_type.GetByteSize(&thread);
if (compiler_type.IsIntegerType (is_signed))
{
@@ -504,8 +508,13 @@
}
else if (compiler_type.IsVectorType(nullptr, nullptr))
{
-size_t byte_size = compiler_type.GetByteSize(&thread);
-if (byte_size <= 16)
+if (IsArmHardFloat(thread) && (byte_size == 8 || byte_size == 16))
+{
+is_vfp_candidate = true;
+vfp_byte_size = 8;
+vfp_count = (byte_size == 8?1:2);
+}
+else if (byte_size <= 16)
{
DataBufferHeap buffer(16, 0);
uint32_t* buffer_ptr = (uint32_t*)buffer.GetBytes();
@@ -574,86 +583,83 @@
}
}
}
-else
+else if (is_complex && float_count == 2)
{
+if (IsArmHardFloat(thread))
+{
+is_vfp_candidate = true;
+vfp_byte_size = byte_size / 2;
+vfp_count = 2;
+}
+else if (!GetReturnValuePassedInMemory(thread, reg_ctx, bit_width / 8, value))
+return return_valobj_sp;
+}
+else
// not handled yet
return return_valobj_sp;
-}
}
else if (compiler_type.IsAggregateType())
{
-size_t byte_size = compiler_type.GetByteSize(&thread);
if (IsArmHardFloat(thread))
{
CompilerType base_type;
const uint32_t homogeneous_count = compiler_type.IsHomogeneousAggregate (&base_type);
if (homogeneous_count > 0 && homogeneous_count <= 4)
{
-if (base_type.IsFloatingPointType(float_count, is_complex))
+if (base_type.IsVectorType(nullptr, nullptr))
+{
+uint64_t base_byte_size = base_type.GetByteSize(nullptr);
+if (base_byte_size == 8 || base_byte_size == 16)
+{
+is_vfp_candidate = true;
+vfp_byte_size = 8;
+vfp_count = (base_type.GetByteSize(nullptr) == 8 ? homogeneous_count : homogeneous_count * 2);
+}
+}
+else if (base_type.IsFloatingPointType(float_count, is_complex))
{
if (float_count == 1 && !is_complex)
{
-ProcessSP process_sp (thread.GetProcess());
-ByteOrder byte_order = process_sp->GetByteOrder();
+is_vfp_candidate = true;
+vfp_byte_size = base_type.GetByteSize(nullptr);
+vfp_count = homogeneous_count;
+}
+}
+}
+else if (homogeneous_count == 0)
+{
+const uint32_t num_children = compiler_type.GetNumFields ();
-DataBufferSP data_sp (new DataBufferHeap(byte_size, 0));
-const size_t base_byte_size = base_type.GetByteSize(nullptr);
-uint32_t data_offset = 0;
+if (num_children > 0 && num_children <=2)
+{
+uint32_t index = 0;
+for (index = 0; index < num_children; index++)
+{
+std::string name;
+base_type = compiler_type.GetFieldAtIndex (index, name, NULL, NULL, NULL);
-for (uint32_t reg_index = 0; reg_index < homogeneous_count; reg_index++)
+if (base_type.IsFloatingPointType(float_count, is_complex))
{
-uint32_t regnum = 0;
-
-
[Lldb-commits] [PATCH] D17716: Add complex and aggregate with vector types return value test cases
omjavaid created this revision.
omjavaid added a reviewer: tberghammer.
omjavaid added a subscriber: lldb-commits.
This patch adds tests to test complex return types and aggregate return types
with vector elements.
http://reviews.llvm.org/D17716
Files:
packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
Index: packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
===
--- packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
+++ packages/Python/lldbsuite/test/functionalities/return-value/call-func.c
@@ -1,3 +1,4 @@
+#include /* Standard Library of Complex Numbers */
// Some convenient things to return:
static char *g_first_pointer = "I am the first";
static char *g_second_pointer = "I am the second";
@@ -345,6 +346,77 @@
return value;
}
+struct two_vectors_8_bytes
+{
+ vector_size_float32_8 first_vector_8_bytes;
+ vector_size_float32_8 second_vector_8_bytes;
+};
+
+struct two_vectors_8_bytes
+return_two_vectors_8_bytes (struct two_vectors_8_bytes value)
+{
+ return value;
+}
+
+struct four_vectors_16_bytes
+{
+ vector_size_float32_16 first_vector_16_bytes;
+ vector_size_float32_16 second_vector_16_bytes;
+ vector_size_float32_16 third_vector_16_bytes;
+ vector_size_float32_16 fourth_vector_16_bytes;
+};
+
+struct four_vectors_16_bytes
+return_four_vectors_16_bytes (struct four_vectors_16_bytes value)
+{
+ return value;
+}
+
+struct four_vectors_32_bytes
+{
+ vector_size_float32_32 first_vector_32_bytes;
+ vector_size_float32_32 second_vector_32_bytes;
+ vector_size_float32_32 third_vector_32_bytes;
+ vector_size_float32_32 fourth_vector_32_bytes;
+};
+
+struct four_vectors_32_bytes
+return_four_vectors_32_bytes (struct four_vectors_32_bytes value)
+{
+ return value;
+}
+
+double complex
+return_one_complex (double complex value)
+{
+ return value;
+}
+
+struct two_complex
+{
+ double complex z0;
+ double complex z1;
+};
+
+struct two_complex
+return_two_complex (struct two_complex value)
+{
+ return value;
+}
+
+struct three_complex
+{
+ float complex z0;
+ float complex z1;
+ float complex z2;
+};
+
+struct three_complex
+return_three_complex (struct three_complex value)
+{
+ return value;
+}
+
int
main ()
{
@@ -395,10 +467,22 @@
return_one_int_one_double_packed ((struct one_int_one_double_packed) {10, 20.0});
return_one_int_one_long ((struct one_int_one_long) {10, 20});
+ return_one_complex ((complex) {(1.1,2.1)});
+ return_two_complex ((struct two_complex) {(7.89, 8.52), (6.31, 9.12)});
+ return_three_complex ((struct three_complex) {(7.89, 8.52), (6.31, 9.12), (1.1,2.1)});
+
return_vector_size_float32_8 (( vector_size_float32_8 ){1.5, 2.25});
return_vector_size_float32_16 (( vector_size_float32_16 ){1.5, 2.25, 4.125, 8.0625});
return_vector_size_float32_32 (( vector_size_float32_32 ){1.5, 2.25, 4.125, 8.0625, 7.89, 8.52, 6.31, 9.12});
+ return_two_vectors_8_bytes ((struct two_vectors_8_bytes ){{1.5, 2.25}, {4.125, 8.0625}});
+ return_four_vectors_16_bytes ((struct four_vectors_16_bytes ){{1.5, 2.25, 4.125, 8.0625}, {7.89, 8.52, 6.31, 9.12},
+{9.80, 1.52, 7.31, 6.33}, {1.188, 4.61, 0.31, 0.12}});
+ return_four_vectors_32_bytes ((struct four_vectors_32_bytes ){{1.5, 2.25, 4.125, 8.0625, 7.89, 8.52, 6.31, 9.12},
+{9.80, 1.52, 7.31, 6.33, 1.188, 4.61, 0.31, 0.12},
+{1.1, 2.15, 0.125, 3.0625, 0.89, 4.52, 2.31, 8.12},
+{0.80, 1.52, 7.77, 2.26, 8.188, 0.61, 5.31, 77.12}});
+
return_ext_vector_size_float32_2 ((ext_vector_size_float32_2){ 16.5, 32.25});
return_ext_vector_size_float32_4 ((ext_vector_size_float32_4){ 16.5, 32.25, 64.125, 128.0625});
return_ext_vector_size_float32_8 ((ext_vector_size_float32_8){ 16.5, 32.25, 64.125, 128.0625, 1.59, 3.57, 8.63, 9.12 });
Index: packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
===
--- packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
+++ packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
@@ -151,9 +151,17 @@
#self.return_and_test_struct_value ("return_one_int_one_double_packed")
self.return_and_test_struct_value ("return_one_int_one_long")
+self.return_and_test_struct_value ("return_one_complex")
+self.return_and_test_struct_value ("return_two_complex")
+self.return_and_test_struct_value ("return_three_complex")
+
self.return_and_test_struct_value ("return_vector_size_float32_8")
self.return_and_test_struct_value ("return_vector_si
Re: [Lldb-commits] [PATCH] D18059: Add IR fixups for RenderScript ABI mismatch between ARMV7 frontend and x86 backend
omjavaid added a reviewer: clayborg. omjavaid added a comment. I dont have a lot of background in this area of the code. Can you kindly take a look. http://reviews.llvm.org/D18059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
omjavaid added a comment.
Seems legit but One cosmetic comment inline.
Also have you tested this patch by running LLDB testsuite on arm in both little
and big endian modes?
Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:157
@@ -149,12 +156,3 @@
{
-if (size > 8)
-return false;
-
-if (size <= 4)
-m_memory[p_address] = value;
-else if (size == 8)
-{
-m_memory[p_address] = (value << 32) >> 32;
-m_memory[p_address + 4] = value << 32;
-}
+m_memory[p_address] = value;
return true;
m_memory is a map with map type uint32? I think we will end up loosing data
here if its larger than 4 bytes. if StoreToPseudoAddress isnt used elsewhere
better change value from uint64 to uint32 ? Also size seems to be a redundant
argument now.
http://reviews.llvm.org/D18984
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
omjavaid accepted this revision. omjavaid added a comment. LGTM http://reviews.llvm.org/D18984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D19252: Handle invalid values of PLT entry size generated by ld + gcc on arm linux targets.
omjavaid created this revision.
omjavaid added reviewers: tberghammer, rengolin, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: danalbert, tberghammer, rengolin, aemerson.
This patch provides a fix for wrong plt entry size generated for binaries built
with gcc and linked with ld for arm linux targets.
Many tests fail on arm-linux targets for this very issues. Luckily on Android
arm32 targets we get a zero size for which there is already a fix available in
the code.
Effect of this patch appears when code jumps into plt code and tries to
calculate frame for current PC. A wrong calculation of plt entry addresses
ranges results in failure to calculate frame hence stepping failures when
dealing with any library functions using procedure linkage table.
LD produces 12 byte plt entries for arm and can also produce 16 byte entries
but by no means plt entry can be 4 bytes which appears while we decode plt
header.
No other architecture in my knowledge uses a PLT slot of less than or equal to
4bytes. I could be wrong but in my knowledge a PLT slot is at least 2
instructions on a 32bit machine s which is 8 bytes and a lot higher for 64 bit
machines so I have made the code change to handle all casses below or equal 4
bytes with manual calculation.
This fixes issues on arm targets.
LGTM? or comments?
http://reviews.llvm.org/D19252
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2510,7 +2510,7 @@
elf_xword plt_entsize = plt_hdr->sh_addralign ?
llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) :
plt_hdr->sh_entsize;
-if (plt_entsize == 0)
+if (plt_entsize <= 4)
{
// The linker haven't set the plt_hdr->sh_entsize field. Try to guess
the size of the plt
// entries based on the number of entries and the size of the plt
section with the
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2510,7 +2510,7 @@
elf_xword plt_entsize = plt_hdr->sh_addralign ?
llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) : plt_hdr->sh_entsize;
-if (plt_entsize == 0)
+if (plt_entsize <= 4)
{
// The linker haven't set the plt_hdr->sh_entsize field. Try to guess the size of the plt
// entries based on the number of entries and the size of the plt section with the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19252: Handle invalid values of PLT entry size generated by ld + gcc on arm linux targets.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL267405: Handle invalid values of PLT entry size generated by
linker (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D19252?vs=54146&id=54845#toc
Repository:
rL LLVM
http://reviews.llvm.org/D19252
Files:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2610,7 +2610,10 @@
elf_xword plt_entsize = plt_hdr->sh_addralign ?
llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) :
plt_hdr->sh_entsize;
-if (plt_entsize == 0)
+// Some linkers e.g ld for arm, fill plt_hdr->sh_entsize field incorrectly.
+// PLT entries relocation code in general requires multiple instruction and
+// should be greater than 4 bytes in most cases. Try to guess correct size
just in case.
+if (plt_entsize <= 4)
{
// The linker haven't set the plt_hdr->sh_entsize field. Try to guess
the size of the plt
// entries based on the number of entries and the size of the plt
section with the
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2610,7 +2610,10 @@
elf_xword plt_entsize = plt_hdr->sh_addralign ?
llvm::alignTo (plt_hdr->sh_entsize, plt_hdr->sh_addralign) : plt_hdr->sh_entsize;
-if (plt_entsize == 0)
+// Some linkers e.g ld for arm, fill plt_hdr->sh_entsize field incorrectly.
+// PLT entries relocation code in general requires multiple instruction and
+// should be greater than 4 bytes in most cases. Try to guess correct size just in case.
+if (plt_entsize <= 4)
{
// The linker haven't set the plt_hdr->sh_entsize field. Try to guess the size of the plt
// entries based on the number of entries and the size of the plt section with the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D19520: rL267291: Architecture change to thumb on parsing arm.attributes causes regression.
omjavaid created this revision.
omjavaid added reviewers: tberghammer, labath.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
rL267291 introduces a lot of regression on arm-linux by fixing module
architecture to thumb if it finds thumb32 tag set.
Tag_THUMB_ISA_use, (=9), uleb128
2 32-bit Thumb instructions were permitted (implies 16-bit instructions
permitted)
Does not mean that there wont be any arm instruction in current module.
Therefore we can not set the architecture to thumb without knowing the value of
Tag_ARM_ISA_use, (=8), uleb128
0 The user did not permit this entity to use ARM instructions
1 The user intended that this entity could use ARM instructions
For most cases Tag_ARM_ISA_use, (=8), uleb128 will be set to 1 that will force
us to use arm as our architecture instead of thumb.
I am removing this code for now may be we can come up with a better solution to
get it over with.
http://reviews.llvm.org/D19520
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1564,19 +1564,6 @@
break;
-case llvm::ARMBuildAttrs::THUMB_ISA_use:
-{
-uint64_t ThumbISA = data.GetULEB128(&Offset);
-
-// NOTE: ignore ThumbISA ==
llvm::ARMBuildAttrs::AllowThumbDerived
-// since that derives it based on the architecutre/profile
-if (ThumbISA == llvm::ARMBuildAttrs::AllowThumb32)
-if (arch_spec.GetTriple().getArch() ==
llvm::Triple::UnknownArch ||
-arch_spec.GetTriple().getArch() ==
llvm::Triple::arm)
-arch_spec.GetTriple().setArch(llvm::Triple::thumb);
-
-break;
-}
case llvm::ARMBuildAttrs::ABI_VFP_args:
{
uint64_t VFPArgs = data.GetULEB128(&Offset);
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1564,19 +1564,6 @@
break;
-case llvm::ARMBuildAttrs::THUMB_ISA_use:
-{
-uint64_t ThumbISA = data.GetULEB128(&Offset);
-
-// NOTE: ignore ThumbISA == llvm::ARMBuildAttrs::AllowThumbDerived
-// since that derives it based on the architecutre/profile
-if (ThumbISA == llvm::ARMBuildAttrs::AllowThumb32)
-if (arch_spec.GetTriple().getArch() == llvm::Triple::UnknownArch ||
-arch_spec.GetTriple().getArch() == llvm::Triple::arm)
-arch_spec.GetTriple().setArch(llvm::Triple::thumb);
-
-break;
-}
case llvm::ARMBuildAttrs::ABI_VFP_args:
{
uint64_t VFPArgs = data.GetULEB128(&Offset);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19520: rL267291: Architecture change to thumb on parsing arm.attributes causes regression.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL267550: rL267291: Architecture change to thumb on parsing
arm.attributes causes… (authored by omjavaid).
Changed prior to commit:
http://reviews.llvm.org/D19520?vs=54961&id=54993#toc
Repository:
rL LLVM
http://reviews.llvm.org/D19520
Files:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1564,19 +1564,6 @@
break;
-case llvm::ARMBuildAttrs::THUMB_ISA_use:
-{
-uint64_t ThumbISA = data.GetULEB128(&Offset);
-
-// NOTE: ignore ThumbISA ==
llvm::ARMBuildAttrs::AllowThumbDerived
-// since that derives it based on the architecutre/profile
-if (ThumbISA == llvm::ARMBuildAttrs::AllowThumb32)
-if (arch_spec.GetTriple().getArch() ==
llvm::Triple::UnknownArch ||
-arch_spec.GetTriple().getArch() ==
llvm::Triple::arm)
-arch_spec.GetTriple().setArch(llvm::Triple::thumb);
-
-break;
-}
case llvm::ARMBuildAttrs::ABI_VFP_args:
{
uint64_t VFPArgs = data.GetULEB128(&Offset);
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1564,19 +1564,6 @@
break;
-case llvm::ARMBuildAttrs::THUMB_ISA_use:
-{
-uint64_t ThumbISA = data.GetULEB128(&Offset);
-
-// NOTE: ignore ThumbISA == llvm::ARMBuildAttrs::AllowThumbDerived
-// since that derives it based on the architecutre/profile
-if (ThumbISA == llvm::ARMBuildAttrs::AllowThumb32)
-if (arch_spec.GetTriple().getArch() == llvm::Triple::UnknownArch ||
-arch_spec.GetTriple().getArch() == llvm::Triple::arm)
-arch_spec.GetTriple().setArch(llvm::Triple::thumb);
-
-break;
-}
case llvm::ARMBuildAttrs::ABI_VFP_args:
{
uint64_t VFPArgs = data.GetULEB128(&Offset);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D11899: Fix AArch64 watchpoint handlers in NativeRegisterContextLinux_arm64
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
There were some bugs that needed to be fixed in watchpoint handling code on
arm64.
Watchpoints were being written to all watchpoint registers and cache was not
being maintained correctly.
This patch fixes up all these issues. Watchpoints now work on arm64 except that
there is race condition which is not allowing lldb to step out of a watchpoint
hit before we can continue again so inferior gets stuck at that point. Manually
disabling the watchpoint then issuing a step and then enabling the watchpoint
again fixes shows that watchpoints are being installed and removed correctly.
http://reviews.llvm.org/D11899
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -172,10 +172,10 @@
IsFPR(unsigned reg) const;
Error
-ReadHardwareDebugInfo(unsigned int &watch_count , unsigned int &break_count);
+ReadHardwareDebugInfo();
Error
-WriteHardwareDebugRegs(lldb::addr_t *addr_buf, uint32_t *cntrl_buf, int type, int count);
+WriteHardwareDebugRegs(int type);
};
} // namespace process_linux
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -391,14 +391,10 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
-NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
-if (!process_sp)
-return false;
-
// Check if our hardware breakpoint and watchpoint information is updated.
if (m_refresh_hwdebug_info)
{
-ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
+ReadHardwareDebugInfo ();
m_refresh_hwdebug_info = false;
}
@@ -443,7 +439,8 @@
m_hbr_regs[bp_index].control = control_value;
m_hbr_regs[bp_index].refcount = 1;
-//TODO: PTRACE CALL HERE for an UPDATE
+// PTRACE call to set corresponding hardware breakpoint register.
+WriteHardwareDebugRegs(1);
}
else
m_hbr_regs[bp_index].refcount++;
@@ -459,6 +456,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+// Check if our hardware breakpoint and watchpoint information is updated.
+if (m_refresh_hwdebug_info)
+{
+ReadHardwareDebugInfo ();
+m_refresh_hwdebug_info = false;
+}
+
if (hw_idx >= m_max_hbp_supported)
return false;
@@ -474,8 +478,8 @@
m_hbr_regs[hw_idx].address = 0;
m_hbr_regs[hw_idx].refcount = 0;
-//TODO: PTRACE CALL HERE for an UPDATE
-return true;
+// PTRACE call to clear corresponding hardware breakpoint register.
+WriteHardwareDebugRegs(1);
}
return false;
@@ -489,6 +493,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+// Check if our hardware breakpoint and watchpoint information is updated.
+if (m_refresh_hwdebug_info)
+{
+ReadHardwareDebugInfo ();
+m_refresh_hwdebug_info = false;
+}
+
return m_max_hwp_supported;
}
@@ -499,33 +510,31 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
-
-NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
-if (!process_sp)
-return false;
-
// Check if our hardware breakpoint and watchpoint information is updated.
if (m_refresh_hwdebug_info)
{
-ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
+ReadHardwareDebugInfo ();
m_refresh_hwdebug_info = false;
}
uint32_t control_value, wp_index;
-
+// Check if we are setting watchpoint other than read/write/access
if (watch_flags != 0x1 && watch_flags != 0x2 && watch_flags != 0x3)
-return 0;//Error ("Invalid read/write bits for watchpoint");
+return LLDB_INVALID_INDEX32;
// Check if size has a valid hardware watchpoint length.
if (size != 1 && size != 2 && size != 4 && size != 8)
-return 0;//Error ("Invalid size for watchpoint");
+return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
// TODO: Add support for watching un-aligned addresses
if (addr & 0x07)
-r
[Lldb-commits] [PATCH] D11902: Fix LLGS to enable read type watchpoints
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
LLGS was forcing lldb to use only write or read+write type watchpoints.
This patch fixes this behavior to enable read, write and read+write types of
watchpoints.
Note: On x86_64 read watchpoints still dont work and they are disabled by
NativeRegisterContextLinux_x86. We ll need to revisit this later because all
three types of watchpoints should also work on x86.
http://reviews.llvm.org/D11902
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2227,6 +2227,7 @@
bool want_breakpoint = true;
bool want_hardware = false;
+uint32_t watch_flags;
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32 (eStoppointInvalid));
@@ -2237,10 +2238,13 @@
case eBreakpointHardware:
want_hardware = true; want_breakpoint = true; break;
case eWatchpointWrite:
+watch_flags = 1;
want_hardware = true; want_breakpoint = false; break;
case eWatchpointRead:
+watch_flags = 2;
want_hardware = true; want_breakpoint = false; break;
case eWatchpointReadWrite:
+watch_flags = 3;
want_hardware = true; want_breakpoint = false; break;
case eStoppointInvalid:
return SendIllFormedResponse(packet, "Z packet had invalid
software/hardware specifier");
@@ -2280,11 +2284,6 @@
}
else
{
-uint32_t watch_flags =
-stoppoint_type == eWatchpointWrite
-? 0x1 // Write
-: 0x3; // ReadWrite
-
// Try to set the watchpoint.
const Error error = m_debugged_process_sp->SetWatchpoint (
addr, size, watch_flags, want_hardware);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -1018,6 +1018,9 @@
if (wp_index >= NumSupportedHardwareWatchpoints())
return Error ("Watchpoint index out of range");
+if (watch_flags == 0x2)
+return Error ("Read watchpoints currently unsupported on x86_64
architecture");
+
if (watch_flags != 0x1 && watch_flags != 0x3)
return Error ("Invalid read/write bits for watchpoint");
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2227,6 +2227,7 @@
bool want_breakpoint = true;
bool want_hardware = false;
+uint32_t watch_flags;
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32 (eStoppointInvalid));
@@ -2237,10 +2238,13 @@
case eBreakpointHardware:
want_hardware = true; want_breakpoint = true; break;
case eWatchpointWrite:
+watch_flags = 1;
want_hardware = true; want_breakpoint = false; break;
case eWatchpointRead:
+watch_flags = 2;
want_hardware = true; want_breakpoint = false; break;
case eWatchpointReadWrite:
+watch_flags = 3;
want_hardware = true; want_breakpoint = false; break;
case eStoppointInvalid:
return SendIllFormedResponse(packet, "Z packet had invalid software/hardware specifier");
@@ -2280,11 +2284,6 @@
}
else
{
-uint32_t watch_flags =
-stoppoint_type == eWatchpointWrite
-? 0x1 // Write
-: 0x3; // ReadWrite
-
// Try to set the watchpoint.
const Error error = m_debugged_process_sp->SetWatchpoint (
addr, size, watch_flags, want_hardware);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -1018,6 +1018,9 @@
if (wp_index >= NumSupportedHardwareWatchpoints())
return Error ("Watchpoint index out of range");
+if (watch_flags == 0x2)
+return Error ("Read watchpoints currently unsupported on x86_64 architecture");
+
if (watch_flags != 0x1 && watch_flags !=
Re: [Lldb-commits] [PATCH] D11899: Fix AArch64 watchpoint handlers in NativeRegisterContextLinux_arm64
omjavaid updated this revision to Diff 31934.
omjavaid added a comment.
I have updated this patch after incorporating suggestions.
Is it good for commit now?
http://reviews.llvm.org/D11899
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -82,6 +82,13 @@
bool
WatchpointIsEnabled(uint32_t wp_index);
+// Debug register type select
+enum DREGType
+{
+eDREGTypeWATCH = 0,
+eDREGTypeBREAK
+};
+
protected:
Error
DoReadRegisterValue(uint32_t offset,
@@ -172,10 +179,10 @@
IsFPR(unsigned reg) const;
Error
-ReadHardwareDebugInfo(unsigned int &watch_count , unsigned int &break_count);
+ReadHardwareDebugInfo();
Error
-WriteHardwareDebugRegs(lldb::addr_t *addr_buf, uint32_t *cntrl_buf, int type, int count);
+WriteHardwareDebugRegs(int hwbType);
};
} // namespace process_linux
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -391,17 +391,9 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
-NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
-if (!process_sp)
-return false;
+// Read hardware breakpoint and watchpoint information.
+ReadHardwareDebugInfo ();
-// Check if our hardware breakpoint and watchpoint information is updated.
-if (m_refresh_hwdebug_info)
-{
-ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
-m_refresh_hwdebug_info = false;
-}
-
uint32_t control_value, bp_index;
// Check if size has a valid hardware breakpoint length.
@@ -443,7 +435,8 @@
m_hbr_regs[bp_index].control = control_value;
m_hbr_regs[bp_index].refcount = 1;
-//TODO: PTRACE CALL HERE for an UPDATE
+// PTRACE call to set corresponding hardware breakpoint register.
+WriteHardwareDebugRegs(eDREGTypeBREAK);
}
else
m_hbr_regs[bp_index].refcount++;
@@ -459,6 +452,9 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+// Read hardware breakpoint and watchpoint information.
+ReadHardwareDebugInfo ();
+
if (hw_idx >= m_max_hbp_supported)
return false;
@@ -474,8 +470,8 @@
m_hbr_regs[hw_idx].address = 0;
m_hbr_regs[hw_idx].refcount = 0;
-//TODO: PTRACE CALL HERE for an UPDATE
-return true;
+// PTRACE call to clear corresponding hardware breakpoint register.
+WriteHardwareDebugRegs(eDREGTypeBREAK);
}
return false;
@@ -489,6 +485,9 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+// Read hardware breakpoint and watchpoint information.
+ReadHardwareDebugInfo ();
+
return m_max_hwp_supported;
}
@@ -499,33 +498,36 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
-
-NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
-if (!process_sp)
-return false;
-
-// Check if our hardware breakpoint and watchpoint information is updated.
-if (m_refresh_hwdebug_info)
-{
-ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
-m_refresh_hwdebug_info = false;
-}
+// Read hardware breakpoint and watchpoint information.
+ReadHardwareDebugInfo ();
uint32_t control_value, wp_index;
+// Check if we are setting watchpoint other than read/write/access
+// Also update watchpoint flag to match AArch64 write-read bit configuration.
+switch (watch_flags)
+{
+case 1:
+watch_flags = 2;
+break;
+case 2:
+watch_flags = 1;
+break;
+case 3:
+break;
+default:
+return LLDB_INVALID_INDEX32;
+}
-if (watch_flags != 0x1 && watch_flags != 0x2 && watch_flags != 0x3)
-return 0;//Error ("Invalid read/write bits for watchpoint");
-
// Check if size has a valid hardware watchpoint length.
if (size != 1 && size != 2 && size != 4 && size != 8)
-return 0;//Error ("Invalid size for watchpoint");
+return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
// TODO: Add support for watching un-align
[Lldb-commits] [PATCH] D11987: Fix to handle AArch64 watchpoint exception before instruction being watched is executed
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
LLDB needs to know whether a watchpoint instruction is executed before or after
the watchpoint exception is delivered.
This patch fixes this behavior for AArch64 and makes sure lldb disables and
steps over the watchpoint instruction after watchpoint exception is delivered.
http://reviews.llvm.org/D11987
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -182,7 +182,9 @@
response.Printf("watchpoint_exceptions_received:after;");
#else
if (host_arch.GetMachine() == llvm::Triple::mips64 ||
-host_arch.GetMachine() == llvm::Triple::mips64el)
+host_arch.GetMachine() == llvm::Triple::mips64el ||
+host_arch.GetMachine() == llvm::Triple::aarch64 ||
+host_arch.GetMachine() == llvm::Triple::aarch64_be)
response.Printf("watchpoint_exceptions_received:before;");
else
response.Printf("watchpoint_exceptions_received:after;");
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -182,7 +182,9 @@
response.Printf("watchpoint_exceptions_received:after;");
#else
if (host_arch.GetMachine() == llvm::Triple::mips64 ||
-host_arch.GetMachine() == llvm::Triple::mips64el)
+host_arch.GetMachine() == llvm::Triple::mips64el ||
+host_arch.GetMachine() == llvm::Triple::aarch64 ||
+host_arch.GetMachine() == llvm::Triple::aarch64_be)
response.Printf("watchpoint_exceptions_received:before;");
else
response.Printf("watchpoint_exceptions_received:after;");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D9703: Adds support for ARM hardware watchpoints
omjavaid updated this revision to Diff 33075.
omjavaid added a comment.
This updated patches correct problems in arm hardware watchpoint support patch
posted earlier.
This patch has been tested on samsung chromebook (ARM - Linux) and PandaBoard
using basic watchpoint test application.
Also it was tested on Nexus 7 Android device.
On chromebook linux we are able to set and clear all types of watchpoints but
on android we end up getting a watchpoint packet error because we are not able
to call hardware watchpoint ptrace functions successfully.
I still dont have a android device on me that has hardware watchpoints enable
but fact that this functionality now works on linux means that we should get
this into our source code.
I am trying to write a test to check hardware watchpoint capabilities of a
platform so we are pretty sure what works on which platform.
Is this good to commit?
http://reviews.llvm.org/D9703
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -183,6 +183,8 @@
#else
if (host_arch.GetMachine() == llvm::Triple::aarch64 ||
host_arch.GetMachine() == llvm::Triple::aarch64_be ||
+host_arch.GetMachine() == llvm::Triple::arm ||
+host_arch.GetMachine() == llvm::Triple::armeb ||
host_arch.GetMachine() == llvm::Triple::mips64 ||
host_arch.GetMachine() == llvm::Triple::mips64el)
response.Printf("watchpoint_exceptions_received:before;");
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
@@ -48,6 +48,47 @@
Error
WriteAllRegisterValues (const lldb::DataBufferSP &data_sp) override;
+//--
+// Hardware breakpoints/watchpoint mangement functions
+//--
+
+uint32_t
+SetHardwareBreakpoint (lldb::addr_t addr, size_t size) override;
+
+bool
+ClearHardwareBreakpoint (uint32_t hw_idx) override;
+
+uint32_t
+NumSupportedHardwareWatchpoints () override;
+
+uint32_t
+SetHardwareWatchpoint (lldb::addr_t addr, size_t size, uint32_t watch_flags) override;
+
+bool
+ClearHardwareWatchpoint (uint32_t hw_index) override;
+
+Error
+ClearAllHardwareWatchpoints () override;
+
+Error
+GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr) override;
+
+lldb::addr_t
+GetWatchpointAddress (uint32_t wp_index) override;
+
+uint32_t
+GetWatchpointSize(uint32_t wp_index);
+
+bool
+WatchpointIsEnabled(uint32_t wp_index);
+
+// Debug register type select
+enum DREGType
+{
+eDREGTypeWATCH = 0,
+eDREGTypeBREAK
+};
+
protected:
void*
GetGPRBuffer() override { return &m_gpr_arm; }
@@ -94,11 +135,32 @@
RegInfo m_reg_info;
FPU m_fpr;
+// Debug register info for hardware breakpoints and watchpoints management.
+struct DREG
+{
+lldb::addr_t address; // Breakpoint/watchpoint address value.
+uint32_t control; // Breakpoint/watchpoint control value.
+uint32_t refcount; // Serves as enable/disable and refernce counter.
+};
+
+struct DREG m_hbr_regs[16]; // Arm native linux hardware breakpoints
+struct DREG m_hwp_regs[16]; // Arm native linux hardware watchpoints
+
+uint32_t m_max_hwp_supported;
+uint32_t m_max_hbp_supported;
+bool m_refresh_hwdebug_info;
+
bool
IsGPR(unsigned reg) const;
bool
IsFPR(unsigned reg) const;
+
+Error
+ReadHardwareDebugInfo();
+
+Error
+WriteHardwareDebugRegs(int hwbType, int hwb_index);
};
} // namespace process_linux
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -13,6 +13,7 @@
#include "lldb/Core/DataBufferHeap.h"
#include "lldb/Core/Error.h"
+#include "lldb/Core/Log.h"
#include "lldb/Core/RegisterValue.h"
#include "Plug
[Lldb-commits] [PATCH] D12328: Error checking correction in AArch64 hardware watchpoint code
omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
This patch fixes a few areas where AArch64 hardware watchpoints were not
emitting errors correctly.
This makes sure any ptrace failures are reflected in any packet responses from
the server.
http://reviews.llvm.org/D12328
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -391,10 +391,15 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
-uint32_t control_value, bp_index;
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
+
+uint32_t control_value = 0, bp_index = 0;
// Check if size has a valid hardware breakpoint length.
if (size != 4)
@@ -436,7 +441,10 @@
m_hbr_regs[bp_index].refcount = 1;
// PTRACE call to set corresponding hardware breakpoint register.
-WriteHardwareDebugRegs(eDREGTypeBREAK);
+error = WriteHardwareDebugRegs(eDREGTypeBREAK);
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
}
else
m_hbr_regs[bp_index].refcount++;
@@ -452,8 +460,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
if (hw_idx >= m_max_hbp_supported)
return false;
@@ -472,6 +485,11 @@
// PTRACE call to clear corresponding hardware breakpoint register.
WriteHardwareDebugRegs(eDREGTypeBREAK);
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
+
+return true;
}
return false;
@@ -485,8 +503,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
return m_max_hwp_supported;
}
@@ -499,10 +522,15 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
-uint32_t control_value, wp_index;
+uint32_t control_value = 0, wp_index = 0;
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match AArch64 write-read bit configuration.
@@ -562,7 +590,10 @@
m_hwp_regs[wp_index].refcount = 1;
// PTRACE call to set corresponding watchpoint register.
-WriteHardwareDebugRegs(eDREGTypeWATCH);
+error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
}
else
m_hwp_regs[wp_index].refcount++;
@@ -578,8 +609,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
+
+if (error.Fail())
+return LLDB_INVALID_INDEX32;
if (wp_index >= m_max_hwp_supported)
return false;
@@ -598,7 +634,11 @@
m_hwp_regs[wp_index].refcount = 0;
// Ptrace call to update hardware debug registers
-WriteHardwareDebugRegs(eDREGTypeWATCH);
+error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+if (error.Fail())
+return false;
+
return true;
}
@@ -613,8 +653,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+Error error;
+
// Read hardware breakpoint and watchpoint information.
-ReadHardwareDebugInfo ();
+error = ReadHardwareDebugInfo ();
+
+if (error.Fail())
+return error;
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
@@ -626,7 +671,10 @@
m_hwp_regs[i].refcount = 0;
// Ptrace call to update hardware debug registers
-WriteHardwareDebugRegs(eDREGTypeWATCH);
+error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+if (error.Fail())
+
