[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-16 Thread Martin Storsjö via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318446: Remove a FIXME about truncated section names 
(authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D39918?vs=122498&id=123223#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39918

Files:
  libunwind/trunk/src/AddressSpace.hpp


Index: libunwind/trunk/src/AddressSpace.hpp
===
--- libunwind/trunk/src/AddressSpace.hpp
+++ libunwind/trunk/src/AddressSpace.hpp
@@ -382,8 +382,6 @@
   found_obj = true;
   } else if (!strncmp((const char *)pish->Name, ".eh_frame",
   IMAGE_SIZEOF_SHORT_NAME)) {
-// FIXME: This section name actually is truncated, ideally we
-// should locate and check the full long name instead.
 info.dwarf_section = begin;
 info.dwarf_section_length = pish->Misc.VirtualSize;
 found_hdr = true;


Index: libunwind/trunk/src/AddressSpace.hpp
===
--- libunwind/trunk/src/AddressSpace.hpp
+++ libunwind/trunk/src/AddressSpace.hpp
@@ -382,8 +382,6 @@
   found_obj = true;
   } else if (!strncmp((const char *)pish->Name, ".eh_frame",
   IMAGE_SIZEOF_SHORT_NAME)) {
-// FIXME: This section name actually is truncated, ideally we
-// should locate and check the full long name instead.
 info.dwarf_section = begin;
 info.dwarf_section_length = pish->Misc.VirtualSize;
 found_hdr = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-16 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Yep, no need for this.


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-16 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

@rnk I guess this is ok now as https://reviews.llvm.org/D40025 is committed and 
done?


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-13 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In https://reviews.llvm.org/D39918#923070, @ruiu wrote:

> Actually I don't have a strong opinion on that topic. It seems like just 
> truncating the section name to ".eh_fram" at the linker is good enough, but 
> how much important is the compatibility with GNU ld?


It's hard to say... Right now, the llvm/lld based mingw distribution is 
incompatible with GNU ld already anyway for other reasons, so doing a fix in 
lld isn't too bad, but ideally I'd like to work towards having things as 
compatible as possible of course. Doesn't feel like a very high priority right 
now though since there's quite enough with more concrete other issues to fix 
first.


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-13 Thread Rui Ueyama via Phabricator via cfe-commits 
ruiu added a comment.

Actually I don't have a strong opinion on that topic. It seems like just 
truncating the section name to ".eh_fram" at the linker is good enough, but how 
much important is the compatibility with GNU ld?


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-13 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In https://reviews.llvm.org/D39918#923059, @ruiu wrote:

> I think this is the right thing to do, but I'd defer it to libunwind's owner 
> to approve the patch.


Removing the comment that is? Yes, since it's probably impossible to implement.

Do you have any suggestion on how to handle the rest of the issue (when the 
actual name is unavailable at runtime and you only have e.g. "/4" as section 
name) for binaries built with debug info enabled?


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-13 Thread Rui Ueyama via Phabricator via cfe-commits 
ruiu added a comment.

I think this is the right thing to do, but I'd defer it to libunwind's owner to 
approve the patch.


https://reviews.llvm.org/D39918



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

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

2017-11-10 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo created this revision.
Herald added a subscriber: aprantl.

If the linker chose to store the full section name instead of truncating it, 
this field doesn't contain a truncated name, but an offset into the string 
table of the binary. The string table isn't loaded/mapped into memory during 
runtime though, so it's not possible to read the full section name, unless we 
try to locate the DLL/EXE on disk that the HMODULE corresponds to and load that 
manually.

This also has the practical consequence that with the current LLD, we must not 
enable debug info (i.e. must link with -s with the mingw frontend). If debug 
info is enabled, the full ".eh_frame" name is written into the string table and 
the the section name itself is just an offset, e.g. "/4".

Alternatives to fixing this consequence are:

- Making LLD always output a truncated ".eh_fram" (maybe only when -lldmingw is 
specified?), even if debug info is enabled. (GNU ld doesn't do this, so this 
approach in libunwind won't work with binaries built with that.)
- Switch to statically registering the .eh_frame sections on startup, instead 
of dynamically enumerating them when needed. This is what libgcc does. This 
doesn't match what libunwind does on other platforms though. This only works as 
long as libunwind is linked dynamically so that all involved DLLs register 
their sections to the same instance of libunwind. (I haven't succeeded in 
building a shared libcxx/libcxxabi/libunwind yet for mingw though.)


https://reviews.llvm.org/D39918

Files:
  src/AddressSpace.hpp


Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -382,8 +382,6 @@
   found_obj = true;
   } else if (!strncmp((const char *)pish->Name, ".eh_frame",
   IMAGE_SIZEOF_SHORT_NAME)) {
-// FIXME: This section name actually is truncated, ideally we
-// should locate and check the full long name instead.
 info.dwarf_section = begin;
 info.dwarf_section_length = pish->Misc.VirtualSize;
 found_hdr = true;


Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -382,8 +382,6 @@
   found_obj = true;
   } else if (!strncmp((const char *)pish->Name, ".eh_frame",
   IMAGE_SIZEOF_SHORT_NAME)) {
-// FIXME: This section name actually is truncated, ideally we
-// should locate and check the full long name instead.
 info.dwarf_section = begin;
 info.dwarf_section_length = pish->Misc.VirtualSize;
 found_hdr = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits