public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH rebase] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL
@ 2022-07-16 16:01 Christian Franke
  2022-07-18  8:15 ` Corinna Vinschen
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Franke @ 2022-07-16 16:01 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

The current check for overlapping non-rebaseable DLLs uses a possible 
outdated base address. It also could not detect multiple overlaps by one 
non-rebaseable DLL.

Patch size may be misleading due to indentation change.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Ensure-that-no-rebaseable-DLL-overlaps-a-non-rebasea.patch --]
[-- Type: text/plain, Size: 9756 bytes --]

From ee8a966cb6da48b45dfb6de3e732dade81ce7fb9 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sat, 16 Jul 2022 17:55:58 +0200
Subject: [PATCH] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL

Restart rebase decision loop with newly sorted list if a DLL is not
rebaseable.  Search for all DLLs which overlap such a DLL.
---
 rebase.c | 203 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 121 insertions(+), 82 deletions(-)

diff --git a/rebase.c b/rebase.c
index 5cda123..39759a9 100644
--- a/rebase.c
+++ b/rebase.c
@@ -625,6 +625,7 @@ merge_image_info ()
 {
   int i, end;
   img_info_t *match;
+  BOOL sorted;
   ULONG64 floating_image_base;
 
   /* Sort new files from command line by name. */
@@ -725,91 +726,129 @@ merge_image_info ()
       img_info_rebase_start = 0;
     }
 
-  /* Now sort the old part of the list by base address. */
-  if (img_info_rebase_start)
-    qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
-	   img_info_cmp);
-  /* Perform several tests on the information fetched from the database
-     to match with reality. */
-  for (i = 0; i < img_info_rebase_start; ++i)
+  for (sorted = FALSE; img_info_rebase_start && !sorted; )
     {
-      ULONG64 cur_base, cur_base_orig;
-      ULONG cur_size, slot_size;
-
-      /* Files with the needs_rebasing or cannot_rebase flags set have been
-	 checked already. */
-      if (img_info_list[i].flag.needs_rebasing
-      	  || img_info_list[i].flag.cannot_rebase)
-	continue;
-      /* Check if the files in the old list still exist.  Drop non-existant
-	 or unaccessible files. */
-      if (access (img_info_list[i].name, F_OK) == -1
-	  || !GetImageInfos64 (img_info_list[i].name, NULL,
-			       &cur_base, &cur_size))
-	{
-	  free (img_info_list[i].name);
-	  memmove (img_info_list + i, img_info_list + i + 1,
-		   (img_info_size - i - 1) * sizeof (img_info_t));
-	  --img_info_rebase_start;
-	  --img_info_size;
-	  continue;
-	}
-      slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
-      cur_base_orig = cur_base;
-      /* If the file has been reinstalled, try to rebase to the same address
-	 in the first place. */
-      if (cur_base != img_info_list[i].base)
+      char overlaps[img_info_rebase_start];
+      memset(overlaps, 0, img_info_rebase_start);
+      /* Now sort the old part of the list by base address. */
+      qsort (img_info_list, img_info_rebase_start, sizeof (img_info_t),
+	     img_info_cmp);
+      /* Perform several tests on the information fetched from the database
+	 to match with reality. */
+      for (sorted = TRUE, i = 0; sorted && i < img_info_rebase_start; ++i)
 	{
-	  img_info_list[i].flag.needs_rebasing = 1;
-	  if (verbose)
-	    fprintf (stderr, "rebasing %s because it's base has changed (due to being reinstalled?)\n", img_info_list[i].name);
-	  /* Set cur_base to the old base to simplify subsequent tests. */
-	  cur_base = img_info_list[i].base;
-	}
-      /* However, if the DLL got bigger and doesn't fit into its slot
-	 anymore, rebase this DLL from scratch. */
-      if (i + 1 < img_info_rebase_start
-	  && cur_base + slot_size + offset > img_info_list[i + 1].base)
-	{
-	  img_info_list[i].base = 0;
-	  if (verbose)
-	    fprintf (stderr, "rebasing %s because it won't fit in it's old slot without overlapping next DLL\n", img_info_list[i].name);
-	}
-      /* Does the previous DLL reach into the address space of this
-	 DLL?  This happens if the previous DLL is not rebaseable. */
-      else if (i > 0 && cur_base < img_info_list[i - 1].base
-				   + img_info_list[i - 1].slot_size)
-	{
-	  img_info_list[i].base = 0;
-	  if (verbose)
-	    fprintf (stderr, "rebasing %s because previous DLL now overlaps\n", img_info_list[i].name);
-	}
-      /* Does the file match the base address requirements?  If not,
-	 rebase from scratch. */
-      else if ((down_flag && cur_base + slot_size + offset > image_base)
-	       || (!down_flag && cur_base < image_base))
-	{
-	  img_info_list[i].base = 0;
-	  if (verbose)
-	    fprintf (stderr, "rebasing %s because it's base address is outside the expected area\n", img_info_list[i].name);
-	}
-      /* Make sure all DLLs with base address 0 have the needs_rebasing
-	 flag set. */
-      if (img_info_list[i].base == 0)
-	img_info_list[i].flag.needs_rebasing = 1;
-      /* Only check for writability if file needs rebasing to keep
-	 Compact OS compression of unchanged files.  Revert rebase
-	 decision if not writeable. */
-      if (img_info_list[i].flag.needs_rebasing && set_cannot_rebase (&img_info_list[i]))
-	{
-	  img_info_list[i].flag.needs_rebasing = 0;
-	  img_info_list[i].base = cur_base_orig;
-	  if (verbose)
-	    fprintf (stderr, "rebasing %s deferred because file is not writable\n", img_info_list[i].name);
+	  ULONG64 cur_base, cur_base_orig;
+	  ULONG cur_size, slot_size;
+
+	  /* If file is not rebaseable and neither --oblivious nor --merge-files
+	     is active, search forward for possible overlaps. */
+	  if (img_info_list[i].flag.cannot_rebase == 1)
+	    {
+	      int j;
+	      /* If cannot_rebase is set, base and size are already valid. */
+	      cur_base = img_info_list[i].base;
+	      slot_size = img_info_list[i].slot_size;
+	      for (j = i + 1; j < img_info_rebase_start; ++j)
+		{
+		  if (img_info_list[j].flag.cannot_rebase)
+		    continue; /* Also not rebaseable. */
+		  if (img_info_list[j].base == 0)
+		    continue; /* Will already be rebased from scratch. */
+		  if (cur_base + slot_size + offset <= img_info_list[j].base)
+		    break; /* This and any further DLLs do not overlap. */
+		  /* Select rebase from scratch when outer loop arrives at j. */
+		  overlaps[j] = 1;
+		}
+	    }
+
+	  /* Files with the needs_rebasing or cannot_rebase flags set have been
+	     checked already. */
+	  if (img_info_list[i].flag.needs_rebasing
+	      || img_info_list[i].flag.cannot_rebase)
+	    continue;
+	  /* Check if the files in the old list still exist.  Drop non-existant
+	     or unaccessible files. */
+	  if (access (img_info_list[i].name, F_OK) == -1
+	      || !GetImageInfos64 (img_info_list[i].name, NULL,
+				   &cur_base, &cur_size))
+	    {
+	      free (img_info_list[i].name);
+	      memmove (img_info_list + i, img_info_list + i + 1,
+		       (img_info_size - i - 1) * sizeof (img_info_t));
+	      memmove (overlaps + i, overlaps + i + 1, img_info_size - i - 1);
+	      --img_info_rebase_start;
+	      --img_info_size;
+	      continue;
+	    }
+	  slot_size = roundup2 (cur_size, ALLOCATION_SLOT);
+	  cur_base_orig = cur_base;
+	  /* If the file has been reinstalled, try to rebase to the same address
+	     in the first place. */
+	  if (cur_base != img_info_list[i].base)
+	    {
+	      img_info_list[i].flag.needs_rebasing = 1;
+	      if (verbose)
+		fprintf (stderr, "rebasing %s because it's base has changed "
+			 "(due to being reinstalled?)\n", img_info_list[i].name);
+	      /* Set cur_base to the old base to simplify subsequent tests. */
+	      cur_base = img_info_list[i].base;
+	    }
+	  /* However, if the DLL got bigger and doesn't fit into its slot
+	     anymore, rebase this DLL from scratch. */
+	  if (i + 1 < img_info_rebase_start
+	      && cur_base + slot_size + offset > img_info_list[i + 1].base)
+	    {
+	      img_info_list[i].base = 0;
+	      if (verbose)
+		fprintf (stderr, "rebasing %s because it won't fit in it's old slot "
+			 "without overlapping next DLL\n", img_info_list[i].name);
+	    }
+	  /* Does a previous DLL reach into the address space of this
+	     DLL?  This happens if the previous DLL is not rebaseable. */
+	  else if (overlaps[i])
+	    {
+	      img_info_list[i].base = 0;
+	      if (verbose)
+		fprintf (stderr, "rebasing %s because a previous non-writable DLL "
+			 "overlaps\n", img_info_list[i].name);
+	    }
+	  /* Does the file match the base address requirements?  If not,
+	     rebase from scratch. */
+	  else if ((down_flag && cur_base + slot_size + offset > image_base)
+		   || (!down_flag && cur_base < image_base))
+	    {
+	      img_info_list[i].base = 0;
+	      if (verbose)
+		fprintf (stderr, "rebasing %s because it's base address is outside the "
+			 "expected area\n", img_info_list[i].name);
+	    }
+	  /* Make sure all DLLs with base address 0 have the needs_rebasing
+	     flag set. */
+	  if (img_info_list[i].base == 0)
+	    img_info_list[i].flag.needs_rebasing = 1;
+	  /* Only check for writability if file needs rebasing to keep
+	     Compact OS compression of unchanged files.  Revert rebase
+	     decision if not writeable. */
+	  if (img_info_list[i].flag.needs_rebasing
+	      && set_cannot_rebase (&img_info_list[i]))
+	    {
+	      img_info_list[i].flag.needs_rebasing = 0;
+	      img_info_list[i].base = cur_base_orig;
+	      if (verbose)
+		fprintf (stderr, "rebasing %s deferred because file is not writable\n",
+			 img_info_list[i].name);
+	      /* List is possibly no longer sorted which could result in false
+	         negative rebase decisions.  Exit loop and restart it with
+		 newly sorted list.  This will also set the overlaps[] array
+		 for this DLL when the restarted loop visits the (possibly new)
+		 index of this DLL.  Infinite retries could not occur because
+		 each turn sets another cannot_rebase flag. */
+	      sorted = FALSE;
+	    }
+	  /* Unconditionally overwrite old with new size. */
+	  img_info_list[i].size = cur_size;
+	  img_info_list[i].slot_size = slot_size;
 	}
-      /* Unconditionally overwrite old with new size. */
-      img_info_list[i].size = cur_size;
-      img_info_list[i].slot_size = slot_size;
     }
   /* The remainder of the function expects img_info_size to be > 0. */
   if (img_info_size == 0)
-- 
2.37.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH rebase] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL
  2022-07-16 16:01 [PATCH rebase] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL Christian Franke
@ 2022-07-18  8:15 ` Corinna Vinschen
  0 siblings, 0 replies; 2+ messages in thread
From: Corinna Vinschen @ 2022-07-18  8:15 UTC (permalink / raw)
  To: cygwin-apps

On Jul 16 18:01, Christian Franke wrote:
> The current check for overlapping non-rebaseable DLLs uses a possible
> outdated base address. It also could not detect multiple overlaps by one
> non-rebaseable DLL.
> 
> Patch size may be misleading due to indentation change.

Yeah, I diff'ed -b it for a better view :)

LGTM, pushed.


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-07-18  8:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-16 16:01 [PATCH rebase] Ensure that no rebaseable DLL overlaps a non-rebaseable DLL Christian Franke
2022-07-18  8:15 ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).