Comment 7 for bug 490792

Revision history for this message
In , Jacob Bramley (jacob-bramley) wrote :

Created an attachment (id=371456)
Generate interworking branches.

--- Patch Summary

* asm_call uses the new BLX method to generate interworking branches.
  - BLX() will generate interworking branches on ARMv4T, which doesn't support the actual BLX instruction.
  - BLX() is clever enough to generate simple (non-interworking) branches for ARMv4(T) when the target is not Thumb. This is always the case for plain ARMv4.

* The processor feature detection has been modified to allow detection of ARMv5 support (along with other features).
  - TODO: I can't test the WinCE code here. It looks Ok, but could someone try this out please?

* The patch works only if asm_call is the only function used to call out of generated code. That appears to be the case as it handles LIR_call, which is generated by insCall, but I'm not entirely familiar with the code so there could be some other way that calls can be generated.

* Existing branch mechanisms for branching within (and directly between) generated code fragments are unchanged, as the target will always be ARM code and interworking is not required.

* The exit branches were generated as "BX"; this won't work for ARMv4, so I've added a fix for this too.

* A number of asm_output() calls have been tidied up to be more informative of what's actually generated.

--- Bug Reproduction

Whilst I could hack the generated makefiles to get TM itself in Thumb, the generated code always seems to be far enough away from the native code that a simple BLX is never generated. The original code used "LDR PC, =addr" in this case, and on ARMv5+ this type of branch supports interworking so the behaviour is correct. (I don't have an ARMv4T platform to test on.) However, this is still a potential bug as we can't guarantee the memory locations of each code element.

To clarify:
 * The bug will manifest on ARMv5+ if the target address is within 32MB of the branch instruction _and_ the target is Thumb (or Thumb-2) code.
 * The bug will manifest on ARMv4T regardless, as LDR PC does not interwork on this architecture.
 * A slightly different bug will manifest on ARMv4, but that should be fixed by my patch.

--- Old Architectures

There's a lot of code in there to handle ARMv4(T). This does make me wonder if it's worth supporting these. What is the policy on this? Can we assume ARMv5+, or do we need the ARMv4 support code?

If we don't need to support ARMv4, I'll strip that stuff out of the patch to make it simpler and cleaner.

Note: I think Tamarin supported ARMv4T, but I don't know about ARMv4.

--- Testing

No additional failures arise from adding this patch on either trace-tests.js or js/tests.