clarify doc for __builtin_return_address

A program I instrumented to help me debug an otherwise unrelated
problem in 5.1.0 has been crashing in calls to
__builtin_return_address. After checking the manual, I didn't
think I was doing anything wrong. I then did some debugging and
found that the function simply isn't safe to call with non-zero
arguments near the top of the stack. That seemed like a bug to
me so I created a small test case and ran it on a few targets
to see if the problem was isolated to just powerpc (where I'm
working at the moment) or more general. It turned out not to
be target-specific. Before opening a bug, I checked Bugzilla
to see if it's already been reported but couldn't find any open
reports. To be sure I wasn't missing something, I expanded my
search to already resolved bugs. That's when I finally found
pr8743 which had been closed years ago as a documentation issue,
after adding the following to the manual:

   This function should only be used with a nonzero argument
   for debugging purposes.

Since I was using the function exactly for this purpose, I'd
like to propose the patch below to clarify the effects of the
function to set the right expectations and help others avoid
the effort it took me to figure out this is by design.

Does anyone have any concerns with this update or is it okay
to check in?

Thanks
Martin

2015-05-21  Martin Sebor  <msebor@redhat.com>

	* extend.texi (Return Address): Clarify possible effects
	of calling the functions with non-zero arguments.

  @deftypefn {Built-in Function} {void *} __builtin_extract_return_addr 
(void *@var{addr})
@@ -7998,7 +7999,8 @@ of the stack has been reached, this function 
returns @code{0} if
  the first frame pointer is properly initialized by the startup code.

  This function should only be used with a nonzero argument for debugging
-purposes.
+purposes since such calls to it can have unpredictable effects, including
+crashing the calling program.
  @end deftypefn

  @node Vector Extensions

On 05/21/2015 01:19 PM, Martin Sebor wrote:

> 2015-05-21  Martin Sebor  <msebor@redhat.com>
>
>      * extend.texi (Return Address): Clarify possible effects
>      of calling the functions with non-zero arguments.
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 7470e40..b37e893 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7959,7 +7959,8 @@ Additional post-processing of the returned value
> may be needed, see
>   @code{__builtin_extract_return_addr}.
>
>   This function should only be used with a nonzero argument for debugging
> -purposes.
> +purposes since such calls to it can have unpredictable effects, including
> +crashing the calling program.
>   @end deftypefn
>
>   @deftypefn {Built-in Function} {void *} __builtin_extract_return_addr
> (void *@var{addr})

I think the problem is that the original sentence parses ambiguously -- 
is it telling you that you must pass a nonzero argument to use it for 
debugging purposes, or telling you that you must use calls with a 
nonzero argument only for debugging?  And adding an additional clause 
onto the end only makes it harder to parse.

I suggest rewriting it as something like

Calling this function with a nonzero argument can have unpredictable 
effects, including crashing the calling program.  Such calls are 
typically only useful in debugging situations.

> @@ -7998,7 +7999,8 @@ of the stack has been reached, this function
> returns @code{0} if
>   the first frame pointer is properly initialized by the startup code.
>
>   This function should only be used with a nonzero argument for debugging
> -purposes.
> +purposes since such calls to it can have unpredictable effects, including
> +crashing the calling program.
>   @end deftypefn
>
>   @node Vector Extensions
>

Same applies here as well.

-Sandra

On 05/21/2015 02:05 PM, Sandra Loosemore wrote:
> On 05/21/2015 01:19 PM, Martin Sebor wrote:
>
>> 2015-05-21  Martin Sebor  <msebor@redhat.com>
>>
>>      * extend.texi (Return Address): Clarify possible effects
>>      of calling the functions with non-zero arguments.
>>
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 7470e40..b37e893 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -7959,7 +7959,8 @@ Additional post-processing of the returned value
>> may be needed, see
>>   @code{__builtin_extract_return_addr}.
>>
>>   This function should only be used with a nonzero argument for debugging
>> -purposes.
>> +purposes since such calls to it can have unpredictable effects,
>> including
>> +crashing the calling program.
>>   @end deftypefn
>>
>>   @deftypefn {Built-in Function} {void *} __builtin_extract_return_addr
>> (void *@var{addr})
>
> I think the problem is that the original sentence parses ambiguously --
> is it telling you that you must pass a nonzero argument to use it for
> debugging purposes, or telling you that you must use calls with a
> nonzero argument only for debugging?  And adding an additional clause
> onto the end only makes it harder to parse.
>
> I suggest rewriting it as something like
>
> Calling this function with a nonzero argument can have unpredictable
> effects, including crashing the calling program.  Such calls are
> typically only useful in debugging situations.

Thanks. I agree that's better. I'll wait for further comments
or approval to commit your version.

Martin

On 05/21/2015 08:19 PM, Martin Sebor wrote:
> A program I instrumented to help me debug an otherwise unrelated
> problem in 5.1.0 has been crashing in calls to
> __builtin_return_address. After checking the manual, I didn't
> think I was doing anything wrong. I then did some debugging and
> found that the function simply isn't safe to call with non-zero
> arguments near the top of the stack. That seemed like a bug to
> me so I created a small test case and ran it on a few targets
> to see if the problem was isolated to just powerpc (where I'm
> working at the moment) or more general. It turned out not to
> be target-specific. Before opening a bug, I checked Bugzilla
> to see if it's already been reported but couldn't find any open
> reports. To be sure I wasn't missing something, I expanded my
> search to already resolved bugs. That's when I finally found
> pr8743 which had been closed years ago as a documentation issue,
> after adding the following to the manual:
> 
>    This function should only be used with a nonzero argument
>    for debugging purposes.
> 
> Since I was using the function exactly for this purpose, I'd
> like to propose the patch below to clarify the effects of the
> function to set the right expectations and help others avoid
> the effort it took me to figure out this is by design.
> 
> Does anyone have any concerns with this update or is it okay
> to check in?

Sounds like a good candidate for a warning.

Thanks,
Pedro Alves