This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Make Target::TrapInstr to be uint8_t
AbandonedPublic

Authored by grimar on Mar 28 2018, 3:55 AM.

Details

Summary

Currently, it is 4 bytes and we do not really need it.
All our targets use 1-byte trap atm.

And sometimes it is inconvenient to have 4 bytes, for example
when we fill gaps with traps in writeTrapInstr
and want to fill memory area. (see D44775/D44943).

Diff Detail

Event Timeline

grimar created this revision.Mar 28 2018, 3:55 AM
espindola added inline comments.Mar 28 2018, 5:34 PM
ELF/Arch/ARM.cpp
63

This is at least odd, as 0xd4 is not a trap on ARM.

Should we just directly represent the fact that the trap instruction has different sizes in different architectures by using an StringRef or ArrayRef?

espindola added inline comments.Mar 28 2018, 5:41 PM
ELF/Arch/ARM.cpp
63

Also, nop on powerpc64 (which we are getting more support for) is 60 00 00 00.

ruiu added a comment.Mar 28 2018, 5:44 PM

This is towards a wrong direction. uint32_t is a deliberate choice because as Rafael pointed out uint8_t doesn't work for most processors. You cannot expect that trap instruction is 1 byte long.

grimar abandoned this revision.Mar 29 2018, 2:02 AM

Given the comments, abandoning.