[cfe-commits] r148874 - in /cfe/trunk: lib/Sema/SemaExprCXX.cpp test/Analysis/reference.cpp

[Nico Weber]

Chandler / Matthieu: I tried that - it gets rid of the copy
constructor error, but since the volatile is casted away, the
Wnull-dereference warning reappears.

John: abort() works, and that's what I'm using locally for now. I
don't know if all compilers are smart enough to not warn about a
missing return statement if the function body contains nothing but
abort() – but I suppose that would be other compiler's bug then.

Another example with the same problem was in a container class, which
looked like:

template<class T> class container {
  T Get(int index) {  if (i < 0 || i >= size_) return *(const T*)NULL;
return *((T*)storage_)[i]; }
};

The idea here is to turn a potentially exploitable invalid read into
at worst a harmless invalid read. Here the semantics change slightly
with the abort() from 'maybe crash on invalid index' to 'definitely
crash on invalid index' (at least for Chromium, that's fine).

So is abort() the recommended solution? Should the diagnostic text
recommend abort() when this warning occurs as part of a return
expression?

Thanks,
Nico

On Sun, Jan 29, 2012 at 4:40 AM, Matthieu Monrocq
<matthieu.monrocq at gmail.com> wrote:
>
>
> Le 29 janvier 2012 09:30, Chandler Carruth <chandlerc at google.com> a écrit :
>
>> FWIW, I've not checked to see whether it could just be replaced with
>> abort, or no body at all. I'll look into it when I can. However...
>>
>> On Sat, Jan 28, 2012 at 10:38 PM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>> You mean like
>>>
>>> template <typename T>
>>> inline T Invalid() {
>>>  return *static_cast<volatile typename remove_reference<T>::type*>(0);
>>
>>
>> The error below can be addressed quite simply:
>>
>> return *const_cast<typename
>> remove_reference<T>::type&>(static_cast<volatile typename
>> remove_reference<T>::type*>(0));
>>
>> You might think that this could just be 'const_cast<T&>(...)' but while
>> this is equivalent according to the standard thanks to reference collapsing,
>> we found that some compilers choked on it spectacularly. =/ I can't remember
>> whether it was an older MSVC, or the GCC in an older version of xcode.
>>
>>
>
> // the reference looks wrong
>
> return *const_cast<typename remove_reference<T>::type&>(static_cast<volatile
> typename remove_reference<T>::type*>(0));
>
> // this looks better
> return *const_cast<typename remove_reference<T>::type*>(static_cast<volatile
> typename remove_reference<T>::type*>(0));
>
> -- Matthieu