This is an archive of the discontinued LLVM Phabricator instance.

[lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div
ClosedPublic

Authored by jwnhy on Apr 1 2023, 12:35 AM.

Details

Summary

This patch resolves an issue where a value
is incorrectly displayed if it is represented
by DW_OP_div.

This issue is caused by lldb evaluating
operands of DW_OP_div as unsigned
and performed unintended unsigned
division.

This issue is resolved by creating two
temporary signed scalar and performing
signed division.

Diff Detail

Event Timeline

jwnhy created this revision.Apr 1 2023, 12:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2023, 12:35 AM
jwnhy requested review of this revision.Apr 1 2023, 12:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2023, 12:35 AM

The change looks good: it matches what the DWARFv5 standard says:

The DW_OP_div operation pops the top two stack values, divides the former second entry by the former top of the stack using signed division, and pushes the result.

Can you please add a regression test for the corrected behavior?

jwnhy updated this revision to Diff 510285.Apr 1 2023, 7:31 PM

Update the patch to include a small
regression test.

jwnhy updated this revision to Diff 512350.Apr 10 2023, 11:55 PM

Thanks a lot for the suggestion.
I added a assembly testcase for "DW_OP_div".
Also, since I don't have the permission to push,
if the patch is good to you, could you please
help me merge it?

Ping.

Michael137 accepted this revision.Apr 27 2023, 5:38 AM

LGTM

lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
4

other tests use llvm-mc as the assembler
E.g., llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s

I think it'll marginally speed things up

7

Can you paste the source code here which you used to derive this test case? For future readers

This revision is now accepted and ready to land.Apr 27 2023, 5:38 AM
jwnhy updated this revision to Diff 517784.Apr 27 2023, 8:43 PM
jwnhy marked an inline comment as done.

Updated the patch to include the original code.

Could you please help me push it to the main codebase?, as I don't have the push right to the repo.

jwnhy marked an inline comment as done.Apr 27 2023, 8:44 PM
jwnhy added inline comments.
lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
4

There are some issues on llvm-mc, it complains about .loc directives.
So I guess I will just go with Clang...

7

Update the patch to include the original sources.

@jwnhy Let me know which name and email I should attribute the patch to

jwnhy marked an inline comment as done.May 1 2023, 7:35 PM

@jwnhy Let me know which name and email I should attribute the patch to

Oh, sorry.
I am LU Hongyi, and the email is jwnhy0@gmail.com, my github account is also jwnhy.

Thank you very much.

This revision was landed with ongoing or failed builds.May 2 2023, 4:39 AM
This revision was automatically updated to reflect the committed changes.

I'll try adding a # REQUIRES: lld

asl added a comment.May 2 2023, 11:42 AM

The test seems to rely on the presence of the linker for the desired target platform, so cannot be used on cross-compile environment.

******************** TEST 'lldb-shell :: SymbolFile/DWARF/x86/DW_OP_div-with-signed.s' FAILED ********************
Script:
--
: 'RUN: at line 3';   /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution --target=x86_64-pc-linux -o /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
: 'RUN: at line 4';   /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/lldb --no-lldbinit -S /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb/test/Shell/lit-lldb-init-quiet /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i"  -o "exit" | /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/FileCheck /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
--
Exit Code: 1
Command Output (stderr):
--
/usr/bin/ld: unrecognised emulation mode: elf_x86_64
Supported emulations: armelf_linux_eabi armelfb_linux_eabi
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Reverted for now until we find fix for test

jwnhy updated this revision to Diff 518975.May 2 2023, 11:08 PM

Update the testcase to resolve cross-platform issue.
This new testcase is hand-crafted with DW_OP_div.
Therefore, it no longer require to be linked and executed.
Just directly call lldb on this object file and use "p g" to
see if the value represented by DW_OP_div is correct or not.

jwnhy added a comment.May 2 2023, 11:09 PM

I have updated the testcase in the patch and remove the linker stage (using "-c").
But I don't have an Arm devices, can anyone help me test it?

jwnhy added a comment.May 2 2023, 11:39 PM

I have updated the testcase in the patch and remove the linker stage (using "-c").
But I don't have an Arm devices, can anyone help me test it?

Asked one of my friend to tested on M1 Arm, "clang -c" works.

I tried the new test case on my Mac but it's now hitting an assertion:

******************** TEST 'lldb-shell :: SymbolFile/DWARF/x86/DW_OP_div-with-signed.s' FAILED ********************
Script:
--
: 'RUN: at line 2';   /Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang --target=specify-a-target-or-use-a-_host-substitution -c --target=x86_64-pc-linux -o /Users/michaelbuch/Git/lldb-build-main-no-mod
ules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp /Users/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
: 'RUN: at line 3';   /Users/michaelbuch/Git/lldb-build-main-no-modules/bin/lldb --no-lldbinit -S /Users/michaelbuch/Git/lldb-build-main-no-modules/tools/lldb/test/Shell/lit-lldb-init-quiet /Users/michaelbuch/Git
/lldb-build-main-no-modules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp -o "expression -T -- g" -o "exit" | /Users/michaelbuch/Git/lldb-build-main-no-modules/bin/FileCheck /Users
/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
--
Exit Code: 1

Command Output (stderr):
--
Assertion failed: ((!RootFile.Name.empty() || MCDwarfFiles.size() >= 1) && "No root file and no .file directives"), function emitV5FileDirTables, file MCDwarf.cpp, line 473.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang --target=specify-a-target-or-use-a-_host-substitution -c --target=x86_64-pc-linux -o /Users/michaelbuch/Git/lldb-build-main-n
o-modules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp /Users/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
 #0 0x0000000109466660 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104682660)
 #1 0x0000000109466bf8 PrintStackTraceSignalHandler(void*) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104682bf8)
 #2 0x00000001094649d0 llvm::sys::RunSignalHandlers() (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1046809d0)
 #3 0x0000000109465e10 llvm::sys::CleanupOnSignal(unsigned long) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104681e10)
 #4 0x00000001093245c8 (anonymous namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1045405c8)
 #5 0x0000000109324a64 CrashRecoverySignalHandler(int) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104540a64)
 #6 0x000000018ddc2a24 (/usr/lib/system/libsystem_platform.dylib+0x18042ea24)
 #7 0x000000018dd93c28 (/usr/lib/system/libsystem_pthread.dylib+0x1803ffc28)
 #8 0x000000018dca1ae8 (/usr/lib/system/libsystem_c.dylib+0x18030dae8)
 #9 0x000000018dca0e44 (/usr/lib/system/libsystem_c.dylib+0x18030ce44)
#10 0x0000000108c3ad04 llvm::MCDwarfLineTableHeader::emitV5FileDirTables(llvm::MCStreamer*, std::__1::optional<llvm::MCDwarfLineStr>&) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e5
6d04)
#11 0x0000000108c39ff8 llvm::MCDwarfLineTableHeader::Emit(llvm::MCStreamer*, llvm::MCDwarfLineTableParams, llvm::ArrayRef<char>, std::__1::optional<llvm::MCDwarfLineStr>&) const (/Users/michaelbuch/Git/lldb-build
-main-no-modules/bin/clang-17+0x103e55ff8)
#12 0x0000000108c3a178 llvm::MCDwarfLineTableHeader::Emit(llvm::MCStreamer*, llvm::MCDwarfLineTableParams, std::__1::optional<llvm::MCDwarfLineStr>&) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/c
lang-17+0x103e56178)
#13 0x0000000108c39920 llvm::MCDwarfLineTable::emitCU(llvm::MCStreamer*, llvm::MCDwarfLineTableParams, std::__1::optional<llvm::MCDwarfLineStr>&) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang
-17+0x103e55920)
#14 0x0000000108c397b4 llvm::MCDwarfLineTable::emit(llvm::MCStreamer*, llvm::MCDwarfLineTableParams) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e557b4)
#15 0x0000000108c80320 llvm::MCObjectStreamer::finishImpl() (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e9c320)
#16 0x0000000108c528e4 llvm::MCELFStreamer::finishImpl() (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e6e8e4)
#17 0x0000000108cb0b88 llvm::MCStreamer::finish(llvm::SMLoc) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103eccb88)
#18 0x0000000108d51fb4 (anonymous namespace)::AsmParser::Run(bool, bool) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103f6dfb4)
#19 0x0000000104e15b74 ExecuteAssemblerImpl((anonymous namespace)::AssemblerInvocation&, clang::DiagnosticsEngine&) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x100031b74)
#20 0x0000000104e117c8 ExecuteAssembler((anonymous namespace)::AssemblerInvocation&, clang::DiagnosticsEngine&) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x10002d7c8)
#21 0x0000000104e10190 cc1as_main(llvm::ArrayRef<char const*>, char const*, void*) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x10002c190)
#22 0x0000000104deb110 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x100007110)
#23 0x0000000104e008ac clang_main(int, char**, llvm::ToolContext const&)::$_0::operator()(llvm::SmallVectorImpl<char const*>&) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x10001c8ac)
#24 0x0000000104e00880 int llvm::function_ref<int (llvm::SmallVectorImpl<char const*>&)>::callback_fn<clang_main(int, char**, llvm::ToolContext const&)::$_0>(long, llvm::SmallVectorImpl<char const*>&) (/Users/mic
haelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x10001c880)
#25 0x000000010a6a9ac0 llvm::function_ref<int (llvm::SmallVectorImpl<char const*>&)>::operator()(llvm::SmallVectorImpl<char const*>&) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1058c5
ac0)
#26 0x000000010a6a9a80 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, bool*) const::
$_1::operator()() const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1058c5a80)
#27 0x000000010a6a9a4c void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char
>, std::__1::allocator<char>>*, bool*) const::$_1>(long) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1058c5a4c)
#28 0x0000000109324468 llvm::function_ref<void ()>::operator()() const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104540468)
#29 0x00000001093243ec llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1045403ec)
#30 0x000000010a6a595c clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, bool*) const (
/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1058c195c)
#31 0x000000010a63d890 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x10585
9890)
#32 0x000000010a63db1c clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*>>&, bool) const (/Users/michaelbuch/Git/lldb-b
uild-main-no-modules/bin/clang-17+0x105859b1c)
#33 0x000000010a65a0d8 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*>>&) (/Users/michaelbuch/Git/lldb-build-main-no
-modules/bin/clang-17+0x1058760d8)
#34 0x0000000104dea6dc clang_main(int, char**, llvm::ToolContext const&) (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1000066dc)
#35 0x0000000104e28918 main (/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x100044918)
#36 0x000000018da3bf28
clang: error: clang integrated assembler command failed with exit code 134 (use -v to see invocation)
clang version 17.0.0 (https://github.com/Michael137/llvm-project.git 398403259a5a42624d382517f05b21f106f591b8)
Target: x86_64-pc-linux
Thread model: posix
InstalledDir: /Users/michaelbuch/Git/lldb-build-main-no-modules/bin
clang: note: diagnostic msg: Error generating preprocessed source(s) - no preprocessable inputs.

--

********************
********************
Failed Tests (1):
  lldb-shell :: SymbolFile/DWARF/x86/DW_OP_div-with-signed.s


Testing Time: 129.47s
  Failed: 1

1 warning(s) in tests
jwnhy updated this revision to Diff 519733.May 4 2023, 8:30 PM

Update the patch to pass the assertion.
Sorry I forget to enable these on my machine.

Given that said, this assertion seems pretty weird to me.
Since this assembly is also generated by clang (with handcrafted DWARF,
this assertion means that any assembly generated from a single file will
be rejected?

Anyway, I have added a dummy ".file 1" directive to pass
this assertion.

jwnhy updated this revision to Diff 519735.May 4 2023, 8:47 PM

Update the patch for better naming.
The assertion is correct, I somehow delete
a file directive from the assembly generated
by clang.

It should work now...Thanks a lot for checking
this.

Thanks for fixing

Just checked and it works on my M1 with assertions.

I relanded the latest version.