windelbouwman / ppci

A compiler for ARM, X86, MSP430, xtensa and more implemented in pure Python
https://ppci.readthedocs.io/en/latest/
BSD 2-Clause "Simplified" License
335 stars 35 forks source link

Fix for issue 97. Aligns all var_args on the strongest alignment constraint #98

Open tstreiff opened 4 years ago

tstreiff commented 4 years ago

The var_arg block is an array of slots, one for each argument. The size of the slot is based on the strongest alignment between "long int" and pointers. Each argument is stored at the address of its slot.

This way va_arg takes the expected value at the current var_arg pointer and increments it by the slot size.

On x86_64, the slot size is 8byte. On i386, MIPS, m68k, the slot size is 4byte.

codecov-commenter commented 4 years ago

Codecov Report

Merging #98 into master will increase coverage by 0.00%. The diff coverage is 96.55%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #98   +/-   ##
=======================================
  Coverage   78.50%   78.51%           
=======================================
  Files         349      349           
  Lines       45526    45533    +7     
=======================================
+ Hits        35741    35750    +9     
+ Misses       9785     9783    -2     
Impacted Files Coverage Δ
ppci/lang/c/semantics.py 83.72% <80.00%> (+0.07%) :arrow_up:
ppci/binutils/linker.py 76.15% <100.00%> (ø)
ppci/binutils/objectfile.py 93.23% <100.00%> (+0.12%) :arrow_up:
ppci/lang/c/codegenerator.py 95.42% <100.00%> (+0.16%) :arrow_up:
ppci/lang/c/eval.py 88.31% <100.00%> (-0.88%) :arrow_down:
ppci/lang/c/nodes/types.py 99.47% <100.00%> (+0.53%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eb4798f...6e9c25b. Read the comment docs.

tstreiff commented 4 years ago

This pull request does not cover another current problem: it is not possible to pass struct args to a varrarg function. Not required for printf, scanf & co but the C standard does not limit the possible types.

It should be easy to fix: ths fix puts each argument in a slot and increments the pointer by the slot size. In case a type is larger than one slot, the caller will have to allocate as many slots as necessary, and will align the next argument to the next slot. The important thing is to always keep the alignment.

I propose that we fix the printf & co problem now (scanf & co are not a problem since they use only pointers) and deal with the minor restriction for struct later.