code review 12568043: cmd/ld: Put the textflag constants in a separate file.

cmd/ld: Put the textflag constants in a separate file.
We can then include this file in assembly to replace
cryptic constants like "7" with meaningful constants
like "(NOPROF|DUPOK|NOSPLIT)".

Converting just pkg/runtime/asm*.s for now.  Dropping NOPROF
and DUPOK from lots of places where they aren't needed.
More .s files to come in a subsequent changelist.

A nonzero number in the textflag field now means
"has not been converted yet".

[khr, 2013-08-06 22:18:23]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[DMorsing, 2013-08-07 07:32:56]

Does *a support ORing constants?

[rsc, 2013-08-07 13:32:57]

LGTM

[khr1, 2013-08-07 16:01:55]

Yes, they all do.  But you need to put the OR in parentheses, like this:

TEXT foo(SB),(NOSPLIT|DUPOK),$64-16

Could be fixed to not require the parentheses, but probably not worth it.


On Wed, Aug 7, 2013 at 12:32 AM, <daniel.morsing@gmail.com> wrote:

> Does *a support ORing constants?
>
>
https://codereview.appspot.**com/12568043/<https://codereview.appspot.com/125...
>

[khr, 2013-08-07 17:23:28]

*** Submitted as https://code.google.com/p/go/source/detail?r=cfc3eab6195a ***

cmd/ld: Put the textflag constants in a separate file.
We can then include this file in assembly to replace
cryptic constants like "7" with meaningful constants
like "(NOPROF|DUPOK|NOSPLIT)".

Converting just pkg/runtime/asm*.s for now.  Dropping NOPROF
and DUPOK from lots of places where they aren't needed.
More .s files to come in a subsequent changelist.

A nonzero number in the textflag field now means
"has not been converted yet".

R=golang-dev, daniel.morsing, rsc, khr
CC=golang-dev
https://codereview.appspot.com/12568043

[mischief, 2013-08-13 09:58:44]

On 2013/08/07 17:23:28, khr wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=cfc3eab6195a ***
> 
> cmd/ld: Put the textflag constants in a separate file.
> We can then include this file in assembly to replace
> cryptic constants like "7" with meaningful constants
> like "(NOPROF|DUPOK|NOSPLIT)".
> 
> Converting just pkg/runtime/asm*.s for now.  Dropping NOPROF
> and DUPOK from lots of places where they aren't needed.
> More .s files to come in a subsequent changelist.
> 
> A nonzero number in the textflag field now means
> "has not been converted yet".
> 
> R=golang-dev, daniel.morsing, rsc, khr
> CC=golang-dev
> https://codereview.appspot.com/12568043

hey there. this is a breaking change for plan 9. not sure why..
http://9.offblast.org/stuff/cfc3eab6195a
should i file an issue?

[rsc, 2013-08-13 13:13:42]

I don't know why this is only breaking on Plan 9. Maybe we pass an extra -I
somewhere on non-Plan 9 systems.

[mischief, 2013-08-13 14:40:55]

On Tue, Aug 13, 2013 at 09:13:41AM -0400, Russ Cox wrote:
> I don't know why this is only breaking on Plan 9. Maybe we pass an extra -I
> somewhere on non-Plan 9 systems.

hi russ,

i think this is working on non-plan9 systems by the assumption that
"../ld/textflag.h" will be resolved relative to
"src/libmach/../cmd/5l/5.out.h" included by 5obj.c.

on linux/amd64 no special -I flags to resolve this file seem to be
passed.

output of 'strace -f -e trace=process -v ./make.bash |& grep exec | grep 5obj.c'
follows.

it seems this is not the only breaking change introduced recently by include
files..

[pid 26709] execve("/usr/bin/gcc", ["gcc", "-Wall",
"-Wstrict-prototypes", "-Wextra", "-Wunused", "-Wuninitialized",
"-Wno-sign-compare", "-Wno-missing-braces", "-Wno-parentheses",
"-Wno-unknown-pragmas", "-Wno-switch", "-Wno-comment",
"-Wno-missing-field-initializers", "-Werror", "-fno-common", "-ggdb",
"-pipe", "-O2", "-c", "-m64", "-I", "/srv/go/include", "-I",
"/srv/go/src/libmach", "-o", "/var/tmp/go-cbuild-cHEobt/5obj.o",
"/srv/go/src/libmach/5obj.c"], ["SSH_AGENT_PID=4145", "SHELL=/bin/bash",
"TERM=dumb", "XDG_SESSION_COOKIE=a82eba6068825"...,
"PLAN9=/usr/local/plan9", "HUSHLOGIN=FALSE", "WINDOWID=14680077",
"USER=mischief", "XTERM_SHELL=/usr/bin/tmux",
"http_proxy=http://127.0.0.1:8118"..., "user=mischief",
"LS_COLORS=rs=0:di=01;34:ln=01;36"...,
"SSH_AUTH_SOCK=/tmp/ssh-lGJIlTbb5"...,
"TMUX=/tmp//tmux-1000/default,427"..., "MAIL=/var/mail/mischief",
"PATH=/srv/go/bin:/srv/go/bin:/ho"..., "PWD=/srv/go/src", "EDITOR=gvim",
"LANG=C", "GS_FONTPATH=/usr/local/plan9/pos"..., "TMUX_PANE=%10",
"XTERM_LOCALE=en_US.UTF-8", "XTERM_VERSION=XTerm(278)",
"HOME=/home/mischief", "SHLVL=2", "home=/home/mischief",
"GOROOT=/srv/go", "prompt=$H=; ", "LOGNAME=mischief",
"GOPATH=/home/mischief/code/go", "BROWSER=xombrero", "WINDOWPATH=7",
"DISPLAY=:0", "XAUTHORITY=/home/mischief/.Xauth"...,
"font=/usr/local/plan9/font/lucm/"..., "_=./cmd/dist/dist",
"GOARCH=amd64", "GOOS=linux", "GOARM=6", "GO386=sse2",
"LANGUAGE=en_US.UTF8", "GOROOT_FINAL=/srv/go"] <unfinished ...>

[rsc, 2013-08-13 14:48:57]

On Tue, Aug 13, 2013 at 10:40 AM, Nick Owens <mischief@offblast.org> wrote:

> hi russ,
>
> i think this is working on non-plan9 systems by the assumption that
> "../ld/textflag.h" will be resolved relative to
> "src/libmach/../cmd/5l/5.out.h" included by 5obj.c.
>
> on linux/amd64 no special -I flags to resolve this file seem to be
> passed.
>

If this is really the bug, then Plan 9 compiler should be fixed.
The fix would probably go in cc/macbody's macinc, but it looks like it is
trying all the possible directories already.

[khm, 2013-08-13 16:42:59]

On 2013/08/13 14:48:57, rsc wrote:
> On Tue, Aug 13, 2013 at 10:40 AM, Nick Owens <mailto:mischief@offblast.org>
wrote:
> 
> > hi russ,
> >
> > i think this is working on non-plan9 systems by the assumption that
> > "../ld/textflag.h" will be resolved relative to
> > "src/libmach/../cmd/5l/5.out.h" included by 5obj.c.
> >
> > on linux/amd64 no special -I flags to resolve this file seem to be
> > passed.
> >
> 
> If this is really the bug, then Plan 9 compiler should be fixed.
> The fix would probably go in cc/macbody's macinc, but it looks like it is
> trying all the possible directories already.

I think the bug is that something is generating paths with .. in them.  On any
system with symlinks or union mounts this is impossible to canonically resolve.

[rsc, 2013-08-13 18:33:43]

On Tue, Aug 13, 2013 at 12:43 PM, <khm@intma.in> wrote:

> I think the bug is that something is generating paths with .. in them.
> On any system with symlinks or union mounts this is impossible to
> canonically resolve.
>

that's not the problem here. and plan 9 does a better job than anything
else. i wrote the code.

russ

[khm, 2013-08-13 22:20:24]

On 2013/08/13 18:33:43, rsc wrote:
> 
> that's not the problem here. and plan 9 does a better job than anything
> else. i wrote the code.
> 
> russ

Except insofar as it does not actually work.  If plan 9 does a better job, why
are you suggesting plan 9 be "fixed"?  Help me understand your desires here

[rsc, 2013-09-03 19:14:50]

On Tue, Aug 13, 2013 at 6:20 PM, <khm@intma.in> wrote:

> On 2013/08/13 18:33:43, rsc wrote:
>
>  that's not the problem here. and plan 9 does a better job than
>>
> anything
>
>> else. i wrote the code.
>>
>
>  russ
>>
>
> Except insofar as it does not actually work.  If plan 9 does a better
> job, why are you suggesting plan 9 be "fixed"?  Help me understand your
> desires here
>

I believe that either the Plan 9 C compiler or the Plan 9 cpp (whichever is
being used in this case) is mishandling the .. in the #include. There are
no symlinks or union mounts here, so the operating system is not the
problem.

Russ

[r, 2013-09-03 21:13:30]

The 9fans list is reporting that it's a bug in include-of-include in 8c.