[PATCH] D17733: [ELF] Fix reading of PC values of FDEs

Simon Atanasyan via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 1 13:30:33 PST 2016


atanasyan added inline comments.

================
Comment at: ELF/OutputSections.cpp:669-670
@@ -668,16 +668,4 @@
 typename EhFrameHeader<ELFT>::uintX_t
 EhFrameHeader<ELFT>::getFdePc(uintX_t EhVA, const FdeData &F) {
-  const endianness E = ELFT::TargetEndianness;
-  assert((F.Enc & 0xF0) != DW_EH_PE_datarel);
-
-  uintX_t FdeOff = EhVA + F.Off + 8;
-  switch (F.Enc & 0xF) {
-  case DW_EH_PE_udata2:
-  case DW_EH_PE_sdata2:
-    return FdeOff + read16<E>(F.PCRel);
-  case DW_EH_PE_udata4:
-  case DW_EH_PE_sdata4:
-    return FdeOff + read32<E>(F.PCRel);
-  case DW_EH_PE_udata8:
-  case DW_EH_PE_sdata8:
-    return FdeOff + read64<E>(F.PCRel);
+  auto Read = [=](uint8_t Size) -> uint64_t {
+    const endianness E = ELFT::TargetEndianness;
----------------
ruiu wrote:
> Do you have to use lambda? It seems you can write without it in a more regular way.
> 
>   const endianness E = ELFT::TargetEndianness;
> 
>   int Size = F.Enc & 0x7;
>   if (Size == DW_EH_PE_absptr)
>     Size = sizeof(uintX_t) == 8 ? DW_EH_PE_udata8 : DW_EH_PE_udata4;
> 
>   uintX_t V;
>   switch (Size) {
>   case DW_EH_PE_udata2:
>     V = read16<E>(F.PCRel);
>   case DW_EH_PE_udata4:
>     V = read32<E>(F.PCRel);
>   case DW_EH_PE_udata8:
>     V = read64<E>(F.PCRel);
>   default:
>     fatal("unknown FDE size encoding");
>   }
> 
>   switch (F.Enc & 0x70) {
>   case DW_EH_PE_absptr:
>     return V;
>   case DW_EH_PE_pcrel:
>     return V + EhVA + F.Off + 8;
>   default:
>     fatal("unknown FDE size relative encoding");
>   }
> 
> Do you have to use lambda? It seems you can write without it in a more regular way.

The lambda is not mandatory. But without it the code is a bit longer because we have to add `break` to each case of the `switch (Size)`.


Repository:
  rL LLVM

http://reviews.llvm.org/D17733





More information about the llvm-commits mailing list