Improve Nullability, consider case of null imageSize

sentry-android-ndk/src/main/java/io/sentry/android/ndk/DebugImagesLoader.java

@@ -27,7 +27,7 @@ public final class DebugImagesLoader implements IDebugImagesLoader {
 
   private final @NotNull NativeModuleListLoader moduleListLoader;
 
-  private static @Nullable List<DebugImage> debugImages;
+  private static volatile @Nullable List<DebugImage> debugImages;
 
   /** we need to lock it because it could be called from different threads */
   protected static final @NotNull AutoClosableReentrantLock debugImagesLock =
@@ -81,23 +81,19 @@ public DebugImagesLoader(
    * Loads debug images for the given set of addresses.
    *
    * @param addresses Set of memory addresses to find debug images for
-   * @return Set of debug images, or null if debug images couldn't be loaded
+   * @return Set of matching debug images, or null if debug images couldn't be loaded
    */
-  public @Nullable Set<DebugImage> loadDebugImagesForAddresses(@NotNull Set<Long> addresses) {
+  public @Nullable Set<DebugImage> loadDebugImagesForAddresses(final @NotNull Set<Long> addresses) {
     try (final @NotNull ISentryLifecycleToken ignored = debugImagesLock.acquire()) {
-      List<DebugImage> allDebugImages = loadDebugImages();
+      final @Nullable List<DebugImage> allDebugImages = loadDebugImages();
       if (allDebugImages == null) {
         return null;
       }
-
-      Set<DebugImage> referencedImages = new HashSet<>();
-      for (Long addr : addresses) {
-        DebugImage image = findImageByAddress(addr, allDebugImages);
-        if (image != null) {
-          referencedImages.add(image);
-        }
+      if (addresses.isEmpty()) {
+        return null;
       }
 
+      final Set<DebugImage> referencedImages = filterImagesByAddresses(allDebugImages, addresses);
       if (referencedImages.isEmpty()) {
         options
             .getLogger()
@@ -118,31 +114,47 @@ public DebugImagesLoader(
    *
    * @return The matching debug image or null if not found
    */
-  private @Nullable DebugImage findImageByAddress(long address, List<DebugImage> images) {
-    int left = 0;
-    int right = images.size() - 1;
-
-    while (left <= right) {
-      int mid = left + (right - left) / 2;
-      DebugImage image = images.get(mid);
-
-      if (image.getImageAddr() == null || image.getImageSize() == null) {
-        return null;
-      }
-
-      long imageStart = Long.parseLong(image.getImageAddr().replace("0x", ""), 16);
-      long imageEnd = imageStart + image.getImageSize();
-
-      if (address >= imageStart && address < imageEnd) {
-        return image;
-      } else if (address < imageStart) {
-        right = mid - 1;
-      } else {
-        left = mid + 1;
+  private @NotNull Set<DebugImage> filterImagesByAddresses(
+      final @NotNull List<DebugImage> images, final @NotNull Set<Long> addresses) {
+    final Set<DebugImage> result = new HashSet<>();
+
+    for (int i = 0; i < images.size(); i++) {
+      final @NotNull DebugImage image = images.get(i);
+      final @Nullable DebugImage nextDebugImage =
+          (i + 1) < images.size() ? images.get(i + 1) : null;
+      final @Nullable String nextDebugImageAddress =
+          nextDebugImage != null ? nextDebugImage.getImageAddr() : null;
+
+      for (final @NotNull Long address : addresses) {
+        final @Nullable String imageAddress = image.getImageAddr();
+
+        if (imageAddress != null) {
+          try {
+            final long imageStart = Long.parseLong(imageAddress.replace("0x", ""), 16);
+            final long imageEnd;
+
+            final @Nullable Long imageSize = image.getImageSize();
+            if (imageSize != null) {
+              imageEnd = imageStart + imageSize;
+            } else if (nextDebugImageAddress != null) {
+              imageEnd = Long.parseLong(nextDebugImageAddress.replace("0x", ""), 16);
+            } else {
+              imageEnd = Long.MAX_VALUE;
+            }
+            if (address >= imageStart && address < imageEnd) {
+              result.add(image);
+              // once image is added we can skip the remaining addresses and go straight to the next
+              // image
+              break;
+            }
+          } catch (NumberFormatException e) {
+            options.getLogger().log(SentryLevel.WARNING, e, "Failed to parse image address.");
+          }
+        }
       }
     }
 
-    return null;
+    return result;
   }
 
   @Override

sentry-android-ndk/src/test/java/io/sentry/android/ndk/DebugImagesLoaderTest.kt

@@ -17,11 +17,13 @@ class DebugImagesLoaderTest {
         val options = SentryAndroidOptions()
 
         fun getSut(): DebugImagesLoader {
-            return DebugImagesLoader(options, nativeLoader)
+            val loader = DebugImagesLoader(options, nativeLoader)
+            loader.clearDebugImages()
+            return loader
         }
     }
 
-    private val fixture = Fixture()
+    private var fixture = Fixture()
 
     @Test
     fun `get images returns image list`() {
@@ -102,11 +104,12 @@ class DebugImagesLoaderTest {
 
         val result = sut.loadDebugImagesForAddresses(
             setOf(0x1500L, 0x2500L)
-        )!!.toList()
+        )
 
+        assertNotNull(result)
         assertEquals(2, result.size)
-        assertEquals(image1.imageAddr, result[0].imageAddr)
-        assertEquals(image2.imageAddr, result[1].imageAddr)
+        assertTrue(result.any { it.imageAddr == image1.imageAddr })
+        assertTrue(result.any { it.imageAddr == image2.imageAddr })
     }
 
     @Test
@@ -152,4 +155,32 @@ class DebugImagesLoaderTest {
 
         assertNull(result)
     }
+
+    @Test
+    fun `invalid image adresses are ignored for loadDebugImagesForAddresses`() {
+        val sut = fixture.getSut()
+
+        val image1 = io.sentry.ndk.DebugImage().apply {
+            imageAddr = "0xNotANumber"
+            imageSize = 0x1000L
+        }
+
+        val image2 = io.sentry.ndk.DebugImage().apply {
+            imageAddr = "0x2000"
+            imageSize = null
+        }
+
+        val image3 = io.sentry.ndk.DebugImage().apply {
+            imageAddr = "0x5000"
+            imageSize = 0x1000L
+        }
+
+        whenever(fixture.nativeLoader.loadModuleList()).thenReturn(arrayOf(image1, image2, image3))
+
+        val hexAddresses = setOf(100L, 0x2000L, 0x2000L, 0x5000L)
+        val result = sut.loadDebugImagesForAddresses(hexAddresses)
+
+        assertNotNull(result)
+        assertEquals(2, result.size)
+    }
 }