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

lua: avoid gcc -Wreturn-local-addr bug #11337

Merged
merged 1 commit into from
Dec 15, 2020
Merged

Conversation

rlibby
Copy link
Contributor

@rlibby rlibby commented Dec 13, 2020

Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation. In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Signed-off-by: Ryan Libby rlibby@FreeBSD.org

Motivation and Context

Part of a small set of changes for building with gcc on FreeBSD.

This change addresses this warning, which also leads to a code generation error.

/usr/src/freebsd/sys/contrib/openzfs/module/lua/ldo.c: In function 'stack_remaining':
/usr/src/freebsd/sys/contrib/openzfs/module/lua/ldo.c:43:10: error: function returns address of local variable [-Werror=return-local-addr]
   return (intptr_t)(&local - (char *)curthread->td_kstack);
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Description

Avoid -Wreturn-local-addr by reformulating the computation in a way that gcc doesn't recognize as returning the address of a local.

This is admittedly ugly, as there is nothing wrong with the previous formulation, and without examining gcc internals it's not obvious why this formulation is any less likely to encounter the bug. However, it is a simple solution to the problem.

An alternative solution which may be more clear may be to define stack_remaining() as taking a parameter representing the amount of stack space needed and then returning a boolean, rather than returning the available space remaining. On the other hand, changing the function prototype would cause slightly more code churn.

Note that -Wno-return-local-addr is not a solution: that suppresses the warning but still leads to wrong code generation.

How Has This Been Tested?

Compile tested in the downstream FreeBSD build with gcc and clang.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation.  In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Signed-off-by: Ryan Libby <rlibby@FreeBSD.org>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 14, 2020
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks for linking to the original gcc bug in the PR and commit comment, that was very helpful. It's good to see this has been fixed in gcc 9.2, and 10. Since we can avoid this with a little minor code restructuring on these buggy versions that does seem preferable.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 15, 2020
@behlendorf behlendorf merged commit d8a09b3 into openzfs:master Dec 15, 2020
ghost pushed a commit to zfsonfreebsd/ZoF that referenced this pull request Dec 23, 2020
Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation.  In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Libby <rlibby@FreeBSD.org>
Closes openzfs#11337
behlendorf pushed a commit that referenced this pull request Dec 23, 2020
Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation.  In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Libby <rlibby@FreeBSD.org>
Closes #11337
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation.  In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Libby <rlibby@FreeBSD.org>
Closes openzfs#11337
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Avoid a bug with gcc's -Wreturn-local-addr warning with some
obfuscation.  In buggy versions of gcc, if a return value is an
expression that involves the address of a local variable, and even if
that address is legally converted to a non-pointer type, a warning may
be emitted and the value of the address may be replaced with zero.
Howerver, buggy versions don't emit the warning or replace the value
when simply returning a local variable of non-pointer type.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90737

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Libby <rlibby@FreeBSD.org>
Closes openzfs#11337
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants