ASAN_OPTIONS=allocator_may_return_null=1 needs to be set

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dana Burkart]

Alexey says:

We have a test crash with ASan, because it tries to allocate too much, and ASan doesn't allow that.

We need to change the default behavior:

==45420==WARNING: AddressSanitizer failed to allocate 0xffffffffaffff6a0 bytes
==45420==AddressSanitizer's allocator is terminating the process instead of returning 0
==45420==If you don't like this behavior set allocator_may_return_null=1
==45420==AddressSanitizer CHECK failed: /SourceCache/clang/clang-603.0.12/src/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:149 "((0)) != (0)" (0x0, 0x0)

[Dana Burkart]

<rdar://problem/20111863>

[Dana Burkart]

Created attachment 248365 [details]
Set allocator_may_return_null=1

[Geoffrey Garen]

Comment on attachment 248365 [details]
Set allocator_may_return_null=1

r=me

[Geoffrey Garen]

Does this fix the plugin testcase?

[Alexey Proskuryakov]

Comment on attachment 248365 [details]
Set allocator_may_return_null=1

Sadly, I don't think that this is going to work - ASAN_OPTIONS is a runtime variable, not a build time one AFAIK.

[Dana Burkart]

(In reply to comment #5)
> Comment on attachment 248365 [details]
> Set allocator_may_return_null=1
> 
> Sadly, I don't think that this is going to work - ASAN_OPTIONS is a runtime
> variable, not a build time one AFAIK.

Yeah, I misunderstood this. This is going to be more difficult since we don't actually set any ASan-specific settings at runtime, and will have to come up with some way to do this.

[Dana Burkart]

Created attachment 248981 [details]
Set ASAN_OPTIONS in run-webkit-tests

[WebKit Commit Bot]

Attachment 248981 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/common/asan/utils.py:32:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Tools/Scripts/webkitpy/common/asan/utils.py:36:  whitespace before '}'  [pep8/E202] [5]
ERROR: Tools/Scripts/webkitpy/common/asan/__init__.py:1:  no newline at end of file  [pep8/W292] [5]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dana Burkart]

Created attachment 248984 [details]
Fix style

[Alexey Proskuryakov]

Comment on attachment 248981 [details]
Set ASAN_OPTIONS in run-webkit-tests

View in context: https://bugs.webkit.org/attachment.cgi?id=248981&action=review

> Tools/Scripts/webkitpy/common/asan/__init__.py:2
>  \ No newline at end of file

Should one be added?

> Tools/Scripts/webkitpy/common/asan/utils.py:13
> +#     * Neither the name of Google Inc. nor the names of its

Is this the right license?

The preferred WebKit license text is at <http://www.webkit.org/coding/bsd-license.html>.

> Tools/Scripts/webkitpy/common/asan/utils.py:34
> +        "abort_on_error" : 1,

I don't think that we want this.

> Tools/Scripts/webkitpy/common/asan/utils.py:36
> +        "handle_segv" : 0

Not sure about this, it should probably be in a separate fix if it's needed at all.

> Tools/Scripts/webkitpy/port/driver.py:320
> +        if ASanUtility.compiled_with_asan(self._port._path_to_driver()):

I just thought... We do we care, can we just always set the environment variable?

[Dana Burkart]

Created attachment 248986 [details]
Really fix style....

[WebKit Commit Bot]

Attachment 248986 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/common/asan/utils.py:36:  whitespace before '}'  [pep8/E202] [5]
ERROR: Tools/Scripts/webkitpy/common/asan/utils.py:50:  blank line at end of file  [pep8/W391] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexey Proskuryakov]

Comment on attachment 248986 [details]
Really fix style....

View in context: https://bugs.webkit.org/attachment.cgi?id=248986&action=review

> Tools/Scripts/webkitpy/port/driver.py:320
> +        if ASanUtility.compiled_with_asan(self._port._path_to_driver()):

In addition to the above comments: this looks like code that is shared across all platforms, and we probably cannot run otool everywhere.

[Dana Burkart]

(In reply to comment #10)
> Comment on attachment 248981 [details]
> Set ASAN_OPTIONS in run-webkit-tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248981&action=review
> 
> > Tools/Scripts/webkitpy/common/asan/__init__.py:2
> >  \ No newline at end of file
> 
> Should one be added?
> 
> > Tools/Scripts/webkitpy/common/asan/utils.py:13
> > +#     * Neither the name of Google Inc. nor the names of its
> 
> Is this the right license?
> 
> The preferred WebKit license text is at
> <http://www.webkit.org/coding/bsd-license.html>.
> 
> > Tools/Scripts/webkitpy/common/asan/utils.py:34
> > +        "abort_on_error" : 1,
> 
> I don't think that we want this.
> 
> > Tools/Scripts/webkitpy/common/asan/utils.py:36
> > +        "handle_segv" : 0
> 
> Not sure about this, it should probably be in a separate fix if it's needed
> at all.
> 

I added "abort_on_error" and "handle_segv", as David Kilzer had added them to his patch. If we don't need them I will take them out.

> > Tools/Scripts/webkitpy/port/driver.py:320
> > +        if ASanUtility.compiled_with_asan(self._port._path_to_driver()):
> 
> I just thought... We do we care, can we just always set the environment
> variable?

I guess we don't care. I'll remove it.

[Dana Burkart]

Created attachment 248987 [details]
Address Alexey's concerns

[Alexey Proskuryakov]

Comment on attachment 248987 [details]
Address Alexey's concerns

View in context: https://bugs.webkit.org/attachment.cgi?id=248987&action=review

> Tools/Scripts/webkitpy/port/driver.py:320
> +        environment['ASAN_OPTIONS'] = ASanUtility.environment_options()

At this point, I think that it would be better to just say 

environment['ASAN_OPTIONS'] = "allocator_may_return_null=1"

[WebKit Commit Bot]

Attachment 248987 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/common/asan/utils.py:25:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dana Burkart]

(In reply to comment #16)
> Comment on attachment 248987 [details]
> Address Alexey's concerns
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248987&action=review
> 
> > Tools/Scripts/webkitpy/port/driver.py:320
> > +        environment['ASAN_OPTIONS'] = ASanUtility.environment_options()
> 
> At this point, I think that it would be better to just say 
> 
> environment['ASAN_OPTIONS'] = "allocator_may_return_null=1"

I agree, but I think that is going to get clunky soon, as I am sure we will be setting more environment variables soon.

[Dana Burkart]

Merged r181714.