Skip to content

Conversation

@ChAoSUnItY
Copy link
Collaborator

@ChAoSUnItY ChAoSUnItY commented May 21, 2025

This patch supports global string initialization, and allows integer values to be used as address value of any pointer type.

Summary by Bito

This pull request enhances global string initialization by allowing integer values as address values for pointer types. Key modifications include parser updates for string literals, improved register allocation for data addresses, and SSA updates for better string formatting. New tests have been added to ensure the functionality works as intended.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

This patch supports global string initialization, and allows integer
values to be used as address value of any pointer type.
@jserv jserv merged commit e5cc94c into sysprog21:master May 21, 2025
6 checks passed
@jserv
Copy link
Collaborator

jserv commented May 21, 2025

Thank @ChAoSUnItY for contributing!

ChAoSUnItY pushed a commit to ChAoSUnItY/shecc that referenced this pull request May 21, 2025
int main(void)
{
printf(data);
data[1] = 'a';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this write operation is invalid because the string literal ("Hello World!") should be read-only and placed in the .rodata section, although the current implementation does not create this section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to C99 6.4.5 String Literals:

  1. It is unspecified whether these arrays are distinct provided their elements have the appropriate
    values. If the program attempts to modify such an array, the behavior is undefined.

The common practice in other well-known compiler (gcc, clang) suggest that this is indeed an invalid operation since this would cause segmentation fault.

The reason I submitted this initial support patch is to allow my local work to declare string litearl in global scope, which is kind of rough but working anyway. Would you like to look into this behavior patch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Understood. I think we can create a patch to add TODO or FIXME comment to inform developers that this write behavior should be fixed and avoided in the future.

ChAoSUnItY pushed a commit to ChAoSUnItY/shecc that referenced this pull request May 21, 2025
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