[devel] [PATCH for apt 2/2] Fix pointer arithmetics
Ivan A. Melnikov
iv на altlinux.org
Пт Дек 6 16:36:47 MSK 2019
On Fri, Dec 06, 2019 at 04:16:06PM +0300, Aleksei Nikiforov wrote:
> This change should fix pointer arithmetic issues for e2k.
> ---
> apt/apt-pkg/Makefile.am | 1 +
> apt/apt-pkg/cacheiterators.h | 14 ++++++++------
> apt/apt-pkg/contrib/mmap.cc | 8 +++++---
> apt/apt-pkg/pkgcachegen.cc | 15 ++++++++-------
> apt/apt-pkg/rebase_pointer.h | 16 ++++++++++++++++
> apt/apt-pkg/rpm/rpmlistparser.cc | 3 ++-
> 6 files changed, 40 insertions(+), 17 deletions(-)
> create mode 100644 apt/apt-pkg/rebase_pointer.h
>
> diff --git a/apt/apt-pkg/Makefile.am b/apt/apt-pkg/Makefile.am
> index 4c0d234..d038d01 100644
> --- a/apt/apt-pkg/Makefile.am
> +++ b/apt/apt-pkg/Makefile.am
> @@ -94,6 +94,7 @@ libapt_pkg_la_SOURCES = \
> pkgsystem.h \
> policy.cc \
> policy.h \
> + rebase_pointer.h \
> repository.cc \
> repository.h \
> scopeexit.h \
> diff --git a/apt/apt-pkg/cacheiterators.h b/apt/apt-pkg/cacheiterators.h
> index a4bf670..51f70c1 100644
> --- a/apt/apt-pkg/cacheiterators.h
> +++ b/apt/apt-pkg/cacheiterators.h
> @@ -34,6 +34,8 @@
> #pragma interface "apt-pkg/cacheiterators.h"
> #endif
>
> +#include <apt-pkg/rebase_pointer.h>
> +
> #include <set>
>
> // Package Iterator
> @@ -89,7 +91,7 @@ class pkgCache::PkgIterator
> {
> if (Owner == 0 || Pkg == 0)
> return;
> - Pkg += static_cast<Package const *>(newMap) - static_cast<Package const *>(oldMap);
> + Pkg = RebasePointer(Pkg, oldMap, newMap);
> }
>
> // Constructors
> @@ -151,7 +153,7 @@ class pkgCache::VerIterator
> {
> if (Owner == 0 || Ver == 0)
> return;
> - Ver += static_cast<Version const *>(newMap) - static_cast<Version const *>(oldMap);
> + Ver = RebasePointer(Ver, oldMap, newMap);
> }
>
> inline VerIterator() : Ver(0), Owner(0) {};
> @@ -224,7 +226,7 @@ class pkgCache::DepIterator
> {
> if (Owner == 0 || Dep == 0)
> return;
> - Dep += static_cast<Dependency const *>(newMap) - static_cast<Dependency const *>(oldMap);
> + Dep = RebasePointer(Dep, oldMap, newMap);
> }
>
> inline DepIterator(pkgCache &Owner,Dependency *Trg,Version * = 0) :
> @@ -283,7 +285,7 @@ class pkgCache::PrvIterator
> {
> if (Owner == 0 || Prv == 0)
> return;
> - Prv += static_cast<Provides const *>(newMap) - static_cast<Provides const *>(oldMap);
> + Prv = RebasePointer(Prv, oldMap, newMap);
> }
>
> inline PrvIterator() : Prv(0), Type(PrvVer), Owner(0) {};
> @@ -346,7 +348,7 @@ class pkgCache::PkgFileIterator
> {
> if (Owner == 0 || File == 0)
> return;
> - File += static_cast<PackageFile const *>(newMap) - static_cast<PackageFile const *>(oldMap);
> + File = RebasePointer(File, oldMap, newMap);
> }
>
> // Constructors
> @@ -387,7 +389,7 @@ class pkgCache::VerFileIterator
> {
> if (Owner == 0 || FileP == 0)
> return;
> - FileP += static_cast<VerFile const *>(newMap) - static_cast<VerFile const *>(oldMap);
> + FileP = RebasePointer(FileP, oldMap, newMap);
> }
>
> inline VerFileIterator() : Owner(0), FileP(0) {};
> diff --git a/apt/apt-pkg/contrib/mmap.cc b/apt/apt-pkg/contrib/mmap.cc
> index cf01be9..ddae2ff 100644
> --- a/apt/apt-pkg/contrib/mmap.cc
> +++ b/apt/apt-pkg/contrib/mmap.cc
> @@ -30,6 +30,7 @@
> #include <apt-pkg/configuration.h>
> #include <apt-pkg/mmap.h>
> #include <apt-pkg/error.h>
> +#include <apt-pkg/rebase_pointer.h>
>
> #include <apti18n.h>
>
> @@ -301,7 +302,7 @@ std::experimental::optional<map_ptrloc> DynamicMMap::Allocate(unsigned long Item
> Pool* oldPools = Pools;
> auto idxResult = RawAllocate(I->Count*ItemSize,ItemSize);
> if (Pools != oldPools)
> - I += Pools - oldPools;
> + I = RebasePointer(I, oldPools, Pools);
>
> // Does the allocation failed ?
> if (!idxResult)
> @@ -371,7 +372,7 @@ bool DynamicMMap::Grow(unsigned long long size)
> Fd->Write(&C,sizeof(C));
> }
>
> - unsigned long const poolOffset = Pools - ((Pool*) Base);
> + const void * const old_base = Base;
>
> if (Fd != 0)
> {
> @@ -408,7 +409,8 @@ bool DynamicMMap::Grow(unsigned long long size)
> memset((char*)Base + WorkSpace, 0, newSize - WorkSpace);
> }
>
> - Pools = (Pool*) Base + poolOffset;
> + if (Base != old_base)
> + Pools = RebasePointer(Pools, old_base, Base);
> WorkSpace = newSize;
>
> return true;
> diff --git a/apt/apt-pkg/pkgcachegen.cc b/apt/apt-pkg/pkgcachegen.cc
> index 654c81c..10a0fd7 100644
> --- a/apt/apt-pkg/pkgcachegen.cc
> +++ b/apt/apt-pkg/pkgcachegen.cc
> @@ -26,6 +26,7 @@
> #include <apt-pkg/strutl.h>
> #include <apt-pkg/sptr.h>
> #include <apt-pkg/pkgsystem.h>
> +#include <apt-pkg/rebase_pointer.h>
>
> #include <apti18n.h>
>
> @@ -116,11 +117,11 @@ void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newM
>
> Cache.ReMap(false);
>
> - CurrentFile += (pkgCache::PackageFile*) newMap - (pkgCache::PackageFile*) oldMap;
> + CurrentFile = RebasePointer(CurrentFile, oldMap, newMap);
>
> for (size_t i = 0; i < _count(UniqHash); ++i)
> if (UniqHash[i] != 0)
> - UniqHash[i] += (pkgCache::StringItem*) newMap - (pkgCache::StringItem*) oldMap;
> + UniqHash[i] = RebasePointer(UniqHash[i], oldMap, newMap);
>
> for (auto i = Dynamic<pkgCache::PkgIterator>::toReMap.begin();
> i != Dynamic<pkgCache::PkgIterator>::toReMap.end(); ++i)
> @@ -271,7 +272,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
>
> if (oldMap != Map.Data())
> {
> - Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
> + Last = RebasePointer(Last, oldMap, Map.Data());
> oldMap = Map.Data();
> }
>
> @@ -297,7 +298,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
> PackageName.c_str(), 1);
>
> if (oldMap != Map.Data())
> - Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
> + Last = RebasePointer(Last, oldMap, Map.Data());
> *Last = *verindex;
>
> Ver->ParentPkg = Pkg.Index();
> @@ -605,7 +606,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
> OldDepLast = &D->NextDepends;
> OldDepVer = Ver;
> } else if (oldMap != Owner->Map.Data())
> - OldDepLast += (map_ptrloc*) Owner->Map.Data() - (map_ptrloc*) oldMap;
> + OldDepLast = RebasePointer(OldDepLast, oldMap, Owner->Map.Data());
>
> // Is it a file dependency?
> if (PackageName[0] == '/')
> @@ -747,8 +748,8 @@ std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteUniqString(const
>
> if (oldMap != Map.Data())
> {
> - Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
> - I += (pkgCache::StringItem*) Map.Data() - (pkgCache::StringItem*) oldMap;
> + Last = RebasePointer(Last, oldMap, Map.Data());
> + I = RebasePointer(I, oldMap, Map.Data());
> }
>
> *Last = *Item;
> diff --git a/apt/apt-pkg/rebase_pointer.h b/apt/apt-pkg/rebase_pointer.h
> new file mode 100644
> index 0000000..efc4074
> --- /dev/null
> +++ b/apt/apt-pkg/rebase_pointer.h
> @@ -0,0 +1,16 @@
> +#ifndef PKGLIB_REBASE_POINTER_H
> +#define PKGLIB_REBASE_POINTER_H
> +
> +template <typename T>
> +static inline T* RebasePointer(T *ptr, const void * const old_base, const void * const new_base)
> +{
> + return reinterpret_cast<T*>(reinterpret_cast<char*>(ptr) + (reinterpret_cast<char const * const>(new_base) - reinterpret_cast<char const * const>(old_base)));
AFAIR standard only allows to substract pointers to elements of the same
array. Wouldn't it be safer to first compute the offset of ptr with
respect to old_base and then add it to new_base?
> +}
> +
> +template <typename T>
> +static inline const T* RebasePointer(const T *ptr, const void * const old_base, const void * const new_base)
> +{
> + return reinterpret_cast<const T*>(reinterpret_cast<const char*>(ptr) + (reinterpret_cast<char const * const>(new_base) - reinterpret_cast<char const * const>(old_base)));
> +}
> +
> +#endif
> diff --git a/apt/apt-pkg/rpm/rpmlistparser.cc b/apt/apt-pkg/rpm/rpmlistparser.cc
> index 9b2e9ad..84b6b8d 100644
> --- a/apt/apt-pkg/rpm/rpmlistparser.cc
> +++ b/apt/apt-pkg/rpm/rpmlistparser.cc
> @@ -25,6 +25,7 @@
> #include <apt-pkg/strutl.h>
> #include <apt-pkg/crc-16.h>
> #include <apt-pkg/tagfile.h>
> +#include <apt-pkg/rebase_pointer.h>
>
> #include <apti18n.h>
>
> @@ -56,7 +57,7 @@ rpmListParser::rpmListParser(RPMHandler *Handler)
>
> for (auto iter: *SeenPackages)
> {
> - tmp.insert(iter + (static_cast<const char *>(newMap) - static_cast<const char *>(oldMap)));
> + tmp.insert(RebasePointer(iter, oldMap, newMap));
> }
>
> SeenPackages->swap(tmp);
> --
> 2.24.0
>
> _______________________________________________
> Devel mailing list
> Devel на lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel
Подробная информация о списке рассылки Devel