[LLDB] Warn about truncated DWARF section names on Windows #145175
Conversation
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesLinking executables with DWARF sections on Windows with the default link.exe will truncate the section names, as sections in PE/COFF executable images can't be longer than 8 bytes. There's no warning about this. When trying to debug these executables in LLDB, no debug info will be loaded, because LLDB won't find the required sections and it doesn't find CodeView debug info. There's no warning here, either (this PR adds one). I ran into this myself while playing around with LLDB:
The fix would be to pass This PR adds a warning when LLDB detects truncated DWARF sections (i.e. sections matching I included the additional note, because to get to this point, one would have to have used Clang on windows-msvc (on MinGW, the default linker doesn't truncate the names), so they should have lld-link available as well. Full diff: https://github.com/llvm/llvm-project/pull/145175.diff 4 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 4984445dcbab9..343d7477c66c7 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1036,12 +1036,19 @@ void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
m_sections_up->AddSection(header_sp);
unified_section_list.AddSection(header_sp);
+ std::vector<llvm::StringRef> truncated_dwarf_sections;
const uint32_t nsects = m_sect_headers.size();
for (uint32_t idx = 0; idx < nsects; ++idx) {
llvm::StringRef sect_name = GetSectionName(m_sect_headers[idx]);
ConstString const_sect_name(sect_name);
SectionType section_type = GetSectionType(sect_name, m_sect_headers[idx]);
+ // Detect unknown sections matching ^\.debug_[a-z]$
+ if (section_type == eSectionTypeOther && sect_name.size() == 8 &&
+ sect_name.starts_with(".debug_") && llvm::isLower(sect_name.back())) {
+ truncated_dwarf_sections.emplace_back(sect_name);
+ }
+
SectionSP section_sp(new Section(
module_sp, // Module to which this section belongs
this, // Object file to which this section belongs
@@ -1071,6 +1078,16 @@ void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
m_sections_up->AddSection(section_sp);
unified_section_list.AddSection(section_sp);
}
+
+ if (!truncated_dwarf_sections.empty()) {
+ module_sp->ReportWarning(
+ "contains {} truncated DWARF sections ({}).\n"
+ "Note: Executable images on Windows can't include the required names "
+ "when linking with the default link.exe. A third party linker like "
+ "lld-link is required (compile with -fuse-ld=lld-link on Clang).",
+ truncated_dwarf_sections.size(),
+ llvm::join(truncated_dwarf_sections, ", "));
+ }
}
}
diff --git a/lldb/test/Shell/ObjectFile/PECOFF/lit.local.cfg b/lldb/test/Shell/ObjectFile/PECOFF/lit.local.cfg
index 9ef350be1dee2..1ae00d07fc3e6 100644
--- a/lldb/test/Shell/ObjectFile/PECOFF/lit.local.cfg
+++ b/lldb/test/Shell/ObjectFile/PECOFF/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.yaml', '.test']
+config.suffixes = ['.yaml', '.test', '.c']
diff --git a/lldb/test/Shell/ObjectFile/PECOFF/truncated-dwarf.c b/lldb/test/Shell/ObjectFile/PECOFF/truncated-dwarf.c
new file mode 100644
index 0000000000000..92cb61050142a
--- /dev/null
+++ b/lldb/test/Shell/ObjectFile/PECOFF/truncated-dwarf.c
@@ -0,0 +1,7 @@
+// REQUIRES: target-windows
+// RUN: %build --compiler=clang-cl --force-dwarf-symbols --force-ms-link -o %t.exe -- %s
+// RUN: %lldb -f %t.exe 2>&1 | FileCheck %s
+
+int main(void) {}
+
+// CHECK: warning: {{.*}} contains 4 truncated DWARF sections (.debug_{{[a-z]}}, .debug_{{[a-z]}}, .debug_{{[a-z]}}, .debug_{{[a-z]}})
diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index caaa14f90af1c..ecb02c1d7c87f 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -173,6 +173,22 @@
help="Specify the C/C++ standard.",
)
+if sys.platform == "win32":
+ parser.add_argument(
+ "--force-dwarf-symbols",
+ dest="force_dwarf_symbols",
+ action="store_true",
+ default=False,
+ help="When compiling with clang-cl on Windows, use DWARF instead of CodeView",
+ )
+ parser.add_argument(
+ "--force-ms-link",
+ dest="force_ms_link",
+ action="store_true",
+ default=False,
+ help="When compiling with clang-cl on Windows, always use link.exe",
+ )
+
args = parser.parse_args(args=sys.argv[1:])
@@ -379,15 +395,20 @@ def __init__(self, toolchain_type, args):
)
if self.mode == "link" or self.mode == "compile-and-link":
- self.linker = (
- self._find_linker("link")
- if toolchain_type == "msvc"
- else self._find_linker("lld-link", args.tools_dir)
- )
+ if toolchain_type == "msvc" or args.force_ms_link:
+ search_paths = []
+ if toolchain_type != "msvc":
+ search_paths.append(
+ os.path.dirname(find_executable("cl", args.tools_dir))
+ )
+ self.linker = self._find_linker("link", search_paths)
+ else:
+ self.linker = self._find_linker("lld-link", args.tools_dir)
if not self.linker:
raise ValueError("Unable to find an appropriate linker.")
self.compile_env, self.link_env = self._get_visual_studio_environment()
+ self.force_dwarf_symbols = args.force_dwarf_symbols
def _find_linker(self, name, search_paths=[]):
compiler_dir = os.path.dirname(self.compiler)
@@ -678,6 +699,8 @@ def _get_compilation_command(self, source, obj):
args.append("/GR-")
args.append("/Z7")
if self.toolchain_type == "clang-cl":
+ if self.force_dwarf_symbols:
+ args.append("-gdwarf")
args.append("-Xclang")
args.append("-fkeep-static-consts")
args.append("-fms-compatibility-version=19")
|
Nerixyz
force-pushed
the
feat/lldb-warn-on-trunc-dwarf-section
branch
from
b58c2be to
d2e8656
Compare
Nerixyz
force-pushed
the
feat/lldb-warn-on-trunc-dwarf-section
branch
from
d2e8656 to
c4ded34
Compare
DavidSpickett
left a comment
DavidSpickett
left a comment
There was a problem hiding this comment.
I've disabled a lot of tests on this bot for what was/is probably this issue, so this will be a helpful warning I think.
There was a problem hiding this comment.
Do we need the size == 8 and isLower checks here? Probably unlikely, but could the truncation change in the future to a different size?
Could we just check the section_type and prefix and issue a warning saying "unrecognized section, possibly got truncated"?
There was a problem hiding this comment.
Probably unlikely, but could the truncation change in the future to a different size?
The truncation will always be 8 bytes, because that's how much space the section name has in the PE/COFF section header - it's basically a char name[8] field. For longer names, the linker has to write the section name into the string table and set the name in the section header to /n where n is the offset in the string table.
Could we just check the section_type and prefix and issue a warning saying "unrecognized section, possibly got truncated"?
That could be an additional diagnostic for unknown DWARF sections (should probably be there for other object files as well). If the section name is 8 bytes, however, we can be sure this is from someone using link.exe.
As an extension, this could be present for .lldbformatters and .lldbsummaries as well. In this specific case, the matching could also consider .lldbfor and .lldbsum, because there are only two.
Do we need the
size == 8andisLowerchecks here?
We need the size check, but I removed the isLower check, because the indicator for DWARF is .debug_.
There was a problem hiding this comment.
For longer names, the linker has to write the section name into the string table and set the name in the section header to /n where n is the offset in the string table.
Besides the point of this PR, but is there any information about whether link.exe does this too, or if it's just an escape route for third parties?
Though even if it did this, our parser would have to be updated to parse it too. I could understand a hypothetical link.exe option being off by default for tool compatibility reasons.
There was a problem hiding this comment.
The truncation will always be 8 bytes, because that's how much space the section name has in the PE/COFF section header - it's basically a char name[8] field. For longer names, the linker has to write the section name into the string table and set the name in the section header to /n where n is the offset in the string table.
If that's guaranteed by the file format and won't ever change, fair enough. Lets at least keep a comment about this here
There was a problem hiding this comment.
Besides the point of this PR, but is there any information about whether link.exe does this too, or if it's just an escape route for third parties?
It is officially documented in the name field of the COFF header.
I don't know when/if link.exe will ever generate this indirect section name (I think it will always emit executable images).
So it's non-standard (lld-link also warns: section name .debug_abbrev is longer than 8 characters and will use a non-standard string table - MinGW's ld does this without any warning), but supported by most tools - even Microsoft's dumpbin will happily output the long section names.
There was a problem hiding this comment.
Besides the point of this PR, but is there any information about whether link.exe does this too, or if it's just an escape route for third parties?
As linked in the official documents, link.exe doesn't do this.
This isn't a syntax invented by third parties though - this uses the same syntax as already is used for relocatable object files, for linked executables as well. That's why Microsoft's own tools happen to end up mostly understanding it too.
Though even if it did this, our parser would have to be updated to parse it too.
I don't think so? Unless they come up with a different, incompatible syntax for doing it, the natural way of handling it would be to do what third party tools do now - reusing the object file syntax. (One argument against it, is that the long section names aren't available as part of the runtime mapped parts of images - so tools that introspect a loaded DLL at runtime can't see these long section names - unless it looks at the origin file on disk, like a debugger does.)
There was a problem hiding this comment.
I was thinking that lldb did not already have support for this format but it sounds like we already do. I know lld will write it out this way because I've seen that non-standard warning a few times before.
First off, this probably meant to say "section names can't be longer than 8 bytes", not the sections themselves. Then, this is on the level of nitpickery, but I think it's good to get the details right: If the names can't be longer than 8 bytes, then it would seem like it would be a hard limit that can't be overcome by using a different linker. As this is the PR description, which ends up as the commit message once this is merged, it's worth the effort to get the wording correct. Not sure what the best wording would be here, though... "Section names in PE/COFF executable images can't be longer than 8 bytes officially, although other linkers do support creating that, and GDB/LLDB do support reading longer names." Or something along those lines? And the same nuance for the warning message:
Perhaps |
Nerixyz
force-pushed
the
feat/lldb-warn-on-trunc-dwarf-section
branch
2 times, most recently
from
39fa8ad to
ebd8542
Compare
DavidSpickett
left a comment
DavidSpickett
left a comment
There was a problem hiding this comment.
LGTM.
Whether it can be extended to other known sections, that's up to you, but in its current DWARF specific form I like it.
|
Actually, add an entry to https://github.com/llvm/llvm-project/blob/main/llvm/docs/ReleaseNotes.md#changes-to-lldb as well. It's not something I'd normally put in the release notes but it may appear in existing workflows when people update. If it's in the release notes we can refer them back to that to prove that it's expected to be there. For example:
|
Nerixyz
force-pushed
the
feat/lldb-warn-on-trunc-dwarf-section
branch
2 times, most recently
from
72f30f6 to
fa67d1d
Compare
Nerixyz
force-pushed
the
feat/lldb-warn-on-trunc-dwarf-section
branch
from
fa67d1d to
a7e33a1
Compare
6 participants
Linking executables with DWARF sections on Windows with the default link.exe will truncate the section names, as section names in PE/COFF executable images can't be longer than 8 bytes officially. There's no warning about this. When trying to debug these executables in LLDB, no debug info will be loaded, because LLDB won't find the required sections and it doesn't find CodeView debug info. There's no warning here, either (this PR adds one).
Other linkers like lld-link, however, support creating these sections and GDB/LLDB and Microsoft's dumpbin utility are able to pick them up.
I ran into this myself while playing around with LLDB:
clang-cl foo.c -gdwarf -o foo.exelldb foo.exeb foo.c:10⚡ → "Unable to resolve breakpoint to any actual locations." (because no debug info is loaded)The fix would be to pass
-fuse-ld=lld-linkto Clang.This PR adds a warning when LLDB detects truncated DWARF sections (i.e. sections matching
.debug_*):I included the additional note, because to get to this point, one would have to have used Clang on windows-msvc (on MinGW, the default linker doesn't truncate the names), so they should have lld-link available as well.