Re: [Development] QtCore missing check for memory allocation
On Tuesday 10 March 2015 10:10:46 Alex Montgomery wrote: Note that we're talking about a standard violation in the first place. The standard says you can replace operator new, so if MSVC doesn't allow you to do it properly, then you can throw the standard out of the window. If an inline operator new (for MSVC only) solved the problem, it would be ok. As in, you'd be ok with individuals locally hacking Qt to work this way, or as in you'd be ok with including a mechanism in a future version of Qt to replace new and delete that (on Windows only) used this methodology? I'm ok with adding small patches that would ease your local maintenance burden, so long as they don't increase ours unduly and as long as they don't have any performance impact by default. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Tuesday 10 March 2015 08:18:42 Koehne Kai wrote: Except that dynamically linked Windows Qt applications (read: most) don't work this way, so Windows users are left out in the cold. DLLs do not allow you to simply replace one new operator across link boundaries. See the comments in this Qt bug: https://bugreports.qt.io/browse/QTBUG-37395 True. [cut] Question is whether you can easily force all Qt modules to use a custom operator new / delete ... I've seen suggestions [1] to define an inline operator new / delete (e.g. in qglobal.h), but that seems to violate the standard explicitly stating that The program's definitions shall not be specified as inline (C++14 17.6.4.3.3 3). Might nevertheless work though, since there's a high chance that qglobal.h is included in all places where Qt allocates/deallocates memory... Note that we're talking about a standard violation in the first place. The standard says you can replace operator new, so if MSVC doesn't allow you to do it properly, then you can throw the standard out of the window. If an inline operator new (for MSVC only) solved the problem, it would be ok. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Note that we're talking about a standard violation in the first place. The standard says you can replace operator new, so if MSVC doesn't allow you to do it properly, then you can throw the standard out of the window. If an inline operator new (for MSVC only) solved the problem, it would be ok. As in, you'd be ok with individuals locally hacking Qt to work this way, or as in you'd be ok with including a mechanism in a future version of Qt to replace new and delete that (on Windows only) used this methodology? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
-Original Message- From: development-bounces+kai.koehne=theqtcompany.com@qt- project.org [mailto:development- bounces+kai.koehne=theqtcompany@qt-project.org] On Behalf Of Alex Montgomery Sent: Monday, March 09, 2015 6:56 PM To: Knoll Lars Cc: development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation On Mon, Mar 9, 2015 at 12:57 AM, Knoll Lars lars.kn...@theqtcompany.com wrote: Yes, the best solution IMO is still to use your own malloc and operator new replacements. In addition, OOM handlers on the OS level can help. Except that dynamically linked Windows Qt applications (read: most) don't work this way, so Windows users are left out in the cold. DLLs do not allow you to simply replace one new operator across link boundaries. See the comments in this Qt bug: https://bugreports.qt.io/browse/QTBUG-37395 True. Is there any strategy for the large body of Qt developers targeting Windows, or is statically linking the only supported method for replacing Qt allocators on Windows? You almost definitely need to recompile Qt , because you can't really override operator new/delete across DLL boundaries (as you already pointed out in the bug report). Question is whether you can easily force all Qt modules to use a custom operator new / delete ... I've seen suggestions [1] to define an inline operator new / delete (e.g. in qglobal.h), but that seems to violate the standard explicitly stating that The program's definitions shall not be specified as inline (C++14 17.6.4.3.3 3). Might nevertheless work though, since there's a high chance that qglobal.h is included in all places where Qt allocates/deallocates memory... Regards Kai [1]: https://social.msdn.microsoft.com/Forums/vstudio/en-US/ab642c88-2d2d-4f5d-9fd7-2341442d5a46/replacing-new-and-delete-how-to-export-the-implementation?forum=vclanguage) ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
It's my understanding that on Windows you link the global operator new/delete replacement into every dll separately. A howto: 1) Implement your replacement global operator new and delete. Just do so in a single .cpp file, you don't need a header. 2) Compile this into a static library. 3) Pass the static library to the configure script as an extra library so that it is linked into every Qt .dll. If you want, also link it into your main .exe or just add the .cpp directly. You don't need to link to Qt statically, but you do need to compile Qt yourself in order to statically link into every dll your custom implementation of global operator new delete. Now, replacing malloc/free on Windows is non-trivial. To my knowledge, your best bet would be to use detours [1], but I have never tried it. Cheers, Louai [1] http://research.microsoft.com/en-us/projects/detours/ From: development-bounces+louai.al-khanji=theqtcompany@qt-project.org development-bounces+louai.al-khanji=theqtcompany@qt-project.org on behalf of Koehne Kai kai.koe...@theqtcompany.com Sent: Tuesday, March 10, 2015 10:18 AM To: Alex Montgomery; Knoll Lars Cc: development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation -Original Message- From: development-bounces+kai.koehne=theqtcompany.com@qt- project.org [mailto:development- bounces+kai.koehne=theqtcompany@qt-project.org] On Behalf Of Alex Montgomery Sent: Monday, March 09, 2015 6:56 PM To: Knoll Lars Cc: development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation On Mon, Mar 9, 2015 at 12:57 AM, Knoll Lars lars.kn...@theqtcompany.com wrote: Yes, the best solution IMO is still to use your own malloc and operator new replacements. In addition, OOM handlers on the OS level can help. Except that dynamically linked Windows Qt applications (read: most) don't work this way, so Windows users are left out in the cold. DLLs do not allow you to simply replace one new operator across link boundaries. See the comments in this Qt bug: https://bugreports.qt.io/browse/QTBUG-37395 True. Is there any strategy for the large body of Qt developers targeting Windows, or is statically linking the only supported method for replacing Qt allocators on Windows? You almost definitely need to recompile Qt , because you can't really override operator new/delete across DLL boundaries (as you already pointed out in the bug report). Question is whether you can easily force all Qt modules to use a custom operator new / delete ... I've seen suggestions [1] to define an inline operator new / delete (e.g. in qglobal.h), but that seems to violate the standard explicitly stating that The program's definitions shall not be specified as inline (C++14 17.6.4.3.3 3). Might nevertheless work though, since there's a high chance that qglobal.h is included in all places where Qt allocates/deallocates memory... Regards Kai [1]: https://social.msdn.microsoft.com/Forums/vstudio/en-US/ab642c88-2d2d-4f5d-9fd7-2341442d5a46/replacing-new-and-delete-how-to-export-the-implementation?forum=vclanguage) ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Hi, On Tue, Mar 10, 2015 at 10:55 AM, Al-Khanji Louai louai.al-kha...@theqtcompany.com wrote: It's my understanding that on Windows you link the global operator new/delete replacement into every dll separately. Well there is a very hackish but easier way, patching libcmt.lib to remove allocation functionality: https://github.com/adobe/chromium/blob/master/base/allocator/prep_libc.sh Then you can plug in your malloc replacement. This is how Chrome uses tcmalloc on Windows. Regards, ismail ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Mon, Mar 9, 2015 at 12:57 AM, Knoll Lars lars.kn...@theqtcompany.com wrote: Yes, the best solution IMO is still to use your own malloc and operator new replacements. In addition, OOM handlers on the OS level can help. Except that dynamically linked Windows Qt applications (read: most) don't work this way, so Windows users are left out in the cold. DLLs do not allow you to simply replace one new operator across link boundaries. See the comments in this Qt bug: https://bugreports.qt.io/browse/QTBUG-37395 Is there any strategy for the large body of Qt developers targeting Windows, or is statically linking the only supported method for replacing Qt allocators on Windows? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Monday 09 March 2015 07:57:18 Knoll Lars wrote:
The problem is that a non throwing new can’t reliably detect OOM issues.
Even if we did check the return value of every 'operator new’ call in Qt,
this would leave us with one big problem that’s more or less impossible to
solve. The main problem is that we call new inside other constructors (ie.
Inside another call to operator new).
The fundamental issue that can’t be solved without exceptions is that this
should cause the outer operator new to fail (as the inner object didn’t
get constructed, thus the outer object is not fully constructed and
usable). But without exceptions there’s no way we can report this back
through the outer constructor call.
You will be glad to know that the compiler is required to add a null check
before calling the constructor in case of a noexpect or non-throwing operator
new.
[C++11 5.3.4 13]
Other than that, if an user of Qt wants a different behavior for malloc and
new, it is easy enough to use the facilities C++ provides, such as providing a
global operator new, or overriding malloc.
If the application provides its own symbol 'malloc', that one will be used.
So Qt does not need to do anything.
--
Olivier
Woboq - Qt services and support - http://woboq.com - http://code.woboq.org#include QtCore/QtCore
extern C void *__libc_malloc(size_t s);
extern C void __libc_free(void *s);
extern C {
void *malloc(size_t s) {
printf(malloc: %d\n, int(s));
return __libc_malloc(s);
}
}
void *operator new(size_t s) {
printf(new: %d\n, int(s));
return __libc_malloc(s);
}
void operator delete(void *v) {
__libc_free(v);
}
struct Test { double a,b; };
int main() {
printf(main\n);
Test t;
QListTest() t t t;
QVectorchar v(88);
}
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On 03/03/15 22:10, Oswald Buddenhagen oswald.buddenha...@theqtcompany.com wrote: On Mon, Mar 02, 2015 at 10:11:33AM -0800, Thiago Macieira wrote: On Monday 02 March 2015 11:21:10 Oswald Buddenhagen wrote: I am actively against it while it's a huge burden on us for little perceived benefit. You have to convince me of the benefit against the cost. i still don't see that huge burden. i see introducing a few (inline) functions in two alternative #ifdef branches, and a global search replace operation. You need to replace all the new too for this to be effective. this aims at the reliably failing builds without exception support, so the non-throwing new's return value would have to be checked to complement the malloc wrapper. The problem is that a non throwing new can’t reliably detect OOM issues. Even if we did check the return value of every 'operator new’ call in Qt, this would leave us with one big problem that’s more or less impossible to solve. The main problem is that we call new inside other constructors (ie. Inside another call to operator new). The fundamental issue that can’t be solved without exceptions is that this should cause the outer operator new to fail (as the inner object didn’t get constructed, thus the outer object is not fully constructed and usable). But without exceptions there’s no way we can report this back through the outer constructor call. while this would be reasonably simple (just replace new with qNew or something like that), it would be a) kinda ugly and b) would expand to lots of code. the other case is a build with exceptions where new throws as normal and qMalloc (used from c++ code) throws as well. malloc calls from c code would obviously need to handle and propagate malloc failures up to a point where we can see them and throw, which isn't much effort for our own code. however, there is a serious problem: stack unwinding resulting from an exception being thrown (even if it just aborts in the end because nobody catches it) requires all objects on the stack to keep their internal state consistent enough so that invoked d'tors won't crash - even when a c'tor throws before initialization finishes. this is clearly at least partially the exception safety which we have deemed out of scope so far because of an unreasonable cost/benefit ratio. i presume we're not going to change our opinion on that point. No. We will not change this. We have tried to get some partial exception safety for some of our code during the Symbian days and it was extremely error prone, a huge pain to maintain and caused an incredible amount of bloat in the created binaries. Doing this (and maintaining it) for all of Qt would require a development team a couple of times the size we currently have, and still would bring new feature development almost to a standstill. regarding 3rd party libraries: some would need to be excluded from the support entirely (because they are uncooperative), while others may need fixes inside or around the libs. generally, whether they return null or throw (while being exception-safe) doesn't matter, as the mechanisms can be translated into each other. i don't think the effort for that would be prohibitive if we had a serious interest in it, but it certainly wouldn't come for free. so to summarize ... color me convinced. replacing malloc it is. Yes, the best solution IMO is still to use your own malloc and operator new replacements. In addition, OOM handlers on the OS level can help. Cheers, Lars ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Mon, Mar 02, 2015 at 10:11:33AM -0800, Thiago Macieira wrote: On Monday 02 March 2015 11:21:10 Oswald Buddenhagen wrote: I am actively against it while it's a huge burden on us for little perceived benefit. You have to convince me of the benefit against the cost. i still don't see that huge burden. i see introducing a few (inline) functions in two alternative #ifdef branches, and a global search replace operation. You need to replace all the new too for this to be effective. this aims at the reliably failing builds without exception support, so the non-throwing new's return value would have to be checked to complement the malloc wrapper. while this would be reasonably simple (just replace new with qNew or something like that), it would be a) kinda ugly and b) would expand to lots of code. the other case is a build with exceptions where new throws as normal and qMalloc (used from c++ code) throws as well. malloc calls from c code would obviously need to handle and propagate malloc failures up to a point where we can see them and throw, which isn't much effort for our own code. however, there is a serious problem: stack unwinding resulting from an exception being thrown (even if it just aborts in the end because nobody catches it) requires all objects on the stack to keep their internal state consistent enough so that invoked d'tors won't crash - even when a c'tor throws before initialization finishes. this is clearly at least partially the exception safety which we have deemed out of scope so far because of an unreasonable cost/benefit ratio. i presume we're not going to change our opinion on that point. regarding 3rd party libraries: some would need to be excluded from the support entirely (because they are uncooperative), while others may need fixes inside or around the libs. generally, whether they return null or throw (while being exception-safe) doesn't matter, as the mechanisms can be translated into each other. i don't think the effort for that would be prohibitive if we had a serious interest in it, but it certainly wouldn't come for free. so to summarize ... color me convinced. replacing malloc it is. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Fri, Feb 27, 2015 at 09:26:14AM -0800, Thiago Macieira wrote: On Friday 27 February 2015 09:20:54 Oswald Buddenhagen wrote: the whole point would be *not* using unwrapped malloc and new(nothrow). this can be trivially verified for our own code with a grepping bot. There's an easier solution. replacing system functions seems a bit messy. but whatever. on the receiving side, i guess a signal handler that longjmp()s out of signal context is about equivalent to a catch block, so it should be a workable solution. The argument is that chain is as strong as its weakest link. If we don't fix all the holes, the bucket will still leak (pardon my switch of metaphors). you can let this be somebody else's worry. our task is to enable a use case, not to make it bullet-proof ourselves. the first step is not being actively hostile to it. I am actively against it while it's a huge burden on us for little perceived benefit. You have to convince me of the benefit against the cost. i still don't see that huge burden. i see introducing a few (inline) functions in two alternative #ifdef branches, and a global search replace operation. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Monday 02 March 2015 11:21:10 Oswald Buddenhagen wrote: I am actively against it while it's a huge burden on us for little perceived benefit. You have to convince me of the benefit against the cost. i still don't see that huge burden. i see introducing a few (inline) functions in two alternative #ifdef branches, and a global search replace operation. You need to replace all the new too for this to be effective. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Friday 27 February 2015 11:15:03 Al-Khanji Louai wrote: In that case they cannot be overwritten without a recompile. Which brings me back to my original comment from yesterday (to which no one replied): How is that different from linking a custom implementation of operator new/operator delete and malloc/free into Qt? It isn't. That's a MUCH easier solution: just replace malloc and make it abort() on failure instead of returning NULL. This has the following advantages: * works for ALL libraries in the process, whether they handle malloc failures or not * works for operator new of all forms, since they just call malloc * requires no code change anywhere -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Friday 27 February 2015 09:20:54 Oswald Buddenhagen wrote: the whole point would be *not* using unwrapped malloc and new(nothrow). this can be trivially verified for our own code with a grepping bot. There's an easier solution. See the reply to Louai. then explain edd2d9bd0a7f5dbe059aea0902d519b728acc01a. Who says that's good code? I fixed a warning, I didn't make the code good. If the compiler starts removing the undefined behaviour, we'll get a test failure and we can easily correct it. We can't do that in a library because we don't know when it will fail. clang suggests to do precisely what the patch did. are you saying it is making an unsafe suggestion? I'm suggesting that Clang suggested something that works for Clang today. That does not mean it will work: * for other compilers * for past or future versions of Clang Undefined behaviour is undefined behaviour. The compiler can implement whatever it chooses. The argument is that even if we happened to catch all failures to malloc() in all libraries, there's also all those pesky system calls that may return ENOMEM and we're not checking those. So wrapping malloc() and operator new are not enough: we need to check system calls too. And extend that to all the calls to libraries that do catch malloc() and system call failures and report errors to us. the failure to check the return value of system calls needs to be addressed irrespective of what we decide about memory management. it's simply irresponsible to not handle syscall return values, and can even lead to security holes. That may be. The argument is that chain is as strong as its weakest link. If we don't fix all the holes, the bucket will still leak (pardon my switch of metaphors). you can let this be somebody else's worry. our task is to enable a use case, not to make it bullet-proof ourselves. the first step is not being actively hostile to it. I am actively against it while it's a huge burden on us for little perceived benefit. You have to convince me of the benefit against the cost. And even if I had no technical arguments, my opinion counts. I'm not pulling this out of thin air. I have over a decade of experience, so if I say that it sounds wrong, it probably is. And I'm the maintainer for QtCore, so I'm the one that needs to be convinced, not the other way around. you can pull an argument to authority when no unanswered technical arguments remain and it's still a draw. I can pull this when I think this isn't technical, but social. The problem I perceive is that the solution you're proposing is a hugely complex task, requiring constant maintenance, places a big burden on all our developers, and is a leaking bucket. You're going to run into Pareto's Law really quickly too. And we have better things to do. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Sure, here's what I know: Regarding operator new: http://en.cppreference.com/w/cpp/memory/new/operator_new C++11 cleans up the exception specifications and specifies that the no-throw single object variant is called by the standard implementations of all other versions. Regarding operator delete: http://en.cppreference.com/w/cpp/memory/new/operator_delete C++11 again cleans up the exception specifications and specifies like with operator new that the single object variant is called by the standard implementations of all other variants. C++14 introduces sized global delete which can be very useful for custom allocators (for a classic example see the small object allocator in Alexandrescu's Modern C++ Design, source code for his library: http://sourceforge.net/p/loki-lib/code/HEAD/tree/trunk/). That the various implementations all internally by default call the single object versions of new/delete has long been the canonical thing to do, but it wasn't actually specified by the standard. The noexcept cleanups are straight-forward but necessary. The really interesting bit to me is the addition of global sized operator delete in C++14. --Louai -Original Message- From: Gunnar Roth [mailto:gunnar.r...@gmx.de] Sent: Friday, February 27, 2015 2:11 PM To: Al-Khanji Louai Cc: development@qt-project.org Subject: Aw: Re: [Development] QtCore missing check for memory allocation Hi, in fact both C++11 and C++14 have improved the ways in which the new/delete operators can be overridden. can you give me some links describing these improvements? regards, Gunnar Roth ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thu, Feb 26, 2015 at 04:12:27PM -0800, Thiago Macieira wrote: On Thursday 26 February 2015 20:54:32 Oswald Buddenhagen wrote: Let me put it this way: who's going to write the unit tests to ensure we get coverage for all those conditionals? Any volunteers? which conditionals? the malloc wrapper would throw/qFatal (depending on the build configuration). your dream of never-failing malloc would be a reality. or we just use new() everywhere. this one already has said wrapper. The conditionals for checking the result of malloc or new (nothrow). the whole point would be *not* using unwrapped malloc and new(nothrow). this can be trivially verified for our own code with a grepping bot. regarding replacing the malloc implementation ... is it possible to link an implementation that would actually throw? No, malloc() cannot ever throw. If you throw and your caller is C code, you crash. Also, malloc() is defined as nothrow() in the glibc headers. yeah, makes sense. Sorry, the compiler *is* allowed to remove undefined behaviour because, by the very definition of undefined behaviour, ANYTHING can happen, including absolutely nothing. then explain edd2d9bd0a7f5dbe059aea0902d519b728acc01a. Who says that's good code? I fixed a warning, I didn't make the code good. If the compiler starts removing the undefined behaviour, we'll get a test failure and we can easily correct it. We can't do that in a library because we don't know when it will fail. clang suggests to do precisely what the patch did. are you saying it is making an unsafe suggestion? The argument is that it implies runtime overhead. See Robin's email for numbers. This is asking for making the code slower on the very devices where it needs to run faster. i don't trust this number. i don't know how qMalloc was implemented, but there is no way a simple forwarding wrapper would add 10% overhead to malloc (esp. in an optimized build). modern processors even have a specific optimization for call forwarding (or whatever it's called properly). The argument is that we're in the habit of not checking it, so even if we had a massive effort to fix all the cases, we'll soon add more that we don't check. The argument is that we have no infrastructure for testing OOM situations to guarantee that we reliably take the action we decided. And no one is volunteering to write the tests. Just sprinkling Q_CHECK_PTR all over the place is no good if we miss critical ones. The argument is that this is a massive change. Most of our modules (except for QtCore, QtConcurrent, QtXmlPatterns) are compiled with exceptions disabled and yet we use the regular, non-nothrow operator new, so we'd need to replace it EVERYWHERE with the nothrow version plus the check. why are you basing your argumentation on the opposite of what we are suggesting? reliable failure is what ulf asked for. throw/qFatal everywhere. The argument is that we use libraries that don't care either for OOM situations. So even if we made the effort ourselves, it might be all for nothing as soon as we make a call to one of those libraries (think glib friends). this is a good argument. luckily, user code that cares can avoid this pitfall to a high degree by either avoiding the functionality at all (compiling without glib, for example), or using improved backend libraries. The argument is that even if we happened to catch all failures to malloc() in all libraries, there's also all those pesky system calls that may return ENOMEM and we're not checking those. So wrapping malloc() and operator new are not enough: we need to check system calls too. And extend that to all the calls to libraries that do catch malloc() and system call failures and report errors to us. the failure to check the return value of system calls needs to be addressed irrespective of what we decide about memory management. it's simply irresponsible to not handle syscall return values, and can even lead to security holes. The argument is that chain is as strong as its weakest link. If we don't fix all the holes, the bucket will still leak (pardon my switch of metaphors). you can let this be somebody else's worry. our task is to enable a use case, not to make it bullet-proof ourselves. the first step is not being actively hostile to it. The argument is that, even with all of this, there's no way to reliably catch an OOM situation, due to overcommit. The application may suddenly disappear. not on sane(ly configured) embedded systems. And even if I had no technical arguments, my opinion counts. I'm not pulling this out of thin air. I have over a decade of experience, so if I say that it sounds wrong, it probably is. And I'm the maintainer for QtCore, so I'm the one that needs to be convinced, not the other way around. you can pull an argument to authority when no unanswered technical arguments remain and it's still a
Re: [Development] QtCore missing check for memory allocation
On Fri, Feb 27, 2015 at 9:20 AM, Oswald Buddenhagen
oswald.buddenha...@theqtcompany.com wrote:
The argument is that it implies runtime overhead. See Robin's email for
numbers. This is asking for making the code slower on the very devices where
it needs to run faster.
i don't trust this number. i don't know how qMalloc was implemented, but
there is no way a simple forwarding wrapper would add 10% overhead to
malloc (esp. in an optimized build).
modern processors even have a specific optimization for call forwarding
(or whatever it's called properly).
I went and dug out the test I had. I remembered the results correctly
(for OS X, for Linux the situation is somehow worse overall?). The
measurement is actually qMalloc + qFree vs malloc + free, so the
overhead is shared across two calls. See the attachments.
Here's the results for OS X 10.10 and Linux (Ubuntu 12.04). Hopefully
they don't get screwed in mail formatting :)
OS X 10.10Size msTotalIterations
qtMalloc 1 0,55 58 1048576
qtMalloc 100,56 60 1048576
qtMalloc 100 0,59 69 1048576
qtMalloc 1 0,00018 90 524288
qtMalloc 100 0,029 59 2048
qtMalloc 1000 0,34 86 256
regularMalloc 1 0,51 55 1048576
regularMalloc 100,51 58 1048576
regularMalloc 100 0,52 55 1048576
regularMalloc 1 0,00013 73 524288
regularMalloc 100 0,029 59 2048
regularMalloc 1000 0,4 97 256
Percentage change
-0,49
-0,49
-0,48
-0,99987
-0,971
-0,6
Linux Size msTotalIterations
qtMalloc 1 0,32 68 2097152
qtMalloc 100,32 68 2097152
qtMalloc 100 0,36 76 2097152
qtMalloc 1 0,00021 57 262144
qtMalloc 100 0,037 76 2048
qtMalloc 1000 0,47 61 128
regularMalloc 1 0,1 92 8388608
regularMalloc 100,11 95 8388608
regularMalloc 100 0,11 99 8388608
regularMalloc 1 0,11 97 8388608
regularMalloc 100 0,11 93 8388608
regularMalloc 1000 0,11 96 8388608
Percentage change
-0,9
-0,89
-0,89
-0,89
-0,89
-0,89
Looking at the instruction counts (Linux only, via callgrind) is
interesting, though, and I don't know how to explain this:
Linux Size Instruction reads
qtMalloc 1 376
qtMalloc 10376
qtMalloc 100 405
qtMalloc 1 2,320
qtMalloc 100 95,208
qtMalloc 1000 938,880
regularMalloc 1 179
regularMalloc 10179
regularMalloc 100 179
regularMalloc 1 179
regularMalloc 100 179
regularMalloc 1000 179
#include QtCore
#include qtest.h
#include qcoreapplication.h
#include qdatetime.h
class MallocBenchmark : public QObject
{
Q_OBJECT
private slots:
void qtMalloc();
void qtMalloc_data();
void regularMalloc();
void regularMalloc_data();
};
void MallocBenchmark::qtMalloc_data()
{
QTest::addColumnint(size);
QTest::newRow(1) 1;
QTest::newRow(10) 1;
QTest::newRow(100) 100;
QTest::newRow(1) 1;
QTest::newRow(100) 100;
QTest::newRow(1000) 1000;
}
void MallocBenchmark::qtMalloc()
{
QFETCH(int, size);
QBENCHMARK {
void *p = ::qMalloc(size);
memset(p, rand(), size);
::qFree(p);
}
}
void MallocBenchmark::regularMalloc_data()
{
qtMalloc_data();
}
void MallocBenchmark::regularMalloc()
{
QFETCH(int, size);
QBENCHMARK {
void *p = malloc(size);
memset(p, rand(), size);
free(p);
}
}
QTEST_MAIN(MallocBenchmark)
#include main.moc
malloccheck.pro
Description: Binary data
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
-Original Message- From: development-bounces+kai.koehne=theqtcompany.com@qt- project.org [mailto:development- bounces+kai.koehne=theqtcompany@qt-project.org] On Behalf Of Robin Burchell Sent: Friday, February 27, 2015 11:47 AM To: development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation On Fri, Feb 27, 2015 at 9:20 AM, Oswald Buddenhagen oswald.buddenha...@theqtcompany.com wrote: The argument is that it implies runtime overhead. See Robin's email for numbers. This is asking for making the code slower on the very devices where it needs to run faster. i don't trust this number. i don't know how qMalloc was implemented, but there is no way a simple forwarding wrapper would add 10% overhead to malloc (esp. in an optimized build). modern processors even have a specific optimization for call forwarding (or whatever it's called properly). qmalloc and friends where implemented in qmalloc.cpp. That is, they can't be inlined, and every call to it from another library will be a cross-library one. A inlined, header-only wrapper should get away with this. Regards Kai ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
In that case they cannot be overwritten without a recompile. Which brings me back to my original comment from yesterday (to which no one replied): How is that different from linking a custom implementation of operator new/operator delete and malloc/free into Qt? These are embedded use-cases anyway, so you wouldn't be using a stock Qt binary. Implementing the above is well-documented, and in fact both C++11 and C++14 have improved the ways in which the new/delete operators can be overridden. Just put your implementation in a static lib and pass it to the configure script as an additional library. On Linux you can also inject such a library using LD_PRELOAD, most operating systems have a similar mechanism. Unlike changing a header, that doesn't require a recompile of Qt. --Louai -Original Message- From: development-bounces+louai.al-khanji=theqtcompany.com@qt- project.org [mailto:development-bounces+louai.al- khanji=theqtcompany@qt-project.org] On Behalf Of Koehne Kai Sent: Friday, February 27, 2015 12:57 PM To: Robin Burchell; development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation -Original Message- From: development-bounces+kai.koehne=theqtcompany.com@qt- project.org [mailto:development- bounces+kai.koehne=theqtcompany@qt-project.org] On Behalf Of Robin Burchell Sent: Friday, February 27, 2015 11:47 AM To: development@qt-project.org Subject: Re: [Development] QtCore missing check for memory allocation On Fri, Feb 27, 2015 at 9:20 AM, Oswald Buddenhagen oswald.buddenha...@theqtcompany.com wrote: The argument is that it implies runtime overhead. See Robin's email for numbers. This is asking for making the code slower on the very devices where it needs to run faster. i don't trust this number. i don't know how qMalloc was implemented, but there is no way a simple forwarding wrapper would add 10% overhead to malloc (esp. in an optimized build). modern processors even have a specific optimization for call forwarding (or whatever it's called properly). qmalloc and friends where implemented in qmalloc.cpp. That is, they can't be inlined, and every call to it from another library will be a cross-library one. A inlined, header-only wrapper should get away with this. Regards Kai ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Hi, in fact both C++11 and C++14 have improved the ways in which the new/delete operators can be overridden. can you give me some links describing these improvements? regards, Gunnar Roth ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thursday 26 February 2015 21:05:31 Oswald Buddenhagen wrote: even assuming that nobody else had an interest in this, you'd still need rather good reasons to effectively sabotage another contributor's interest, especially considering the majority situation. I do have good reasons. See the other email. That implies discussion on technical content, which is what we're doing. As we're reaching no consensus, the Chief Maintainer should be asked to weigh in. as i see it, the discussion has just begun. calling for consensus - let alone chief maintainer intervention - seems a tiny bit premature. it also seems like a bit of an own goal, considering the CM's affiliation His affiliation is irrelevant. The CM has to weigh the technical benefits as well as the strategic benefits to the ecosystem and the cost/burden this places on the contributors and maintainers. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
25.02.2015, 22:09, Oswald Buddenhagen oswald.buddenha...@theqtcompany.com: On Wed, Feb 25, 2015 at 08:38:21AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 17:20:54 Christian Kandeler wrote: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Which is one of the reasons why we don't check for malloc failures. Modern memory allocators with overcommitting and OOM killers mean that it's very hard to track down actual OOM situations. exactly because of this, in the embedded world it is common to disable overcommit. When using such a large library as Qt is quite hard to get along with disabled overcommit on embedded device (because you need to keep your application's VmSize under than size of physical memory). -- Regards, Konstantin ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thu, Feb 26, 2015 at 10:25:42AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 20:07:58 Oswald Buddenhagen wrote: as ulf pointed out, a rather trivial wrapper which ensures deterministic behavior is hardly a burden. And I disagree that it's hardly a burden. I am saying it is overhead. Let me put it this way: who's going to write the unit tests to ensure we get coverage for all those conditionals? Any volunteers? which conditionals? the malloc wrapper would throw/qFatal (depending on the build configuration). your dream of never-failing malloc would be a reality. or we just use new() everywhere. this one already has said wrapper. regarding replacing the malloc implementation ... is it possible to link an implementation that would actually throw? The only reliable way of causing a segfault is raise(SIGSEGV). You can't reliably trigger a memory problem because, by the very definition of it, the compiler is allowed to assume it doesn't happen. you can assign to a volatile pointer and deref it. the compiler is not allowed to optimize that away. we established that much last time we discussed this topic. Sorry, the compiler *is* allowed to remove undefined behaviour because, by the very definition of undefined behaviour, ANYTHING can happen, including absolutely nothing. then explain edd2d9bd0a7f5dbe059aea0902d519b728acc01a. [...] Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. I understand your arguments, but I still disagree we should act. well, and i say you are wrong. see the problem with this kind of argumentation? We've both exposed our technical argumentation for our suggestions but arrived at no consensus. you missed the point. you didn't provide arguments, only an opinion (at the point of the discussion this quote refers to). it isn't constructive to state your position without backing it up. if you stand by your previously made arguments, post links, or refer to a search engine (possibly suggesting keywords). not everyone witnessed the previous discussions, or remembers them. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thu, Feb 26, 2015 at 11:33:17AM -0800, Thiago Macieira wrote: On Thursday 26 February 2015 20:16:43 Oswald Buddenhagen wrote: There's no feature on Linux to do that. Overcommit is always enabled. wrong. https://www.kernel.org/doc/Documentation/vm/overcommit-accounting Have you read the file you linked to? did you? Options are: 0Heuristic overcommitting 1Always overcommit 2Overcommit by swap + 50% of RAM That sounds to me like you can't turn it off. that sounds to me like you didn't read it. because if you did, you'd know that 50% is merely the default. also, linux isn't the only os around. True, but it's the vast majority of the embedded OSes out there and only growing. given tqtc's strategic interest in the embedded market, a purely majority-based approach is hardly justifiable. One contributor company's strategy does not bind the others or the project. The project governance rules apply to the project. even assuming that nobody else had an interest in this, you'd still need rather good reasons to effectively sabotage another contributor's interest, especially considering the majority situation. That implies discussion on technical content, which is what we're doing. As we're reaching no consensus, the Chief Maintainer should be asked to weigh in. as i see it, the discussion has just begun. calling for consensus - let alone chief maintainer intervention - seems a tiny bit premature. it also seems like a bit of an own goal, considering the CM's affiliation ... ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 21:29:31 Marc Mutz wrote: -- why should now people have a slower library because of all those checks? Q_CHECK_PTR does not necessarily make the library slower. It only needs to be applied to malloc/calloc and nothrow operator new, because ordinary new already throws. We can port such code to normal operator new instead. Writing the checks is useless unless we also write the code to recover from such situations. For the case of exceptions, we need to make sure that the code survives stack unwinding somewhat gracefully until the exception handler. For the case of nullptrs, we need to ensure that functions exit indicating errors or setting error states in the objects. The checks themselves may not be the performance bottleneck. Rearranging the code around to make sure that the check is useful is the bottleneck. Unless we take the suggestion elsewhere in this thread to reliably and immediately terminate the application. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thursday 26 February 2015 19:22:56 Konstantin Tokarev wrote: When using such a large library as Qt is quite hard to get along with disabled overcommit on embedded device (because you need to keep your application's VmSize under than size of physical memory). That's not necessary. Let me take an example: $ ~/src/owntools/rssanalyse/rssanalyse.pl `pidof qtcreator` Total mapped memory: 5445164 kB of which is padding: 1988700 kB of which swapped out: 132204 kB of which likely stack: 32908 kB mapped, 140 kB resident (80 kB main, 60 kB aux threads) of which are resident: 732556 kB, 722644 kB proportionally shared total anonymous: 704784 kB (96.2%) Shared: 17628 kB total (2.4% of RSS) Breakdown: 2596 kB clean (14.7%), 15032 kB dirty (85.3%) 2540 kB code (14.4%), 56 kB RO data (0.3%), 15032 kB RW data (85.3%) 0 kB heap (0.0%), 0 kB stack (0.0%), 0 kB other (0.0%) Private: 714928 kB total (97.6% of RSS) Breakdown: 27796 kB clean (3.9%), 687132 kB dirty (96.1%) 9576 kB code (1.3%), 6324 kB RO data (0.9%), 2080 kB RW data (0.3%) 696344 kB heap (97.4%), 140 kB stack (0.0%), 464 kB other (0.1%) Qt Creator's VSZ is 5.3 GB but it has nearly 2 GB of that used for padding only (memory regions that with no read, write or execute permissions). That leaves 3.3 GB of virtual memory that is either readable, writable or executable. Even then, the program's actual, physical memory usage is 715 MB. Even if you add the amount of memory swapped out (128 MB), that still leaves 2.5 GB of VSZ that isn t backed by main RAM. A quick sampling of the process's smaps shows there are large chunks of mapped memory that have hardly been used. I expect many of those come from threading -- whenever you start a thread, glibc maps a 64 MB chunk of heap zone and 8 MB of stack, but only a few kB of those are ever used. The RSS is the current working set size for Qt Creator. That is, it runs fine with this amount of RAM. It may run with less RAM, if you subtract the clean portions of the RSS (29 MB) or if you swap out the dirty portions. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 20:09:21 Oswald Buddenhagen wrote: On Wed, Feb 25, 2015 at 08:38:21AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 17:20:54 Christian Kandeler wrote: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Which is one of the reasons why we don't check for malloc failures. Modern memory allocators with overcommitting and OOM killers mean that it's very hard to track down actual OOM situations. exactly because of this, in the embedded world it is common to disable overcommit. There's no feature on Linux to do that. Overcommit is always enabled. also, linux isn't the only os around. True, but it's the vast majority of the embedded OSes out there and only growing. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 20:07:58 Oswald Buddenhagen wrote: On Wed, Feb 25, 2015 at 08:01:54AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 16:48:44 Ulf Hermann wrote: We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. I disagree on both accounts. Could you elaborate a bit? I disagree that we should do Q_CHECK_PTR after every memory allocation and I disagree that Q_CHECK_PTR should do something in all circumstances. elaborate, not restate more verbosely. I already have, on multiple previous occasions, explained why I don't think this is a useful use of our brain cycles and of CPU cycles. as ulf pointed out, a rather trivial wrapper which ensures deterministic behavior is hardly a burden. And I disagree that it's hardly a burden. I am saying it is overhead. Let me put it this way: who's going to write the unit tests to ensure we get coverage for all those conditionals? Any volunteers? That's undefined behaviour. If you write code: if (!p) *(char*)p = 42; // crash I'm not saying we should access p in this case, but rather some fixed place of which we know we cannot access it, to *reliably* raise a segfault. Maybe there is even a more elegant way to trigger a segfault than accessing invalid memory. The only reliable way of causing a segfault is raise(SIGSEGV). You can't reliably trigger a memory problem because, by the very definition of it, the compiler is allowed to assume it doesn't happen. you can assign to a volatile pointer and deref it. the compiler is not allowed to optimize that away. we established that much last time we discussed this topic. Sorry, the compiler *is* allowed to remove undefined behaviour because, by the very definition of undefined behaviour, ANYTHING can happen, including absolutely nothing. In any case, raise(SIGSEGV) on Unix is fine. We'd just have to find something equivalent for Windows. I see no need for trying to figure out an address that crashes -- instead, let's use a system call. It's hardly going to be performance-critical anyway. [...] Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. I understand your arguments, but I still disagree we should act. well, and i say you are wrong. see the problem with this kind of argumentation? We've both exposed our technical argumentation for our suggestions but arrived at no consensus. What now? Since we're talking about a Qt-wide policy decision, the Chief Maintainer should be asked to weigh in. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Thu, Feb 26, 2015 at 10:27:05AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 20:09:21 Oswald Buddenhagen wrote: On Wed, Feb 25, 2015 at 08:38:21AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 17:20:54 Christian Kandeler wrote: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Which is one of the reasons why we don't check for malloc failures. Modern memory allocators with overcommitting and OOM killers mean that it's very hard to track down actual OOM situations. exactly because of this, in the embedded world it is common to disable overcommit. There's no feature on Linux to do that. Overcommit is always enabled. wrong. https://www.kernel.org/doc/Documentation/vm/overcommit-accounting also, linux isn't the only os around. True, but it's the vast majority of the embedded OSes out there and only growing. given tqtc's strategic interest in the embedded market, a purely majority-based approach is hardly justifiable. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
How is that different from linking a custom implementation of operator new/operator delete and malloc/free into Qt? These are embedded use-cases anyway, so you wouldn’t be using a stock Qt binary. Implementing the above is well-documented, and in fact both C++11 and C++14 have improved the ways in which the new/delete can be overridden. Just put your implementation in a static lib and pass it to the configure script as an additional library. --Louai Maybe a bit off-topic: can we introduce our own (probably customizable) memory allocation API, something like Apple's CFAllocator [1], and move Qt internals to use it instead of malloc/realloc/free (and maybe instead of some new/delete-s, based on some policies)? Then we could let the user to decide if he needs a OOM safety, i.e. with QAllocator::setDefaultAllocator(..) Such a centralized memory allocation routines also could be very useful in debugging or memory consumption monitoring. Regards, Konstantin [1] https://developer.apple.com/library/mac/documentation/CoreFoundation/Reference/CFAllocatorRef/ 2015-02-26 0:29 GMT+04:00 Marc Mutz marc.m...@kdab.commailto:marc.m...@kdab.com: On Wednesday 25 February 2015 16:30:56 Giuseppe D'Angelo wrote: And on the other hand, this assumption of having infinite memory has held for a while Only in the niche that Qt exists in. Now, the question is whether Qt does not play a role in systems that handle oom gracefully because Qt doesn't handle them gacefully or whether it doesn't play a role in those systems because there are no such systems. But I'm sure there are embedded systems that fall in between the desktop model of inifinite VM space and hard-realtime systems that allow no dynamic memory allocations after program initialisation. For such systems, recovering from oom is actually a plausible alternative. Relying on the oom killer in a soft realtime application is asking for trouble. It can literally take an hour for the system to recover. I just experienced a 50min thrashing when I dared to start up a 4GB VM without waiting for qtcreator to fully quit. The oom killer killed qt-creator, but it _did_ take 50mins. As Ossi said, overcommitting is likely turned off in those systems. BTW, IIRC, you also get nullptr mallocs (without thrashing swap first when you run into user-defined ulimits. If Qt wants to be(come) relevant to that part of the embedded space (or that part of server space that still uses ulimit), we better stop acting like there's only infinite memory. -- why should now people have a slower library because of all those checks? Q_CHECK_PTR does not necessarily make the library slower. It only needs to be applied to malloc/calloc and nothrow operator new, because ordinary new already throws. We can port such code to normal operator new instead. Thanks, Marc -- Marc Mutz marc.m...@kdab.commailto:marc.m...@kdab.com | Senior Software Engineer KDAB (Deutschland) GmbH Co.KG, a KDAB Group Company www.kdab.comhttp://www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-Independent Software Solutions ___ Development mailing list Development@qt-project.orgmailto:Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Anyhow, what's the problem with that? The problem is that memory allocation fails and Qt continues without checking the pointers ending up in a SEGV. Would a fix be in Q_CHECK_PTR to abort in this case (exception or qFatal) or should QVector get an additional check for null-pointer in addition to Q_CHECK_PTR? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
I noticed that in qglobal.h Q_CHECK_PTR may be a noop in case QT_NO_DEBUG is set. Q_CHECK_PTR is used to check if memory allocations succeeded (e.g. QVector::reallocateData). Until 9d44645eae144fcfefa0de2455d41f04d29c40d4 (September 2014) most of QVector's allocations weren't checked at all and surprisingly no one had complained about that before I did. The common theme is If you need so much space you better design your own data structure. I find that argument lacking because memory allocation can fail for a number of reasons, not only because you have requested a too large single chunk of memory. Furthermore people keep saying What can we do if we detect a failed memory allocation? Qt is in an invalid state then and we have to crash anyway. I somewhat agree to that, but we should really crash reliably without writing or reading random user memory before. We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. Is QT_NO_DEBUG really disabling the check for valid memory allocation? You need QT_NO_EXCEPTIONS and QT_NO_DEBUG for Q_CHECK_PTR to be a qt_noop(). I can't say I like this situation, but the point seems to be that you cannot throw bad_alloc if you've compiled without exceptions. I would argue for just crashing in this case, by accessing a known-bad address. That's quite a behavior change, of course. Ulf ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Il 25/02/2015 12:39, Rainer Keller ha scritto: Is QT_NO_DEBUG really disabling the check for valid memory allocation? It also depends on exception support, but yes. Also unlike Q_ASSERT there isn't an equivalent of QT_FORCE_ASSERTS to make the check present in non-debug no-exceptions builds... Anyhow, what's the problem with that? -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Software Engineer KDAB (UK) Ltd., a KDAB Group company Tel. UK +44-1738-450410, Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-independent software solutions smime.p7s Description: Firma crittografica S/MIME ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
because memory allocation can fail for a number of reasons, not only because you have requested a too large single chunk of memory. Exactly, for example if other applications are consuming most of the memory. Furthermore people keep saying What can we do if we detect a failed memory allocation? Qt is in an invalid state then and we have to crash anyway. At least show a useful error message in any case. Right now the message is SEGV. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. I disagree on both accounts. Could you elaborate a bit? That every memory allocation may be relaxed a bit as there might be places where the code can deal with 0 or where we pass the pointer straight on to user code and can expect the user to check it. The default should be checking for 0, though. That's undefined behaviour. If you write code: if (!p) *(char*)p = 42; // crash I'm not saying we should access p in this case, but rather some fixed place of which we know we cannot access it, to *reliably* raise a segfault. Maybe there is even a more elegant way to trigger a segfault than accessing invalid memory. The point is this: With the current behavior you're not actually guaranteed to get a segfault. The client code might not access *p, but rather p[some large number], and that might hit valid memory. Or it might store p, do whatever funny arithmetic on it with the assumption that it's not 0, and run into some other incorrect behavior. An attacker could specifically search for such a case and overwrite some important piece of information like this. We don't want that. Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. Ulf ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 14:45:02 Rainer Keller wrote: At least show a useful error message in any case. Right now the message is SEGV. Which is useful. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 13:35:17 Ulf Hermann wrote: We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. I disagree on both accounts. Is QT_NO_DEBUG really disabling the check for valid memory allocation? You need QT_NO_EXCEPTIONS and QT_NO_DEBUG for Q_CHECK_PTR to be a qt_noop(). I can't say I like this situation, but the point seems to be that you cannot throw bad_alloc if you've compiled without exceptions. I would argue for just crashing in this case, by accessing a known-bad address. That's quite a behavior change, of course. That's undefined behaviour. If you write code: if (!p) *(char*)p = 42; // crash The compiler will simply eliminate your code because it *knows* that you never dereference null pointers. The only thing you can reliably do is raise(SIGSEGV). -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 16:48:44 Ulf Hermann wrote: We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. I disagree on both accounts. Could you elaborate a bit? I disagree that we should do Q_CHECK_PTR after every memory allocation and I disagree that Q_CHECK_PTR should do something in all circumstances. That every memory allocation may be relaxed a bit as there might be places where the code can deal with 0 or where we pass the pointer straight on to user code and can expect the user to check it. The default should be checking for 0, though. I really disagree. If we wanted to find bad memory allocations, we should turn exceptions back on and write exception-safe code. Anything else is putting the burden on the common code path. That's undefined behaviour. If you write code: if (!p) *(char*)p = 42; // crash I'm not saying we should access p in this case, but rather some fixed place of which we know we cannot access it, to *reliably* raise a segfault. Maybe there is even a more elegant way to trigger a segfault than accessing invalid memory. The only reliable way of causing a segfault is raise(SIGSEGV). You can't reliably trigger a memory problem because, by the very definition of it, the compiler is allowed to assume it doesn't happen. The point is this: With the current behavior you're not actually guaranteed to get a segfault. The client code might not access *p, but rather p[some large number], and that might hit valid memory. Or it might store p, do whatever funny arithmetic on it with the assumption that it's not 0, and run into some other incorrect behavior. An attacker could specifically search for such a case and overwrite some important piece of information like this. We don't want that. Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. I understand your arguments, but I still disagree we should act. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Il 25/02/2015 13:35, Ulf Hermann ha scritto: I noticed that in qglobal.h Q_CHECK_PTR may be a noop in case QT_NO_DEBUG is set. Q_CHECK_PTR is used to check if memory allocations succeeded (e.g. QVector::reallocateData). Until 9d44645eae144fcfefa0de2455d41f04d29c40d4 (September 2014) most of QVector's allocations weren't checked at all and surprisingly no one had complained about that before I did. The common theme is If you need so much space you better design your own data structure. I find that argument lacking because memory allocation can fail for a number of reasons, not only because you have requested a too large single chunk of memory. Furthermore people keep saying What can we do if we detect a failed memory allocation? Qt is in an invalid state then and we have to crash anyway. I somewhat agree to that, but we should really crash reliably without writing or reading random user memory before. We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. That's a much bigger commitment than changing QVector, though. All of Qt's codebase assumes infinite memory, so that allocations never fail. In other words, only a handful of places currently check for such OOM conditions, and it's unclear what should happen in case a OOM is detected (apart from crashing). And on the other hand, this assumption of having infinite memory has held for a while -- why should now people have a slower library because of all those checks? Is QT_NO_DEBUG really disabling the check for valid memory allocation? You need QT_NO_EXCEPTIONS and QT_NO_DEBUG for Q_CHECK_PTR to be a qt_noop(). I can't say I like this situation, but the point seems to be that you cannot throw bad_alloc if you've compiled without exceptions. I would argue for just crashing in this case, by accessing a known-bad address. That's quite a behavior change, of course. Or a simple call to qFatal()...? -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Software Engineer KDAB (UK) Ltd., a KDAB Group company Tel. UK +44-1738-450410, Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-independent software solutions smime.p7s Description: Firma crittografica S/MIME ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 17:20:54 Christian Kandeler wrote: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Which is one of the reasons why we don't check for malloc failures. Modern memory allocators with overcommitting and OOM killers mean that it's very hard to track down actual OOM situations. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On 02/25/2015 04:30 PM, Giuseppe D'Angelo wrote: Il 25/02/2015 13:35, Ulf Hermann ha scritto: I noticed that in qglobal.h Q_CHECK_PTR may be a noop in case QT_NO_DEBUG is set. Q_CHECK_PTR is used to check if memory allocations succeeded (e.g. QVector::reallocateData). Until 9d44645eae144fcfefa0de2455d41f04d29c40d4 (September 2014) most of QVector's allocations weren't checked at all and surprisingly no one had complained about that before I did. The common theme is If you need so much space you better design your own data structure. I find that argument lacking because memory allocation can fail for a number of reasons, not only because you have requested a too large single chunk of memory. Furthermore people keep saying What can we do if we detect a failed memory allocation? Qt is in an invalid state then and we have to crash anyway. I somewhat agree to that, but we should really crash reliably without writing or reading random user memory before. We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. That's a much bigger commitment than changing QVector, though. All of Qt's codebase assumes infinite memory, so that allocations never fail. In other words, only a handful of places currently check for such OOM conditions, and it's unclear what should happen in case a OOM is detected (apart from crashing). Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Christian ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Am 25.02.2015 um 16:48 schrieb Ulf Hermann: The point is this: With the current behavior you're not actually guaranteed to get a segfault. The client code might not access *p, but rather p[some large number], and that might hit valid memory. Or it might store p, do whatever funny arithmetic on it with the assumption that it's not 0, and run into some other incorrect behavior. An attacker could specifically search for such a case and overwrite some important piece of information like this. We don't want that. Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. The commonly accepted solution to that problem is using memory debuggers like Valgrind. They are sufficiently sophisticated to tell you exactly where your bad pointer comes from. Ideally you have the resources to let your CI run your automated tests with such debugger. Writing allocation-safe code was considering a good idea years ago. Sadly the people supporting that approach totally forgot that checking each memory allocation dramatically inflates cyclomatic complexity of your code, rendering it unmaintainable quickly. Have a look at libdbus to get an idea of the overhead allocation-safe code causes. Ciao, Mathias ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
You are contradicting yourself. Just a few minutes ago you considered it a problem, if you don't know the exact location of a failed allocation. Now when it comes to overcommiting you say not knowing the location is not a problem. I am confused now. When overcommitting, the kernel will give you a range of virtual memory that isn't actually backed by physical memory. It will then map the memory as you access it. If you try to access too much of it and the kernel runs out of physical memory, it will kill the process. You can't use that mechanism to overwrite pieces of memory you didn't want to overwrite - or at least I don't see a way to do so. Ulf ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Am 25.02.2015 um 17:34 schrieb Ulf Hermann: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Overcommitting is not dangerous as it will reliably lead to the process getting killed if it goes wrong. If you can exploit that, we have a much worse problem, and not only in Qt. You are contradicting yourself. Just a few minutes ago you considered it a problem, if you don't know the exact location of a failed allocation. Now when it comes to overcommiting you say not knowing the location is not a problem. I am confused now. Ciao, Mathias ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
The commonly accepted solution to that problem is using memory debuggers like Valgrind. They are sufficiently sophisticated to tell you exactly where your bad pointer comes from. Ideally you have the resources to let your CI run your automated tests with such debugger. Bad code which accesses invalid memory locations without triggering allocation failures is not my problem here. Writing allocation-safe code was considering a good idea years ago. Sadly the people supporting that approach totally forgot that checking each memory allocation dramatically inflates cyclomatic complexity of your code, rendering it unmaintainable quickly. Have a look at libdbus to get an idea of the overhead allocation-safe code causes. If we just consider any allocation failure as fatal we can get by with exactly one more line of code per malloc. We can also write a malloc wrapper to do that for us and end up with virtually no added code (not considering operator new with nothrow for now). We don't need to handle allocation failures gracefully; we've never done that. We shouldn't tolerate undefined behavior on allocation failures, though. Ulf ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Overcommitting is not dangerous as it will reliably lead to the process getting killed if it goes wrong. If you can exploit that, we have a much worse problem, and not only in Qt. Ulf ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 17:36:39 Mathias Hasselmann wrote: [...] Writing allocation-safe code was considering a good idea years ago. Sadly the people supporting that approach totally forgot that checking each memory allocation dramatically inflates cyclomatic complexity of your code, rendering it unmaintainable quickly. Have a look at libdbus to get an idea of the overhead allocation-safe code causes. libdbus is written in an inferior language. In C++, it is good practice to write exception-safe code anyway, and then it doesn't really matter that any statement may throw: the code stays elegant and robust _because_ you assume any statment can throw unless proven otherwise. -- Marc Mutz marc.m...@kdab.com | Senior Software Engineer KDAB (Deutschland) GmbH Co.KG, a KDAB Group Company www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-Independent Software Solutions ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wed, Feb 25, 2015 at 08:01:54AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 16:48:44 Ulf Hermann wrote: We should thus do Q_CHECK_PTR on every memory allocation in Qt and we should fix Q_CHECK_PTR so that it works under all circumstances. I disagree on both accounts. Could you elaborate a bit? I disagree that we should do Q_CHECK_PTR after every memory allocation and I disagree that Q_CHECK_PTR should do something in all circumstances. elaborate, not restate more verbosely. That every memory allocation may be relaxed a bit as there might be places where the code can deal with 0 or where we pass the pointer straight on to user code and can expect the user to check it. The default should be checking for 0, though. I really disagree. If we wanted to find bad memory allocations, we should turn exceptions back on and write exception-safe code. Anything else is putting the burden on the common code path. as ulf pointed out, a rather trivial wrapper which ensures deterministic behavior is hardly a burden. That's undefined behaviour. If you write code: if (!p) *(char*)p = 42; // crash I'm not saying we should access p in this case, but rather some fixed place of which we know we cannot access it, to *reliably* raise a segfault. Maybe there is even a more elegant way to trigger a segfault than accessing invalid memory. The only reliable way of causing a segfault is raise(SIGSEGV). You can't reliably trigger a memory problem because, by the very definition of it, the compiler is allowed to assume it doesn't happen. you can assign to a volatile pointer and deref it. the compiler is not allowed to optimize that away. we established that much last time we discussed this topic. [...] Q_CHECK_PTR should mean If the pointer is 0 either throw an exception or abort right away. Don't just continue. I understand your arguments, but I still disagree we should act. well, and i say you are wrong. see the problem with this kind of argumentation? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wed, Feb 25, 2015 at 08:38:21AM -0800, Thiago Macieira wrote: On Wednesday 25 February 2015 17:20:54 Christian Kandeler wrote: Also, you are not even guaranteed to get a null pointer/bad_alloc due to things like Linux overcommitting. Which is one of the reasons why we don't check for malloc failures. Modern memory allocators with overcommitting and OOM killers mean that it's very hard to track down actual OOM situations. exactly because of this, in the embedded world it is common to disable overcommit. also, linux isn't the only os around. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wednesday 25 February 2015 16:30:56 Giuseppe D'Angelo wrote: And on the other hand, this assumption of having infinite memory has held for a while Only in the niche that Qt exists in. Now, the question is whether Qt does not play a role in systems that handle oom gracefully because Qt doesn't handle them gacefully or whether it doesn't play a role in those systems because there are no such systems. But I'm sure there are embedded systems that fall in between the desktop model of inifinite VM space and hard-realtime systems that allow no dynamic memory allocations after program initialisation. For such systems, recovering from oom is actually a plausible alternative. Relying on the oom killer in a soft realtime application is asking for trouble. It can literally take an hour for the system to recover. I just experienced a 50min thrashing when I dared to start up a 4GB VM without waiting for qtcreator to fully quit. The oom killer killed qt-creator, but it _did_ take 50mins. As Ossi said, overcommitting is likely turned off in those systems. BTW, IIRC, you also get nullptr mallocs (without thrashing swap first when you run into user-defined ulimits. If Qt wants to be(come) relevant to that part of the embedded space (or that part of server space that still uses ulimit), we better stop acting like there's only infinite memory. -- why should now people have a slower library because of all those checks? Q_CHECK_PTR does not necessarily make the library slower. It only needs to be applied to malloc/calloc and nothrow operator new, because ordinary new already throws. We can port such code to normal operator new instead. Thanks, Marc -- Marc Mutz marc.m...@kdab.com | Senior Software Engineer KDAB (Deutschland) GmbH Co.KG, a KDAB Group Company www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-Independent Software Solutions ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
Maybe a bit off-topic: can we introduce our own (probably customizable) memory allocation API, something like Apple's CFAllocator [1], and move Qt internals to use it instead of malloc/realloc/free (and maybe instead of some new/delete-s, based on some policies)? Then we could let the user to decide if he needs a OOM safety, i.e. with QAllocator::setDefaultAllocator(..) Such a centralized memory allocation routines also could be very useful in debugging or memory consumption monitoring. Regards, Konstantin [1] https://developer.apple.com/library/mac/documentation/CoreFoundation/Reference/CFAllocatorRef/ 2015-02-26 0:29 GMT+04:00 Marc Mutz marc.m...@kdab.com: On Wednesday 25 February 2015 16:30:56 Giuseppe D'Angelo wrote: And on the other hand, this assumption of having infinite memory has held for a while Only in the niche that Qt exists in. Now, the question is whether Qt does not play a role in systems that handle oom gracefully because Qt doesn't handle them gacefully or whether it doesn't play a role in those systems because there are no such systems. But I'm sure there are embedded systems that fall in between the desktop model of inifinite VM space and hard-realtime systems that allow no dynamic memory allocations after program initialisation. For such systems, recovering from oom is actually a plausible alternative. Relying on the oom killer in a soft realtime application is asking for trouble. It can literally take an hour for the system to recover. I just experienced a 50min thrashing when I dared to start up a 4GB VM without waiting for qtcreator to fully quit. The oom killer killed qt-creator, but it _did_ take 50mins. As Ossi said, overcommitting is likely turned off in those systems. BTW, IIRC, you also get nullptr mallocs (without thrashing swap first when you run into user-defined ulimits. If Qt wants to be(come) relevant to that part of the embedded space (or that part of server space that still uses ulimit), we better stop acting like there's only infinite memory. -- why should now people have a slower library because of all those checks? Q_CHECK_PTR does not necessarily make the library slower. It only needs to be applied to malloc/calloc and nothrow operator new, because ordinary new already throws. We can port such code to normal operator new instead. Thanks, Marc -- Marc Mutz marc.m...@kdab.com | Senior Software Engineer KDAB (Deutschland) GmbH Co.KG, a KDAB Group Company www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-Independent Software Solutions ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtCore missing check for memory allocation
On Wed, Feb 25, 2015 at 10:17 PM, Konstantin Ritt ritt...@gmail.com wrote: Maybe a bit off-topic: can we introduce our own (probably customizable) memory allocation API, something like Apple's CFAllocator [1], and move Qt internals to use it instead of malloc/realloc/free (and maybe instead of some new/delete-s, based on some policies)? Before you go all-out on something like that, please make sure you benchmark and consider the cost. Memory allocation is already something that is quite slow, and yet, done a lot - all over the place (too much, even, but that's a rant for another day). IIRC when I pushed for deprecating qMalloc/qFree in the past, I measured (on Linux) a ~10% time penalty for small allocations, lowering to 0-5% as allocation size increased. That's a way too hefty penalty to ask people to pay on something so universal - and that was literally just a function wrapping malloc, no debugging or monitoring. Furthermore, aside from performance, I don't think that reintroducing a wrapper is a smart way to do it for much the same reason why I wanted to get rid of them in the first place: they weren't universally used, leading some people to get the false impression that intercepting qMalloc would give them the kind of diagnostic information you're talking about. If you want to debug or monitor, then I'd suggest doing it a lot more centrally with something dedicated to that task, see e.g. heaptrack (http://milianw.de/tag/heaptrack). ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] QtCore missing check for memory allocation
Hi, I noticed that in qglobal.h Q_CHECK_PTR may be a noop in case QT_NO_DEBUG is set. Q_CHECK_PTR is used to check if memory allocations succeeded (e.g. QVector::reallocateData). Is QT_NO_DEBUG really disabling the check for valid memory allocation? Rainer ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
