Re: [eigen] Checking for integer overflow in size computations

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

I just realized this introduces an unwanted performance regression as
sizes are checked at runtime even when they are known at compile time.
I've been too optimistic in assuming that the compiler would always
inline my helper function. Patch coming soon.

Benoit

2011/10/17 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> Pushed 626705504d3ef1d81e968bfe901816f2b38144ad with this commit message:
>
> Throw std::bad_alloc even when exceptions are disabled, by doing new
> int[size_t(-1)].
> Don't throw exceptions on aligned_malloc(0) (just because malloc's
> retval is null doesn't mean error, if size==0).
> Remove EIGEN_NO_EXCEPTIONS option, use only compiler standard defines.
> Either exceptions are enabled or they aren't.
>
> Sounds OK to everybody?
>
> Benoit
>
> 2011/10/17 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
>> 2011/10/17 Eamon Nerbonne <eamon@xxxxxxxxxxxx>:
>>> If you're building without exceptions and an exception gets thrown,
>>
>> It's impossible to compile without exceptions code that explicitly
>> throws exceptions, as that results in a compilation error:
>>
>> a.cpp: In function ‘int main()’:
>> a.cpp:5:24: error: exception handling disabled, use -fexceptions to enable
>>
>> For this reason, currently in Eigen our code throwing exceptions is
>> #ifdef EIGEN_EXCEPTIONS which means it's not compiled at all when
>> exceptions are disabled.
>>
>> That said, I tried a simple c++ program with a huge new[] to get an
>> exception thrown for me, compiled with -fno-exceptions, and that
>> indeed worked: a std::bad_alloc exception was thrown and killed my
>> test program.
>>
>> My understanding is that the only way to get exactly the same behavior
>> from code that compiles with -fno-exceptions would be to call into
>> code that was compiled with exceptions. A way to simulate that is to
>> just do int *a = new int[size_t(-1)];
>>
>> Patch coming.
>>
>> Benoit
>>
>>> it just
>>> means app termination (unless you're being really weird and catching the
>>> exception anyhow, not sure what happens then...), which is probably the
>>> right behavior in face of OOM anyhow.
>>>
>>> Also, even if exceptions are off by default in some projects, it's really
>>> handy to use them anyhow, since they make debugging that easier - i.e. a
>>> scenario in which devs can easily trigger on exceptions, regardless of
>>> wether the subsequent cleanup will honor destructors or not.  By constrast,
>>> a nullptr return value won't necessarily be easy to notice unless
>>> potentially much later.
>>>
>>> --eamon@xxxxxxxxxxxx - Tel#:+31-6-15142163
>>>
>>>
>>> On Mon, Oct 17, 2011 at 00:36, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>>> wrote:
>>>>
>>>> 2011/10/16 Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx>:
>>>> > On 16.10.2011 22:21, Benoit Jacob wrote:
>>>> >>
>>>> >> Note that there is a small performance overhead associated with that.
>>>> >> I tried to minimize it, but there is still 1 integer division in the
>>>> >> size=rows*cols overflow check. I hope that's still negligible compared
>>>> >> to the cost of malloc.
>>>> >
>>>> > I have not looked into your patch yet, but wouldn't a multiplication in
>>>> > int64/int128 also do the job?
>>>>
>>>> I don't know that all platforms have an integer type twice bigger than
>>>> size_t?
>>>>
>>>> On x86-64 are there 128 bit integers? I didn't know that.
>>>> Also I'm afraid there might exist 32bit platforms without 64bit integers.
>>>>
>>>> > I guess that would be much cheaper than a
>>>> > division. Anyways I would agree that the overhead should be negligible.
>>>> > If
>>>> > someone really cares for that overhead, we might introduce a flag such
>>>> > as
>>>> > EIGEN_DONT_CHECK_INTEGER_OVERFLOW.
>>>> >
>>>> >> Like the standard behavior of operator new and of our own aligned_new
>>>> >> routines, these checks throw std::bad_alloc on error and don't do
>>>> >> anything else. In particular, we still don't guarantee that
>>>> >> aligned_new returns null on error. It feels weird to me to rely
>>>> >> entirely on exceptions to report these errors, but this is how c++
>>>> >> works (as long as one doesn't use nothrow-new) and what we've been
>>>> >> doing. Is this OK?
>>>> >
>>>> > I don't want to start a philosophical discussion about exceptions vs
>>>> > return-type checking, but honestly I guess that the C++ way is much more
>>>> > reliable, since about 99% of malloc users hardly ever check for NULL
>>>> > pointers ("Come on, how likely is it that I can't get ** MB?").
>>>> > Furthermore, I guess hardly anyone disables exceptions in C++, unless he
>>>> > really knows what he's doing.
>>>>
>>>> Both Mozilla and KDE are examples of large projects building with
>>>> -fno-exceptions. That's the kind of fact that makes me uncomfortable
>>>> about C++ exceptions.
>>>>
>>>> Benoit
>>>>
>>>>
>>>
>>>
>>
>