[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-11-01 Thread Stefan Gränitz via lldb-commits 

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-11-01 Thread Stefan Gränitz via lldb-commits 


@@ -791,11 +791,10 @@ void ObjectFilePECOFF::AppendFromCOFFSymbolTable(
   for (const auto _ref : m_binary->symbols()) {
 const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref);
 auto name_or_error = sym_ref.getName();
-if (auto err = name_or_error.takeError()) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get "
-   "symbol table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+if (!name_or_error) {
+  LLDB_LOG_ERROR(log, name_or_error.takeError(),
+ "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to "

weliveindetail wrote:

As above

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-11-01 Thread Stefan Gränitz via lldb-commits 

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-11-01 Thread Stefan Gränitz via lldb-commits 


@@ -271,9 +271,9 @@ void ObjectFileCOFF::ParseSymtab(lldb_private::Symtab 
) {
 const auto COFFSymRef = m_object->getCOFFSymbol(SymRef);
 
 Expected NameOrErr = SymRef.getName();
-if (auto error = NameOrErr.takeError()) {
-  LLDB_LOG(log, "ObjectFileCOFF: failed to get symbol name: {0}",
-   llvm::fmt_consume(std::move(error)));
+if (!NameOrErr) {
+  LLDB_LOG_ERROR(log, NameOrErr.takeError(),
+ "ObjectFileCOFF: failed to get symbol name: {0}");

weliveindetail wrote:

Hey Ismail! Right, this seems unconventional compared to other logging code in 
LLDB. It's not something that changed when moving from `LLDB_LOG` to 
`LLDB_LOG_ERROR` though. We should keep printing the same info, but I agree 
that this is worth fixing while I am here.

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Med Ismail Bennani via lldb-commits 

https://github.com/medismailben requested changes to this pull request.


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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Med Ismail Bennani via lldb-commits 


@@ -791,11 +791,10 @@ void ObjectFilePECOFF::AppendFromCOFFSymbolTable(
   for (const auto _ref : m_binary->symbols()) {
 const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref);
 auto name_or_error = sym_ref.getName();
-if (auto err = name_or_error.takeError()) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get "
-   "symbol table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+if (!name_or_error) {
+  LLDB_LOG_ERROR(log, name_or_error.takeError(),
+ "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to "

medismailben wrote:

Same here

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Med Ismail Bennani via lldb-commits 


@@ -271,9 +271,9 @@ void ObjectFileCOFF::ParseSymtab(lldb_private::Symtab 
) {
 const auto COFFSymRef = m_object->getCOFFSymbol(SymRef);
 
 Expected NameOrErr = SymRef.getName();
-if (auto error = NameOrErr.takeError()) {
-  LLDB_LOG(log, "ObjectFileCOFF: failed to get symbol name: {0}",
-   llvm::fmt_consume(std::move(error)));
+if (!NameOrErr) {
+  LLDB_LOG_ERROR(log, NameOrErr.takeError(),
+ "ObjectFileCOFF: failed to get symbol name: {0}");

medismailben wrote:

I don't think you need to specify `ObjectFileCOFF:` anymore since 
`LLDB_LOG_ERROR` will take care of adding the file name and the function symbol 
for you.

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Med Ismail Bennani via lldb-commits 

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Jonas Devlieghere via lldb-commits 

https://github.com/JDevlieghere approved this pull request.

Thanks Stefan!

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord approved this pull request.


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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Saleem Abdulrasool via lldb-commits 

https://github.com/compnerd approved this pull request.


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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread David Spickett via lldb-commits 

https://github.com/DavidSpickett approved this pull request.

Assuming I understand correctly that previously, the error was only consumed 
when logging was enabled, this LGTM.

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Stefan Gränitz via lldb-commits 

weliveindetail wrote:

Sorry, I had a few unrelated patched in the pipe. I force-pushed to fix that.

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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread via lldb-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff c280bed85a8b9cd6ebf48f9f2923890edc7039f0 
5091d1ffec05ec65e4e762d2bea8735d0de7b6fb -- 
lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
llvm/include/llvm-c/Orc.h llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h 
llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h 
llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h 
llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h 
llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp 
llvm/lib/ExecutionEngine/JITLink/aarch32.cpp 
llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp
``





View the diff from clang-format here.


``diff
diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp 
b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index b31dd5c22..674220025 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -627,7 +627,7 @@ static Symbol (LinkGraph , Section 
,
   constexpr uint8_t AlignmentThumb = 2;
   constexpr orc::ExecutorAddr MaxAddrThumb{~uint32_t(AlignmentThumb - 1)};
   constexpr uint64_t PointerSize = 4;
-  constexpr char NullPointerContent[] { 0x00, 0x00, 0x00, 0x00 };
+  constexpr char NullPointerContent[]{0x00, 0x00, 0x00, 0x00};
   auto  = G.createContentBlock(PointerSection, NullPointerContent,
  MaxAddrThumb, AlignmentThumb, 0);
   if (InitialTarget)
diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp 
b/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp
index 2adf6df9b..4b7b3f9be 100644
--- a/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp
+++ b/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp
@@ -56,9 +56,9 @@ static Expected getELFStubTarget(LinkGraph , 
Block ) {
 return E.takeError();
   auto  = E->getTarget();
   if (!GOTSym.isDefined())
-return make_error(
-"Stubs entry in " + G.getName() + " does not point to GOT entry",
-inconvertibleErrorCode());
+return make_error("Stubs entry in " + G.getName() +
+   " does not point to GOT entry",
+   inconvertibleErrorCode());
   if (!isELFGOTSection(GOTSym.getBlock().getSection()))
 return make_error(
 "Stubs entry in " + G.getName() + ", \"" +

``




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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Stefan Gränitz (weliveindetail)


Changes

I actually ran into it with a downstream fork:
```
llvm::Error::fatalUncheckedError(), llvm-project\llvm\lib\Support\Error.cpp, 
line 117
ObjectFilePECOFF::AppendFromCOFFSymbolTable(), 
llvm-project\lldb\source\Plugins\ObjectFile\PECOFF\ObjectFilePECOFF.cpp, line 
806
ObjectFilePECOFF::ParseSymtab(), 
llvm-project\lldb\source\Plugins\ObjectFile\PECOFF\ObjectFilePECOFF.cpp, line 
777
```

If logging is disabled `LLDB_LOG_ERROR` calls `llvm::consumeError()`, which 
marks the error as checked. All `llvm::Error`s must be checked before 
destruction. This patch fixes one more such case in 
`ObjectFileCOFF::ParseSymtab()`.

---
Full diff: https://github.com/llvm/llvm-project/pull/70793.diff


11 Files Affected:

- (modified) lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp (+3-3) 
- (modified) lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp (+4-5) 


``diff
diff --git a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp 
b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
index 03c454bf3efab14..a7ad5d27b237f12 100644
--- a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
@@ -271,9 +271,9 @@ void ObjectFileCOFF::ParseSymtab(lldb_private::Symtab 
) {
 const auto COFFSymRef = m_object->getCOFFSymbol(SymRef);
 
 Expected NameOrErr = SymRef.getName();
-if (auto error = NameOrErr.takeError()) {
-  LLDB_LOG(log, "ObjectFileCOFF: failed to get symbol name: {0}",
-   llvm::fmt_consume(std::move(error)));
+if (!NameOrErr) {
+  LLDB_LOG_ERROR(log, NameOrErr.takeError(),
+ "ObjectFileCOFF: failed to get symbol name: {0}");
   continue;
 }
 
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 7fb10a69391c566..be0020cad5bee8e 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -791,11 +791,10 @@ void ObjectFilePECOFF::AppendFromCOFFSymbolTable(
   for (const auto _ref : m_binary->symbols()) {
 const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref);
 auto name_or_error = sym_ref.getName();
-if (auto err = name_or_error.takeError()) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get "
-   "symbol table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+if (!name_or_error) {
+  LLDB_LOG_ERROR(log, name_or_error.takeError(),
+ "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to "
+ "get symbol table entry name: {0}");
   continue;
 }
 const llvm::StringRef sym_name = *name_or_error;

``




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

[Lldb-commits] [lldb] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF (PR #70793)

2023-10-31 Thread Stefan Gränitz via lldb-commits 

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/70793

From a4fecdd4bd9b12b1151d62892bcad7bab6833ce0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Tue, 31 Oct 2023 13:12:21 +0100
Subject: [PATCH] [lldb] Fix missing comsumeError() with LLDB_LOG in
 ObjectFileCOFF/PECOFF

---
 lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp   | 6 +++---
 .../Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp   | 9 -
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp 
b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
index 03c454bf3efab14..a7ad5d27b237f12 100644
--- a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp
@@ -271,9 +271,9 @@ void ObjectFileCOFF::ParseSymtab(lldb_private::Symtab 
) {
 const auto COFFSymRef = m_object->getCOFFSymbol(SymRef);
 
 Expected NameOrErr = SymRef.getName();
-if (auto error = NameOrErr.takeError()) {
-  LLDB_LOG(log, "ObjectFileCOFF: failed to get symbol name: {0}",
-   llvm::fmt_consume(std::move(error)));
+if (!NameOrErr) {
+  LLDB_LOG_ERROR(log, NameOrErr.takeError(),
+ "ObjectFileCOFF: failed to get symbol name: {0}");
   continue;
 }
 
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 7fb10a69391c566..be0020cad5bee8e 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -791,11 +791,10 @@ void ObjectFilePECOFF::AppendFromCOFFSymbolTable(
   for (const auto _ref : m_binary->symbols()) {
 const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref);
 auto name_or_error = sym_ref.getName();
-if (auto err = name_or_error.takeError()) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get "
-   "symbol table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+if (!name_or_error) {
+  LLDB_LOG_ERROR(log, name_or_error.takeError(),
+ "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to "
+ "get symbol table entry name: {0}");
   continue;
 }
 const llvm::StringRef sym_name = *name_or_error;

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