[PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

Hello.

It's bit confusing for a use that -fsanitize-recover=address does not recover
an instrumented binary. As a default value of halt_on_error is set to 0 for address sanitizer,
the binary fails on a first error.

Following patch attempts to explain the ENV variable.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-Document-ASAN_OPTIONS-halt_on_error-env-variable.patch --]
[-- Type: text/x-patch, Size: 1319 bytes --]

From 95f694a92428759773e5259323e82cbf49eade34 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 11 May 2016 16:05:49 +0200
Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.

gcc/ChangeLog:

2016-05-11  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
	and ASAN_OPTIONS="halt_on_error=1".
---
 gcc/doc/invoke.texi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a54a0af..722647a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9998,6 +9998,12 @@ for which this feature is experimental.
 accepted, the former enables recovery for all sanitizers that support it,
 the latter disables recovery for all sanitizers that support it.
 
+The error recovery mode can be overwritten by @code{halt_on_error=1} provided
+to one of the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}
+or @env{UBSAN_OPTIONS}. The default value is set to @code{halt_on_error=1},
+only the address sanitizer by default does not recover and @code{halt_on_error=0}
+must be provided.
+
 Syntax without explicit @var{opts} parameter is deprecated.  It is equivalent to
 @smallexample
 -fsanitize-recover=undefined,float-cast-overflow,float-divide-by-zero
-- 
2.8.2

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Jakub Jelinek]

On Wed, May 11, 2016 at 04:13:27PM +0200, Martin Liška wrote:
> It's bit confusing for a use that -fsanitize-recover=address does not recover
> an instrumented binary. As a default value of halt_on_error is set to 0 for address sanitizer,
> the binary fails on a first error.
> 
> Following patch attempts to explain the ENV variable.
> 
> Ready for trunk?
> Thanks,
> Martin

> >From 95f694a92428759773e5259323e82cbf49eade34 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 11 May 2016 16:05:49 +0200
> Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.
> 
> gcc/ChangeLog:
> 
> 2016-05-11  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
> 	and ASAN_OPTIONS="halt_on_error=1".
> ---
>  gcc/doc/invoke.texi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index a54a0af..722647a 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -9998,6 +9998,12 @@ for which this feature is experimental.
>  accepted, the former enables recovery for all sanitizers that support it,
>  the latter disables recovery for all sanitizers that support it.
>  
> +The error recovery mode can be overwritten by @code{halt_on_error=1} provided
> +to one of the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}
> +or @env{UBSAN_OPTIONS}. The default value is set to @code{halt_on_error=1},
> +only the address sanitizer by default does not recover and @code{halt_on_error=0}
> +must be provided.

It can be overridden (not overwritten?) only in one way I believe, i.e. the
code must be built with -fsanitize-recover=<whatever> (whether by default or
not) and the recovery must be enabled in the library (by default or using
env var) for successful recovery.  If you compile without recovery, then no
matter what you do on the env var side it still will be fatal.
So the docs need to make that clear.

	Jakub

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Yury Gribov]

On 05/11/2016 05:13 PM, Martin Liška wrote:
> Hello.
>
> It's bit confusing for a use that -fsanitize-recover=address does not recover
> an instrumented binary. As a default value of halt_on_error is set to 0 for address sanitizer,
> the binary fails on a first error.

I'm the guy behind -fsanitize-recover=address so let me explain.

Error recovery requires changes both to compiler (insert calls to 
recovering __asan_report_error_X_noabort rather than noreturning 
__asan_report_error_X) and runtime (do not abort when detecting overflow 
inside intercepted API like memcpy). -fsanitize-recover controls the 
compiler side, whereas halt_on_error=0 controls the runtime side.

Unfortunately currently there is no way to inform runtime library that 
compiled code would like it to continue execution after detecting error. 
Actually it's not clear how to do that properly because different parts 
of application could be compiled with different recovery settings (e.g. 
a.c with recovery and b.c without) making it hard to understand what 
behavior user would expect from runtime library interceptors.

> Following patch attempts to explain the ENV variable.
>
> Ready for trunk?

LGTM (but I'm not a maintainer and do not have approve right).

> Thanks,
> Martin
>

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]

On 05/11/2016 04:20 PM, Jakub Jelinek wrote:
> On Wed, May 11, 2016 at 04:13:27PM +0200, Martin Liška wrote:
>> It's bit confusing for a use that -fsanitize-recover=address does not recover
>> an instrumented binary. As a default value of halt_on_error is set to 0 for address sanitizer,
>> the binary fails on a first error.
>>
>> Following patch attempts to explain the ENV variable.
>>
>> Ready for trunk?
>> Thanks,
>> Martin
> 
>> >From 95f694a92428759773e5259323e82cbf49eade34 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Wed, 11 May 2016 16:05:49 +0200
>> Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.
>>
>> gcc/ChangeLog:
>>
>> 2016-05-11  Martin Liska  <mliska@suse.cz>
>>
>> 	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
>> 	and ASAN_OPTIONS="halt_on_error=1".
>> ---
>>  gcc/doc/invoke.texi | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index a54a0af..722647a 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -9998,6 +9998,12 @@ for which this feature is experimental.
>>  accepted, the former enables recovery for all sanitizers that support it,
>>  the latter disables recovery for all sanitizers that support it.
>>  
>> +The error recovery mode can be overwritten by @code{halt_on_error=1} provided
>> +to one of the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}
>> +or @env{UBSAN_OPTIONS}. The default value is set to @code{halt_on_error=1},
>> +only the address sanitizer by default does not recover and @code{halt_on_error=0}
>> +must be provided.
> 
> It can be overridden (not overwritten?) only in one way I believe, i.e. the
> code must be built with -fsanitize-recover=<whatever> (whether by default or
> not) and the recovery must be enabled in the library (by default or using
> env var) for successful recovery.  If you compile without recovery, then no
> matter what you do on the env var side it still will be fatal.
> So the docs need to make that clear.
> 
> 	Jakub
> 

Thank you Jakub for the note. What about the second version of the patch?

Thanks,
Martin

[-- Attachment #2: 0001-Document-ASAN_OPTIONS-halt_on_error-env-variable-v2.patch --]
[-- Type: text/x-patch, Size: 1277 bytes --]

From da688c187067dc5c475a4ab5b844c11c4bcd0494 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 11 May 2016 16:05:49 +0200
Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.

gcc/ChangeLog:

2016-05-11  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
	and ASAN_OPTIONS="halt_on_error=1".
---
 gcc/doc/invoke.texi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a54a0af..282367d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9998,6 +9998,12 @@ for which this feature is experimental.
 accepted, the former enables recovery for all sanitizers that support it,
 the latter disables recovery for all sanitizers that support it.
 
+Even if a recovery mode is turned on, it can be overridden by providing
+@code{halt_on_error=1} to one of the following environment variables:
+@env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}, @env{UBSAN_OPTIONS}.
+The default value is set to @code{halt_on_error=0} for all options,
+except @env{ASAN_OPTIONS}.
+
 Syntax without explicit @var{opts} parameter is deprecated.  It is equivalent to
 @smallexample
 -fsanitize-recover=undefined,float-cast-overflow,float-divide-by-zero
-- 
2.8.2

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Jakub Jelinek]

On Wed, May 11, 2016 at 04:47:46PM +0200, Martin Liška wrote:
> Thank you Jakub for the note. What about the second version of the patch?
> 
> Thanks,
> Martin

> >From da688c187067dc5c475a4ab5b844c11c4bcd0494 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 11 May 2016 16:05:49 +0200
> Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.
> 
> gcc/ChangeLog:
> 
> 2016-05-11  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
> 	and ASAN_OPTIONS="halt_on_error=1".
> ---
>  gcc/doc/invoke.texi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index a54a0af..282367d 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -9998,6 +9998,12 @@ for which this feature is experimental.
>  accepted, the former enables recovery for all sanitizers that support it,
>  the latter disables recovery for all sanitizers that support it.
>  
> +Even if a recovery mode is turned on, it can be overridden by providing
> +@code{halt_on_error=1} to one of the following environment variables:
> +@env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}, @env{UBSAN_OPTIONS}.
> +The default value is set to @code{halt_on_error=0} for all options,
> +except @env{ASAN_OPTIONS}.
> +

I think it better should say that:
Even if a recovery mode is turned on the compiler side, it needs to be also
enabled on the runtime library side, otherwise the failures are still fatal.
The runtime library defaults to ... and this can be overridden through ...
or so.

	Jakub

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Sandra Loosemore]

On 05/11/2016 08:56 AM, Jakub Jelinek wrote:
> On Wed, May 11, 2016 at 04:47:46PM +0200, Martin Liška wrote:
>> Thank you Jakub for the note. What about the second version of the patch?
>>
>> Thanks,
>> Martin
>
>> >From da688c187067dc5c475a4ab5b844c11c4bcd0494 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Wed, 11 May 2016 16:05:49 +0200
>> Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.
>>
>> gcc/ChangeLog:
>>
>> 2016-05-11  Martin Liska  <mliska@suse.cz>
>>
>> 	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
>> 	and ASAN_OPTIONS="halt_on_error=1".
>> ---
>>   gcc/doc/invoke.texi | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index a54a0af..282367d 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -9998,6 +9998,12 @@ for which this feature is experimental.
>>   accepted, the former enables recovery for all sanitizers that support it,
>>   the latter disables recovery for all sanitizers that support it.
>>
>> +Even if a recovery mode is turned on, it can be overridden by providing
>> +@code{halt_on_error=1} to one of the following environment variables:
>> +@env{ASAN_OPTIONS}, @env{TSAN_OPTIONS}, @env{UBSAN_OPTIONS}.
>> +The default value is set to @code{halt_on_error=0} for all options,
>> +except @env{ASAN_OPTIONS}.
>> +
>
> I think it better should say that:
> Even if a recovery mode is turned on the compiler side, it needs to be also
> enabled on the runtime library side, otherwise the failures are still fatal.
> The runtime library defaults to ... and this can be overridden through ...
> or so.

Yes, please.  I cannot understand either of the first two versions of 
the patch.

-Sandra

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

On 05/11/2016 04:56 PM, Jakub Jelinek wrote:
> I think it better should say that:
> Even if a recovery mode is turned on the compiler side, it needs to be also
> enabled on the runtime library side, otherwise the failures are still fatal.
> The runtime library defaults to ... and this can be overridden through ...
> or so.
> 
> 	Jakub

Thanks for the hint, I've just installed v3 as r236172.

Martin

[-- Attachment #2: 0001-Document-ASAN_OPTIONS-halt_on_error-env-variable-v3.patch --]
[-- Type: text/x-patch, Size: 1437 bytes --]

From 12336cb48bafffca7330e815d831d9c8ab1d0396 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 11 May 2016 16:05:49 +0200
Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.

gcc/ChangeLog:

2016-05-11  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
	and ASAN_OPTIONS="halt_on_error=1".
---
 gcc/doc/invoke.texi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a54a0af..c126644 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9998,6 +9998,14 @@ for which this feature is experimental.
 accepted, the former enables recovery for all sanitizers that support it,
 the latter disables recovery for all sanitizers that support it.
 
+Even if a recovery mode is turned on the compiler side, it needs to be also
+enabled on the runtime library side, otherwise the failures are still fatal.
+The runtime library defaults to @code{halt_on_error=0} for
+ThreadSanitizer and UndefinedBehaviorSanitizer, while default value for
+AddressSanitizer is @code{halt_on_error=1}. This can overridden through
+the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS},
+@env{UBSAN_OPTIONS}.
+
 Syntax without explicit @var{opts} parameter is deprecated.  It is equivalent to
 @smallexample
 -fsanitize-recover=undefined,float-cast-overflow,float-divide-by-zero
-- 
2.8.2

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Jakub Jelinek]

On Thu, May 12, 2016 at 02:36:44PM +0200, Martin Liška wrote:
> On 05/11/2016 04:56 PM, Jakub Jelinek wrote:
> > I think it better should say that:
> > Even if a recovery mode is turned on the compiler side, it needs to be also
> > enabled on the runtime library side, otherwise the failures are still fatal.
> > The runtime library defaults to ... and this can be overridden through ...
> > or so.
> 
> Thanks for the hint, I've just installed v3 as r236172.
> 
> Martin

> >From 12336cb48bafffca7330e815d831d9c8ab1d0396 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 11 May 2016 16:05:49 +0200
> Subject: [PATCH] Document ASAN_OPTIONS="halt_on_error" env variable.
> 
> gcc/ChangeLog:
> 
> 2016-05-11  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/invoke.texi: Explain connection between -fsanitize-recover=address
> 	and ASAN_OPTIONS="halt_on_error=1".
> ---
>  gcc/doc/invoke.texi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index a54a0af..c126644 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -9998,6 +9998,14 @@ for which this feature is experimental.
>  accepted, the former enables recovery for all sanitizers that support it,
>  the latter disables recovery for all sanitizers that support it.
>  
> +Even if a recovery mode is turned on the compiler side, it needs to be also
> +enabled on the runtime library side, otherwise the failures are still fatal.
> +The runtime library defaults to @code{halt_on_error=0} for
> +ThreadSanitizer and UndefinedBehaviorSanitizer, while default value for
> +AddressSanitizer is @code{halt_on_error=1}. This can overridden through

s/can/can be/

> +the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS},
> +@env{UBSAN_OPTIONS}.

I think it isn't obvious that one needs to put halt_on_error=0 or
halt_on_error=1 into those options and what to do if you need multiple
options in there.

	Jakub

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Martin Liška]

On 05/12/2016 02:44 PM, Jakub Jelinek wrote:
> I think it isn't obvious that one needs to put halt_on_error=0 or
> halt_on_error=1 into those options and what to do if you need multiple
> options in there.

What about changing the last sentence to:

This can be overridden through a corresponding environment variable.

Thanks
Martin

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Jakub Jelinek]

On Fri, May 13, 2016 at 12:26:57PM +0200, Martin Liška wrote:
> On 05/12/2016 02:44 PM, Jakub Jelinek wrote:
> > I think it isn't obvious that one needs to put halt_on_error=0 or
> > halt_on_error=1 into those options and what to do if you need multiple
> > options in there.
> 
> What about changing the last sentence to:
> 
> This can be overridden through a corresponding environment variable.

I think it is better to be explicit, so something like:
This can be overridden through setting the halt_on_error flag in
the corresponding environment variable.

	Jakub

Re: [PATCH, DOC] Document ASAN_OPTIONS="halt_on_error" env variable.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On 05/13/2016 01:03 PM, Jakub Jelinek wrote:
> On Fri, May 13, 2016 at 12:26:57PM +0200, Martin Liška wrote:
>> On 05/12/2016 02:44 PM, Jakub Jelinek wrote:
>>> I think it isn't obvious that one needs to put halt_on_error=0 or
>>> halt_on_error=1 into those options and what to do if you need multiple
>>> options in there.
>>
>> What about changing the last sentence to:
>>
>> This can be overridden through a corresponding environment variable.
> 
> I think it is better to be explicit, so something like:
> This can be overridden through setting the halt_on_error flag in
> the corresponding environment variable.
> 
> 	Jakub
> 

Good, I've just installed following patch as r236202.

Martin

[-- Attachment #2: 0001-Enhance-explanation-of-halt_on_error.patch --]
[-- Type: text/x-patch, Size: 1303 bytes --]

From f5ed058b7bacb1f75407765d2511fe1507901214 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 13 May 2016 12:27:07 +0200
Subject: [PATCH] Enhance explanation of halt_on_error.

gcc/ChangeLog:

2016-05-13  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Enhance explanation of error recovery
	of sanitizers.
---
 gcc/doc/invoke.texi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c126644..97e5060 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10002,9 +10002,8 @@ Even if a recovery mode is turned on the compiler side, it needs to be also
 enabled on the runtime library side, otherwise the failures are still fatal.
 The runtime library defaults to @code{halt_on_error=0} for
 ThreadSanitizer and UndefinedBehaviorSanitizer, while default value for
-AddressSanitizer is @code{halt_on_error=1}. This can overridden through
-the following environment variables: @env{ASAN_OPTIONS}, @env{TSAN_OPTIONS},
-@env{UBSAN_OPTIONS}.
+AddressSanitizer is @code{halt_on_error=1}. This can be overridden through
+setting the @code{halt_on_error} flag in the corresponding environment variable.
 
 Syntax without explicit @var{opts} parameter is deprecated.  It is equivalent to
 @smallexample
-- 
2.8.2