date:20220626

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

2022-06-26 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

In D128268#3608197 , @labath wrote:

> In D128268#3604555 , @mstorsjo 
> wrote:
>
>> I found that this duality was introduced in 
>> 5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 
>>  and 
>> ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 / D4658 
>>  (CC @zturner), what do you make out of the 
>> reasonings in those commits?
>
> The first patch seems like it's just working around some mismatches in the 
> windows dynamic loader plugin. I think a better approach would be to have the 
> dynamic loader claim both architectures, though I don't think that is 
> necessary if we're just consistent about what we use. I don't see anything 
> wrong with the second patch (the darwin platform does something similar for 
> arm architectures, even though I'm not convinced it's necessary (the reason 
> it's necessary for darwin is because there they actually make a distinction 
> between armv6XX and armv7YY and treat those as different architectures).

The odd thing about the second one is the added hardcoded 
`AddArch(ArchSpec("i686-pc-windows"));` and 
`AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.

Anyway, it does seem to work fine in my quick test to get rid of this duality, 
so I'll post a patch that does that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128268/new/

https://reviews.llvm.org/D128268

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128617: [lldb] Stop passing both i386 and i686 in parallel as architectures on Windows

2022-06-26 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: labath, zturner, DavidSpickett.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

When an object file returns multiple architectures, it is treated
as a fat binary - which really isn't the case of i386 vs i686 where
the object file actually has one architecture.

This allows getting rid of hardcoded architecture triples in
PlatformWindows.

The parallel i386 and i686 architecture strings stem from
5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 
 and
ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 / D4658 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128617

Files:
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(env);
-specs.Append(module_spec);
 break;
   case MachineArmNt:
 spec.SetTriple("armv7-pc-windows");
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -137,8 +137,6 @@
   case PDB_Machine::x86:
 module_arch.SetTriple("i386-pc-windows");
 specs.Append(module_spec);
-module_arch.SetTriple("i686-pc-windows");
-specs.Append(module_spec);
 break;
   case PDB_Machine::ArmNT:
 module_arch.SetTriple("armv7-pc-windows");


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(

[Lldb-commits] [lldb] 96d1b4d - [lld] Don't use Optional::hasValue (NFC)

2022-06-26 Thread Kazu Hirata via lldb-commits


Author: Kazu Hirata
Date: 2022-06-26T19:29:40-07:00
New Revision: 96d1b4ddb2cc37b900692215f7598ff5970b0baa

URL: 
https://github.com/llvm/llvm-project/commit/96d1b4ddb2cc37b900692215f7598ff5970b0baa
DIFF: 
https://github.com/llvm/llvm-project/commit/96d1b4ddb2cc37b900692215f7598ff5970b0baa.diff

LOG: [lld] Don't use Optional::hasValue (NFC)

This patch replaces x.hasValue() with x where x is contextually
convertible to bool.

Added: 


Modified: 
lldb/include/lldb/Target/MemoryRegionInfo.h
lldb/source/API/SBMemoryRegionInfo.cpp
lldb/source/Breakpoint/BreakpointIDList.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Core/DataFileCache.cpp
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Target/UnixSignals.cpp
lldb/source/Utility/SelectHelper.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index acca66e838337..4e978d33b05dd 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -124,7 +124,7 @@ class MemoryRegionInfo {
   void SetPageSize(int pagesize) { m_pagesize = pagesize; }
 
   void SetDirtyPageList(std::vector pagelist) {
-if (m_dirty_pages.hasValue())
+if (m_dirty_pages)
   m_dirty_pages.getValue().clear();
 m_dirty_pages = std::move(pagelist);
   }

diff  --git a/lldb/source/API/SBMemoryRegionInfo.cpp 
b/lldb/source/API/SBMemoryRegionInfo.cpp
index d0f74374476e0..e811bf31c7223 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -135,7 +135,7 @@ uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
   uint32_t num_dirty_pages = 0;
   const llvm::Optional> &dirty_page_list =
   m_opaque_up->GetDirtyPageList();
-  if (dirty_page_list.hasValue())
+  if (dirty_page_list)
 num_dirty_pages = dirty_page_list.getValue().size();
 
   return num_dirty_pages;
@@ -147,7 +147,7 @@ addr_t 
SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {
   addr_t dirty_page_addr = LLDB_INVALID_ADDRESS;
   const llvm::Optional> &dirty_page_list =
   m_opaque_up->GetDirtyPageList();
-  if (dirty_page_list.hasValue() && idx < dirty_page_list.getValue().size())
+  if (dirty_page_list && idx < dirty_page_list.getValue().size())
 dirty_page_addr = dirty_page_list.getValue()[idx];
 
   return dirty_page_addr;

diff  --git a/lldb/source/Breakpoint/BreakpointIDList.cpp 
b/lldb/source/Breakpoint/BreakpointIDList.cpp
index b434056993640..dd16d3b6388c4 100644
--- a/lldb/source/Breakpoint/BreakpointIDList.cpp
+++ b/lldb/source/Breakpoint/BreakpointIDList.cpp
@@ -192,7 +192,7 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
 auto start_bp = BreakpointID::ParseCanonicalReference(range_from);
 auto end_bp = BreakpointID::ParseCanonicalReference(range_to);
 
-if (!start_bp.hasValue() ||
+if (!start_bp ||
 !target->GetBreakpointByID(start_bp->GetBreakpointID())) {
   new_args.Clear();
   result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n",
@@ -200,7 +200,7 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
   return;
 }
 
-if (!end_bp.hasValue() ||
+if (!end_bp ||
 !target->GetBreakpointByID(end_bp->GetBreakpointID())) {
   new_args.Clear();
   result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n",

diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index 4081e87f2ddb9..93c41ee5a0be3 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -137,14 +137,14 @@ class CommandObjectFrameDiagnose : public 
CommandObjectParsed {
 
 ValueObjectSP valobj_sp;
 
-if (m_options.address.hasValue()) {
-  if (m_options.reg.hasValue() || m_options.offset.hasValue()) {
+if (m_options.address) {
+  if (m_options.reg || m_options.offset) {
 result.AppendError(
 "`frame diagnose --address` is incompatible with other 
arguments.");
 return false;
   }
   valobj_sp = frame_sp->GuessValueForAddress(m_options.address.getValue());
-} else if (m_options.reg.hasValue()) {
+} else if (m_options.reg) {
   valobj_sp = frame_sp->GuessValueForRegisterAndOffset(
   m_options.reg.getValue(), m_options.offset.value_or(0));
 } else {

diff  --git a/lldb/source/Commands/CommandObjectMem

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

2022-06-26 Thread Pavel Kosov via Phabricator via lldb-commits

kpdev42 updated this revision to Diff 440109.
kpdev42 added a comment.
Herald added a subscriber: Michael137.

Address review notes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126359/new/

https://reviews.llvm.org/D126359

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/c/fpeval/Makefile
  lldb/test/API/lang/c/fpeval/TestFPEval.py
  lldb/test/API/lang/c/fpeval/main.c

Index: lldb/test/API/lang/c/fpeval/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/main.c
@@ -0,0 +1,6 @@
+int main (int argc, char const *argv[])
+{
+double a = 42.0;
+double b = 2.0;
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -0,0 +1,54 @@
+"""Tests IR interpreter handling of basic floating point operations (fadd, fsub, etc)."""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class FPEvalTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+"""Test floating point expressions while jitter is disabled."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '40'])
+self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '21'])
+self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '84'])
+self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a != b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+
Index: lldb/test/API/lang/c/fpeval/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -167,6 +167,18 @@
 const Constant *constant = dyn_cast(value);
 
 if (constant) {
+  if (constant->getValueID() == Value::ConstantFPVal) {
+if (auto *cfp = dyn_cast(constant)) {
+  if (cfp->getType()->isDoubleTy())
+scalar = cfp->getValueAPF().convertToDouble();
+  else if (cfp->getType()->isFloatTy())
+scalar = cfp->getValueAPF().convertToFloat();
+  else
+return false;
+  return true;
+}
+return false;
+  }
   APInt value_apint;
 
   if (!ResolveConstantValue(value_apint, constant))
@@ -189,9 +201,18 @@
 
 lldb::offset_t offset = 0;
 if (value_size <= 8) {
-  uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
-  return AssignToMatchType(scalar, llvm::APInt(64, u64value),
-   value->getType());
+  Type *ty = value->getType();
+  if (ty->isDoubleTy()) {
+scalar = value_extractor.GetDouble(&offset);
+return true;
+  } else if (ty->isFloatTy()) {
+scalar = value_extractor.GetFloat(&offset);
+return true;
+  } else {
+uint64_t u64value = val

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Lldb-commits] [PATCH] D128617: [lldb] Stop passing both i386 and i686 in parallel as architectures on Windows

[Lldb-commits] [lldb] 96d1b4d - [lld] Don't use Optional::hasValue (NFC)

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

4 matches

Site Navigation

Mail list logo

Footer information