Improve arm64 ABI handling across the board #387
Conversation
|
This resolves #367 |
| Specifies whether functions accepting a return value location should | ||
| return it back. |
There was a problem hiding this comment.
Specifies whether functions whose return value is saved in a memory area pointed by an argument passed by the caller (SPTAR) should also return such pointer on ReturnValueLocationRegister/ReturnValueLocationOnStack.
| @@ -145,6 +145,13 @@ type: struct | |||
| ARM ABI), the value of `StackAlignment` should be equal to 4. | |||
| type: uint64_t | |||
|
|
|||
| - name: MinimumStackWordSize | |||
There was a problem hiding this comment.
I'd avoid the "word" nomenclature, it's quite overloaded.
| - name: MinimumStackWordSize | ||
| doc: | | ||
| States the minimum possible stack argument size in bytes. | ||
| When the factual type of an argument is below this value, it should be |
There was a problem hiding this comment.
Was supposed to be actual, but I suppose I was thinking about some kind of facts at the time 😆
| This allows a non-position-based ABI to take advantage of one of the main | ||
| traits of position-based ABI: ability to use pointers to copy to pass | ||
| big aggregates. |
There was a problem hiding this comment.
These descriptions are very difficult to understand.
Can you make an example of the difference between an ABI using true and one using false?
| @@ -79,7 +79,7 @@ class model::Segment : public model::generated::Segment { | |||
|
|
|||
| public: | |||
| bool contains(MetaAddress Address) const { | |||
| auto EndAddress = StartAddress() + VirtualSize(); | |||
| auto EndAddress = StartAddress().toGeneric() + VirtualSize(); | |||
There was a problem hiding this comment.
Wait segments should already be Generic
There was a problem hiding this comment.
I saw AArch64 segments failing because of alignment. Hence these changes. I guess PDB importer did something weird? 🤔
There was a problem hiding this comment.
Do we want to have an assert about segments being generic instead?
There was a problem hiding this comment.
It should be in verify
| inline constexpr model::ABI::Values | ||
| getDefaultForMachO(model::Architecture::Values V) { | ||
| switch (V) { | ||
| case model::Architecture::aarch64: | ||
| return model::ABI::Apple_AAPCS64; | ||
| default: | ||
| return getDefaultFallback(V); | ||
| } | ||
| } |
There was a problem hiding this comment.
getDefaultFallback handles x86-64 ABIs, correct?
There was a problem hiding this comment.
getDefaultFallback is the "current default" in a way. This lets you only specify "overrides" for the architectures that needs something special. This limits logic duplication somewhat.
There was a problem hiding this comment.
I guess we could be even stricter with it and explicitly disallow some combinations (for example MIPS in a Mach-O binary), but for now I opted to preserve the current behaviour.
Sister PRs: