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

missing DWARF symbol table #3717

Merged
merged 2 commits into from
Aug 1, 2022

Conversation

crazy-max
Copy link
Member

Regression from #3320 which always strips DWARF symbol table.

I encounter this issue while trying to debug #3621 but also to improve our shell out operations with debug/buildinfo pkg (available since Go 1.18) for plugins discovery.

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max
Copy link
Member Author

Ah ok now I remember why we removed DWARF symbol table: https://github.com/docker/cli/runs/7479620604?check_suite_focus=true#step:4:2067

 > [linux/amd64->darwin/arm64 build 3/3] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=from=dockercore/golang-cross:xx-sdk-extras,target=/xx-sdk,src=/xx-sdk     --mount=type=tmpfs,target=cli/winresources     xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     xx-verify $([ "static" = "static" ] && echo "--static") /out/docker:
#0 1.584 Building static docker-darwin-arm64
#0 1.603 + go build -o /out/docker-darwin-arm64 -tags  osusergo pkcs11 -ldflags  -X "github.com/docker/cli/cli/version.GitCommit=f1d40cae1b" -X "github.com/docker/cli/cli/version.BuildTime=2022-07-23T07:51:49Z" -X "github.com/docker/cli/cli/version.Version=22.06.0-beta.0-56-gf1d40cae1b.m" -buildmode=pie github.com/docker/cli/cmd/docker
#67 517.7 # github.com/docker/cli/cmd/docker
#67 517.7 /usr/local/go/pkg/tool/linux_amd64/link: /usr/local/go/pkg/tool/linux_amd64/link: running dsymutil failed: exec: "dsymutil": executable file not found in $PATH
#67 517.7 

By default on Darwin the linker will invoke dsymutil to put the debug info directly into the executable. This invocation of dsymutil is disabled by the linker's -w or -s option 😣

I think our osxsdk used for cross comp darwin binaries might be too old:

cli/Dockerfile

Line 62 in f1615fa

--mount=from=dockercore/golang-cross:xx-sdk-extras,target=/xx-sdk,src=/xx-sdk \

I will take a look

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2022

Codecov Report

Merging #3717 (2854529) into master (f1615fa) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3717      +/-   ##
==========================================
- Coverage   59.06%   59.06%   -0.01%     
==========================================
  Files         289      289              
  Lines       24673    24665       -8     
==========================================
- Hits        14573    14568       -5     
+ Misses       9226     9224       -2     
+ Partials      874      873       -1     

@crazy-max
Copy link
Member Author

Ok so we have to install the llvm debian package when using glibc but minimal version required with osxsdk is llvm 12 and bullseye is late: llvm (1:11.0-51+nmu5).

I took a look at https://apt.llvm.org/ but does not seem to include dsymutil 😞:

FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-bullseye AS build-base-bullseye
COPY --from=xx / /
RUN apt-get update && apt-get install --no-install-recommends -y bash clang lld file lsb-release software-properties-common
RUN wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && ./llvm.sh 12 all && rm -f llvm.sh
WORKDIR /go/src/github.com/docker/cli
 > [linux/amd64->darwin/arm64 build 3/3] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=from=dockercore/golang-cross:xx-sdk-extras,target=/xx-sdk,src=/xx-sdk     --mount=type=tmpfs,target=cli/winresources     xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     xx-verify $([ "static" = "static" ] && echo "--static") /out/docker:
#0 0.369 Building static docker-darwin-arm64
#0 0.371 + go build -o /out/docker-darwin-arm64 -tags  osusergo pkcs11 -ldflags  -X "github.com/docker/cli/cli/version.GitCommit=d02816e4fa" -X "github.com/docker/cli/cli/version.BuildTime=2022-07-23T11:44:35Z" -X "github.com/docker/cli/cli/version.Version=22.06.0-beta.0-55-gd02816e4fa.m" -buildmode=pie github.com/docker/cli/cmd/docker
#0 2.803 # github.com/docker/cli/cmd/docker
#0 2.803 /usr/local/go/pkg/tool/linux_amd64/link: /usr/local/go/pkg/tool/linux_amd64/link: running dsymutil failed: exec: "dsymutil": executable file not found in $PATH
#0 2.803

So I think we have to use the osxcross toolchain that includes all the tooling we need. Unfortunately our dockercore/golang-cross:xx-sdk-extras image only includes the osxsdk but I have created an image that includes the osxcross toolchain and the osxsdk: https://github.com/crazy-max/docker-osxcross#usage and everything looks good now using both with alpine and debian.

@tonistiigi Maybe instead of the debian variant we could switch to ubuntu:22.04 that supports llvm 12 and includes dsymutil: https://packages.ubuntu.com/jammy/amd64/llvm-12/filelist but armel cross pkgs are dropped in this release 😣: moby/moby#43529 (comment)

Dockerfile Outdated
Comment on lines 73 to 80
if [ "$BASE_VARIANT" != "alpine" ] && [ "$(xx-info os)" = "darwin" ]; then
# on debian, use osxcross toolchain that includes the llvm-dsymutil tool
# and right minimal llvm version required
# more info: https://github.com/docker/cli/pull/3717
export PATH="/osxcross/bin:$PATH"
export LD_LIBRARY_PATH="/osxcross/lib:$LD_LIBRARY_PATH"
export CC="o64-clang"
export CXX="o64-clang++"
fi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonistiigi If you're ok with osxcross we could add a detection folder like /xx-sdk on xx so it could set itself the required env vars to use osxcross and we could remove this part in the Dockerfile? Maybe also the ability to support macports like https://github.com/crazy-max/goxx/blob/main/rootfs/usr/local/bin/goxx-macports. WDYT?

@crazy-max crazy-max force-pushed the keep-dwarf-symbol-table branch 4 times, most recently from 1edf995 to 75dabed Compare July 25, 2022 08:46
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default on Darwin the linker will invoke dsymutil to put the debug info directly into the executable. This invocation of dsymutil is disabled by the linker's -w or -s option

Is this something that could be fixed in Go?

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@crazy-max
Copy link
Member Author

crazy-max commented Jul 25, 2022

@tonistiigi Can't repro everytime but we currently have this issue with BuildKit: https://github.com/docker/cli/runs/7496406536?check_suite_focus=true#step:4:389

#59 [linux/amd64->windows/amd64 build 3/3] RUN --mount=type=bind,target=.     --mount=type=cache,target=/root/.cache     --mount=from=osxcross,src=/osxcross,target=/osxcross     --mount=from=osxcross,src=/osxsdk,target=/xx-sdk     --mount=type=tmpfs,target=cli/winresources <<EOT (set -e...)
#59 ERROR: failed to calculate checksum of ref m922mgc9ac45dw652zvglr6tl::w8dtv6wnagnwlcaa7arrzhowq: "/osxsdk": not found

But /osxsdk folder exists for windows/amd64 platform:

$ undock --rm-dist --platform=windows/amd64 crazymax/osxcross:11.3-r7-alpine ./extract
$ tree -nh ./extract
[ 512]  ./extract/
├── [ 512]  osxcross
└── [ 512]  osxsdk

2 directories, 0 files

I wonder is this is not linked to the infamous empty layer issue we had in the past or moby/buildkit#2973.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max force-pushed the keep-dwarf-symbol-table branch 2 times, most recently from 093023f to 963e446 Compare July 25, 2022 09:46
@crazy-max
Copy link
Member Author

I wonder is this is not linked to the infamous empty layer issue we had in the past or moby/buildkit#2973.

Looking at https://github.com/docker/cli/runs/7497061435?check_suite_focus=true#step:4:330:

#36 [linux/amd64 goversioninfo 1/1] RUN --mount=type=cache,target=/root/.cache/go-build     --mount=type=cache,target=/go/pkg/mod     GOBIN=/out GO111MODULE=on go install "github.com/josephspurrier/goversioninfo/cmd/goversioninfo@v1.3.0"
#36 CACHED

It seems some instructions are cached even if no cache exporters are in place and the builder is a new one with a fresh BuildKit state in the GHA workflow.

With cache disabled everything looks good: https://github.com/docker/cli/actions/runs/2731390831

@crazy-max crazy-max marked this pull request as draft July 25, 2022 17:56
@crazy-max crazy-max marked this pull request as ready for review July 25, 2022 19:51
@crazy-max
Copy link
Member Author

crazy-max commented Jul 25, 2022

Ok so after bisect we actually strip DWARF symbol table in 20.10. So not a regression but this is needed for docker-archive/compose-cli#2151. Needs to noop llvm-strip for darwin/amd64 platform with glibc as discussed with @tonistiigi. Can't repro with alpine and LLVM 12.

@crazy-max crazy-max requested a review from thaJeztah July 25, 2022 19:53
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
scripts/build/.variables Show resolved Hide resolved
scripts/build/.variables Show resolved Hide resolved
@crazy-max
Copy link
Member Author

crazy-max commented Jul 28, 2022

@thaJeztah Comment added. We also discussed with @tonistiigi about this and we could fallback to strip from cctools for dawrin if llvm upstream doesn't work. this could be bundled in xx with ld64 so we could remove this hack.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max requested a review from thaJeztah July 28, 2022 20:49
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

@tonistiigi this LGTY?

@thaJeztah thaJeztah added this to the 22.06.0 milestone Jul 28, 2022
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look more deeply into what that darwin issue is. Is it something broken in old LLVM etc. Is it Debian weirdness? If needed we can get the Apple strip from https://github.com/tpoechtrager/cctools-port/blob/master/cctools/misc/strip.c with ld64 in xx.

@tonistiigi tonistiigi merged commit 5c511f4 into docker:master Aug 1, 2022
@crazy-max crazy-max deleted the keep-dwarf-symbol-table branch August 1, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants