Possible issue with ARM/MC/MachO fixup

Hi everyone.

In ARMAsmBackend.cpp, in routine DarwinARMAsmBackend::ApplyFixup()
there is a curious call to getFixupKindNumBytes() - which can return
1,2, 3, or 4 depending upon the FixupKind

The code in ApplyFixup() seems to be lifted from the X86.

AFAIK, the initial Fixup.Offset() is always divisible by 4, at least
for ARM mode - i.e. it is always at the instruction boundary.

it looks like adjustFixupValue() is meant to create a mask that can be
bitwise OR'ed into the full instruction, which is 2 or 4 bytes
So what does it mean to patch 1 or 3 bytes starting from the
instruction boundary?

I don't know enough about Macho to tell whether this is a bug or not.
If it isn't, apologies for the noise.

-Jason

Hi everyone.

In ARMAsmBackend.cpp, in routine DarwinARMAsmBackend::ApplyFixup()
there is a curious call to getFixupKindNumBytes() - which can return
1,2, 3, or 4 depending upon the FixupKind

The code in ApplyFixup() seems to be lifted from the X86.

AFAIK, the initial Fixup.Offset() is always divisible by 4, at least
for ARM mode - i.e. it is always at the instruction boundary.

it looks like adjustFixupValue() is meant to create a mask that can be
bitwise OR'ed into the full instruction, which is 2 or 4 bytes
So what does it mean to patch 1 or 3 bytes starting from the
instruction boundary?

It's just unused in the current implementation. The Fixup offset (the one that originates from the addFixup()) should always be zero.

I don't know enough about Macho to tell whether this is a bug or not.
If it isn't, apologies for the noise.

It's not a bug, per se, but the way the ARM fixups are handled needs some refactoring. The fixups shouldn't be quite as "smart" as they are and instead there should be one fixup per contiguous range of bits that need adjusted, and instructions with non-contiguous regions that need fixing up should use multiple fixups on the same instructions. In that situation, it'll be important to use the Fixup offset field to specify the starting byte for the fixup. Otherwise MC gets confused (it wants the fixups in order). The idea is that the adjustFixupValue() function will go away entirely and the MCExpr attached to the fixup will do all of the necessary value fiddling to get the right bits to apply to the instruction.

-Jim