Skip to content

Conversation

motiejus
Copy link
Contributor

@motiejus motiejus commented May 18, 2022

I can't get the current -Wl,-z<...> pass to the linker. Assume any C file that has int main:

$ zig cc -Wl,-znow ./test.c -o test
warning: unsupported linker arg: -znow

And, surely, the linker line did not have -z now.

Cross-checking with clang/gcc:

$ clang-13 -v -Wl,-znow ./test.c -o test |& tail -1
 "/usr/bin/ld" --hash-style=both --build-id --eh-frame-hdr -m elf_x86_64 <...> -znow -zrelro <...>

Same for -zrelro, -znoexecstack and others.

After this commit zig does the right thing: observe -z now and -z relro in the linker line:

 $ ~/code/zig/build/zig cc -v -Wl,-zrelro -Wl,-znow ./test.c -o test |& tail -1
ld.lld -error-limit=0 -O0 -z stack-size=16777216 --gc-sections --eh-frame-hdr -z now -z relro <...>

@Luukdegram
Copy link
Contributor

I don’t think this is the right change as it also affects other linkers such as Wasm, Coff. For example to set the stack size you’d use -z stack-size=<x>. This change would break that. Note that lld’s (or really GNU ld since lld is a drop-in replacement) documentation also specifies -z option. Perhaps Clang supports both ways, but do we want that for Zig as well?

@motiejus
Copy link
Contributor Author

Ah, I didn't see the plain -z option. I will rework this diff. Thanks @Luukdegram

I can't get the current `-Wl,-z<...>` pass to the linker. Assume any C
file that has `int main`:

    $ zig cc -Wl,-znow ./test.c -o test
    warning: unsupported linker arg: -znow

And, surely, the linker line did not have `-z now`.

Cross-checking with clang/gcc:

    $ clang-13 -v -Wl,-znow ./test.c -o test |& tail -1
     "/usr/bin/ld" --hash-style=both --build-id --eh-frame-hdr -m elf_x86_64 <...> -znow -zrelro <...>

Same for -zrelro, -znoexecstack and others.

After this commit zig does the right thing: observe `-z now` and `-z
relro` in the linker line:

     $ ~/code/zig/build/zig cc -v -Wl,-zrelro -Wl,-znow ./test.c -o test |& tail -1
    ld.lld -error-limit=0 -O0 -z stack-size=16777216 --gc-sections --eh-frame-hdr -z now -z relro <...>
@motiejus
Copy link
Contributor Author

Thanks again, @Luukdegram , for pointing out -z. I think I cleaned up the diff by now.

@motiejus
Copy link
Contributor Author

This is ready for re-review.

@Luukdegram
Copy link
Contributor

Not a big fan of having multiple ways to specify the same thing where one of them isn't even documented, but perhaps @andrewrk has a different opinion on that. We now also introduce a difference for the same flag between zig cc and zig build-{x}. For example, if the user learns they can do zig cc -znow and then later try to apply it doing zig build-exe example.c -znow they'll be met with an error.

motiejus added a commit to motiejus/go that referenced this pull request May 23, 2022
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:

    $ git grep -h -- '-Wl,-z' master | cut -b-76
            re(`-Wl,-z,(no)?execstack`),
            re(`-Wl,-z,relro`),
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,nodelete")
                            argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                    argv = append(argv, "-Wl,-znow")
                    argv = append(argv, "-Wl,-znocopyreloc")
    // gcc -g multiple-code-sections.c -Wl,--emit-relocs -Wl,--discard-none -Wl,
    // gcc -g multiple-code-sections.c -Wl,-zmax-page-size=1 -fno-asynchronous-u

However, not adding a space between `-z` and the param is not
documented:

llvm-13:
```
$ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
     -z option
             Linker option extensions.
```

gnu ld:

```
$ man ld | grep -E -A1 -w -- "^ +-z"
       -z keyword
           The recognized keywords are:
--
       -z defs
           Report unresolved symbol references from regular object files.  This is done even if the linker is creating a non-symbolic
--
       -z muldefs
           Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
--
       -z
       --imagic
```

... and thus should be avoided.

`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`, as documented.

Closes ziglang/zig#11669
@motiejus
Copy link
Contributor Author

Perhaps this is Go's fault. Go almost always uses -z PARAM.

@alexrp
Copy link
Member

alexrp commented May 23, 2022

I'm confused. What's wrong with zig cc -Wl,-z,now?

@motiejus
Copy link
Contributor Author

I'm confused. What's wrong with zig cc -Wl,-z,now?

I like it, but the use-in-the-wild differs.

Let's see what Go has to say about it in golang/go#53030.

motiejus added a commit to motiejus/go that referenced this pull request May 24, 2022
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:

    $ git grep -h -- '-Wl,-z' master | cut -b-76
            re(`-Wl,-z,(no)?execstack`),
            re(`-Wl,-z,relro`),
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,nodelete")
                            argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                    argv = append(argv, "-Wl,-znow")
                    argv = append(argv, "-Wl,-znocopyreloc")

However, not adding a space between `-z` and the param is not
documented:

llvm-13:
```
$ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
     -z option
             Linker option extensions.
```

gnu ld:

```
$ man ld | grep -E -A1 -w -- "^ +-z"
       -z keyword
           The recognized keywords are:
--
       -z defs
           Report unresolved symbol references from regular object files.  This is done even if the linker is creating a non-symbolic
--
       -z muldefs
           Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
--
       -z
       --imagic
```

... and thus should be avoided.

`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`.

Closes ziglang/zig#11669
motiejus added a commit to motiejus/go that referenced this pull request Jun 7, 2022
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:

    $ git grep -h -- '-Wl,-z' master | cut -b-76
            re(`-Wl,-z,(no)?execstack`),
            re(`-Wl,-z,relro`),
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,nodelete")
                            argv = append(argv, "-Wl,-z,relro")
                                    argv = append(argv, "-Wl,-z,relro")
                    argv = append(argv, "-Wl,-znow")
                    argv = append(argv, "-Wl,-znocopyreloc")

However, not adding a space between `-z` and the param is not
documented:

llvm-13:

    $ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
         -z option
                 Linker option extensions.

gnu ld:

    $ man ld | grep -E -A1 -w -- "^ +-z"
           -z keyword
               The recognized keywords are:
    --
           -z defs
               Report unresolved symbol references from regular object files.  This is done even if the linker is creating a non-symbolic
    --
           -z muldefs
               Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
    --
           -z
           --imagic

... and thus should be avoided.

`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`.

Closes ziglang/zig#11669
gopherbot pushed a commit to golang/go that referenced this pull request Jun 7, 2022
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:

    $ git grep -E -- '-Wl,-z[^,]' master | wc -l
    4
    $ git grep -E -- '-Wl,-z,' master | wc -l
    7

However, not adding a space between `-z` and the param is not
documented:

llvm-13:

    $ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
         -z option
                 Linker option extensions.

gnu ld:

    $ man ld | grep -E -A1 -w -- "^ +-z"
           -z keyword
               The recognized keywords are:
    --
           -z defs
               Report unresolved symbol references from regular object files.  This is done even if the linker is creating a non-symbolic
    --
           -z muldefs
               Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
    --
           -z
           --imagic

... and thus should be avoided.

`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`, as documented.

Closes ziglang/zig#11669

Change-Id: I758054ecaa3ce01a72600bf65d7f7b5c3ec46d09
GitHub-Last-Rev: e068e00
GitHub-Pull-Request: #53030
Reviewed-on: https://go-review.googlesource.com/c/go/+/407834
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Cherry Mui <[email protected]>
@motiejus
Copy link
Contributor Author

motiejus commented Jun 7, 2022

Go will do the right thing now with golang/go@38607c5 , closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants