From: Aleksei Nikiforov <darktemplar@altlinux.org>
To: devel@lists.altlinux.org
Subject: Re: [devel] [PATCH for apt 2/2] Fix pointer arithmetics
Date: Fri, 6 Dec 2019 18:32:27 +0300
Message-ID: <ade56823-6605-397f-2954-060ddaf58ca1@altlinux.org> (raw)
In-Reply-To: <20191206133647.dculnmwkd3yf2wjp@titan.localdomain>
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 <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?
>
>
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<T*>(reinterpret_cast<char*>(ptr) +
(reinterpret_cast<char const * const>(new_base) - reinterpret_cast<char
const * const>(old_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, 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 mailing list
> Devel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel
>
next prev parent reply other threads:[~2019-12-06 15:32 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-06 13:16 [devel] [PATCH for apt 1/2] Add option for debugging DynamicMMap::Allocate Aleksei Nikiforov
2019-12-06 13:16 ` [devel] [PATCH for apt 2/2] Fix pointer arithmetics Aleksei Nikiforov
2019-12-06 13:36 ` Ivan A. Melnikov
2019-12-06 15:32 ` Aleksei Nikiforov [this message]
2019-12-06 15:36 ` [devel] [PATCH for apt 2/2 v2] " Aleksei Nikiforov
2019-12-07 14:52 ` Andrey Savchenko
2019-12-08 22:56 ` Dmitry V. Levin
2019-12-09 6:54 ` Aleksei Nikiforov
2019-12-12 19:20 ` Andrey Savchenko
2019-12-13 7:58 ` Aleksei Nikiforov
2019-12-08 23:21 ` Dmitry V. Levin
2019-12-09 7:08 ` Aleksei Nikiforov
2019-12-10 0:07 ` Dmitry V. Levin
2019-12-10 8:18 ` Aleksei Nikiforov
2019-12-10 10:20 ` Dmitry V. Levin
2019-12-10 10:58 ` Aleksei Nikiforov
2019-12-10 22:20 ` Dmitry V. Levin
2019-12-11 7:50 ` Aleksei Nikiforov
2019-12-12 19:43 ` Andrey Savchenko
2019-12-13 8:01 ` Aleksei Nikiforov
2019-12-08 23:31 ` [devel] [PATCH for apt 2/2] " Dmitry V. Levin
2019-12-09 7:09 ` Aleksei Nikiforov
2019-12-09 7:16 ` [devel] [PATCH for apt 2/2 v3] " Aleksei Nikiforov
2019-12-09 23:54 ` [devel] [PATCH apt 0/3] Fix 0.5.15lorg2-alt70~9 fallout Dmitry V. Levin
2019-12-09 23:56 ` [devel] [PATCH apt 1/3] apt-pkg/cacheiterators.h: revert #include <set> Dmitry V. Levin
2019-12-09 23:56 ` [devel] [PATCH apt 2/3] apt-pkg/contrib/mmap.cc: revert confusing change of Flags to this->Flags Dmitry V. Levin
2019-12-09 23:56 ` [devel] [PATCH apt 3/3] Fix UB in pointer arithmetic Dmitry V. Levin
2019-12-10 8:18 ` Aleksei Nikiforov
2019-12-08 22:50 ` [devel] [PATCH for apt 1/2] Add option for debugging DynamicMMap::Allocate Dmitry V. Levin
2019-12-09 6:58 ` Aleksei Nikiforov
2019-12-09 10:24 ` Dmitry V. Levin
2019-12-09 11:03 ` [devel] [PATCH for apt 1/2 v3] Add Debug::DynamicMMap::Allocate option Aleksei Nikiforov
2019-12-09 22:59 ` Dmitry V. Levin
2019-12-09 7:00 ` [devel] [PATCH for apt 1/2 v2] Add option for debugging Debug::DynamicMMap::Allocate Aleksei Nikiforov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ade56823-6605-397f-2954-060ddaf58ca1@altlinux.org \
--to=darktemplar@altlinux.org \
--cc=devel@lists.altlinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
ALT Linux Team development discussions
This inbox may be cloned and mirrored by anyone:
git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
public-inbox-index devel
Example config snippet for mirrors.
Newsgroup available over NNTP:
nntp://lore.altlinux.org/org.altlinux.lists.devel
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git