Hi Ronald and all,
Sorry for jumping into this thread so late, but there's one thing I think
it still worth clarifying.
Ronald pointed out:
> In any case, the only part that still bothers me (except for the fact
that
> alignment is done at all, which I don't particularly like) is in lines
> 137 through 143 of this part of the code:
> 129 void XMemory::operator delete(void* p, MemoryManager* manager)
> 130 {
> 131 assert(manager != 0);
> 132
> 133 if (p != 0)
> 134 {
> 135 size_t headerSize =
XMLPlatformUtils::alignPointerForNewBlockAllocation(sizeof(MemoryManager*));
> 136 void* const block = (char*)p - headerSize;
> 137 assert(*(MemoryManager**)block == manager);
> 138 /***
> 139 * NOTE: for compiler which can't properly trace the
memory manager used in the
> 140 * placement new, we use the memory manager
embedded in the memory rather
> 141 * than the one passed in
> 142 */
> 143 MemoryManager* pM = *(MemoryManager**)block;
> 144 pM->deallocate(block);
> 145 }
> 146 }
> i.e. the code doesn't trust the parameters of the function. I think that
is
> wrong because I might very well like to write a memory manager that can
take
> over the management from another one - and there is no reason why I
shouldn't.
None at all, except that, as the comment in lines 138-142 tries to point
out, there is at least one buggy compiler which can't handle this. To
whit, the version of the Intel 7.1 64-bit Windows compiler we were using
would choke on this in certain situations, and that's the workaround we
were obliged to adopt. Apparently, this will be fixed in the 8.0 version
of that compiler, so once we step up, perhaps we can make the code do the
right thing, as it did originally.
Cheers,
Neil
Neil Graham
XML Parser Development
IBM Toronto Lab
Phone: 905-413-3519, T/L 969-3519
E-mail: [EMAIL PROTECTED]
Ronald Landheer-Cieslak
<[EMAIL PROTECTED] To: [EMAIL PROTECTED]
eforge.net> cc:
Subject: Re: Memory management:
the XMemory class
10/27/2003 12:54 PM
Please respond to
xerces-c-dev
On Mon, Oct 27, 2003 at 06:46:01AM -0800, James Berry wrote:
> On Oct 27, 2003, at 6:00 AM, Ronald Landheer-Cieslak wrote:
>
> > Q: what's the alignment I should use?
> > A: something smaller than the size of a pointer - like zero!
> >Result: part of - or everything of - the pointer can (and will) be
> >overwritten
> >with user data. Thinking of it a bit more, I think a patch should be
> >done to
> >XMLPlatformUtils::alignPointerForNewBlockAllocation that checks whether
> >XML_PLATFORM_NEW_BLOCK_ALIGNMENT < the pointer size, and if so makes
> >it return
> >zero. The caller should check whether 0 was returned and not assume
> >anything
> >about the pointer stored in the data area if it is. Like that, the
> >semantics
> >become well-defined and the question gets a correct answer.
> You've misread the code. The alignment has no effect on how much space
> is reserved for the block header. Space for the block header is
> allocated anyway. The alignment simply dictates the alignment of the
> data following the block header.
>
> An alignment of < pointer size is perfectly legal for some
> architectures, which might allow alignment of 1, 2, etc.
>
> A bogus configuration that specifies alignment of zero will:
>
> (1) Not cause any overwriting of data.
> (2) Will likely give an immediate divide by zero runtime error
in
> execution as the expression
> size_t current = ptrSize % alignment;
> is evaluated.
>
> (I've added a comment to the alignPointerForNewBlockAllocation code,
> noting that a XML_PLATFORM_NEW_BLOCK_ALIGNMENT declaration of zero is
> illegal).
Severe cafeine deprivation strikes again :(
In deed I mis-read the XMLPlatformUtils::alignPointerForNewBlockAllocation
function..
> I don't agree. alignPointerForNewBlockAllocation does not return false
> information, nor should it return zero, ever, unless input of zero is
> given to it. It works correctly.
You're right - I've got my eyes open this time ;)
> The problem that needs to be understood is what is causing spurious
> crashes in your environment. You have guessed that an improper
> alignment specification for your architecture may be at fault. If
> that's proven to be the case then we can move forward to dealing with
> it.
Yes, well, that much seems to be proven, as going to a 16-byte alignment
eliminated the problem for me.
In any case, the only part that still bothers me (except for the fact that
alignment is done at all, which I don't particularly like) is in lines
137 through 143 of this part of the code:
129 void XMemory::operator delete(void* p, MemoryManager* manager)
130 {
131 assert(manager != 0);
132
133 if (p != 0)
134 {
135 size_t headerSize =
XMLPlatformUtils::alignPointerForNewBlockAllocation(sizeof(MemoryManager*));
136 void* const block = (char*)p - headerSize;
137 assert(*(MemoryManager**)block == manager);
138 /***
139 * NOTE: for compiler which can't properly trace the memory
manager used in the
140 * placement new, we use the memory manager embedded
in the memory rather
141 * than the one passed in
142 */
143 MemoryManager* pM = *(MemoryManager**)block;
144 pM->deallocate(block);
145 }
146 }
i.e. the code doesn't trust the parameters of the function. I think that is
wrong because I might very well like to write a memory manager that can
take
over the management from another one - and there is no reason why I
shouldn't.
Personally, I think that if client code passes in a parameter it is up to
that
code to make sure it's OK - if not, there's no reason not to go KO.
As for alignment being done at all: I'd also like it to be possible not to
use
the header and to always use the default managers, but that could be just
me.
If such a functionality seems interesting to anyone, I'm willing to provide
a
patch :)
rlc
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
