Fixes to get Dart VM compiling on Ubuntu 13.04, Debian Wheezy.

Fixes to get Dart VM compiling on Ubuntu 13.04, Debian Wheezy.
* Convince gcc 4.7.x that len is initialized before use in callers of ReadFileFully()
* Cast RawObject* to intptr_t to avoid strict-aliasing error on gcc 4.7.x
* Replace two calls to strerror_r() with strerror() on Linux. When _GNU_SOURCE is
defined to be non-zero (as it is on Dart Linux builds), strerror_r() is
defined to return a char*, which is the correct error string to use. When
_GNU_SOURCE is zero, the XSI-compliant definition is used and the error
message is returned in the char* param. Surrounding error logging uses
strerror() which solves the problem.
* Replace int literals with char literals in test string definition.
* co19 math/tan_A01_t01 passes on Ubuntu 13.04.

Original issues:
https://code.google.com/p/dart/issues/detail?id=12085
https://code.google.com/p/dart/issues/detail?id=8807

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25890
Committed: https://code.google.com/p/dart/source/detail?r=25929

[siva, 2013-08-06 22:53:43]

https://codereview.chromium.org/22381002/diff/4001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/22381002/diff/4001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:373: const uint8_t* text_buffer = NULL;
Not sure I understand why *file_len = 1 needs to be moved up.
It is not used in the chunk of code above....

https://codereview.chromium.org/22381002/diff/4001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/22381002/diff/4001/runtime/vm/object.h#newcod...
runtime/vm/object.h:5993:
reinterpret_cast<intptr_t>(native_fields->ptr()->data_) + byte_offset);
Please add a comment here as to why the multiple reinterpret_cast is required.

[Chris Bracken, 2013-08-07 06:12:57]

https://codereview.chromium.org/22381002/diff/4001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/22381002/diff/4001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:373: const uint8_t* text_buffer = NULL;
On 2013/08/06 22:53:43, siva wrote:
> Not sure I understand why *file_len = 1 needs to be moved up.
> It is not used in the chunk of code above....

ReadStringFromFile() and LoadScript() both call this function with an
uninitialized stack-allocated local as the out-param file_len, then subsequently
pass that local as a param to another function. In cases where OpenFile()
returns NULL, file_len is left uninitialized. As a result, the compiler issues
an error against the two callers above:

error: ‘len’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]

Both of the callers above actually check the return value of this function for
NULL and bail out before ever using len, but the compiler doesn't see that the
combination of the two result in no cases where len is used uninitialized in our
code, hence the error.

An alternative solution would be to initialize the local len vars in both
callers and leave this code as it was.

https://codereview.chromium.org/22381002/diff/4001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/22381002/diff/4001/runtime/vm/object.h#newcod...
runtime/vm/object.h:5993:
reinterpret_cast<intptr_t>(native_fields->ptr()->data_) + byte_offset);
On 2013/08/06 22:53:43, siva wrote:
> Please add a comment here as to why the multiple reinterpret_cast is required.

Rewrote with a simpler approach for indexing into RawTypedData's variable-length
data_ that avoids the nested casts and the strict-aliasing warning that came
from dereferencing data_ (uint8_t* cast to intptr_t*)

[siva, 2013-08-07 15:59:26]

lgtm

[Chris Bracken, 2013-08-07 17:28:15]

Committed patchset #3 manually as r25890 (presubmit successful).

[Ivan Posva, 2013-08-07 18:33:44]

Please fix the multi-threading issues.

-Ivan

https://codereview.chromium.org/22381002/diff/14001/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/22381002/diff/14001/runtime/bin/file_linux.cc...
runtime/bin/file_linux.cc:50: Log::PrintErr("%s\n", strerror(errno));
strerror is not multithread safe and you should be using strerror_r as the
original code was doing.

https://codereview.chromium.org/22381002/diff/14001/runtime/bin/socket_linux.cc
File runtime/bin/socket_linux.cc (right):

https://codereview.chromium.org/22381002/diff/14001/runtime/bin/socket_linux....
runtime/bin/socket_linux.cc:384: Log::PrintErr("%s\n", strerror(errno));
ditto

[srdjan, 2013-08-07 18:36:43]

https://codereview.chromium.org/22381002/diff/14001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/22381002/diff/14001/runtime/vm/object.h#newco...
runtime/vm/object.h:5991: return
*(reinterpret_cast<intptr_t*>(native_fields->ptr()->data_) + index);
From Slava:

nit: you can use array syntax for this

reinterpret_cast<intptr_t*>(native_fields->ptr()->data_)[index]

[Chris Bracken, 2013-08-07 18:53:43]

On 2013/08/07 18:33:44, Ivan Posva wrote:
> Please fix the multi-threading issues.
> 
> -Ivan
> 
> https://codereview.chromium.org/22381002/diff/14001/runtime/bin/file_linux.cc
> File runtime/bin/file_linux.cc (right):
> 
>
https://codereview.chromium.org/22381002/diff/14001/runtime/bin/file_linux.cc...
> runtime/bin/file_linux.cc:50: Log::PrintErr("%s\n", strerror(errno));
> strerror is not multithread safe and you should be using strerror_r as the
> original code was doing.
> 
>
https://codereview.chromium.org/22381002/diff/14001/runtime/bin/socket_linux.cc
> File runtime/bin/socket_linux.cc (right):
> 
>
https://codereview.chromium.org/22381002/diff/14001/runtime/bin/socket_linux....
> runtime/bin/socket_linux.cc:384: Log::PrintErr("%s\n", strerror(errno));
> ditto

Will do; since we use the GNU-compliant version of strerror_r() (as opposed to
the XSI-compliant version) we should be relying on the returned char* as opposed
to the out-val param. Will also update other uses of the non-reentrant
strerror() in the same files.

[Chris Bracken, 2013-08-08 02:14:29]

On 2013/08/07 18:36:43, srdjan wrote:
> https://codereview.chromium.org/22381002/diff/14001/runtime/vm/object.h
> File runtime/vm/object.h (right):
> 
>
https://codereview.chromium.org/22381002/diff/14001/runtime/vm/object.h#newco...
> runtime/vm/object.h:5991: return
> *(reinterpret_cast<intptr_t*>(native_fields->ptr()->data_) + index);
> From Slava:
> 
> nit: you can use array syntax for this
> 
> reinterpret_cast<intptr_t*>(native_fields->ptr()->data_)[index]

Done.

[Chris Bracken, 2013-08-08 16:40:50]

Committed patchset #4 manually as r25929 (presubmit successful).