[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

In D134518#3811218 , @labath wrote:

> Ok, so is there any harm in keeping them this way?
>
> The symbols may not be very useful, but I would not say that they are 
> useless. If, for whatever reason, the user finds himself inspecting the part 
> of the memory covered by the forwarder string, then with this symbol, that 
> memory would symbolicate as `forwarded_symbol+X`, which seems nice.

I guess you're right, we can keep these symbols. Though do you think it make 
sense to synthesize a demangled name for the symbol, let's say `ExportName 
(forwarded to kernel32.LoadLibrary)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D134518#3811206 , @alvinhochun 
wrote:

> In D134518#3811160 , @labath wrote:
>
>> Where is the "dll description string" that they point to? Could they be made 
>> to point to that?
>
> The current symbol address is already pointing to it.

Ok, so is there any harm in keeping them this way?

The symbols may not be very useful, but I would not say that they are useless. 
If, for whatever reason, the user finds himself inspecting the part of the 
memory covered by the forwarder string, then with this symbol, that memory 
would symbolicate as `forwarded_symbol+X`, which seems nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

In D134518#3811155 , @mstorsjo wrote:

> In D134518#3811153 , @labath wrote:
>
>> They may not be useful (at the moment), but if they're not actively causing 
>> harm (e.g. stopping some other feature from functioning, or if we're badly 
>> misrepresenting them in the symtab output), then I think we should keep 
>> them. You never know -- maybe someone will find a use for them.
>
> Hm, maybe, but they're just plain names even without an associated address? 
> @alvinhochun If we keep them, do we need to adjust the code below to handle 
> the fact that they're addressless?

Hmm I'm not sure how. They do have an address pointing to the forwarder string, 
but they are useless as it is now.

Mind that the DLL containing the forwarder export does not actually export the 
symbol. When another EXE or DLL imports the forwarder symbol, Windows sees that 
it is a forwarder and loads the forwarded target instead. Unless a real 
exported function is imported from this DLL, it isn't even loaded into the 
process (at least that's what I gather from Process Explorer's module search 
with "api-ms" -- DLLs using this as a prefix contains only forwarder symbols to 
ucrtbase.dll or other system DLLs).

If anyone wants to list these symbols they can always do it with 
`llvm-objdump`. (`llvm-readobj` isn't handling this right now but I will make a 
patch.)

In D134518#3811160 , @labath wrote:

> Where is the "dll description string" that they point to? Could they be made 
> to point to that?

The current symbol address is already pointing to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Where is the "dll description string" that they point to? Could they be made to 
point to that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D134518#3811153 , @labath wrote:

> They may not be useful (at the moment), but if they're not actively causing 
> harm (e.g. stopping some other feature from functioning, or if we're badly 
> misrepresenting them in the symtab output), then I think we should keep them. 
> You never know -- maybe someone will find a use for them.

Hm, maybe, but they're just plain symbols even without an associated address? 
@alvinhochun If we keep them, do we need to adjust the code below to handle the 
fact that they're addressless?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

They may not be useful (at the moment), but if they're not actively causing 
harm (e.g. stopping some other feature from functioning, or if we're badly 
misrepresenting them in the symtab output), then I think we should keep them. 
You never know -- maybe someone will find a use for them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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


[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun created this revision.
alvinhochun added reviewers: labath, DavidSpickett, mstorsjo.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Forwarder exports do not point to a real function or variable. Instead
they point to a string describing which DLL and symbol to forward to.
Any imports which uses them will be redirected by the loader
transparently, therefore it is not necessary for LLDB to know about them
when debugging.

Depends on D134426 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134518

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -868,6 +868,18 @@
 llvm::cantFail(entry.getOrdinal(ordinal));
 symbol.SetID(ordinal);
 
+bool is_forwarder;
+llvm::cantFail(entry.isForwarder(is_forwarder));
+if (is_forwarder) {
+  // Forwarder exports are redirected by the loader transparently, so LLDB
+  // has no use for them.
+  LLDB_LOG(log,
+   "ObjectFilePECOFF::AppendFromExportTable - skipping forwarder "
+   "symbol '{0}'",
+   sym_name);
+  continue;
+}
+
 uint32_t function_rva;
 if (auto err = entry.getExportRVA(function_rva)) {
   LLDB_LOG(log,


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -868,6 +868,18 @@
 llvm::cantFail(entry.getOrdinal(ordinal));
 symbol.SetID(ordinal);
 
+bool is_forwarder;
+llvm::cantFail(entry.isForwarder(is_forwarder));
+if (is_forwarder) {
+  // Forwarder exports are redirected by the loader transparently, so LLDB
+  // has no use for them.
+  LLDB_LOG(log,
+   "ObjectFilePECOFF::AppendFromExportTable - skipping forwarder "
+   "symbol '{0}'",
+   sym_name);
+  continue;
+}
+
 uint32_t function_rva;
 if (auto err = entry.getExportRVA(function_rva)) {
   LLDB_LOG(log,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits