[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-12 Thread Kamlesh Kumar via lldb-commits

https://github.com/kamleshbhalui created 
https://github.com/llvm/llvm-project/pull/81451

Storing Larger bitwidth causes problem when byteswapping.

>From 51347e2de532261cfe980b299baeceb7747b7d48 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Mon, 12 Feb 2024 13:30:32 +0530
Subject: [PATCH] [lldb] Store proper integer bitwidth in Scalar Type

Storing Larger bitwidth causes problem when byteswapping.
---
 lldb/source/Expression/DWARFExpression.cpp | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index fe4928d4f43a43..1546612be34ac7 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -857,10 +857,18 @@ static Scalar DerefSizeExtractDataHelper(uint8_t 
*addr_bytes,
   DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
 
   lldb::offset_t addr_data_offset = 0;
-  if (size <= 8)
-return addr_data.GetMaxU64(&addr_data_offset, size);
-  else
-return addr_data.GetAddress(&addr_data_offset);
+  switch (size) {
+  case 1:
+return addr_data.GetU8(&addr_data_offset);
+  case 2:
+return addr_data.GetU16(&addr_data_offset);
+  case 4:
+return addr_data.GetU32(&addr_data_offset);
+  case 8:
+return addr_data.GetU64(&addr_data_offset);
+  default:
+ return addr_data.GetAddress(&addr_data_offset);
+  }
 }
 
 bool DWARFExpression::Evaluate(

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


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-12 Thread Kamlesh Kumar via lldb-commits

https://github.com/kamleshbhalui updated 
https://github.com/llvm/llvm-project/pull/81451

>From 51347e2de532261cfe980b299baeceb7747b7d48 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Mon, 12 Feb 2024 13:30:32 +0530
Subject: [PATCH 1/2] [lldb] Store proper integer bitwidth in Scalar Type

Storing Larger bitwidth causes problem when byteswapping.
---
 lldb/source/Expression/DWARFExpression.cpp | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index fe4928d4f43a43..1546612be34ac7 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -857,10 +857,18 @@ static Scalar DerefSizeExtractDataHelper(uint8_t 
*addr_bytes,
   DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
 
   lldb::offset_t addr_data_offset = 0;
-  if (size <= 8)
-return addr_data.GetMaxU64(&addr_data_offset, size);
-  else
-return addr_data.GetAddress(&addr_data_offset);
+  switch (size) {
+  case 1:
+return addr_data.GetU8(&addr_data_offset);
+  case 2:
+return addr_data.GetU16(&addr_data_offset);
+  case 4:
+return addr_data.GetU32(&addr_data_offset);
+  case 8:
+return addr_data.GetU64(&addr_data_offset);
+  default:
+ return addr_data.GetAddress(&addr_data_offset);
+  }
 }
 
 bool DWARFExpression::Evaluate(

>From a04e9ea70fd29cd5fb0599c0e2f106c17656da57 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Mon, 12 Feb 2024 14:21:44 +0530
Subject: [PATCH 2/2] clang-formatted

---
 lldb/source/Expression/DWARFExpression.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 1546612be34ac7..6839e05bb115bd 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -867,7 +867,7 @@ static Scalar DerefSizeExtractDataHelper(uint8_t 
*addr_bytes,
   case 8:
 return addr_data.GetU64(&addr_data_offset);
   default:
- return addr_data.GetAddress(&addr_data_offset);
+return addr_data.GetAddress(&addr_data_offset);
   }
 }
 

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


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-12 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

There is not a test scenario in lldb itself.


https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-12 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

> This uses `DataExtractor::GetMaxU64` which already does this under the hood. 
> What does this do that isn't already being done? It may help if you add a 
> test case to show what you are trying to fix.

The problem with GetMaxU64 is that it always returns uint64_t even though 
actual type was uint32_t, so when byteswap is performed it becomes invalid 
integer, to fixed this we need to store correct bitwidth not higher.
i.e.
Suppose there is actual 32 bit integer i.e. 0x
`GetMaxU64` will return 0x (prmoted to uint64_t from uint32_t)
 and when performing byteswap on this it becomes 0x which is 
invalid.


https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

@clayborg @bulbazord 
we have extension in lldb to support cobol code debugging, we require 
byteswapping there. upstream version of lldb does not do byteswapping on scalar 
so no problem seen.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

I think this PR https://reviews.llvm.org/D121408 introduced the current code, 
which causing the problem.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

> > > This uses `DataExtractor::GetMaxU64` which already does this under the 
> > > hood. What does this do that isn't already being done? It may help if you 
> > > add a test case to show what you are trying to fix.
> > 
> > 
> > @clayborg @bulbazord The problem with GetMaxU64 is that it always returns 
> > uint64_t even though actual type was uint32_t, so when byteswap is 
> > performed it becomes invalid integer, to fixed this we need to store 
> > correct bitwidth not higher. i.e. Suppose there is actual 32 bit integer 
> > i.e. 0x `GetMaxU64` will return 0x (prmoted to 
> > uint64_t from uint32_t) and when performing byteswap on this it becomes 
> > 0x which is invalid.
> 
> So you are saying someone is taking the resulting Scalar and trying to 
> byteswap it? Errors can still happen then in this class for int8_t/uint8_t 
> and int16_t/uint16_t as there are no overloads for these in the `Scalar` 
> class. We only currently have:
> 
> ```
>   Scalar(int v);
>   Scalar(unsigned int v);
>   Scalar(long v);
>   Scalar(unsigned long v);
>   Scalar(long long v);
>   Scalar(unsigned long long v);
> ```
> 
> It would suggest if we are going to make a requirement that Scalar will hold 
> the correct bit widths, then we need to stop using "int" and "long" and 
> switch to using the `int*_t` and `uint*_t` typedefs to make things clear:
> 
> ```
>   Scalar(int8_t v);
>   Scalar(int16_t v);
>   Scalar(int32_t v);
>   Scalar(int64_t v);
>   Scalar(uint8_t v);
>   Scalar(uint16_t v);
>   Scalar(uint32_t v);
>   Scalar(uint64_t v);
> ```
> 
> Then we have all of the bit widths handled correctly.

@clayborg I agree, we should use suggested type to fix it for all.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

> > @clayborg @bulbazord we have extension in lldb to support cobol code 
> > debugging, we require byteswapping there. upstream version of lldb does not 
> > do byteswapping on scalar so no problem seen.
> 
> Maybe not, but this should still be testable with a unit test. If somebody 
> modifies or refactors this code in the future without a test, this might 
> break in a similar way and you would have to do all this work again. :)

Will try to write a unit test for same.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits


@@ -857,10 +857,18 @@ static Scalar DerefSizeExtractDataHelper(uint8_t 
*addr_bytes,
   DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
 
   lldb::offset_t addr_data_offset = 0;
-  if (size <= 8)
-return addr_data.GetMaxU64(&addr_data_offset, size);
-  else
+  switch (size) {
+  case 1:
+return addr_data.GetU8(&addr_data_offset);
+  case 2:
+return addr_data.GetU16(&addr_data_offset);
+  case 4:
+return addr_data.GetU32(&addr_data_offset);
+  case 8:
+return addr_data.GetU64(&addr_data_offset);
+  default:
 return addr_data.GetAddress(&addr_data_offset);
+  }

kamleshbhalui wrote:

I will write a unit test.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

2024-02-14 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

> I am fine with this patch if we fix the Scalar class to obey the bit width as 
> suggested above. I do worry though that other Scalar values in the expression 
> might get set to 64 bit values all of the time and cause problems with DWARF 
> expressions. Do you have a code snippet of where this is used where you rely 
> on the bit width? Usually `DW_OP_*` opcodes act on a pointer width integer, 
> but some do actually specify a width (some of the deref operations).

Yes here it is 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/APInt.cpp#L715.

https://github.com/llvm/llvm-project/pull/81451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 (PR #110822)

2024-10-02 Thread Kamlesh Kumar via lldb-commits

https://github.com/kamleshbhalui created 
https://github.com/llvm/llvm-project/pull/110822

On AArch64, TLS variables do not have DT_Location entries generated in the 
debug information due to the lack of dtpoff relocation support, unlike on 
x86_64. LLDB relies on this location info to calculate the TLS address, leading 
to issues when debugging TLS variables on AArch64.
However, GDB can successfully calculate the TLS address without relying on this 
debug info, by using the symbol’s address and manually calculating the offset. 
We adopt a similar approach for LLDB.

Fixes #71666

>From 899c26d1a70dd332c3a0d5cd1887fac205752888 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Thu, 19 Sep 2024 18:57:14 +0200
Subject: [PATCH] [LLDB] Enable TLS Variable Debugging Without Location Info on
 AArch64
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On AArch64, TLS variables do not have DT_Location entries generated in the
debug information due to the lack of dtpoff relocation support, unlike on 
x86_64.
LLDB relies on this location info to calculate the TLS address, leading to 
issues
when debugging TLS variables on AArch64.
However, GDB can successfully calculate the TLS address without relying on this 
debug info,
by using the symbol’s address and manually calculating the offset. We adopt a 
similar approach for LLDB.
---
 lldb/include/lldb/Symbol/Variable.h   |  2 ++
 lldb/source/Core/ValueObjectVariable.cpp  | 33 ++-
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 14 +---
 .../Utility/RegisterInfoPOSIX_arm64.cpp   | 13 
 .../Process/Utility/RegisterInfos_arm64.h |  5 +--
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp |  5 ++-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  8 +
 lldb/source/Symbol/Variable.cpp   | 11 +++
 8 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/lldb/include/lldb/Symbol/Variable.h 
b/lldb/include/lldb/Symbol/Variable.h
index c437624d1ea6d7..4ad06baeb684a4 100644
--- a/lldb/include/lldb/Symbol/Variable.h
+++ b/lldb/include/lldb/Symbol/Variable.h
@@ -79,6 +79,8 @@ class Variable : public UserID, public 
std::enable_shared_from_this {
 return m_location_list;
   }
 
+  bool IsThreadLocal() const;
+
   // When given invalid address, it dumps all locations. Otherwise it only 
dumps
   // the location that contains this address.
   bool DumpLocations(Stream *s, const Address &address);
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 29aefb270c92c8..393ddc1960ab47 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -254,7 +254,38 @@ bool ValueObjectVariable::UpdateValue() {
   m_resolved_value.SetContext(Value::ContextType::Invalid, nullptr);
 }
   }
-
+  if (m_error.Fail() && variable->IsThreadLocal()) {
+ExecutionContext exe_ctx(GetExecutionContextRef());
+Thread *thread = exe_ctx.GetThreadPtr();
+lldb::ModuleSP module_sp = GetModule();
+if (!thread) {
+  m_error = Status::FromErrorString("no thread to evaluate TLS within");
+  return m_error.Success();
+}
+std::vector symbol_indexes;
+module_sp->GetSymtab()->FindAllSymbolsWithNameAndType(
+ConstString(variable->GetName()), lldb::SymbolType::eSymbolTypeAny,
+symbol_indexes);
+Symbol *symbol = module_sp->GetSymtab()->SymbolAtIndex(symbol_indexes[0]);
+lldb::addr_t tls_file_addr =
+symbol->GetAddress().GetOffset() +
+symbol->GetAddress().GetSection()->GetFileAddress();
+const lldb::addr_t tls_load_addr =
+thread->GetThreadLocalData(module_sp, tls_file_addr);
+if (tls_load_addr == LLDB_INVALID_ADDRESS)
+  m_error = Status::FromErrorString(
+  "no TLS data currently exists for this thread");
+else {
+  Value old_value(m_value);
+  m_value.GetScalar() = tls_load_addr;
+  m_value.SetContext(Value::ContextType::Variable, variable);
+  m_value.SetValueType(Value::ValueType::LoadAddress);
+  m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
+  SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() ||
+m_value.GetScalar() != old_value.GetScalar());
+  SetValueIsValid(m_error.Success());
+}
+  }
   return m_error.Success();
 }
 
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 51e4b3e6728f23..5a78ad2286f87a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -771,9 +771,12 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const 
lldb::ModuleSP module_sp,
 "GetThreadLocalData info: link_map=0x%" PRIx64
 ", thread info metadata: "
 "modid_offset=0x%" 

[Lldb-commits] [lldb] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 (PR #110822)

2024-10-02 Thread Kamlesh Kumar via lldb-commits

https://github.com/kamleshbhalui updated 
https://github.com/llvm/llvm-project/pull/110822

>From f36d8a2346be90b6bf5d68a7cec0bcf2d41cbd99 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Thu, 19 Sep 2024 18:57:14 +0200
Subject: [PATCH] [LLDB] Enable TLS Variable Debugging Without Location Info on
 AArch64
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On AArch64, TLS variables do not have DT_Location entries generated in the
debug information due to the lack of dtpoff relocation support, unlike on 
x86_64.
LLDB relies on this location info to calculate the TLS address, leading to 
issues
when debugging TLS variables on AArch64.
However, GDB can successfully calculate the TLS address without relying on this 
debug info,
by using the symbol’s address and manually calculating the offset. We adopt a 
similar approach for LLDB.
---
 lldb/include/lldb/Symbol/Variable.h   |  2 ++
 lldb/source/Core/ValueObjectVariable.cpp  | 33 ++-
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 14 +---
 .../Utility/RegisterInfoPOSIX_arm64.cpp   | 15 +
 .../Process/Utility/RegisterInfos_arm64.h | 11 +--
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp |  7 ++--
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  9 +
 lldb/source/Symbol/Variable.cpp   | 13 
 8 files changed, 85 insertions(+), 19 deletions(-)

diff --git a/lldb/include/lldb/Symbol/Variable.h 
b/lldb/include/lldb/Symbol/Variable.h
index c437624d1ea6d7..4ad06baeb684a4 100644
--- a/lldb/include/lldb/Symbol/Variable.h
+++ b/lldb/include/lldb/Symbol/Variable.h
@@ -79,6 +79,8 @@ class Variable : public UserID, public 
std::enable_shared_from_this {
 return m_location_list;
   }
 
+  bool IsThreadLocal() const;
+
   // When given invalid address, it dumps all locations. Otherwise it only 
dumps
   // the location that contains this address.
   bool DumpLocations(Stream *s, const Address &address);
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 29aefb270c92c8..393ddc1960ab47 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -254,7 +254,38 @@ bool ValueObjectVariable::UpdateValue() {
   m_resolved_value.SetContext(Value::ContextType::Invalid, nullptr);
 }
   }
-
+  if (m_error.Fail() && variable->IsThreadLocal()) {
+ExecutionContext exe_ctx(GetExecutionContextRef());
+Thread *thread = exe_ctx.GetThreadPtr();
+lldb::ModuleSP module_sp = GetModule();
+if (!thread) {
+  m_error = Status::FromErrorString("no thread to evaluate TLS within");
+  return m_error.Success();
+}
+std::vector symbol_indexes;
+module_sp->GetSymtab()->FindAllSymbolsWithNameAndType(
+ConstString(variable->GetName()), lldb::SymbolType::eSymbolTypeAny,
+symbol_indexes);
+Symbol *symbol = module_sp->GetSymtab()->SymbolAtIndex(symbol_indexes[0]);
+lldb::addr_t tls_file_addr =
+symbol->GetAddress().GetOffset() +
+symbol->GetAddress().GetSection()->GetFileAddress();
+const lldb::addr_t tls_load_addr =
+thread->GetThreadLocalData(module_sp, tls_file_addr);
+if (tls_load_addr == LLDB_INVALID_ADDRESS)
+  m_error = Status::FromErrorString(
+  "no TLS data currently exists for this thread");
+else {
+  Value old_value(m_value);
+  m_value.GetScalar() = tls_load_addr;
+  m_value.SetContext(Value::ContextType::Variable, variable);
+  m_value.SetValueType(Value::ValueType::LoadAddress);
+  m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
+  SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() ||
+m_value.GetScalar() != old_value.GetScalar());
+  SetValueIsValid(m_error.Success());
+}
+  }
   return m_error.Success();
 }
 
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 51e4b3e6728f23..5a78ad2286f87a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -771,9 +771,12 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const 
lldb::ModuleSP module_sp,
 "GetThreadLocalData info: link_map=0x%" PRIx64
 ", thread info metadata: "
 "modid_offset=0x%" PRIx32 ", dtv_offset=0x%" PRIx32
-", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32 "\n",
+", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32
+", tls_file_addr=0x%" PRIx64 ", module name=%s "
+"\n",
 link_map, metadata.modid_offset, metadata.dtv_offset,
-metadata.tls_offset, metadata.dtv_slot_size);
+metadata.tls_offset, metadata.dtv_slot_size, tls_file_addr,
+module_sp->GetFileSpec().G

[Lldb-commits] [lldb] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 (PR #110822)

2024-10-02 Thread Kamlesh Kumar via lldb-commits

https://github.com/kamleshbhalui updated 
https://github.com/llvm/llvm-project/pull/110822

>From ba3071566ac697e750606c7fc941e5f6fc738367 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar 
Date: Thu, 19 Sep 2024 18:57:14 +0200
Subject: [PATCH] [LLDB] Enable TLS Variable Debugging Without Location Info on
 AArch64
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On AArch64, TLS variables do not have DT_Location entries generated in the
debug information due to the lack of dtpoff relocation support, unlike on 
x86_64.
LLDB relies on this location info to calculate the TLS address, leading to 
issues
when debugging TLS variables on AArch64.
However, GDB can successfully calculate the TLS address without relying on this 
debug info,
by using the symbol’s address and manually calculating the offset. We adopt a 
similar approach for LLDB.
---
 lldb/include/lldb/Symbol/Variable.h   |  2 ++
 lldb/source/Core/ValueObjectVariable.cpp  | 33 ++-
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 14 +---
 .../Utility/RegisterInfoPOSIX_arm64.cpp   | 15 +
 .../Process/Utility/RegisterInfos_arm64.h | 11 +--
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp |  7 ++--
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  9 +
 lldb/source/Symbol/Variable.cpp   | 13 
 8 files changed, 85 insertions(+), 19 deletions(-)

diff --git a/lldb/include/lldb/Symbol/Variable.h 
b/lldb/include/lldb/Symbol/Variable.h
index c437624d1ea6d7..4ad06baeb684a4 100644
--- a/lldb/include/lldb/Symbol/Variable.h
+++ b/lldb/include/lldb/Symbol/Variable.h
@@ -79,6 +79,8 @@ class Variable : public UserID, public 
std::enable_shared_from_this {
 return m_location_list;
   }
 
+  bool IsThreadLocal() const;
+
   // When given invalid address, it dumps all locations. Otherwise it only 
dumps
   // the location that contains this address.
   bool DumpLocations(Stream *s, const Address &address);
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index 29aefb270c92c8..393ddc1960ab47 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -254,7 +254,38 @@ bool ValueObjectVariable::UpdateValue() {
   m_resolved_value.SetContext(Value::ContextType::Invalid, nullptr);
 }
   }
-
+  if (m_error.Fail() && variable->IsThreadLocal()) {
+ExecutionContext exe_ctx(GetExecutionContextRef());
+Thread *thread = exe_ctx.GetThreadPtr();
+lldb::ModuleSP module_sp = GetModule();
+if (!thread) {
+  m_error = Status::FromErrorString("no thread to evaluate TLS within");
+  return m_error.Success();
+}
+std::vector symbol_indexes;
+module_sp->GetSymtab()->FindAllSymbolsWithNameAndType(
+ConstString(variable->GetName()), lldb::SymbolType::eSymbolTypeAny,
+symbol_indexes);
+Symbol *symbol = module_sp->GetSymtab()->SymbolAtIndex(symbol_indexes[0]);
+lldb::addr_t tls_file_addr =
+symbol->GetAddress().GetOffset() +
+symbol->GetAddress().GetSection()->GetFileAddress();
+const lldb::addr_t tls_load_addr =
+thread->GetThreadLocalData(module_sp, tls_file_addr);
+if (tls_load_addr == LLDB_INVALID_ADDRESS)
+  m_error = Status::FromErrorString(
+  "no TLS data currently exists for this thread");
+else {
+  Value old_value(m_value);
+  m_value.GetScalar() = tls_load_addr;
+  m_value.SetContext(Value::ContextType::Variable, variable);
+  m_value.SetValueType(Value::ValueType::LoadAddress);
+  m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
+  SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() ||
+m_value.GetScalar() != old_value.GetScalar());
+  SetValueIsValid(m_error.Success());
+}
+  }
   return m_error.Success();
 }
 
diff --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 51e4b3e6728f23..5a78ad2286f87a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -771,9 +771,12 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const 
lldb::ModuleSP module_sp,
 "GetThreadLocalData info: link_map=0x%" PRIx64
 ", thread info metadata: "
 "modid_offset=0x%" PRIx32 ", dtv_offset=0x%" PRIx32
-", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32 "\n",
+", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32
+", tls_file_addr=0x%" PRIx64 ", module name=%s "
+"\n",
 link_map, metadata.modid_offset, metadata.dtv_offset,
-metadata.tls_offset, metadata.dtv_slot_size);
+metadata.tls_offset, metadata.dtv_slot_size, tls_file_addr,
+module_sp->GetFileSpec().G

[Lldb-commits] [lldb] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 (PR #110822)

2024-10-03 Thread Kamlesh Kumar via lldb-commits

kamleshbhalui wrote:

> The implementation of Variable::IsThreadLocal is most likely a non-starter, 
> as it introduces a relatively expensive operation on the common variable 
> update path for all variables. Doing this (once) at variable creation would 
> be better, but I still have very big reservations about this approach. 
> AFAICT, there's no way to guarantee that the variable you find this way will 
> be the one that is actually described by the DWARF. You could even have more 
> than one variable with the same name if they're `static thread_local`.
> 
> This really sounds like something where the compiler should give us more to 
> go on -- instead of us trying to divine this information out of thin air.

I could pass that info to variable creation to simplify the check on update 
path and about second question even gdb could not debug  static thread_local.

https://github.com/llvm/llvm-project/pull/110822
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits