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.

Sign up for GitHub

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

Jump to bottom

Add -fstrict-flex-arrays. Fixes #249 #263

Merged
merged 8 commits into from
Nov 8, 2023
Merged

Add -fstrict-flex-arrays. Fixes #249 #263

merged 8 commits into from
Nov 8, 2023

Conversation

david-a-wheeler
Copy link
Contributor

No description provided.

@david-a-wheeler
Add -fstrict-flex-arrays. Fixes #249
210bbfc 
Signed-off-by: David A. Wheeler <[email protected]>
@david-a-wheeler david-a-wheeler mentioned this pull request Oct 18, 2023
Closed
@david-a-wheeler
Copy link
Contributor Author

@kees - this seem correct? We're trying to encourage others to do what the Linux kernel has already done.

@david-a-wheeler david-a-wheeler mentioned this pull request Oct 18, 2023
Closed
@david-a-wheeler
Remove erroneous space
3b01970 
Signed-off-by: David A. Wheeler <[email protected]>
@kees
Copy link

kees commented Oct 18, 2023

Yes, this looks good, though perhaps more commentary on using "C99 flexible arrays" over the GNU Extension of 0-sized arrays maybe worth calling out.

@david-a-wheeler
Small tweaks from review
c6098dd 
Signed-off-by: David A. Wheeler <[email protected]>
@david-a-wheeler
Copy link
Contributor Author

I made changes per review. I think it's ready to merge. Any last comments?

@eslerm
Copy link

eslerm commented Oct 19, 2023

Could we add the LSS talk Progress On Bounds Checking in C and the Linux Kernel by Kees Cook and Gustavo A.R. Silva

@david-a-wheeler
Copy link
Contributor Author

eslerm commented 5 days ago - Could we add the LSS talk Progress On Bounds Checking in C and the Linux Kernel by Kees Cook and Gustavo A.R. Silva

Sure! Done!

gkunz
gkunz approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@gkunz gkunz left a comment

Choose a reason for hiding this comment

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

LGTM

david-a-wheeler and others added 3 commits November 3, 2023 12:21
@david-a-wheeler
Add more about flexible arrays per discussion
c025b57 
Signed-off-by: David A. Wheeler <[email protected]>
@david-a-wheeler
Fix markdown_lint error
0bc886b 
Signed-off-by: David A. Wheeler <[email protected]>
@thomasnyman
Reorder -fstrict-flex-arrays variants to show recommended variant first
74936c8 
Minor rewording to the description accordingly and fix description of
-fstrict-flex-arrays=2 to correctly omit [1] case. Remove separate note
on compiler versions as this is now apparent from the table.

Signed-off-by: Thomas Nyman <[email protected]>
@thomasnyman
Copy link
Contributor

Reordered -fstrict-flex-arrays variants in detailed table to show the recommended -fstrict-flex-arrays=3 variant first.

Did some minor rewording to the description accordingly and fixed the description of -fstrict-flex-arrays=2 to correctly omit the [1] case. With the version information in the table, the separate separate note on compiler versions was not redundant so I removed it to shorten the description a bit.

Also did consistency pass for the references and moved the footnotes to the relevant parts of the description.

Added new references to the GCC Manual for the level descriptions found for -Wstrict-flex-arrays and the interaction with the -fsanitize=bounds option.

@thomasnyman
Use consistent style for references in -fstrix-flex-arrays description
7d26a9e 
Move footnotes to the relevant parts of the description. Add references
to the GCC Manual for the level descriptions found for
-Wstrict-flex-arrays and the -fsanitize=bounds option.

Signed-off-by: Thomas Nyman <[email protected]>
@thomasnyman thomasnyman merged commit 9708dec into main Nov 8, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siddhesh siddhesh siddhesh left review comments

@eslerm eslerm eslerm approved these changes

@gkunz gkunz gkunz approved these changes

@thomasnyman thomasnyman thomasnyman approved these changes

Assignees
No one assigned
Labels
Product: Compiler Hardening Guide
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@david-a-wheeler @kees @eslerm @thomasnyman @siddhesh @gkunz