Code review - Issue 4527098: code review 4527098: gc: enable building under clang/2.9https://codereview.appspot.com/2011-06-08T00:28:57+00:00rietveld
Message from unknown
2011-06-06T07:07:08+00:00dfcurn:md5:f9b965ae4d1f2adebf9abbb5f0e3f6b5
Message from unknown
2011-06-06T07:07:13+00:00dfcurn:md5:ea830775d0915a8f8fe5a208d20ccdbd
Message from jeff@somethingsimilar.com
2011-06-06T07:16:40+00:00jmhodgesurn:md5:39ca0b6e8cb9a5e84ccbae8d99279c0c
http://codereview.appspot.com/4527098/diff/4/src/cmd/gc/subr.c
File src/cmd/gc/subr.c (left):
http://codereview.appspot.com/4527098/diff/4/src/cmd/gc/subr.c#oldcode108
src/cmd/gc/subr.c:108: *(int*)0 = 0;
Alternatively, this could be done with `*(volatile int*)0 = 0;` if a gcc builtin isn't acceptable.
Message from unknown
2011-06-06T07:20:23+00:00dfcurn:md5:7f6a7f2cb42d67cf68b02ace9469583c
Message from dave@cheney.net
2011-06-06T07:20:25+00:00dfcurn:md5:60ce6171c831defd86fc60c5530aac00
Hello jeff@somethingsimilar.com (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
https://go.googlecode.com/hg/
Message from r@golang.org
2011-06-06T07:55:51+00:00rurn:md5:8a9bec5601cc739a4f0902fafac94d00
http://codereview.appspot.com/4527098/diff/3001/src/cmd/gc/subr.c
File src/cmd/gc/subr.c (right):
http://codereview.appspot.com/4527098/diff/3001/src/cmd/gc/subr.c#newcode108
src/cmd/gc/subr.c:108: __builtin_trap();
is there a reason not to call abort()?
Message from ality@pbrane.org
2011-06-06T08:03:19+00:00alityurn:md5:42c876ebf699bc62b595423d29c509a4
dave@cheney.net once said:
> - *(int*)0 = 0;
> + __builtin_trap();
I'm suprised clang won't let you compile this.
Can you do this another way? The only other use of
__builtin goo is in 8g/gsubr.c:/^regalloc and I've
been meaning to replace it with getcallerpc (once I
import the code from plan9port's lib9).
I know there are a few others, aside from myself, who
would like to compile the Go toolchain on Plan 9.
Thanks,
Anthony
Message from jeff@somethingsimilar.com
2011-06-06T08:18:04+00:00jmhodgesurn:md5:f8b210cf913c8dfc917084c7a263df0e
On 2011/06/06 07:55:51, r wrote:
> http://codereview.appspot.com/4527098/diff/3001/src/cmd/gc/subr.c
> File src/cmd/gc/subr.c (right):
>
> http://codereview.appspot.com/4527098/diff/3001/src/cmd/gc/subr.c#newcode108
> src/cmd/gc/subr.c:108: __builtin_trap();
> is there a reason not to call abort()?
Ah, my current read of this would say that it's okay.
This may be a sleep-deprived mind but I think we found something interesting here. My read is that the SIGSEGV that was meant to be raised here would be caught by the signal(SIGSEGV, fault) call in lex.c's main(). But if that is true, we eventually wind up back in hcrash() like so: lex.c's fault(int) -> to subr.c's fatal(char *...) -> subr.c's hcrash(). So, by fixing this attempted SIGSEGV, we could be introducing a stack blow up.
In any case, abort() does seem okay (and probably preferable).
Message from jeff@somethingsimilar.com
2011-06-06T08:22:33+00:00jmhodgesurn:md5:82870b419e92e89c9ecec96c59105e93
Message from unknown
2011-06-06T08:52:55+00:00dfcurn:md5:5f2e3a3f989327c0a5820f6925256d14
Message from dave@cheney.net
2011-06-06T08:53:32+00:00dfcurn:md5:851796298eb06431392b617b2cf9a3ad
Thanks for the feedback. PTAL.
Message from jeff@somethingsimilar.com
2011-06-06T08:57:58+00:00jmhodgesurn:md5:d292310618abd03640abcae703cb4d31
LGTM
Message from r@golang.org
2011-06-06T09:51:08+00:00rurn:md5:313c962c8e3649c141e0f9bdd063c3f1
LGTM
Message from r@golang.org
2011-06-06T09:53:48+00:00rurn:md5:13b4f2962fd17caeb9ca59d9ceb1bccc
*** Submitted as http://code.google.com/p/go/source/detail?r=6414e4f6b736 ***
gc: enable building under clang/2.9
To build under clang, pass the path to clang in CC when
calling ./make.bash
CC=/opt/llvm/llvm-2.9/bin/clang ./make.bash
Credit goes to jmhodges for suggestions.
R=jeff, r, ality
CC=golang-dev
http://codereview.appspot.com/4527098
Committer: Rob Pike <r@golang.org>
Message from dave@cheney.net
2011-06-06T09:54:20+00:00dfcurn:md5:2d5cb0925e1a1020e697a1b593613b59
Thanks Rob.
Message from rsc@golang.org
2011-06-07T19:19:55+00:00rscurn:md5:2ed21df0a41dbb5909d010d0b6d36c49
I'm sad to see abort instead of *(int*)0 = 0.
The former deposits you in the bowels of glibc
while the latter leaves you in the actual program
you want to debug.
Does clang let you do *(volatile int*)0 = 0 ?
Russ
Message from jeff@somethingsimilar.com
2011-06-07T22:04:25+00:00jmhodgesurn:md5:c1509a0f1cfa56f9bd292437934a2b8a
Totally does, as my first comment pointed out!
On Tue, Jun 7, 2011 at 12:19 PM, Russ Cox <rsc@golang.org> wrote:
> I'm sad to see abort instead of *(int*)0 = 0.
> The former deposits you in the bowels of glibc
> while the latter leaves you in the actual program
> you want to debug.
>
> Does clang let you do *(volatile int*)0 = 0 ?
>
> Russ
>
Message from dave@cheney.net
2011-06-08T00:28:57+00:00dfcurn:md5:bdff60b480dc8bd87c8e8f2f226be0c3
Hi Russ,
Please try http://codereview.appspot.com/4517143
Cheers
Dave
On Wed, Jun 8, 2011 at 5:19 AM, Russ Cox <rsc@golang.org> wrote:
> I'm sad to see abort instead of *(int*)0 = 0.
> The former deposits you in the bowels of glibc
> while the latter leaves you in the actual program
> you want to debug.
>
> Does clang let you do *(volatile int*)0 = 0 ?
>
> Russ
>