Quantcast

alloc.h global operator new replacement violating alignment requirements

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

alloc.h global operator new replacement violating alignment requirements

Stephan Bergmann
Forgive me if this has already been discussed or even fixed in later
versions:  At least the Firebird 3.0 we build as part of LibreOffice
defines global operator new replacement functions in
src/common/classes/alloc.h (forwarding to MemoryPool) that do not in
general fulfil the alignment requirements for such functions.

Came across this when Firebird compiled with a recent trunk Clang (with
-O, and DEBUG_GDS_ALLOC being undefined) on x86_64-unknown-linux-gnu
causes SEGV from misaligned MOVAPS instructions.

See the mail thread starting at
<http://lists.llvm.org/pipermail/cfe-dev/2017-February/052676.html>
"[cfe-dev] operator new alignment assumptions" for some details.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Alex Peshkoff
On 02/16/17 15:52, Stephan Bergmann wrote:

> Forgive me if this has already been discussed or even fixed in later
> versions:  At least the Firebird 3.0 we build as part of LibreOffice
> defines global operator new replacement functions in
> src/common/classes/alloc.h (forwarding to MemoryPool) that do not in
> general fulfil the alignment requirements for such functions.
>
> Came across this when Firebird compiled with a recent trunk Clang (with
> -O, and DEBUG_GDS_ALLOC being undefined) on x86_64-unknown-linux-gnu
> causes SEGV from misaligned MOVAPS instructions.
>

Yes - allocated memory is aligned at 8 bytes boundary now.
I've tried to set alignment to 16 but looks like that's far not 5 lines
patch - sometimes we were choosing between 4/8 bytes alignment, but last
years only 8 bytes alignment was used.
May be finding specific compiler flag to avoid this instruction is
simpler choice for today?



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Stephan Bergmann
On 02/17/2017 11:48 AM, Alex Peshkoff wrote:

> On 02/16/17 15:52, Stephan Bergmann wrote:
>> Forgive me if this has already been discussed or even fixed in later
>> versions:  At least the Firebird 3.0 we build as part of LibreOffice
>> defines global operator new replacement functions in
>> src/common/classes/alloc.h (forwarding to MemoryPool) that do not in
>> general fulfil the alignment requirements for such functions.
>>
>> Came across this when Firebird compiled with a recent trunk Clang (with
>> -O, and DEBUG_GDS_ALLOC being undefined) on x86_64-unknown-linux-gnu
>> causes SEGV from misaligned MOVAPS instructions.
>>
>
> Yes - allocated memory is aligned at 8 bytes boundary now.
> I've tried to set alignment to 16 but looks like that's far not 5 lines
> patch - sometimes we were choosing between 4/8 bytes alignment, but last
> years only 8 bytes alignment was used.
> May be finding specific compiler flag to avoid this instruction is
> simpler choice for today?

(The way I work around this for now in LibreOffice is by always defining
DEBUG_GDS_ALLOC when building with Clang on Linux X86-64,
<https://cgit.freedesktop.org/libreoffice/core/commit/?id=8ea07101c1613d213fd7cea17f094a947b14cd00>
"external/firebird: Work around operator new alignment violations".
Since LibreOffice builds intended for widespread distribution are on
Linux usually done with GCC not Clang, this shouldn't have a performance
impact.)

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Alex Peshkoff
On 02/17/17 14:15, Stephan Bergmann wrote:

> On 02/17/2017 11:48 AM, Alex Peshkoff wrote:
>> On 02/16/17 15:52, Stephan Bergmann wrote:
>>> Forgive me if this has already been discussed or even fixed in later
>>> versions:  At least the Firebird 3.0 we build as part of LibreOffice
>>> defines global operator new replacement functions in
>>> src/common/classes/alloc.h (forwarding to MemoryPool) that do not in
>>> general fulfil the alignment requirements for such functions.
>>>
>>> Came across this when Firebird compiled with a recent trunk Clang (with
>>> -O, and DEBUG_GDS_ALLOC being undefined) on x86_64-unknown-linux-gnu
>>> causes SEGV from misaligned MOVAPS instructions.
>>>
>> Yes - allocated memory is aligned at 8 bytes boundary now.
>> I've tried to set alignment to 16 but looks like that's far not 5 lines
>> patch - sometimes we were choosing between 4/8 bytes alignment, but last
>> years only 8 bytes alignment was used.
>> May be finding specific compiler flag to avoid this instruction is
>> simpler choice for today?
> (The way I work around this for now in LibreOffice is by always defining
> DEBUG_GDS_ALLOC when building with Clang on Linux X86-64,
> <https://cgit.freedesktop.org/libreoffice/core/commit/?id=8ea07101c1613d213fd7cea17f094a947b14cd00>
> "external/firebird: Work around operator new alignment violations".
> Since LibreOffice builds intended for widespread distribution are on
> Linux usually done with GCC not Clang, this shouldn't have a performance
> impact.)

Afraid that this work around is not too stable (not to say more).
Certainly that macro does change details of memory allocation and may
provide 16-bit alignment in some particular case but be sure it's not
more than side effect and does not gurantee such alignment.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Alex Peshkoff
In reply to this post by Stephan Bergmann
On 02/17/17 14:15, Stephan Bergmann wrote:

This idea seems to be much better than DEBUG_GDS_ALLOC:

> Since LibreOffice builds intended for widespread distribution are on
> Linux usually done with GCC not Clang, this shouldn't have a performance
> impact.)

But what makes me surprised - I know that at least one of FB developers
often does build firebird using clang (Adriano - is it so?). And we
never used to meet such issues before. What changed? New version? Too
good optimization flags?



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Stephan Bergmann
In reply to this post by Alex Peshkoff
On 02/17/2017 12:21 PM, Alex Peshkoff wrote:

> On 02/17/17 14:15, Stephan Bergmann wrote:
>> On 02/17/2017 11:48 AM, Alex Peshkoff wrote:
>>> On 02/16/17 15:52, Stephan Bergmann wrote:
>>>> Forgive me if this has already been discussed or even fixed in later
>>>> versions:  At least the Firebird 3.0 we build as part of LibreOffice
>>>> defines global operator new replacement functions in
>>>> src/common/classes/alloc.h (forwarding to MemoryPool) that do not in
>>>> general fulfil the alignment requirements for such functions.
>>>>
>>>> Came across this when Firebird compiled with a recent trunk Clang (with
>>>> -O, and DEBUG_GDS_ALLOC being undefined) on x86_64-unknown-linux-gnu
>>>> causes SEGV from misaligned MOVAPS instructions.
>>>>
>>> Yes - allocated memory is aligned at 8 bytes boundary now.
>>> I've tried to set alignment to 16 but looks like that's far not 5 lines
>>> patch - sometimes we were choosing between 4/8 bytes alignment, but last
>>> years only 8 bytes alignment was used.
>>> May be finding specific compiler flag to avoid this instruction is
>>> simpler choice for today?
>> (The way I work around this for now in LibreOffice is by always defining
>> DEBUG_GDS_ALLOC when building with Clang on Linux X86-64,
>> <https://cgit.freedesktop.org/libreoffice/core/commit/?id=8ea07101c1613d213fd7cea17f094a947b14cd00>
>> "external/firebird: Work around operator new alignment violations".
>> Since LibreOffice builds intended for widespread distribution are on
>> Linux usually done with GCC not Clang, this shouldn't have a performance
>> impact.)
>
> Afraid that this work around is not too stable (not to say more).
> Certainly that macro does change details of memory allocation and may
> provide 16-bit alignment in some particular case but be sure it's not
> more than side effect and does not gurantee such alignment.

Note that I'm not relying on any 16-byte alignment there, but on the
fact that Clang happens to make those aggressive alignment assumptions
only for the replaceable global allocation functions.  And with
DEBUG_GDS_ALLOC defined, the global allocation functions defined in
alloc.h are no longer replacements of such replaceable allocation functions.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Stephan Bergmann
In reply to this post by Alex Peshkoff
On 02/17/2017 12:24 PM, Alex Peshkoff wrote:

> On 02/17/17 14:15, Stephan Bergmann wrote:
>
> This idea seems to be much better than DEBUG_GDS_ALLOC:
>
>> Since LibreOffice builds intended for widespread distribution are on
>> Linux usually done with GCC not Clang, this shouldn't have a performance
>> impact.)
>
> But what makes me surprised - I know that at least one of FB developers
> often does build firebird using clang (Adriano - is it so?). And we
> never used to meet such issues before. What changed? New version? Too
> good optimization flags?

I think this aggressive alignment-assumption behavior is new with
upcoming Clang 4.  (And you need at least -O optimization for such
problematic MOVAPS code to be produced.)


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Adriano dos Santos Fernandes-3
On 17/02/2017 09:39, Stephan Bergmann wrote:

> On 02/17/2017 12:24 PM, Alex Peshkoff wrote:
>> On 02/17/17 14:15, Stephan Bergmann wrote:
>>
>> This idea seems to be much better than DEBUG_GDS_ALLOC:
>>
>>> Since LibreOffice builds intended for widespread distribution are on
>>> Linux usually done with GCC not Clang, this shouldn't have a performance
>>> impact.)
>> But what makes me surprised - I know that at least one of FB developers
>> often does build firebird using clang (Adriano - is it so?). And we
>> never used to meet such issues before. What changed? New version? Too
>> good optimization flags?
> I think this aggressive alignment-assumption behavior is new with
> upcoming Clang 4.  (And you need at least -O optimization for such
> problematic MOVAPS code to be produced.)
>
>
I'm using clang 3.8 (from Ubuntu 16.10) and always building FB in debug
mode.


Adriano


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: alloc.h global operator new replacement violating alignment requirements

Paul Beach
In reply to this post by Stephan Bergmann
On 17/02/2017 09:39, Stephan Bergmann wrote:

> On 02/17/2017 12:24 PM, Alex Peshkoff wrote:
>> On 02/17/17 14:15, Stephan Bergmann wrote:
>>
>> This idea seems to be much better than DEBUG_GDS_ALLOC:
>>
>>> Since LibreOffice builds intended for widespread distribution are on
>>> Linux usually done with GCC not Clang, this shouldn't have a performance
>>> impact.)
>> But what makes me surprised - I know that at least one of FB developers
>> often does build firebird using clang (Adriano - is it so?). And we
>> never used to meet such issues before. What changed? New version? Too
>> good optimization flags?
> I think this aggressive alignment-assumption behavior is new with
> upcoming Clang 4.  (And you need at least -O optimization for such
> problematic MOVAPS code to be produced.)

Clang is also the default compiler on MacOSX nowadays ....

Paul

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Loading...