This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Add a new verification mode
ClosedPublic

Authored by JDevlieghere on Mar 30 2023, 12:37 AM.

Details

Summary

This patch a new verification mode called "auto" that runs the DWARF verifier on the input and if the input is valid, also runs the DWARF verifier on the output. The goal is to catch cases where dsymutil turns valid into invalid DWARF. This patch makes this verification mode the default when assertions or expensive checks are enabled.

Diff Detail

Event Timeline

JDevlieghere created this revision.Mar 30 2023, 12:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2023, 12:37 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
JDevlieghere requested review of this revision.Mar 30 2023, 12:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2023, 12:37 AM
avl added inline comments.Mar 30 2023, 10:16 AM
llvm/include/llvm/DWARFLinker/DWARFLinker.h
247

onInputVerificationFailure looks to be a bit more precise name.

351
825

It looks like this variable is never set.

llvm/lib/DWARFLinker/DWARFLinker.cpp
2896–2898

as we have a call to the handler here it is probably better to move warning reporting code into this handler.

2897

if VerificationHandler is not set then we need to have a check for null here, or provide already existing default implementation.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
276

default state for the second parameter is DWARFContext::ProcessDebugRelocations::Process. Is it necessary to explicitly specify it here?

llvm/tools/dsymutil/dsymutil.cpp
707–708

probably move this check and warning into the verifyOutput() function? to have a single place where Options.LinkOpts.NoOutput is checked.

avl added a comment.Mar 30 2023, 10:32 AM

as DWARFv5 is not fully supported yet - probably not run input and output verifier for 5 version?

JDevlieghere marked 7 inline comments as done.

Address Alexey's code review feedback.

Skip output verification for DWARF 5

avl accepted this revision.Mar 30 2023, 12:26 PM

LGTM.

llvm/tools/dsymutil/dsymutil.cpp
502

AFAIU, common spelling would be DWARFv5

This revision is now accepted and ready to land.Mar 30 2023, 12:26 PM
This revision was landed with ongoing or failed builds.Mar 31 2023, 9:59 AM
This revision was automatically updated to reflect the committed changes.
dyung added a subscriber: dyung.Mar 31 2023, 3:26 PM

@JDevlieghere, your change is causing 6 test failures on the PS5 Windows bot, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/216/builds/19140

haowei added a subscriber: haowei.Mar 31 2023, 3:38 PM

We are seeing test failures on Fuchsia's Windows bots after you changes are landed:

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "rm" "-rf" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir"
$ ":" "RUN: at line 2"
$ "mkdir" "-p" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir"
$ ":" "RUN: at line 3"
$ "cat" "C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic.macho.x86_64"
$ ":" "RUN: at line 4"
$ "c:\src\llvm-project\build\bin\dsymutil.exe" "-accelerator=Pub" "-oso-prepend-path=C:\src\llvm-project\llvm\test\tools\dsymutil\X86/.." "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic"
# command stderr:
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic1.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.761849100) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic2.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.765848900) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic3.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.768848800) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic1.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.761849100) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic2.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.765848900) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic3.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.768848800) and debug map (2014-12-03 17:01:27.000000000)

$ ":" "RUN: at line 5"
$ "c:\src\llvm-project\build\bin\llvm-dwarfdump.exe" "-a" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic.dSYM"
$ "c:\src\llvm-project\build\bin\filecheck.exe" "C:\src\llvm-project\llvm\test\tools\dsymutil\X86/basic-linking-x86.test"
$ ":" "RUN: at line 6"
$ "c:\src\llvm-project\build\bin\dsymutil.exe" "-accelerator=Pub" "--update" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic.dSYM"
# command stderr:
error: C:\src\temp\lit-tmp-0ajrr046\dsym.tmp2328a.dwarf: permission denied
error: command failed with exit status: 1

1 example of the build: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8785076599579542465/overview

Even with your latest patch c9f37c450056d6438e0ccc653580600d827ecd80, the test failure persist. Could you revert your changes if you need some time to investigate please?

I've switched back to no verification as the default (c2d2f724fe708a650b16c9d2f1f1a47156e5b2ea) while I investigate the permission issue.