This is an archive of the discontinued LLVM Phabricator instance.

[gn build] Add symbol_level to adjust debug info level
ClosedPublic

Authored by aeubanks on Dec 9 2020, 11:51 AM.

Details

Summary

is_debug by default makes symbol_level = 2 and !is_debug means by
default symbol_level = 0.

Diff Detail

Event Timeline

aeubanks requested review of this revision.Dec 9 2020, 11:51 AM
aeubanks created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2020, 11:51 AM
aeubanks updated this revision to Diff 310624.Dec 9 2020, 1:01 PM

don't pass both /Zi and /Zd

Harbormaster completed remote builds in B81695: Diff 310627.
thakis added inline comments.Dec 16 2020, 12:40 PM
llvm/utils/gn/build/BUILD.gn
75

I think this kind of suggests that what we have here is kind of a linear scale with a few steps, and controlling that via a collection of bools is a bit unwieldy.

Maybe we should get the route of having a symbol_level thing that can be 0, 1, or 2 (with 0 being the default for !is_debug builds, and 2 being the default for is_debug builds)? That'd match other GN-based builds, and it maps very clearly to a -g flag (at least on non-win clang).

And then my CL to turn on gdb indices would turn on indices for symbol level 2 (which would be the default for debug builds), and you could set symbol_level=1 and everyone would be happy, maybe.

So is_debug = true would just be an alias for symbol_level = 2 and is_debug = false would be symbol_level = 0? Or should we just remove is_debug and have people only use symbol_level? It seems weird that you could specify is_debug = true and symbol_level = 0 (and likewise is_debug = false and symbol_level = 2).

So is_debug = true would just be an alias for symbol_level = 2 and is_debug = false would be symbol_level = 0? Or should we just remove is_debug and have people only use symbol_level? It seems weird that you could specify is_debug = true and symbol_level = 0 (and likewise is_debug = false and symbol_level = 2).

is_debug = true would be an alias for symbol_level = 2; is_optimized = false, right.

aeubanks updated this revision to Diff 312290.Dec 16 2020, 1:07 PM

add a new symbol_level and use that instead

aeubanks retitled this revision from [gn build] Add gn arg to only emit line numbers for debug info to [gn build] Add symbol_level to adjust debug info level.Dec 16 2020, 1:07 PM
aeubanks edited the summary of this revision. (Show Details)
thakis accepted this revision.Dec 16 2020, 4:22 PM

lg with the Zd thing addressed. Thanks!

llvm/utils/gn/build/BUILD.gn
101

I think you don't need this. As far as I know, Zd is a cl.exe-level option that causes it to pass /DEBUG to link.exe and does nothing else. But we don't invoke cl.exe for links, we invoke link.exe directly. link.exe /debug will produce enough symbol information to create symbolized backtraces, but not enough to have line numbers. So you can either do nothing for symbol_level=1 with cflags and just pass /debug to ldflags like we already do, or you could do if (symbol_level == 1 && is_clang) cflags += -gline-tables-only (or -g1, same thing) so that we get the same behavior on linux and windows at least with clang as host compiler.

This revision is now accepted and ready to land.Dec 16 2020, 4:22 PM
aeubanks added inline comments.Dec 16 2020, 5:02 PM
llvm/utils/gn/build/BUILD.gn
101

using clang, /Zd (without /Zi) does give line info.

In clang/test/Driver/cl-options.c I see

// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"

// RUN: %clang_cl /Zd /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7GMLT %s
// Z7GMLT: "-gcodeview"
// Z7GMLT: "-debug-info-kind=line-tables-only"
thakis added inline comments.Dec 16 2020, 5:42 PM
llvm/utils/gn/build/BUILD.gn
101

Huh, weird. https://reviews.llvm.org/rL274991 / https://reviews.llvm.org/rL275826 suggest a) some interesting backstory that I either never knew or since forgot b) that this used to be a cl.exe flag but is no longer. Does current cl.exe still accept /Zd?

aeubanks updated this revision to Diff 312374.Dec 16 2020, 9:29 PM

use /Zi and -gline-tables-only

llvm/utils/gn/build/BUILD.gn
101

Thanks for the backgroun, looks like cl.exe doesn't recognize /Zd.

Now we always pass /Zi and add -gline-tables-only for is_clang && symbol_level == 1.

thakis accepted this revision.Dec 17 2020, 6:12 AM

lgtm++

This revision was automatically updated to reflect the committed changes.