test: test for issue 30908 broke the noopt builder

[ALTree]

The new test introduced in CL 168957 (as 6582ee9) broke the noopt builder:

https://build.golang.org/log/e35b8d6bf5441c59c6b5f34c198c133b100f57e9

[ALTree]

cc @thanm

[thanm]

I can reproduce this fairly easily with toy programs. Ex:

go build -gcflags="os=-l -N" -ldflags="-strictdups=2" himom.go

Essentially what's happening here is that we have a set of functions that are being compiled in multiple places (ex: error.Error), and importantly both in the runtime and in code outside of the runtime.

On the no-opt builder, only the runtime is being being built with optimization, so the text symbol will have one length (and will have corresponding DWARF .debug_info section), and then when the same routine is compiled in no-opt mode, we'll see different length text (since optimization is off) and corresponding different .debug_info.

It's definitely useful to know about this problem, because it essentially says that programs built with a mix of optimization levels (some but not all packages built with "-l -N") are going to have DWARF that is at best suspect and at worst corrupted.

I will work on developing a fix.

The option that comes to mind would be to hash the contents of the .text section, and store it in the text symbol in some way (either as part of the symbol name, or stored in the symbol data in some fashion). The linker would then select a specific .text symbol, and once that is done we can include just the .debug_info (and related sections) that correspond to that version. Once we've determined which .text syms are being kept and which are being discarded, then we can do the sanity checking of the debug symbols then.

[randall77]

It sounds to me like the exception that allows the runtime to be compiled optimized even when compiling with -l -N, is broken. That exception shouldn't extend to error.Error, for example. (Or if it does, it should always, even when not compiling the runtime.)

[thanm]

That's an interesting idea -- so you're suggesting that the compiler honor "-l -N" for the various builtin functions while not honoring it for the rest of the runtime.

Another possibility would be to just whitelist these functions from the checking. It is a relatively small set, looks like

type..eq.[4]string<1>
type..eq.[6]string<1>
type..eq.[8]string<1>
type..hash.[4]string<1>
type..hash.[6]string<1>
type..hash.[8]string<1>
go.builtin.error.Error<1>

[randall77]

That's an interesting idea -- so you're suggesting that the compiler honor "-l -N" for the various builtin functions while not honoring it for the rest of the runtime.

Right.
Or it could ignore "-l -N" for the builtin functions just as it does for the runtime. The important point is to be consistent about that choice.

[gopherbot]

Change https://golang.org/cl/169161 mentions this issue: test: fix fixedbugs/issue30908.go to work with no-opt builder

[gopherbot]

Change https://golang.org/cl/169160 mentions this issue: cmd/link: change -strictdups checking to handle mix of flags