REPOSITORY rL LLVM ================ Comment at: src/Unwind/AddressSpace.hpp:67 @@ +66,3 @@ +// locally. +#ifndef ElfW +#define ElfW(type) ElfW2(__INTPTR_WIDTH__, type) ---------------- I tend to prefer #if !defined() as it makes it easier to extend in the future if necessary.
================ Comment at: src/Unwind/AddressSpace.hpp:70 @@ +69,3 @@ +#define ElfW2(width, type) ElfW3(width, type) +#define ElfW3(width, type) Elf##width##_##type +#endif ---------------- It would be nice if you renamed `ElfW2` and `ElfW3` to something more generic (`GLUE`, `GLUE_EXPANDED`?). They don't have anything to do with ELF specifically. ================ Comment at: src/Unwind/AddressSpace.hpp:71 @@ -63,1 +70,3 @@ +#define ElfW3(width, type) Elf##width##_##type +#endif #include "EHHeaderParser.hpp" ---------------- jroelofs wrote: > ed wrote: > > compnerd wrote: > > > Hmm..Im not sure that this is the best way to approach this. What if you > > > want to do remote unwinding, where your host is 32-bit and your remote is > > > 64-bit or vice-versa. This breaks in that case, as you are using > > > `__INTPTR_WIDTH__`. Or am I overlooking something? > > This piece of code always has to be executed on the target. This macro is > > simply needed to fulfil the contract of `dl_iterate_phdr()`. This function > > is executed on the target system to obtain the address of the ELF program > > headers of the running process. > > > > Using a width unequal to `__INTPTR_WIDTH__` wouldn't make any sense. In > > fact, it would only generate compiler errors, for the reason that `struct > > dl_phdr_info::dlpi_phdr` is defined as a `const ElfW(Phdr) *`. See the > > following man page for details: > > > > http://linux.die.net/man/3/dl_iterate_phdr > @compnerd: Does the remote unwinding stuff even build? I don't see a > declaration for `template <typename T> class Pointer32;`, nor for `class > LittleEndian;`... > > Either way, I think @ed is right here: no matter the pointer size of the > target process, if we link against `dl_iterate_phdr()`, we have to use the > pointer size for *this* process, not the pointer size for the *other* > process. This will work unless the ElfW's are used in the interface between > `LocalAddressSpace` and `OtherAddressSpace<Foo>`. Ah, right, this would be run on the remote thread. Fair enough. http://reviews.llvm.org/D8169 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
