'Re: kde-4.0.4 segfaults on netbsd sparc'

[prev in list] [next in list] [prev in thread] [next in thread] 

List: kde-devel
Subject: Re: kde-4.0.4 segfaults on netbsd sparc
From: Michael Pyne <mpyne () purinchu ! net>
Date: 2008-05-27 2:45:28
Message-ID: 200805262245.33160.mpyne () purinchu ! net
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]

[Attachment #6 (multipart/alternative)]


On Sunday 25 May 2008, Sebastian Sauer wrote:
> raymond.meyer@rambler.ru wrote:
> > I can't start kde on netbsd sparc machine, because kdelibs contains code
> > that causes segfault. It terminates with "Bus error" which indicates
> > unaligned memory access. Below is the backtrace.
> >
> > The problem is with kdelibs-4.0.4/kdeui/icons/kpixmapcache.cpp. On line
> > 159 it has
> >
> > *mSize = *((quint32*)(mMemory + mSizeEntryOffset));
> >
> > So looks like a pointer to char is cast to a pointer to some form of
> > uint32_t and then dereferenced. Can anyone suggest a proper fix?
>
> iirc mMemory is the start of a buffer used to cache icons and at
> mMemory+mSizeEntryOffset is a 4byte quint32 that is used to know how much
> icons are within those buffer.
>
> https://bugs.kde.org/show_bug.cgi?id=160284
>
> workaround may to undef USE_MMAP, I also still seek for a proper fix :-(

I'll also attach this to bug 160284, but the attached patch may work (if that 
is the only thing causing unaligned accesses).

The patch works by adding padding between the header and the start of the data 
(what mSizeEntryOffset is supposed to be pointing to). I assume that any 
pointer which is a multiple of sizeof(void*) is a valid aligned address.

The version of the pixmap cache has been bumped so in theory this should "just 
work" but I deleted the pixmap cache many times over developing this so I'm 
not sure. (/var/tmp/kde-cache-$USER/kpc/kde-icon-cache* for me).

Regards,
 - Michael Pyne

[Attachment #9 (text/html)]

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" \
"http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" \
content="1" /><style type="text/css"> p, li { white-space: pre-wrap; }
</style></head><body style=" font-family:'Consolas'; font-size:11pt; font-weight:400; \
font-style:normal;"> On Sunday \
25 May 2008, Sebastian Sauer wrote: &gt; raymond.meyer@rambler.ru wrote: &gt; &gt; I can't start kde \
on netbsd sparc machine, because kdelibs contains code &gt; &gt; that causes segfault. It terminates \
with "Bus error" which indicates &gt; &gt; unaligned memory access. Below is the backtrace. &gt; &gt; &gt; &gt; The problem is with \
kdelibs-4.0.4/kdeui/icons/kpixmapcache.cpp. On line &gt; &gt; 159 it has &gt; &gt; &gt; &gt; *mSize = \
*((quint32*)(mMemory + mSizeEntryOffset)); &gt; &gt; &gt; &gt; So looks like a pointer to char is cast \
to a pointer to some form of &gt; &gt; uint32_t and then dereferenced. Can anyone suggest a \
proper fix? &gt; &gt; iirc mMemory is the \
start of a buffer used to cache icons and at &gt; mMemory+mSizeEntryOffset is a 4byte quint32 \
that is used to know how much &gt; icons are within those buffer. &gt; &gt; \
https://bugs.kde.org/show_bug.cgi?id=160284 &gt; &gt; workaround may to undef USE_MMAP, I also \
still seek for a proper fix :-( I'll also attach this to bug \
160284, but the attached patch may work (if that is the only thing causing unaligned \
accesses). The patch works by adding padding between the header and the start \
of the data (what mSizeEntryOffset is supposed to be pointing to). I assume that any \
pointer which is a multiple of sizeof(void*) is a valid aligned address. The version of the pixmap \
cache has been bumped so in theory this should "just work" but I deleted the pixmap \
cache many times over developing this so I'm not sure. \
(/var/tmp/kde-cache-$USER/kpc/kde-icon-cache* for me). Regards, - Michael \
Pyne</body></html>


["0001-kpixmapcache-fix-unaligned.patch" (text/x-patch)]

Index: kiconcache.cpp
===================================================================

--- kiconcache.cpp	(revision 812533)
+++ kiconcache.cpp	(working copy)
@@ -128,7 +128,7 @@
     quint32 version;
     stream >> version;
     if (version != KDE_ICONCACHE_VERSION) {
-        kDebug(264) << "Obsolete iconcache version, will recreate";
+        kDebug(264) << "Obsolete iconcache version" << version << "will recreate";
         return false;
     }
 
Index: kpixmapcache.cpp
===================================================================
--- kpixmapcache.cpp	(revision 812533)
+++ kpixmapcache.cpp	(working copy)
@@ -56,7 +56,7 @@
 #define USE_MMAP
 #endif
 
-#define KPIXMAPCACHE_VERSION 0x000206
+#define KPIXMAPCACHE_VERSION 0x000207
 
 namespace {
 
@@ -208,7 +208,6 @@
 
 } // namespace
 
-
 class KPixmapCache::Private
 {
 public:
@@ -251,9 +250,10 @@
 
     KPixmapCache* q;
 
-    static const char* kpc_magic;
+    static const char *const kpc_magic;
     static const int kpc_magic_len;
     static const int kpc_header_len;
+
     quint32 mHeaderSize;  // full size of the index header, including custom (subclass') header data
     quint32 mIndexRootOffset;  // offset of the first entry in index file
 
@@ -368,17 +368,30 @@
 };
 
 // Magic in the cache files
-const char* KPixmapCache::Private::kpc_magic = "KDE PIXMAP CACHE ";
+
+const char *const KPixmapCache::Private::kpc_magic = "KDE PIXMAP CACHE ";
 const int KPixmapCache::Private::kpc_magic_len = qstrlen(KPixmapCache::Private::kpc_magic);
-// Whole header is magic + version (4 bytes)
-const int KPixmapCache::Private::kpc_header_len = KPixmapCache::Private::kpc_magic_len + 4;
 
+// Add in padding such that the next byte after the header is already pointer
+// aligned.  i.e. if this architecture requires 4 byte alignment for pointers
+// and kpc_magic is 22 bytes, make the header 24 bytes long.
+// The header itself is the magic plus the 4 bytes for the version ID.
+const int KPixmapCache::Private::kpc_header_len =
+    KPixmapCache::Private::kpc_magic_len + 4
 
+    // This is the amount of padding required:
+    + ( // Wow, this looks like LISP...
+        (sizeof(void*) - ((KPixmapCache::Private::kpc_magic_len + 4) % sizeof(void*)))
+    ) % sizeof(void*);
+
+
 KPixmapCache::Private::Private(KPixmapCache* _q)
 {
     q = _q;
     mRemovalThread = 0;
 
+    // the size offset must be to a properly aligned memory address, see above
+    // for how we get kpc_header_len to do that.
     KPCMemoryDevice::setSizeEntryOffset(kpc_header_len);  // ugly
 }
 
@@ -457,7 +470,7 @@
         info->file = 0;
         return false;
     }
-    info->memory = reinterpret_cast<char*>(indexMem);
+    info->memory = static_cast<char*>(indexMem);
 #ifdef HAVE_MADVISE
     madvise(info->memory, info->size, MADV_WILLNEED);
 #endif
@@ -659,11 +672,11 @@
         QDataStream stream(&f);
 
         // Check header and version
-        char buf[100];
+        QByteArray buf(kpc_magic_len + 1, '\0');
         quint32 version;
-        stream.readRawData(buf, kpc_magic_len);
+        stream.readRawData(buf.data(), kpc_magic_len);
         stream >> version;
-        if (qstrncmp(buf, kpc_magic, kpc_magic_len) == 0) {
+        if (qstrncmp(buf.constData(), kpc_magic, kpc_magic_len) == 0) {
             if (version == KPIXMAPCACHE_VERSION) {
                 return true;
             } else if (version < KPIXMAPCACHE_VERSION) {
@@ -1123,6 +1136,8 @@
     QDataStream istream(&indexfile);
     istream.writeRawData(d->kpc_magic, d->kpc_magic_len);
     istream << (quint32)KPIXMAPCACHE_VERSION;
+    for(int i = d->kpc_magic_len + sizeof(quint32); i < d->kpc_header_len; ++i)
+        istream << quint8(0); // Add padding
     // Write invalid file size, valid one will be written later by mmap code
     d->mIndexMmapInfo.size = 0;
     istream << d->mIndexMmapInfo.size;
@@ -1140,8 +1155,12 @@
         return false;
     }
     QDataStream dstream(&datafile);
+    kDebug(264) << "Magic len:" << d->kpc_magic_len;
+    kDebug(264) << "Header len:" << d->kpc_header_len;
     dstream.writeRawData(d->kpc_magic, d->kpc_magic_len);
     dstream << (quint32)KPIXMAPCACHE_VERSION;
+    for(int i = d->kpc_magic_len + sizeof(quint32); i < d->kpc_header_len; ++i)
+        dstream << quint8(0); // Add padding
     d->mDataMmapInfo.size = 0;
     dstream << d->mDataMmapInfo.size;
 

["signature.asc" (application/pgp-signature)]

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread] 
