Re: [libunwind] r297174 - Improve readability and correctness of the OS specific libunwind bits.

2017-03-08 Thread Asiri Rathnayake via cfe-commits 
@Renato: What's your take on Ed's idea?

We use phab for all sorts of reviews, but it should be possible to figure
out which repository a review is intended to land on and add cfe-commits or
llvm-commits appropriately.

Although, for throw-away reviews, it might generate too much spam.

Cheers,

/ Asiri


On Wed, Mar 8, 2017 at 3:18 PM, Ed Schouten  wrote:

> Hi Asiri,
>
> 2017-03-07 20:42 GMT+01:00 Asiri Rathnayake :
> > Could you please always include cfe-commits as a subscriber in you phab
> > reviews?
> >
> > We would like to be aware of these changes in advance before they land.
>
> Sure thing! I'll try to do that from now on.
>
> That said, if the policy is to add cfe-commits@ to all
> Clang/libunwind-related code reviews, would it make sense to configure
> Phabricator's Herald to set this up for us automatically? Looking at
> https://reviews.llvm.org/herald/new/, I suspect that can only be
> configured with admin rights.
>
> Regards,
> --
> Ed Schouten 
> Nuxi, 's-Hertogenbosch, the Netherlands
> KvK-nr.: 62051717
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r297174 - Improve readability and correctness of the OS specific libunwind bits.

2017-03-08 Thread Ed Schouten via cfe-commits 
Hi Asiri,

2017-03-07 20:42 GMT+01:00 Asiri Rathnayake :
> Could you please always include cfe-commits as a subscriber in you phab
> reviews?
>
> We would like to be aware of these changes in advance before they land.

Sure thing! I'll try to do that from now on.

That said, if the policy is to add cfe-commits@ to all
Clang/libunwind-related code reviews, would it make sense to configure
Phabricator's Herald to set this up for us automatically? Looking at
https://reviews.llvm.org/herald/new/, I suspect that can only be
configured with admin rights.

Regards,
-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libunwind] r297174 - Improve readability and correctness of the OS specific libunwind bits.

2017-03-07 Thread Asiri Rathnayake via cfe-commits 
Hi Ed,

Could you please always include cfe-commits as a subscriber in you phab
reviews?

We would like to be aware of these changes in advance before they land.

Thanks.

/ Asiri


On 7 Mar 2017 6:27 p.m., "Ed Schouten via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Author: ed
> Date: Tue Mar  7 12:15:52 2017
> New Revision: 297174
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297174=rev
> Log:
> Improve readability and correctness of the OS specific libunwind bits.
>
> All of the access to __exidx_*, dl_iterate_phdr(), etc. is specific to
> the findUnwindSections() function. Right now all of the includes and
> declarations related to them are scattered throughout the source file.
> For example, for , we have a full list of operating systems
> guarding the #include, even though the code that uses dl_iterate_phdr()
> miraculously doesn't use the same list.
>
> Change the code so that findUnwindSections() is preceded by a block of
> #ifdefs that share the same structure as the function itself. First
> comes all of the macOS specific bits, followed by bare-metal ARM,
> followed by ELF EHABI + DWARF.
>
> This actually allows us to build a copy of libunwind without any
> specific ifdefs for NetBSD, CloudABI, etc. It likely also unbreaks the
> build of libunwind on FreeBSD/armv6, though I can't confirm.
>
> Reviewed by:compnerd
> Differential Revision:  https://reviews.llvm.org/D30696
>
> Modified:
> libunwind/trunk/src/AddressSpace.hpp
>
> Modified: libunwind/trunk/src/AddressSpace.hpp
> URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/
> AddressSpace.hpp?rev=297174=297173=297174=diff
> 
> ==
> --- libunwind/trunk/src/AddressSpace.hpp (original)
> +++ libunwind/trunk/src/AddressSpace.hpp Tue Mar  7 12:15:52 2017
> @@ -34,32 +34,6 @@ namespace libunwind {
>  #include "dwarf2.h"
>  #include "Registers.hpp"
>
> -#if _LIBUNWIND_ARM_EHABI
> -#if defined(_LIBUNWIND_IS_BAREMETAL)
> -// When statically linked on bare-metal, the symbols for the EH table are
> looked
> -// up without going through the dynamic loader.
> -extern char __exidx_start;
> -extern char __exidx_end;
> -#else
> -#include 
> -#endif // !defined(_LIBUNWIND_IS_BAREMETAL)
> -#endif // _LIBUNWIND_ARM_EHABI
> -
> -#if defined(__CloudABI__) || defined(__FreeBSD__) || defined(__Fuchsia__)
> ||  \
> -defined(__linux__) || defined(__NetBSD__)
> -#if _LIBUNWIND_SUPPORT_DWARF_UNWIND && _LIBUNWIND_SUPPORT_DWARF_INDEX
> -#include 
> -// Macro for machine-independent access to the ELF program headers. This
> -// macro is not available on some systems (e.g., FreeBSD). On these
> -// systems the data structures are just called Elf_XXX. Define ElfW()
> -// locally.
> -#if !defined(ElfW)
> -#define ElfW(type) Elf_##type
> -#endif
> -#include "EHHeaderParser.hpp"
> -#endif
> -#endif
> -
>  namespace libunwind {
>
>  /// Used by findUnwindSections() to return info about needed sections.
> @@ -291,6 +265,7 @@ LocalAddressSpace::getEncodedP(pint_t 
>  }
>
>  #ifdef __APPLE__
> +
>struct dyld_unwind_sections
>{
>  const struct mach_header*   mh;
> @@ -336,6 +311,30 @@ LocalAddressSpace::getEncodedP(pint_t 
>return true;
>  }
>#endif
> +
> +#elif _LIBUNWIND_ARM_EHABI && defined(_LIBUNWIND_IS_BAREMETAL)
> +
> +// When statically linked on bare-metal, the symbols for the EH table are
> looked
> +// up without going through the dynamic loader.
> +extern char __exidx_start;
> +extern char __exidx_end;
> +
> +#elif _LIBUNWIND_ARM_EHABI || _LIBUNWIND_SUPPORT_DWARF_UNWIND
> +
> +// ELF-based systems may use dl_iterate_phdr() to access sections
> +// containing unwinding information. The ElfW() macro for pointer-size
> +// independent ELF header traversal is not provided by  on some
> +// systems (e.g., FreeBSD). On these systems the data structures are
> +// just called Elf_XXX. Define ElfW() locally.
> +#include 
> +#if !defined(ElfW)
> +#define ElfW(type) Elf_##type
> +#endif
> +
> +#if _LIBUNWIND_SUPPORT_DWARF_UNWIND
> +#include "EHHeaderParser.hpp"
> +#endif
> +
>  #endif
>
>  inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r297174 - Improve readability and correctness of the OS specific libunwind bits.

2017-03-07 Thread Ed Schouten via cfe-commits 
Author: ed
Date: Tue Mar  7 12:15:52 2017
New Revision: 297174

URL: http://llvm.org/viewvc/llvm-project?rev=297174=rev
Log:
Improve readability and correctness of the OS specific libunwind bits.

All of the access to __exidx_*, dl_iterate_phdr(), etc. is specific to
the findUnwindSections() function. Right now all of the includes and
declarations related to them are scattered throughout the source file.
For example, for , we have a full list of operating systems
guarding the #include, even though the code that uses dl_iterate_phdr()
miraculously doesn't use the same list.

Change the code so that findUnwindSections() is preceded by a block of
#ifdefs that share the same structure as the function itself. First
comes all of the macOS specific bits, followed by bare-metal ARM,
followed by ELF EHABI + DWARF.

This actually allows us to build a copy of libunwind without any
specific ifdefs for NetBSD, CloudABI, etc. It likely also unbreaks the
build of libunwind on FreeBSD/armv6, though I can't confirm.

Reviewed by:compnerd
Differential Revision:  https://reviews.llvm.org/D30696

Modified:
libunwind/trunk/src/AddressSpace.hpp

Modified: libunwind/trunk/src/AddressSpace.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=297174=297173=297174=diff
==
--- libunwind/trunk/src/AddressSpace.hpp (original)
+++ libunwind/trunk/src/AddressSpace.hpp Tue Mar  7 12:15:52 2017
@@ -34,32 +34,6 @@ namespace libunwind {
 #include "dwarf2.h"
 #include "Registers.hpp"
 
-#if _LIBUNWIND_ARM_EHABI
-#if defined(_LIBUNWIND_IS_BAREMETAL)
-// When statically linked on bare-metal, the symbols for the EH table are 
looked
-// up without going through the dynamic loader.
-extern char __exidx_start;
-extern char __exidx_end;
-#else
-#include 
-#endif // !defined(_LIBUNWIND_IS_BAREMETAL)
-#endif // _LIBUNWIND_ARM_EHABI
-
-#if defined(__CloudABI__) || defined(__FreeBSD__) || defined(__Fuchsia__) ||  \
-defined(__linux__) || defined(__NetBSD__)
-#if _LIBUNWIND_SUPPORT_DWARF_UNWIND && _LIBUNWIND_SUPPORT_DWARF_INDEX
-#include 
-// Macro for machine-independent access to the ELF program headers. This
-// macro is not available on some systems (e.g., FreeBSD). On these
-// systems the data structures are just called Elf_XXX. Define ElfW()
-// locally.
-#if !defined(ElfW)
-#define ElfW(type) Elf_##type
-#endif
-#include "EHHeaderParser.hpp"
-#endif
-#endif
-
 namespace libunwind {
 
 /// Used by findUnwindSections() to return info about needed sections.
@@ -291,6 +265,7 @@ LocalAddressSpace::getEncodedP(pint_t 
 }
 
 #ifdef __APPLE__ 
+
   struct dyld_unwind_sections
   {
 const struct mach_header*   mh;
@@ -336,6 +311,30 @@ LocalAddressSpace::getEncodedP(pint_t 
   return true;
 }
   #endif
+
+#elif _LIBUNWIND_ARM_EHABI && defined(_LIBUNWIND_IS_BAREMETAL)
+
+// When statically linked on bare-metal, the symbols for the EH table are 
looked
+// up without going through the dynamic loader.
+extern char __exidx_start;
+extern char __exidx_end;
+
+#elif _LIBUNWIND_ARM_EHABI || _LIBUNWIND_SUPPORT_DWARF_UNWIND
+
+// ELF-based systems may use dl_iterate_phdr() to access sections
+// containing unwinding information. The ElfW() macro for pointer-size
+// independent ELF header traversal is not provided by  on some
+// systems (e.g., FreeBSD). On these systems the data structures are
+// just called Elf_XXX. Define ElfW() locally.
+#include 
+#if !defined(ElfW)
+#define ElfW(type) Elf_##type
+#endif
+
+#if _LIBUNWIND_SUPPORT_DWARF_UNWIND
+#include "EHHeaderParser.hpp"
+#endif
+
 #endif
 
 inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,


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