Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tstreiff
Copy link
Contributor

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.

…traints (long/pointer) so that var_args callers and callees use the same rules
@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2020

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%) ⬆️
ppci/binutils/linker.py 76.15% <100.00%> (ø)
ppci/binutils/objectfile.py 93.23% <100.00%> (+0.12%) ⬆️
ppci/lang/c/codegenerator.py 95.42% <100.00%> (+0.16%) ⬆️
ppci/lang/c/eval.py 88.31% <100.00%> (-0.88%) ⬇️
ppci/lang/c/nodes/types.py 99.47% <100.00%> (+0.53%) ⬆️

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
Copy link
Contributor Author

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.

…e to merge: select only objectfiles defining globally at least one yet unresolved symbol
… supported in initializer constants. Also extend the integer operations to enum types (which are integer types). Does not fix for & (address of)
alignment = max(alignment, va_alignment)
# each argument is stored in a slot, aligned on the strongest alignment
size = self.va_alignment * len(var_args)
alignment = self.va_alignment
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about structs larger than the va_alignment size, is this allowed? Or can only basic types be passed via vararg way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no type restrictions in the C standard, although I have never seen composite types passed to vararg functions.
Struct/union are possible since they support argument-passing by value, and assignment.

In function calls; we can copy their value starting at one slot and occupying several slots. After the copy we realign the vararg pointer to the beginning of next slot.
In va_arg macro, we use assignment to get the value, we increment the vararg pointer by the type size + the required padding to realign the vararg pointer to the beginning of next slot.

@@ -250,6 +255,11 @@ def has_symbol(self, name):
""" Check if this object file has a symbol with name 'name' """
return name in self.symbol_map

def is_defined_global(self, name):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this to defines_global

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's more consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants