lib/utils/printf: Fix printf on release builds #1799
Conversation
|
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? |
|
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. |
|
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.
168eab6 to
48529cc
Compare
|
ok - comment block updated. |
|
Very detailed, looks good to me. |
|
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! |
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)