[lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value

[JDevlieghere]

Summary

Currently, SBTarget::SetModuleLoadAddress cannot accept the large slides needed to load images in high memory. This function should always have taken an unsigned as the slide as it immediately passes it to Target::SetSectionLoadAddress which takes an unsigned. This patch adds an overload and check that the slide is positive for the signed variant.

rdar://101355155

[jasonmolenda]

LGTM. We can detect this This is one of those cases where I'd like to mark SBTarget::SetModuleLoadAddress as deprecated. I like that you detect this situation in SBTarget::SetLoadAddress - we could cast to unsigned and do the right thing, or would the compiler get unhappy? - but when I've seen these kinds of mistakes in the past, python detects a too-large unsigned value being passed to a signed argument and throws an error before we even get to the SetModuleLoadAddress, so it doesn't help much I expect for Python SB API users.

[JDevlieghere]

In D147482#4248858, @jasonmolenda wrote:

LGTM. We can detect this This is one of those cases where I'd like to mark SBTarget::SetModuleLoadAddress as deprecated. I like that you detect this situation in SBTarget::SetLoadAddress - we could cast to unsigned and do the right thing, or would the compiler get unhappy? - but when I've seen these kinds of mistakes in the past, python detects a too-large unsigned value being passed to a signed argument and throws an error before we even get to the SetModuleLoadAddress, so it doesn't help much I expect for Python SB API users.

Yeah, what you described is exactly what would happen before this patch, the function will "just work" from C++ but not from Python. I added the check to discourage C++ users from relying on the old function, but it does change the SB API's behavior.

[JDevlieghere]

Writing this made me realize we can solve this more elegantly. We can have both overloads and deprecate the one that takes an int64_t, mark it as deprecated and not expose it to SWIG.

lldb/include/lldb/API/SBTarget.h

#endif
   ///     The section whose base load address will be cleared within
   ///     this target.
   ///
   /// \return
   ///      An error to indicate success, fail, and any reason for
   ///     failure.
   lldb::SBError ClearSectionLoadAddress(lldb::SBSection section);
 
+#ifndef SWIG
   /// Slide all file addresses for all module sections so that \a module
   /// appears to loaded at these slide addresses.
   ///
   /// When you need all sections within a module to be loaded at a
   /// rigid slide from the addresses found in the module object file,
   /// this function will allow you to easily and quickly slide all
   /// module sections.
   ///
   /// \param[in] module
   ///     The module to load.
   ///
   /// \param[in] sections_offset
   ///     An offset that will be applied to all section file addresses
   ///     (the virtual addresses found in the object file itself).
   ///
   /// \return
   ///     An error to indicate success, fail, and any reason for
   ///     failure.
+  [[deprecated("Use SetModuleLoadAddress(lldb::SBModule, uint64_t)")]]
   lldb::SBError SetModuleLoadAddress(lldb::SBModule module,
                                      int64_t sections_offset);
+#endif
+
+  /// Slide all file addresses for all module sections so that \a module
+  /// appears to loaded at these slide addresses.
+  ///
+  /// When you need all sections within a module to be loaded at a
+  /// rigid slide from the addresses found in the module object file,
+  /// this function will allow you to easily and quickly slide all
+  /// module sections.
+  ///
+  /// \param[in] module
+  ///     The module to load.
+  ///
+  /// \param[in] sections_offset
+  ///     An offset that will be applied to all section file addresses
+  ///     (the virtual addresses found in the object file itself).
+  ///
+  /// \return
+  ///     An error to indicate success, fail, and any reason for
+  ///     failure.
+  lldb::SBError SetModuleLoadAddress(lldb::SBModule module,
+                                     uint64_t sections_offset);
 
   /// Clear the section base load addresses for all sections in a module.
   ///
   /// \param[in] module
   ///     The module to unload.
   ///
   /// \return
   ///     An error to indicate success, fail, and any reason for

lldb/source/API/SBTarget.cpp

SBError SBTarget::ClearSectionLoadAddress(lldb::SBSection section) {
   }
   return sb_error;
 }
 
 SBError SBTarget::SetModuleLoadAddress(lldb::SBModule module,
                                        int64_t slide_offset) {
   LLDB_INSTRUMENT_VA(this, module, slide_offset);
 
+  if (slide_offset < 0) {
+    SBError sb_error;
+    sb_error.SetErrorStringWithFormat("slide must be positive");
+    return sb_error;
+  }
+
+  return SetModuleLoadAddress(module, static_cast<uint64_t>(slide_offset));
+}
+
+SBError SBTarget::SetModuleLoadAddress(lldb::SBModule module,
+                                               uint64_t slide_offset) {
+
   SBError sb_error;
 
   TargetSP target_sp(GetSP());
   if (target_sp) {
     ModuleSP module_sp(module.GetSP());
     if (module_sp) {
       bool changed = false;
       if (module_sp->SetLoadAddress(*target_sp, slide_offset, true, changed)) {

lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py

def test_mulitple_slides(self):
 
         # View the first element of `first` and `second` while
         # they have no load address set.
         self.expect("expression/d ((int*)&first)[0]", substrs=['= 5'])
         self.expect("expression/d ((int*)&second)[0]", substrs=['= 6'])
         self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), lldb.LLDB_INVALID_ADDRESS)
         self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target), lldb.LLDB_INVALID_ADDRESS)
 
-
         # View the first element of `first` and `second` with
         # no slide applied, but with load address set.
         #
         # In memory, we have something like
         #    0x1000 - 0x17ff  first[]
         #    0x1800 - 0x1fff  second[]
-        target.SetModuleLoadAddress(module, 0)
+        error = target.SetModuleLoadAddress(module, 0)
+        self.assertSuccess(error)
         self.expect("expression/d ((int*)&first)[0]", substrs=['= 5'])
         self.expect("expression/d ((int*)&second)[0]", substrs=['= 6'])
         self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
                          first_sym.GetStartAddress().GetFileAddress())
         self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target),
                          second_sym.GetStartAddress().GetFileAddress())
 
         # Slide it a little bit less than the size of the first array.
         #
         # In memory, we have something like
         #    0xfc0 - 0x17bf  first[]
         #    0x17c0 - 0x1fbf second[]
         #
         # but if the original entries are still present in lldb, 
         # the beginning address of second[] will get a load address
         # of 0x1800, instead of 0x17c0 (0x1800-64) as we need to get.
-        target.SetModuleLoadAddress(module, first_size - 64)
+        error = target.SetModuleLoadAddress(module, first_size - 64)
+        self.assertSuccess(error)
         self.expect("expression/d ((int*)&first)[0]", substrs=['= 5'])
         self.expect("expression/d ((int*)&second)[0]", substrs=['= 6'])
         self.assertNotEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
                          first_sym.GetStartAddress().GetFileAddress())
         self.assertNotEqual(second_sym.GetStartAddress().GetLoadAddress(target),
                          second_sym.GetStartAddress().GetFileAddress())
 
         # Slide it back to the original vmaddr.
-        target.SetModuleLoadAddress(module, 0)
+        error = target.SetModuleLoadAddress(module, 0)
+        self.assertSuccess(error)
         self.expect("expression/d ((int*)&first)[0]", substrs=['= 5'])
         self.expect("expression/d ((int*)&second)[0]", substrs=['= 6'])
         self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
                          first_sym.GetStartAddress().GetFileAddress())
         self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target),
                          second_sym.GetStartAddress().GetFileAddress())
 
+        # Make sure we can use a slide > INT64_MAX.
+        error = target.SetModuleLoadAddress(module, 0xffffffff12345678)
+        self.assertSuccess(error)