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

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

Fixed; it turns out I had omitted a inline keyword on that helper
function. Adding that inline keyword solved the problem (changeset
4b14b9286929). I'm baffled that GCC didn't decide to inline it as it
was called from a fixed-size matrix ctor which otherwise had zero
cost!

Benoit

2011/10/24 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
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>