From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=unavailable autolearn_force=no version=3.4.1 To: devel@lists.altlinux.org References: <20191206131606.83061-1-darktemplar@altlinux.org> <20191206131606.83061-2-darktemplar@altlinux.org> <20191206133647.dculnmwkd3yf2wjp@titan.localdomain> From: Aleksei Nikiforov Message-ID: Date: Fri, 6 Dec 2019 18:32:27 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20191206133647.dculnmwkd3yf2wjp@titan.localdomain> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Language: ru Content-Transfer-Encoding: 8bit Subject: Re: [devel] [PATCH for apt 2/2] Fix pointer arithmetics X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Dec 2019 15:32:32 -0000 Archived-At: List-Archive: List-Post: 06.12.2019 16:36, Ivan A. Melnikov пишет: > 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 >> + >> #include >> >> // Package Iterator >> @@ -89,7 +91,7 @@ class pkgCache::PkgIterator >> { >> if (Owner == 0 || Pkg == 0) >> return; >> - Pkg += static_cast(newMap) - static_cast(oldMap); >> + Pkg = RebasePointer(Pkg, oldMap, newMap); >> } >> >> // Constructors >> @@ -151,7 +153,7 @@ class pkgCache::VerIterator >> { >> if (Owner == 0 || Ver == 0) >> return; >> - Ver += static_cast(newMap) - static_cast(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(newMap) - static_cast(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(newMap) - static_cast(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(newMap) - static_cast(oldMap); >> + File = RebasePointer(File, oldMap, newMap); >> } >> >> // Constructors >> @@ -387,7 +389,7 @@ class pkgCache::VerFileIterator >> { >> if (Owner == 0 || FileP == 0) >> return; >> - FileP += static_cast(newMap) - static_cast(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 >> #include >> #include >> +#include >> >> #include >> >> @@ -301,7 +302,7 @@ std::experimental::optional 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 >> #include >> #include >> +#include >> >> #include >> >> @@ -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::toReMap.begin(); >> i != Dynamic::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 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 >> +static inline T* RebasePointer(T *ptr, const void * const old_base, const void * const new_base) >> +{ >> + return reinterpret_cast(reinterpret_cast(ptr) + (reinterpret_cast(new_base) - reinterpret_cast(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? > > It can be done. Arithmetically it should be same result, but it'd also take some changes to get rid of 'const' specifiers since they'd clash with non-const variables (ptr). - return reinterpret_cast(reinterpret_cast(ptr) + (reinterpret_cast(new_base) - reinterpret_cast(old_base))); + return reinterpret_cast(reinterpret_cast(new_base) + (reinterpret_cast(ptr) - reinterpret_cast(old_base))); >> +} >> + >> +template >> +static inline const T* RebasePointer(const T *ptr, const void * const old_base, const void * const new_base) >> +{ >> + return reinterpret_cast(reinterpret_cast(ptr) + (reinterpret_cast(new_base) - reinterpret_cast(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 >> #include >> #include >> +#include >> >> #include >> >> @@ -56,7 +57,7 @@ rpmListParser::rpmListParser(RPMHandler *Handler) >> >> for (auto iter: *SeenPackages) >> { >> - tmp.insert(iter + (static_cast(newMap) - static_cast(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 mailing list > Devel@lists.altlinux.org > https://lists.altlinux.org/mailman/listinfo/devel >