- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15.1k
 
[lldb][test] Make TestFindRangesInMemory.py more robust #152817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
`GetHeapRanges()` could return two overlapping ranges because it did not check whether `heap_pointer1` lies within the range returned for `heap_pointer2`. This could result in a test failure in `test_find_ranges_in_memory_two_matches` when `process.FindRangesInMemory()` returned 3 instead of 2. The patch ensures that `GetHeapRanges()` returns either two non-overlapping ranges or one range covering both heap pointers.
| 
          
 @llvm/pr-subscribers-lldb Author: Igor Kudrin (igorkudrin) Changes
 The patch ensures that  The issue was probably introduced in #111951 Full diff: https://github.com/llvm/llvm-project/pull/152817.diff 1 Files Affected: 
 diff --git a/lldb/test/API/python_api/find_in_memory/address_ranges_helper.py b/lldb/test/API/python_api/find_in_memory/address_ranges_helper.py
index dcceca6d8a5c5..102f2b0edd4c6 100644
--- a/lldb/test/API/python_api/find_in_memory/address_ranges_helper.py
+++ b/lldb/test/API/python_api/find_in_memory/address_ranges_helper.py
@@ -55,27 +55,34 @@ def GetRangeFromAddrValue(test_base, addr, shrink=False):
     return lldb.SBAddressRange(start, size)
 
 
-def IsWithinRange(addr, size, range, target):
-    start_addr = range.GetBaseAddress().GetLoadAddress(target)
-    end_addr = start_addr + range.GetByteSize()
-    addr = addr.GetValueAsUnsigned()
-    return addr >= start_addr and addr + size <= end_addr
-
-
 def GetHeapRanges(test_base, shrink=False):
     frame = test_base.thread.GetSelectedFrame()
 
     ex = frame.EvaluateExpression("heap_pointer1")
     test_base.assertTrue(ex.IsValid())
-    range = GetRangeFromAddrValue(test_base, ex, shrink)
-    addr_ranges = lldb.SBAddressRangeList()
-    addr_ranges.Append(range)
+    range1 = GetRangeFromAddrValue(test_base, ex, shrink)
+    range1_start = range1.GetBaseAddress().GetLoadAddress(test_base.target)
+    range1_end = range1_start + range1.GetByteSize()
 
     ex = frame.EvaluateExpression("heap_pointer2")
     test_base.assertTrue(ex.IsValid())
-    size = len(DOUBLE_INSTANCE_PATTERN_HEAP)
-    if not IsWithinRange(ex, size, addr_ranges[0], test_base.target):
-        addr_ranges.Append(GetRangeFromAddrValue(test_base, ex, shrink))
+    range2 = GetRangeFromAddrValue(test_base, ex, shrink)
+    range2_start = range2.GetBaseAddress().GetLoadAddress(test_base.target)
+    range2_end = range2_start + range2.GetByteSize()
+
+    addr_ranges = lldb.SBAddressRangeList()
+
+    if range1_end < range2_start or range2_end < range1_start:
+        # The ranges do not overlap; add them both.
+        addr_ranges.Append(range1)
+        addr_ranges.Append(range2)
+    else:
+        # Merge overlapping ranges.
+        base = min(range1_start, range2_start)
+        end = max(range1_end, range2_end)
+        start = lldb.SBAddress(base, test_base.target)
+        size = end - base
+        addr_ranges.Append(lldb.SBAddressRange(start, size))
 
     return addr_ranges
 
 | 
    
| 
           Help me understand, how does   | 
    
          
 Assume the addresses of   | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that makes sense to me. Thanks for the explanation!
GetHeapRanges()could return two overlapping ranges because it did not check whetherheap_pointer1lies within the range returned forheap_pointer2. This could result in a test failure intest_find_ranges_in_memory_two_matcheswhenprocess.FindRangesInMemory()returned 3 instead of 2.The patch ensures that
GetHeapRanges()returns either two non-overlapping ranges or one range covering both heap pointers.The issue was probably introduced in #111951