Avoid undefined pointer arithmetic while growing mmap

The undefined behaviour sanitizer complains with:
runtime error: addition of unsigned offset to 0x… overflowed to 0x…

Compilers and runtime do the right thing in any case and it is a
codepath that can (and ideally should) be avoided for speed reasons
alone, but fixing it can't hurt (too much).

4 files changed, 29 insertions, 46 deletions

diff --git a/apt-pkg/cacheiterators.h b/apt-pkg/cacheiterators.h
index 6261b5089..466492442 100644
--- a/apt-pkg/cacheiterators.h
+++ b/apt-pkg/cacheiterators.h
@@ -82,10 +82,10 @@ template<typename Str, typename Itr> class APT_PUBLIC pkgCache::Iterator :
 	inline unsigned long Index() const {return S - OwnerPointer();}
 	inline map_pointer<Str> MapPointer() const {return map_pointer<Str>(Index()) ;}
 
-	void ReMap(void const * const oldMap, void const * const newMap) {
+	void ReMap(void const * const oldMap, void * const newMap) {
 		if (Owner == 0 || S == 0)
 			return;
-		S += static_cast<Str const *>(newMap) - static_cast<Str const *>(oldMap);
+		S = static_cast<Str *>(newMap) + (S - static_cast<Str const *>(oldMap));
 	}
 
 	// Constructors - look out for the variable assigning
@@ -350,12 +350,12 @@ class APT_PUBLIC pkgCache::DepIterator : public Iterator<Dependency, DepIterator
 	};
 	inline DependencyProxy operator->() const {return (DependencyProxy) { S2->Version, S2->Package, S->ID, S2->Type, S2->CompareOp, S->ParentVer, S->DependencyData, S->NextRevDepends, S->NextDepends, S2->NextData };}
 	inline DependencyProxy operator->() {return (DependencyProxy) { S2->Version, S2->Package, S->ID, S2->Type, S2->CompareOp, S->ParentVer, S->DependencyData, S->NextRevDepends, S->NextDepends, S2->NextData };}
-	void ReMap(void const * const oldMap, void const * const newMap)
+	void ReMap(void const * const oldMap, void * const newMap)
 	{
 		Iterator<Dependency, DepIterator>::ReMap(oldMap, newMap);
 		if (Owner == 0 || S == 0 || S2 == 0)
 			return;
-		S2 += static_cast<DependencyData const *>(newMap) - static_cast<DependencyData const *>(oldMap);
+		S2 = static_cast<DependencyData *>(newMap) + (S2 - static_cast<DependencyData const *>(oldMap));
 	}
 
 	//Nice printable representation
diff --git a/apt-pkg/contrib/mmap.cc b/apt-pkg/contrib/mmap.cc
index 020491172..642e20473 100644
--- a/apt-pkg/contrib/mmap.cc
+++ b/apt-pkg/contrib/mmap.cc
@@ -393,7 +393,7 @@ unsigned long DynamicMMap::Allocate(unsigned long ItemSize)
       bool const newError = _error->PendingError();
       _error->MergeWithStack();
       if (Pools != oldPools)
-	 I += Pools - oldPools;
+	 I = Pools + (I - oldPools);
 
       // Does the allocation failed ?
       if (Result == 0 && newError)
diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index 26cf7fc68..6f2e3268c 100644
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -156,13 +156,14 @@ pkgCacheGenerator::~pkgCacheGenerator()
    Map.Sync(0,sizeof(pkgCache::Header));
 }
 									/*}}}*/
-void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newMap, size_t oldSize) {/*{{{*/
+void pkgCacheGenerator::ReMap(void const * const oldMap, void * const newMap, size_t oldSize) {/*{{{*/
+   if (oldMap == newMap)
+      return;
+
    // Prevent multiple remaps of the same iterator. If seen.insert(iterator)
    // returns (something, true) the iterator was not yet seen and we can
    // remap it.
    std::unordered_set<void *> seen;
-   if (oldMap == newMap)
-      return;
 
    if (_config->FindB("Debug::pkgCacheGen", false))
       std::clog << "Remapping from " << oldMap << " to " << newMap << std::endl;
@@ -170,42 +171,24 @@ void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newM
    Cache.ReMap(false);
 
    if (CurrentFile != nullptr)
-      CurrentFile += static_cast<pkgCache::PackageFile const *>(newMap) - static_cast<pkgCache::PackageFile const *>(oldMap);
+      CurrentFile = static_cast<pkgCache::PackageFile *>(newMap) + (CurrentFile - static_cast<pkgCache::PackageFile const *>(oldMap));
    if (CurrentRlsFile != nullptr)
-      CurrentRlsFile += static_cast<pkgCache::ReleaseFile const *>(newMap) - static_cast<pkgCache::ReleaseFile const *>(oldMap);
-
-   for (std::vector<pkgCache::GrpIterator*>::const_iterator i = Dynamic<pkgCache::GrpIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::GrpIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::PkgIterator*>::const_iterator i = Dynamic<pkgCache::PkgIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::PkgIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::VerIterator*>::const_iterator i = Dynamic<pkgCache::VerIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::VerIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::DepIterator*>::const_iterator i = Dynamic<pkgCache::DepIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::DepIterator>::toReMap.end(); ++i)
-	if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::DescIterator*>::const_iterator i = Dynamic<pkgCache::DescIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::DescIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::PrvIterator*>::const_iterator i = Dynamic<pkgCache::PrvIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::PrvIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::PkgFileIterator*>::const_iterator i = Dynamic<pkgCache::PkgFileIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::PkgFileIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
-   for (std::vector<pkgCache::RlsFileIterator*>::const_iterator i = Dynamic<pkgCache::RlsFileIterator>::toReMap.begin();
-	i != Dynamic<pkgCache::RlsFileIterator>::toReMap.end(); ++i)
-      if (std::get<1>(seen.insert(*i)) == true)
-	 (*i)->ReMap(oldMap, newMap);
+      CurrentRlsFile = static_cast<pkgCache::ReleaseFile *>(newMap) + (CurrentRlsFile - static_cast<pkgCache::ReleaseFile const *>(oldMap));
+
+#define APT_REMAP(TYPE) \
+   for (auto * const i : Dynamic<TYPE>::toReMap) \
+      if (seen.insert(i).second) \
+	 i->ReMap(oldMap, newMap)
+   APT_REMAP(pkgCache::GrpIterator);
+   APT_REMAP(pkgCache::PkgIterator);
+   APT_REMAP(pkgCache::VerIterator);
+   APT_REMAP(pkgCache::DepIterator);
+   APT_REMAP(pkgCache::DescIterator);
+   APT_REMAP(pkgCache::PrvIterator);
+   APT_REMAP(pkgCache::PkgFileIterator);
+   APT_REMAP(pkgCache::RlsFileIterator);
+#undef APT_REMAP
+
    for (APT::StringView* ViewP : Dynamic<APT::StringView>::toReMap) {
       if (std::get<1>(seen.insert(ViewP)) == false)
 	 continue;
@@ -453,7 +436,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
 			   Pkg.Name(), "NewVersion", 1);
 
    if (oldMap != Map.Data())
-	 LastVer += static_cast<map_pointer<pkgCache::Version> const *>(Map.Data()) - static_cast<map_pointer<pkgCache::Version> const *>(oldMap);
+	 LastVer = static_cast<map_pointer<pkgCache::Version> *>(Map.Data()) + (LastVer - static_cast<map_pointer<pkgCache::Version> const *>(oldMap));
    *LastVer = verindex;
 
    if (unlikely(List.NewVersion(Ver) == false))
@@ -1073,7 +1056,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg,
       for (pkgCache::DepIterator D = Ver.DependsList(); D.end() == false; ++D)
 	 OldDepLast = &D->NextDepends;
    } else if (oldMap != Map.Data())
-      OldDepLast += static_cast<map_pointer<pkgCache::Dependency> const *>(Map.Data()) - static_cast<map_pointer<pkgCache::Dependency> const *>(oldMap);
+      OldDepLast = static_cast<map_pointer<pkgCache::Dependency> *>(Map.Data()) + (OldDepLast - static_cast<map_pointer<pkgCache::Dependency> const *>(oldMap));
 
    Dep->NextDepends = *OldDepLast;
    *OldDepLast = Dependency;
diff --git a/apt-pkg/pkgcachegen.h b/apt-pkg/pkgcachegen.h
index 62c171be4..f4781d715 100644
--- a/apt-pkg/pkgcachegen.h
+++ b/apt-pkg/pkgcachegen.h
@@ -146,7 +146,7 @@ class APT_HIDDEN pkgCacheGenerator					/*{{{*/
 			MMap **OutMap,pkgCache **OutCache, bool AllowMem = false);
    APT_PUBLIC static bool MakeOnlyStatusCache(OpProgress *Progress,DynamicMMap **OutMap);
 
-   void ReMap(void const * const oldMap, void const * const newMap, size_t oldSize);
+   void ReMap(void const * const oldMap, void * const newMap, size_t oldSize);
    bool Start();
 
    pkgCacheGenerator(DynamicMMap *Map,OpProgress *Progress);