Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flag: index out of range when argc is 0 #32775

Closed
kneht opened this issue Jun 25, 2019 · 13 comments
Closed

flag: index out of range when argc is 0 #32775

kneht opened this issue Jun 25, 2019 · 13 comments
Milestone

Comments

@kneht
Copy link

@kneht kneht commented Jun 25, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go-1.12"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.12/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

When golang compiled binary invoked by execve with 0 arguments like this:

test.cpp code
#include 

int main() {
execve("test.go.out", NULL, NULL);
}

https://play.golang.org/p/zo3YJZTkBu_A

What did you expect to see?

nothing

What did you see instead?

panic: runtime error: index out of range

goroutine 1 [running]:
flag.init.ializers()
/usr/lib/go-1.12/src/flag/flag.go:1009 +0x172

As for documentation, it's sort of guaranteed that os.Args[0] is a program name:
https://golang.org/pkg/os/#pkg-variables

When argc is 0, at least for Linux platform program name can be fetched from procfs(/proc/self/comm), probably here:

argslice = make([]string, argc)
for i := int32(0); i < argc; i++ {
argslice[i] = gostringnocopy(argv_index(argv, i))
}

@robpike

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Jun 25, 2019

Not sure there's anything to fix. That binary compiles and runs fine; it's the caller that's making a mistake: argv should never be empty.

@gregory-m

This comment has been minimized.

Copy link
Contributor

@gregory-m gregory-m commented Jun 26, 2019

From execve(2) man page:

argv is an array of argument strings passed to the new program. By convention, the first of these strings should contain the filename associated with the file being executed

On Linux, argv can be specified as NULL, which has the same effect as
specifying this argument as a pointer to a list containing a single NULL pointer.
Do not take advantage of this misfeature!
It is nonstandard and nonportable: on most other UNIX systems doing this
will result in an error (EFAULT).

So, I'am also not sure if we need to fix something.

@kneht

This comment has been minimized.

Copy link
Author

@kneht kneht commented Jul 1, 2019

I've spotted this issue when my binary was ran by rsyslog.
Documentation looks inconsistent, as argv[0] doesn't have a program name, but whatever is provided to argv[0].
Module flag apparently is not only in charge of parsing args, but also flags when there is no argv[0] by throwing runtime error.
I'm new to golang and don't really have horse in this race, but from what I see at least len(argv) in flag module needs to be checked.

@gregory-m

This comment has been minimized.

Copy link
Contributor

@gregory-m gregory-m commented Jul 1, 2019

What config was used for rsyslog?

Something like this:

module(load="omprog")
action(type="omprog"
       binary="/pathto/omprog.py --parm1=\"value 1\" --parm2=\"value2\""
       template="RSYSLOG_TraditionalFileFormat")
@kneht

This comment has been minimized.

Copy link
Author

@kneht kneht commented Jul 1, 2019

yep, binary was invoked by omprog with no parameters.
like binary="/pathto/omprog.py"

@kneht

This comment has been minimized.

Copy link
Author

@kneht kneht commented Jul 1, 2019

It was fixed in latest versions of rsyslog apparently:
rsyslog/rsyslog@8d807e3

mmexternal bugfix: potentially missing argv[0]
This is incorrect and can cause problems with some language, namely go.
@gregory-m

This comment has been minimized.

Copy link
Contributor

@gregory-m gregory-m commented Jul 1, 2019

I searched Github and found another examples of execve("cmd", NULL, NULL); usage. It defentely not best practice but maybe it good idea to not panic in such case? @ianlancetaylor @robpike

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 1, 2019

It seems to me that the C getopt function will also crash in this case. It really doesn't seem worth worrying about.

But I guess if someone sends a patch for 1.14 we can take a look.

@kneht

This comment has been minimized.

Copy link
Author

@kneht kneht commented Jul 1, 2019

getopt(0,NULL,NULL) will work as argc is 0 obviously,
I can try to work on patch, but I would need some guidance on where this issue needs to be fixed.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 1, 2019

In the initialization of CommandLine in the flag package.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jul 2, 2019

Change https://golang.org/cl/184637 mentions this issue: flag: handle nil os.Args on CommandLine initialization

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@smasher164 smasher164 modified the milestones: Backlog, Go1.14 Oct 11, 2019
@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Nov 8, 2019

Thank you for filing this issue @kneht and for the patience and everyone for chiming in. Welcome to the Go project!

I agree with what @robpike and @ianlancetaylor say: I don't think we should do anything here.

You are incorrectly using execve and yet execve asks that you pass at least one argument to char *const argv[] with the various standards and OSes stating in e.g.

Posix

The all encompassing standard says
https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
Screen Shot 2019-11-07 at 10 06 16 PM

Linux

http://man7.org/linux/man-pages/man2/execve.2.html
Screen Shot 2019-11-07 at 10 02 36 PM

Windows

https://docs.microsoft.com/en-us/cpp/c-runtime-library/exec-wexec-functions?view=vs-2019
Screen Shot 2019-11-07 at 10 05 48 PM

Darwin

https://man.openbsd.org/execve.2
Screen Shot 2019-11-07 at 10 13 08 PM

FreeBSD

https://www.freebsd.org/cgi/man.cgi?query=execve&sektion=2
Screen Shot 2019-11-07 at 10 12 16 PM

and running a C variant on my MacbookPro computer

#include <unistd.h>

int main(int argc, char **argv) {
    getopt(0, NULL, NULL);
}

produces a SEGMENTATION FAULT as per

$ gcc call.c -o call && ./call
Segmentation fault: 11

Thus I shall close this issue, but nonetheless thank you for filing it, for the discourse, patience and for the CL. Looking forward to continuing to interact more with you on Go.

@odeke-em odeke-em closed this Nov 8, 2019
@kneht

This comment has been minimized.

Copy link
Author

@kneht kneht commented Nov 8, 2019

Hi @odeke-em, thank you for complete reply.

Based on links you've posted, standard and Linux documentation says that argv[0] should contain the filename, which sounds like recommendation to me.

Obviously you can check in evecve documentation for every OS and run any tests on personal computer of your choice, but it doesn't matter much as more then 50% of golang users are on Linux, at least based on survey 2018.

Given that fix has been implemented by Max and pre approved by Ian I guess it's up to them if this change needs to be merged.

Thanks for your contributions @odeke-em .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.