Re: Go patch committed: Fix error reporting for invalid builtin calls

Re: Go patch committed: Fix error reporting for invalid builtin calls

[Andreas Schwab]

Ian Lance Taylor <iant@golang.org> writes:

> This patch from Chris Manghane fixes the Go frontend error reporting
> for invalid builtin calls, by not losing track of whether the call is
> erroneous.  This fixes https://golang.org/issue/11561.  Bootstrapped
> and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
> mainline.

../../../libgo/runtime/mprof.goc: In function 'runtime_Stack':
../../../libgo/runtime/mprof.goc:408:5: error: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Werror=frame-address]
  sp = runtime_getcallersp(&b);
     ^

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Go patch committed: Fix error reporting for invalid builtin calls

[Richard Biener]

On Mon, Aug 3, 2015 at 11:10 AM, Andreas Schwab <schwab@suse.de> wrote:
> Ian Lance Taylor <iant@golang.org> writes:
>
>> This patch from Chris Manghane fixes the Go frontend error reporting
>> for invalid builtin calls, by not losing track of whether the call is
>> erroneous.  This fixes https://golang.org/issue/11561.  Bootstrapped
>> and ran Go testsuite on x86_64-unknown-linux-gnu.  Committed to
>> mainline.
>
> ../../../libgo/runtime/mprof.goc: In function 'runtime_Stack':
> ../../../libgo/runtime/mprof.goc:408:5: error: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Werror=frame-address]
>   sp = runtime_getcallersp(&b);
>      ^

Introduced by Martin Sebors patch

2015-08-02  Martin Sebor  <msebor@redhat.com>

        * c-family/c.opt (-Wframe-address): New warning option.
        * doc/invoke.texi (Wframe-address): Document it.
        * doc/extend.texi (__builtin_frame_address, __builtin_return_address):
        Clarify possible effects of calling the functions with non-zero
        arguments and mention -Wframe-address.
        * builtins.c (expand_builtin_frame_address): Handle -Wframe-address.


Richard.

> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Re: Go patch committed: Fix error reporting for invalid builtin calls

[Ian Lance Taylor]

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

On Mon, Aug 3, 2015 at 2:10 AM, Andreas Schwab <schwab@suse.de> wrote:

> ../../../libgo/runtime/mprof.goc: In function 'runtime_Stack':
> ../../../libgo/runtime/mprof.goc:408:5: error: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Werror=frame-address]
>   sp = runtime_getcallersp(&b);

Fixed by this patch by Chris Manghane.  The call was not actually
necessary.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.  This fixes PR
67101.

Ian

[-- Attachment #2: foo.txt --]
[-- Type: text/plain, Size: 1080 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 226510)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-2bf7c643a1d2f8503070c8e6cb87852026e32400
+d5aad2f400a0f21724e33e4ae48e1583ed8b1a87
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/mprof.goc
===================================================================
--- libgo/runtime/mprof.goc	(revision 226510)
+++ libgo/runtime/mprof.goc	(working copy)
@@ -402,10 +402,9 @@ func ThreadCreateProfile(p Slice) (n int
 }
 
 func Stack(b Slice, all bool) (n int) {
-	byte *pc, *sp;
+	byte *pc;
 	bool enablegc;
 	
-	sp = runtime_getcallersp(&b);
 	pc = (byte*)(uintptr)runtime_getcallerpc(&b);
 
 	if(all) {
@@ -423,7 +422,6 @@ func Stack(b Slice, all bool) (n int) {
 		g->writebuf = (byte*)b.__values;
 		g->writenbuf = b.__count;
 		USED(pc);
-		USED(sp);
 		runtime_goroutineheader(g);
 		runtime_traceback();
 		runtime_printcreatedby(g);

Re: [gofrontend-dev] Re: Go patch committed: Fix error reporting for invalid builtin calls

[Michael Hudson-Doyle]

Now I get

../../../gcc/libgo/runtime/mprof.goc: In function ‘runtime_Stack’:
../../../gcc/libgo/runtime/mprof.goc:437:19: error: ‘enablegc’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
   mstats.enablegc = enablegc;
                   ^
../../../gcc/libgo/runtime/mprof.goc:406:7: note: ‘enablegc’ was declared here
  bool enablegc;
       ^

Am I doing something wrong?

Cheers,
mwh

On 4 August 2015 at 05:55, Ian Lance Taylor <iant@golang.org> wrote:
> On Mon, Aug 3, 2015 at 2:10 AM, Andreas Schwab <schwab@suse.de> wrote:
>
>> ../../../libgo/runtime/mprof.goc: In function 'runtime_Stack':
>> ../../../libgo/runtime/mprof.goc:408:5: error: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Werror=frame-address]
>>   sp = runtime_getcallersp(&b);
>
> Fixed by this patch by Chris Manghane.  The call was not actually
> necessary.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline.  This fixes PR
> 67101.
>
> Ian
>
> --
> You received this message because you are subscribed to the Google Groups "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to gofrontend-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [gofrontend-dev] Re: Go patch committed: Fix error reporting for invalid builtin calls

[Ian Lance Taylor]

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

On Mon, Aug 3, 2015 at 7:24 PM, Michael Hudson-Doyle
<michael.hudson@canonical.com> wrote:
>
> Now I get
>
> ../../../gcc/libgo/runtime/mprof.goc: In function ‘runtime_Stack’:
> ../../../gcc/libgo/runtime/mprof.goc:437:19: error: ‘enablegc’ may be
> used uninitialized in this function [-Werror=maybe-uninitialized]
>    mstats.enablegc = enablegc;
>                    ^
> ../../../gcc/libgo/runtime/mprof.goc:406:7: note: ‘enablegc’ was declared here
>   bool enablegc;

I don't know why I am not seeing this, but I've committed this patch
that should fix it.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 804 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 226533)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-a850225433a66a58613c22185c3b09626f5545eb
+bdd98c601f2c8dbd0bf821548ba09c038f7645c4
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/mprof.goc
===================================================================
--- libgo/runtime/mprof.goc	(revision 226525)
+++ libgo/runtime/mprof.goc	(working copy)
@@ -403,7 +403,7 @@ func ThreadCreateProfile(p Slice) (n int
 
 func Stack(b Slice, all bool) (n int) {
 	byte *pc;
-	bool enablegc;
+	bool enablegc = false;
 	
 	pc = (byte*)(uintptr)runtime_getcallerpc(&b);