This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Convert {read,write}*be to endianness-aware read/write.
ClosedPublic

Authored by MaskRay on Mar 7 2018, 2:24 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.Mar 7 2018, 2:24 PM
MaskRay added a reviewer: ruiu.Mar 7 2018, 2:33 PM
ruiu added inline comments.Mar 7 2018, 2:40 PM
ELF/Arch/PPC64.cpp
116 ↗(On Diff #137481)

I think it is better to define write32 like this

void write32(uint8_t *Loc, uint32_t Val) { endian::write32(Loc, Val, Config->Endianness); }

and use it, instead of passing E for each function call.

MaskRay added inline comments.Mar 7 2018, 7:15 PM
ELF/Arch/PPC64.cpp
116 ↗(On Diff #137481)

Where should I put write16 write32 read16 read32? ELF/Target.h TargetInfo?

MaskRay updated this revision to Diff 137596.Mar 8 2018, 9:53 AM

Add endianness-aware read/write utilities to ELF/Target.h

MaskRay marked an inline comment as done.Mar 8 2018, 9:54 AM
MaskRay added inline comments.
ELF/Target.cpp
118 ↗(On Diff #137596)

If this is performance critical, I can use inline functions.

MaskRay marked 2 inline comments as done.Mar 8 2018, 10:02 AM
ruiu added inline comments.Mar 8 2018, 3:26 PM
ELF/Target.h
184 ↗(On Diff #137599)

Please use uint8_t * instead of void * for P.

MaskRay added inline comments.Mar 8 2018, 7:07 PM
ELF/Target.h
184 ↗(On Diff #137599)
../projects/lld/ELF/SyntheticSections.cpp:1767:5: error: no matching function for call to 'write32'
    write32(Buckets + I->BucketIdx, I->Sym->DynsymIndex);
    ^~~~~~~
../projects/lld/ELF/Target.h:200:13: note: candidate function not viable: no known conversion from 'uint32_t *' (aka 'unsigned int *') to 'uint8_t *' (aka 'unsigned char *') for 1st argument
inline void write32(uint8_t *P, uint32_t V) {
            ^
../projects/lld/ELF/SyntheticSections.cpp:1841:3: error: no matching function for call to 'write32'
  write32(P++, NumSymbols); // nbucket
  ^~~~~~~
../projects/lld/ELF/Target.h:200:13: note: candidate function not viable: no known conversion from 'uint32_t *' (aka 'unsigned int *') to 'uint8_t *' (aka 'unsigned char *') for 1st argument
inline void write32(uint8_t *P, uint32_t V) {
            ^
../projects/lld/ELF/SyntheticSections.cpp:1842:3: error: no matching function for call to 'write32'
  write32(P++, NumSymbols); // nchain

uint8_t* uint32_t* uint16_t* typed pointers are all used. void * is a common type they will accept.

ruiu accepted this revision.Mar 8 2018, 8:04 PM

LGTM

ELF/Target.h
184 ↗(On Diff #137599)

Ah thanks. I didn't know that.

This revision is now accepted and ready to land.Mar 8 2018, 8:04 PM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.