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 Date: Sat, 7 Dec 2019 17:52:01 +0300 From: Andrey Savchenko To: ALT Linux Team development discussions Message-Id: <20191207175201.13ca3df6c2cfb01ef559b083@altlinux.org> In-Reply-To: <20191206153655.86334-1-darktemplar@altlinux.org> References: <20191206133647.dculnmwkd3yf2wjp@titan.localdomain> <20191206153655.86334-1-darktemplar@altlinux.org> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; i686-pc-linux-gnu) Mime-Version: 1.0 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg="PGP-SHA512"; boundary="Signature=_Sat__7_Dec_2019_17_52_01_+0300_wG3VkOP0I5LPleFm" Subject: Re: [devel] [PATCH for apt 2/2 v2] 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: Sat, 07 Dec 2019 14:52:10 -0000 Archived-At: List-Archive: List-Post: --Signature=_Sat__7_Dec_2019_17_52_01_+0300_wG3VkOP0I5LPleFm Content-Type: text/plain; charset=US-ASCII Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 >=20 > 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 =3D \ > 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=20 > =20 > +#include > + > #include > =20 > // Package Iterator > @@ -85,11 +87,11 @@ class pkgCache::PkgIterator > inline unsigned long long Index() const {return Pkg - Owner->PkgP;}; > OkState State() const; > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || Pkg =3D=3D 0) > return; > - Pkg +=3D static_cast(newMap) - static_cast(oldMap); > + Pkg =3D RebasePointer(Pkg, oldMap, newMap); > } > =20 > // Constructors > @@ -147,11 +149,11 @@ class pkgCache::VerIterator > bool Automatic() const; > VerFileIterator NewestFile() const; > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || Ver =3D=3D 0) > return; > - Ver +=3D static_cast(newMap) - static_cast(oldMap); > + Ver =3D RebasePointer(Ver, oldMap, newMap); > } > =20 > inline VerIterator() : Ver(0), Owner(0) {}; =20 > @@ -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);}; > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || Dep =3D=3D 0) > return; > - Dep +=3D static_cast(newMap) - static_cast(oldMap); > + Dep =3D RebasePointer(Dep, oldMap, newMap); > } > =20 > inline DepIterator(pkgCache &Owner,Dependency *Trg,Version * =3D 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= ;}; > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || Prv =3D=3D 0) > return; > - Prv +=3D static_cast(newMap) - static_cast(oldMap); > + Prv =3D RebasePointer(Prv, oldMap, newMap); > } > =20 > inline PrvIterator() : Prv(0), Type(PrvVer), Owner(0) {}; > @@ -342,11 +344,11 @@ class pkgCache::PkgFileIterator > bool IsOk(); > string RelStr(); > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || File =3D=3D 0) > return; > - File +=3D static_cast(newMap) - static_cast(oldMap); > + File =3D RebasePointer(File, oldMap, newMap); > } > =20 > // Constructors > @@ -383,11 +385,11 @@ class pkgCache::VerFileIterator > inline PkgFileIterator File() const {return PkgFileIterator(*Owner,Fi= leP->File + Owner->PkgFileP);}; > inline unsigned long long Index() const {return FileP - Owner->VerFil= eP;}; > =20 > - void ReMap(void const * const oldMap, void const * const newMap) > + void ReMap(void *oldMap, void *newMap) > { > if (Owner =3D=3D 0 || FileP =3D=3D 0) > return; > - FileP +=3D static_cast(newMap) - static_cast(oldMap); > + FileP =3D RebasePointer(FileP, oldMap, newMap); > } > =20 > 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 > #include > #include > +#include > =20 > #include > =20 > @@ -301,7 +302,7 @@ std::experimental::optional DynamicMMap::= Allocate(unsigned long Item > Pool* oldPools =3D Pools; > auto idxResult =3D RawAllocate(I->Count*ItemSize,ItemSize); > if (Pools !=3D oldPools) > - I +=3D Pools - oldPools; > + I =3D RebasePointer(I, oldPools, Pools); > =20 > // Does the allocation failed ? > if (!idxResult) > @@ -371,7 +372,7 @@ bool DynamicMMap::Grow(unsigned long long size) > Fd->Write(&C,sizeof(C)); > } > =20 > - unsigned long const poolOffset =3D Pools - ((Pool*) Base); > + void *old_base =3D Base; > =20 > if (Fd !=3D 0) > { > @@ -408,7 +409,8 @@ bool DynamicMMap::Grow(unsigned long long size) > memset((char*)Base + WorkSpace, 0, newSize - WorkSpace); > } > =20 > - Pools =3D (Pool*) Base + poolOffset; > + if (Base !=3D old_base) > + Pools =3D RebasePointer(Pools, old_base, Base); > WorkSpace =3D newSize; > =20 > 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 > #include > #include > +#include > =20 > #include > =20 > @@ -109,18 +110,18 @@ pkgCacheGenerator::~pkgCacheGenerator() > Map.Sync(0,sizeof(pkgCache::Header)); > } > /*}}}*/ > -void pkgCacheGenerator::ReMap(void const * const oldMap, void const * co= nst newMap) > +void pkgCacheGenerator::ReMap(void *oldMap, void *newMap) > { > if (oldMap =3D=3D newMap) > return; > =20 > Cache.ReMap(false); > =20 > - CurrentFile +=3D (pkgCache::PackageFile*) newMap - (pkgCache::Package= File*) oldMap; > + CurrentFile =3D RebasePointer(CurrentFile, oldMap, newMap); > =20 > for (size_t i =3D 0; i < _count(UniqHash); ++i) > if (UniqHash[i] !=3D 0) > - UniqHash[i] +=3D (pkgCache::StringItem*) newMap - (pkgCache::St= ringItem*) oldMap; > + UniqHash[i] =3D RebasePointer(UniqHash[i], oldMap, newMap); > =20 > for (auto i =3D Dynamic::toReMap.begin(); > i !=3D Dynamic::toReMap.end(); ++i) > @@ -147,7 +148,7 @@ void pkgCacheGenerator::ReMap(void const * const oldM= ap, void const * const newM > // CacheGenerator::WriteStringInMap /*{{{*/ > std::experimental::optional pkgCacheGenerator::WriteStringIn= Map(const char *String, > unsigned long Len) { > - void const * const oldMap =3D Map.Data(); > + void *oldMap =3D Map.Data(); > const auto index =3D Map.WriteString(String, Len); > if (index) > ReMap(oldMap, Map.Data()); > @@ -156,7 +157,7 @@ std::experimental::optional pkgCacheGener= ator::WriteStringInMap(cons > /*}}}*/ > // CacheGenerator::WriteStringInMap /*{{{*/ > std::experimental::optional pkgCacheGenerator::WriteStringIn= Map(const char *String) { > - void const * const oldMap =3D Map.Data(); > + void *oldMap =3D Map.Data(); > const auto index =3D Map.WriteString(String); > if (index) > ReMap(oldMap, Map.Data()); > @@ -164,7 +165,7 @@ std::experimental::optional pkgCacheGener= ator::WriteStringInMap(cons > } > /*}}}*/ > std::experimental::optional pkgCacheGenerator::AllocateInMap= (unsigned long size) {/*{{{*/ > - void const * const oldMap =3D Map.Data(); > + void *oldMap =3D Map.Data(); > const auto index =3D Map.Allocate(size); > if (index) > ReMap(oldMap, Map.Data()); > @@ -229,7 +230,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, > pkgCache::VerIterator Ver =3D Pkg.VersionList(); > Dynamic DynVer(Ver); > map_ptrloc *Last =3D &Pkg->VersionList; > - const void * oldMap =3D Map.Data(); > + void *oldMap =3D Map.Data(); > int Res =3D 1; > for (; Ver.end() =3D=3D false; Last =3D &Ver->NextVer, Ver++) > { > @@ -271,7 +272,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, > =20 > if (oldMap !=3D Map.Data()) > { > - Last +=3D (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap; > + Last =3D RebasePointer(Last, oldMap, Map.Data()); > oldMap =3D Map.Data(); > } > =20 > @@ -297,7 +298,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, > PackageName.c_str(), 1); > =20 > if (oldMap !=3D Map.Data()) > - Last +=3D (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap; > + Last =3D RebasePointer(Last, oldMap, Map.Data()); > *Last =3D *verindex; > =20 > Ver->ParentPkg =3D Pkg.Index(); > @@ -553,7 +554,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCac= he::VerIterator &Ver, > unsigned int Op, > unsigned int Type) > { > - void const * const oldMap =3D Owner->Map.Data(); > + void *oldMap =3D Owner->Map.Data(); > pkgCache &Cache =3D Owner->Cache; > =20 > // Get a structure > @@ -605,7 +606,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCac= he::VerIterator &Ver, > OldDepLast =3D &D->NextDepends; > OldDepVer =3D Ver; > } else if (oldMap !=3D Owner->Map.Data()) > - OldDepLast +=3D (map_ptrloc*) Owner->Map.Data() - (map_ptrloc*) ol= dMap; > + OldDepLast =3D RebasePointer(OldDepLast, oldMap, Owner->Map.Data()= ); > =20 > // Is it a file dependency? > if (PackageName[0] =3D=3D '/') > @@ -739,7 +740,7 @@ std::experimental::optional pkgCacheGener= ator::WriteUniqString(const > } > =20 > // Get a structure > - void const * const oldMap =3D Map.Data(); > + void *oldMap =3D Map.Data(); > const auto Item =3D AllocateInMap(sizeof(pkgCache::StringItem)); > const auto idxString =3D WriteStringInMap(S, Size); > if ((!Item) || (!idxString)) > @@ -747,8 +748,8 @@ std::experimental::optional pkgCacheGener= ator::WriteUniqString(const > =20 > if (oldMap !=3D Map.Data()) > { > - Last +=3D (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap; > - I +=3D (pkgCache::StringItem*) Map.Data() - (pkgCache::StringItem*= ) oldMap; > + Last =3D RebasePointer(Last, oldMap, Map.Data()); > + I =3D RebasePointer(I, oldMap, Map.Data()); > } > =20 > *Last =3D *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 function; > + typedef std::function function; > =20 > static std::set toReMap; > =20 > @@ -93,7 +93,7 @@ class pkgCacheGenerator > toReMap.erase(this); > } > =20 > - 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 =3D false;}; > =20 > - void ReMap(void const * const oldMap, void const * const newMap); > + void ReMap(void *oldMap, void *newMap); > =20 > 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 > +static inline T* RebasePointer(T *ptr, void *old_base, void *new_base) > +{ > + return reinterpret_cast(reinterpret_cast(new_base) + (rein= terpret_cast(ptr) - reinterpret_cast(old_base))); > +} > + > +template > +static inline const T* RebasePointer(const T *ptr, void *old_base, void = *new_base) > +{ > + return reinterpret_cast(reinterpret_cast(new_base) += (reinterpret_cast(ptr) - reinterpret_cast(old_base))); > +} > + > +#endif > diff --git a/apt/apt-pkg/rpm/rpmlistparser.cc b/apt/apt-pkg/rpm/rpmlistpa= rser.cc > index 9b2e9ad..bd13b99 100644 > --- a/apt/apt-pkg/rpm/rpmlistparser.cc > +++ b/apt/apt-pkg/rpm/rpmlistparser.cc > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > =20 > #include > =20 > @@ -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; > =20 > for (auto iter: *SeenPackages) > { > - tmp.insert(iter + (static_cast(newMap) - stati= c_cast(oldMap))); > + tmp.insert(RebasePointer(iter, oldMap, newMap)); > } > =20 > SeenPackages->swap(tmp); > diff --git a/apt/apt-pkg/rpm/rpmpackagedata.cc b/apt/apt-pkg/rpm/rpmpacka= gedata.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 =3D VerMap.begin(); iter1 !=3D VerMap.end(); ++ite= r1) > { Best regards, Andrew Savchenko --Signature=_Sat__7_Dec_2019_17_52_01_+0300_wG3VkOP0I5LPleFm Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE63ZIHsdeM+1XgNer9lNaM7oe5I0FAl3rvJEACgkQ9lNaM7oe 5I0qEA/+N2vNCFtlmzQmidIXSyMrHqgc8+6sC4IlAKgISxNjvtcHdBO8x9NOB95Y nCcMzYPYYcuQ15L0y9INJ92v4MLN78NZz4adZmg90g8xFKFx511BuuR2KA/+MXwm J44X10sq4HbzKBe9RmiF5pLig+bLeJcb5c6GzsjczxY+Wf2Vn6W6MiZWeLHHcEYV AvM5LkCjdwpyqRdcPXU0IVqw3Nx3XqU/+uHmQMjhA+hbBrq6pWZF6qNmkSHjSS0C klja8NGnUgPW5hSTPCGtC7E6CDePFA/6a6w293MuRiZF6ak7hRyPqZ+pJlFXaopO 4L/inFkna9VTSUk3lcMQnYLTpTx6K4tP4XomAJfrD1qhFg2Ki0XDqJQqHvmdopU0 ST4aoMYzpAbuMG0B0hwrOERHz1x8dAv79uz+XN4616ryJUSupG/CmkmyIG1VZGzQ DzE5xNYfEAH9c+3fxqye5YJpc+ejGlpTJTvigXu1NWuEzS0mGE3dIxLkl0i6dMJ3 /8JKVffx/GgdcJjVDBJKtyaKCcFIcOEQaM0qelJdV0TvecS08eraWJQhLSFdHgYi QO/OUn317kfMBnUfzbc/d+mpAvOh+b980yHiBOYoRxI1dYh6RiM2/l03CF0WlVHp wtesJ5/2bwXoxnowR+Bd9agcZyfRtv5ihQXuSPihZWp57hMNkV0= =Rb0e -----END PGP SIGNATURE----- --Signature=_Sat__7_Dec_2019_17_52_01_+0300_wG3VkOP0I5LPleFm--