[PATCH] D17733: [ELF] Fix reading of PC values of FDEs
Rafael EspĂndola via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 1 13:38:22 PST 2016
Small preference for a lambda, but LGTM without it too if Rui prefers
it that way.
Cheers,
Rafael
On 1 March 2016 at 16:30, Simon Atanasyan <simon at atanasyan.com> wrote:
> 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