8000 lib/utils/printf: Fix printf on release builds by dhylands · Pull Request #1799 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

lib/utils/printf: Fix printf on release builds #1799

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

Closed
wants to merge 1 commit into from

Conversation

dhylands
Copy link
Contributor

When using newer glibc's the compiler automatically sets
_FORTIFY_SOURCE when building with -O1 and this causes
a special inlined version of printf to be declared which
then bypasses our version of printf.

This is probably the preferred way to fix #1771 (and #1798)

@dpgeorge
Copy link
Member

This is ok with me, but perhaps needs a bit of extra comment, like "... so that our version of printf is not bypassed". @pfalcon what do you think?

@pfalcon
Copy link
Contributor
pfalcon commented Jan 27, 2016

I think that, while seemingly minor, this is a complex issue. I probably made about the same way as @dhylands did regarding figuring what _FORTIFY_SOURCE is. While it's not widely known or have clear, easily accessible documentation (so you need to dig for it), it's enabled by default in few distros (Ubuntu, Gentoo, etc.). Trying to do something with it can be considered playing against distros. So, I considered to submit similar patch, but so far didn't do that, instead being on a stage of figuring out how other projects may have dealt with such situation (e.g. I found that Wine at one point had an issue with that, but couldn't trace exact resolution, but initial response from the project was along line of "we don't care about your broken distro" and from distro "we don't care about your broken project").

So, if we apply such patch, it should have ~10 lines of comments, shortly describing what _FORTIFY_SOURCE is, linking to more details, telling why it affects, and then specifically why we dare to just undefine it. And the best way would be to see if micropython is included in some distros, and seek advice from them.

@dhylands
Copy link
Contributor Author

I'll go ahead and write up a comment block and update the PR.

When using newer glibc's the compiler automatically sets
_FORTIFY_SOURCE when building with -O1 and this causes
a special inlined version of printf to be declared which
then bypasses our version of printf.
@dhylands
Copy link
Contributor Author

ok - comment block updated.

@dpgeorge
Copy link
Member

Very detailed, looks good to me.

@dpgeorge dpgeorge mentioned this pull request Jan 28, 2016
4 tasks
@pfalcon
Copy link
Contributor
pfalcon commented Jan 28, 2016

Quite detailed description indeed ;-). Fixed a typo and added link to original gcc patchset, https://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html . Thanks!

@pfalcon pfalcon closed this Jan 28, 2016
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.

verbose output order
3 participants
0