Thank you for reviewing, John and Richard.
rjmccall wrote: > Is it actually a good idea for this to affect the array-cookie decision? This is an ABI break which will potentially force all clients to compile with the same setting for this flag. My change might be confusing. The doesDeleteInterceptorWantSize() never gets true in this patch. It gets true only when "UseSize" == "DeleteInterceptWantsSize" is true, but it's always false in this patch. If it gets true (though never happen in this change), it will give std::type_info to __intercept_delete__. It doesn't happen by default. I was planning to add another option -fintercept-allocation-args=<choice> soon in another patch to make "UseSize" true by -fintercept-allocation-args=size. The array-cookie decision is changed only by user's choice with the option. In other words, the array-cookie decision doesn't change without any user's choice. If it's confusing, I'll delete this part from this patch. Also, if there is better implementation to give an allocated type to __intercept_delete__ for an array, I'd like to know that and change the implementation. rsmith wrote: > void* __intercept_delete__(void*, const std::type_info&); > I'm concerned about passing the type in here: the type need not match the type passed to the new interceptor. Also, do you really want to intercept placement-new? > > I wonder whether this would be better addressed by adding a specific form of operator new, which is passed the type_info in addition to the usual parameters? For the first question, yes I (we) need. For example, I tried ignoring placement-new in WebKit and Chromium (our main target) at first. It missed ~30% allocated objects (in size). WebKit and Chromium allocate unignorable memory via placement-new. Also for "the type need not match the type passed to the new interceptor," I'm actually not sure of that. May the type differ from the allocated type? The type_info argument in __intercept_delete__ is not required for "our" purpose in fact. I added the type_info since it's more convenient for other users. I can remove it if it's a problem, but I wonder if we could keep it in some safe way for convenience. For the second question, I have also tried that. It was a natural choice since it's similar to the size_t argument in standard operator delete. We however found that it's hard to intercept "in-class" operator new. We'd like to intercept all allocations without changing so many lines of code, but WebKit and Chromium have many in-class operator new. By a specific form of operator new, we (and other users) have to add many operator new for every in-class operator new. We thought and tried many choices (including above choices) to intercept all allocations. They are discussed in the document https://docs.google.com/a/chromium.org/document/d/1zyJ3FJhheJ5iSxYaMXARkZcR61tx6r44euMJbWw8jFY/edit. I wonder if I could have you opinions. http://llvm-reviews.chandlerc.com/D298 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
