[devel] [PATCH for apt 2/2 v2] Fix pointer arithmetics
Andrey Savchenko
bircoph на altlinux.org
Сб Дек 7 17:52:01 MSK 2019
Hi all!
On Fri, 6 Dec 2019 18:36:55 +0300 Aleksei Nikiforov wrote:
> This change should fix pointer arithmetic issues for e2k.
This commit message is misleading. Pointer arithmetic is broken on
all architectures, but by pure chance in happens to work correctly
when gcc is used. On e2k the lcc compiler is used, so problem
manifests itself. But this doesn't change the fact that pointer
arithmetic was flawed on all architectures.
> ---
> apt/apt-pkg/Makefile.am | 1 +
> apt/apt-pkg/cacheiterators.h | 26 ++++++++++++++------------
> apt/apt-pkg/contrib/mmap.cc | 8 +++++---
> apt/apt-pkg/pkgcachegen.cc | 29 +++++++++++++++--------------
> apt/apt-pkg/pkgcachegen.h | 6 +++---
> apt/apt-pkg/rebase_pointer.h | 16 ++++++++++++++++
> apt/apt-pkg/rpm/rpmlistparser.cc | 5 +++--
> apt/apt-pkg/rpm/rpmpackagedata.cc | 2 +-
> 8 files changed, 58 insertions(+), 35 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..118838e 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
> @@ -85,11 +87,11 @@ class pkgCache::PkgIterator
> inline unsigned long long Index() const {return Pkg - Owner->PkgP;};
> OkState State() const;
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> if (Owner == 0 || Pkg == 0)
> return;
> - Pkg += static_cast<Package const *>(newMap) - static_cast<Package const *>(oldMap);
> + Pkg = RebasePointer(Pkg, oldMap, newMap);
> }
>
> // Constructors
> @@ -147,11 +149,11 @@ class pkgCache::VerIterator
> bool Automatic() const;
> VerFileIterator NewestFile() const;
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> 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) {};
> @@ -220,11 +222,11 @@ class pkgCache::DepIterator
> inline const char *CompType() {return Owner->CompType(Dep->CompareOp);};
> inline const char *DepType() {return Owner->DepType(Dep->Type);};
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> 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) :
> @@ -279,11 +281,11 @@ class pkgCache::PrvIterator
> inline PkgIterator OwnerPkg() const {return PkgIterator(*Owner,Owner->PkgP + Owner->VerP[Prv->Version].ParentPkg);};
> inline unsigned long long Index() const {return Prv - Owner->ProvideP;};
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> 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) {};
> @@ -342,11 +344,11 @@ class pkgCache::PkgFileIterator
> bool IsOk();
> string RelStr();
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> if (Owner == 0 || File == 0)
> return;
> - File += static_cast<PackageFile const *>(newMap) - static_cast<PackageFile const *>(oldMap);
> + File = RebasePointer(File, oldMap, newMap);
> }
>
> // Constructors
> @@ -383,11 +385,11 @@ class pkgCache::VerFileIterator
> inline PkgFileIterator File() const {return PkgFileIterator(*Owner,FileP->File + Owner->PkgFileP);};
> inline unsigned long long Index() const {return FileP - Owner->VerFileP;};
>
> - void ReMap(void const * const oldMap, void const * const newMap)
> + void ReMap(void *oldMap, void *newMap)
> {
> 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..35f1a25 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);
> + void *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..37364e9 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>
>
> @@ -109,18 +110,18 @@ pkgCacheGenerator::~pkgCacheGenerator()
> Map.Sync(0,sizeof(pkgCache::Header));
> }
> /*}}}*/
> -void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newMap)
> +void pkgCacheGenerator::ReMap(void *oldMap, void *newMap)
> {
> if (oldMap == newMap)
> return;
>
> 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)
> @@ -147,7 +148,7 @@ void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newM
> // CacheGenerator::WriteStringInMap /*{{{*/
> std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteStringInMap(const char *String,
> unsigned long Len) {
> - void const * const oldMap = Map.Data();
> + void *oldMap = Map.Data();
> const auto index = Map.WriteString(String, Len);
> if (index)
> ReMap(oldMap, Map.Data());
> @@ -156,7 +157,7 @@ std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteStringInMap(cons
> /*}}}*/
> // CacheGenerator::WriteStringInMap /*{{{*/
> std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteStringInMap(const char *String) {
> - void const * const oldMap = Map.Data();
> + void *oldMap = Map.Data();
> const auto index = Map.WriteString(String);
> if (index)
> ReMap(oldMap, Map.Data());
> @@ -164,7 +165,7 @@ std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteStringInMap(cons
> }
> /*}}}*/
> std::experimental::optional<map_ptrloc> pkgCacheGenerator::AllocateInMap(unsigned long size) {/*{{{*/
> - void const * const oldMap = Map.Data();
> + void *oldMap = Map.Data();
> const auto index = Map.Allocate(size);
> if (index)
> ReMap(oldMap, Map.Data());
> @@ -229,7 +230,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
> pkgCache::VerIterator Ver = Pkg.VersionList();
> Dynamic<pkgCache::VerIterator> DynVer(Ver);
> map_ptrloc *Last = &Pkg->VersionList;
> - const void * oldMap = Map.Data();
> + void *oldMap = Map.Data();
> int Res = 1;
> for (; Ver.end() == false; Last = &Ver->NextVer, Ver++)
> {
> @@ -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();
> @@ -553,7 +554,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
> unsigned int Op,
> unsigned int Type)
> {
> - void const * const oldMap = Owner->Map.Data();
> + void *oldMap = Owner->Map.Data();
> pkgCache &Cache = Owner->Cache;
>
> // Get a structure
> @@ -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] == '/')
> @@ -739,7 +740,7 @@ std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteUniqString(const
> }
>
> // Get a structure
> - void const * const oldMap = Map.Data();
> + void *oldMap = Map.Data();
> const auto Item = AllocateInMap(sizeof(pkgCache::StringItem));
> const auto idxString = WriteStringInMap(S, Size);
> if ((!Item) || (!idxString))
> @@ -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/pkgcachegen.h b/apt/apt-pkg/pkgcachegen.h
> index 77075a6..0faf6e1 100644
> --- a/apt/apt-pkg/pkgcachegen.h
> +++ b/apt/apt-pkg/pkgcachegen.h
> @@ -72,7 +72,7 @@ class pkgCacheGenerator
> class DynamicFunction
> {
> public:
> - typedef std::function<void(const void *, const void *)> function;
> + typedef std::function<void(void *, void *)> function;
>
> static std::set<DynamicFunction*> toReMap;
>
> @@ -93,7 +93,7 @@ class pkgCacheGenerator
> toReMap.erase(this);
> }
>
> - void call(const void *oldMap, const void *newMap)
> + void call(void *oldMap, void *newMap)
> {
> if (m_function)
> m_function(oldMap, newMap);
> @@ -140,7 +140,7 @@ class pkgCacheGenerator
> // CNC:2003-03-18
> inline void ResetFileDeps() {FoundFileDeps = false;};
>
> - void ReMap(void const * const oldMap, void const * const newMap);
> + void ReMap(void *oldMap, void *newMap);
>
> pkgCacheGenerator(DynamicMMap *Map,OpProgress *Progress);
> ~pkgCacheGenerator();
> diff --git a/apt/apt-pkg/rebase_pointer.h b/apt/apt-pkg/rebase_pointer.h
> new file mode 100644
> index 0000000..f6b3c15
> --- /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, void *old_base, void *new_base)
> +{
> + return reinterpret_cast<T*>(reinterpret_cast<char*>(new_base) + (reinterpret_cast<char*>(ptr) - reinterpret_cast<char*>(old_base)));
> +}
> +
> +template <typename T>
> +static inline const T* RebasePointer(const T *ptr, void *old_base, void *new_base)
> +{
> + return reinterpret_cast<const T*>(reinterpret_cast<char*>(new_base) + (reinterpret_cast<const char*>(ptr) - reinterpret_cast<char*>(old_base)));
> +}
> +
> +#endif
> diff --git a/apt/apt-pkg/rpm/rpmlistparser.cc b/apt/apt-pkg/rpm/rpmlistparser.cc
> index 9b2e9ad..bd13b99 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>
>
> @@ -50,13 +51,13 @@ rpmListParser::rpmListParser(RPMHandler *Handler)
> {
> SeenPackages.reset(new SeenPackagesType);
> m_SeenPackagesRealloc.reset(new pkgCacheGenerator::DynamicFunction(
> - [this] (void const *oldMap, void const *newMap)
> + [this] (void *oldMap, void *newMap)
> {
> SeenPackagesType tmp;
>
> 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);
> diff --git a/apt/apt-pkg/rpm/rpmpackagedata.cc b/apt/apt-pkg/rpm/rpmpackagedata.cc
> index 61c15d5..d6c677b 100644
> --- a/apt/apt-pkg/rpm/rpmpackagedata.cc
> +++ b/apt/apt-pkg/rpm/rpmpackagedata.cc
> @@ -23,7 +23,7 @@ RPMPackageData::RPMPackageData()
> #endif
> {
> m_VerMapRealloc.reset(new pkgCacheGenerator::DynamicFunction(
> - [this] (void const *oldMap, void const *newMap)
> + [this] (void *oldMap, void *newMap)
> {
> for (auto iter1 = VerMap.begin(); iter1 != VerMap.end(); ++iter1)
> {
Best regards,
Andrew Savchenko
----------- следующая часть -----------
Было удалено вложение не в текстовом формате...
Имя : отсутствует
Тип : application/pgp-signature
Размер : 833 байтов
Описание: отсутствует
Url : <http://lists.altlinux.org/pipermail/devel/attachments/20191207/d4faa000/attachment.bin>
Подробная информация о списке рассылки Devel