| Message ID | alpine.DEB.2.10.1411261513490.27454@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
On 26 Nov 2014 15:14, Joseph Myers wrote: > This patch fixes a warning > > ../include/features.h:328:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] > > seen when building for ARM. This warning comes from libc-do-syscall.S > being built for nscd: the nscd build uses _FORTIFY_SOURCE, while .S > files aren't built with -O, and the combination produces a warning. > As _FORTIFY_SOURCE doesn't do anything for .S files, undefining it in > libc-do-syscall.S seems the simplest solution. should we update features.h to avoid that define when __ASSEMBLER__ is defined ? seems like this warning can impact external code too. -mike
On Sun, 1 Mar 2015, Mike Frysinger wrote: > On 26 Nov 2014 15:14, Joseph Myers wrote: > > This patch fixes a warning > > > > ../include/features.h:328:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] > > > > seen when building for ARM. This warning comes from libc-do-syscall.S > > being built for nscd: the nscd build uses _FORTIFY_SOURCE, while .S > > files aren't built with -O, and the combination produces a warning. > > As _FORTIFY_SOURCE doesn't do anything for .S files, undefining it in > > libc-do-syscall.S seems the simplest solution. > > should we update features.h to avoid that define when __ASSEMBLER__ is defined ? > seems like this warning can impact external code too. I don't know. To what extent do we support users including glibc header files from assembler code?
On 03/02/2015 06:13 PM, Joseph Myers wrote: > On Sun, 1 Mar 2015, Mike Frysinger wrote: > >> On 26 Nov 2014 15:14, Joseph Myers wrote: >>> This patch fixes a warning >>> >>> ../include/features.h:328:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] >>> >>> seen when building for ARM. This warning comes from libc-do-syscall.S >>> being built for nscd: the nscd build uses _FORTIFY_SOURCE, while .S >>> files aren't built with -O, and the combination produces a warning. >>> As _FORTIFY_SOURCE doesn't do anything for .S files, undefining it in >>> libc-do-syscall.S seems the simplest solution. >> >> should we update features.h to avoid that define when __ASSEMBLER__ is defined ? >> seems like this warning can impact external code too. > > I don't know. To what extent do we support users including glibc header > files from assembler code? To the extent that it builds glibc without warnings? Cheers, Carlos.
On Thu, 5 Mar 2015, Carlos O'Donell wrote: > >> should we update features.h to avoid that define when __ASSEMBLER__ is defined ? > >> seems like this warning can impact external code too. > > > > I don't know. To what extent do we support users including glibc header > > files from assembler code? > > To the extent that it builds glibc without warnings? That answer bears no relation to my intended question. Users = users of glibc including headers in non-glibc applications, built outside of the glibc build process. Such uses are completely independent of whether glibc itself builds without warnings.
On 03/05/2015 05:14 PM, Joseph Myers wrote: > On Thu, 5 Mar 2015, Carlos O'Donell wrote: > >>>> should we update features.h to avoid that define when __ASSEMBLER__ is defined ? >>>> seems like this warning can impact external code too. >>> >>> I don't know. To what extent do we support users including glibc header >>> files from assembler code? >> >> To the extent that it builds glibc without warnings? > > That answer bears no relation to my intended question. Users = users of > glibc including headers in non-glibc applications, built outside of the > glibc build process. Such uses are completely independent of whether > glibc itself builds without warnings. If compiling libc-do-syscall.S generates a warning, we should fix it so it doesn't generate a warning. Any fixes beyond those which fix the warning are superfluous. We should not do any active work to support including glibc header files from assembly unless we have specific requests from users for specific use cases. Does that answer your question? Cheers, Carlos.
On 05 Mar 2015 17:22, Carlos O'Donell wrote: > On 03/05/2015 05:14 PM, Joseph Myers wrote: > > On Thu, 5 Mar 2015, Carlos O'Donell wrote: > >>>> should we update features.h to avoid that define when __ASSEMBLER__ is defined ? > >>>> seems like this warning can impact external code too. > >>> > >>> I don't know. To what extent do we support users including glibc header > >>> files from assembler code? > >> > >> To the extent that it builds glibc without warnings? > > > > That answer bears no relation to my intended question. Users = users of > > glibc including headers in non-glibc applications, built outside of the > > glibc build process. Such uses are completely independent of whether > > glibc itself builds without warnings. > > If compiling libc-do-syscall.S generates a warning, we should fix it so it > doesn't generate a warning. Any fixes beyond those which fix the warning > are superfluous. > > We should not do any active work to support including glibc header files > from assembly unless we have specific requests from users for specific > use cases. > > Does that answer your question? not exactly ... Joseph fixed the warning in this one .S file by undefining the symbol. since the warning originated in features.h and can be triggered by any .S file (in glibc or non-glibc code), i suggested maybe we change the features.h header instead so we never have to worry about it again. i don't see that as superfluous ... more fixing it in the right place. to the larger question Joseph and you raise, i don't have an answer for that. i don't think we've even thought about it beyond specific requests. -mike
On 03/05/2015 06:51 PM, Mike Frysinger wrote: > Joseph fixed the warning in this one .S file by undefining the symbol. > since the warning originated in features.h and can be triggered by any > .S file (in glibc or non-glibc code), i suggested maybe we change the > features.h header instead so we never have to worry about it again. i > don't see that as superfluous ... more fixing it in the right place. I agree. My point is that we should do what's right for glibc first. If there are two ways to fix this in glibc, and one way involves modifying any assembly that includes the header, and the other solves the problem once, then the changes to header are a better solution, for glibc, in my opinion. This doesn't take into account any user requirements to include the header in assembly files. > to the larger question Joseph and you raise, i don't have an answer for > that. i don't think we've even thought about it beyond specific requests. Exactly my point, and we shouldn't worry about it until we get such requests. Cheers, Carlos.
On Fri, 6 Mar 2015, Carlos O'Donell wrote: > I agree. My point is that we should do what's right for glibc first. > If there are two ways to fix this in glibc, and one way involves modifying > any assembly that includes the header, and the other solves the problem > once, then the changes to header are a better solution, for glibc, in my > opinion. This doesn't take into account any user requirements to include > the header in assembly files. What about arranging for the configured CFLAGS to be used when building .S files, so that -O options are active and the warning is avoided that way? A comment in Makeconfig notes how -std=* can't be used for .S files, but the -O options ought to work for them.
On 06 Mar 2015 22:15, Joseph Myers wrote: > On Fri, 6 Mar 2015, Carlos O'Donell wrote: > > I agree. My point is that we should do what's right for glibc first. > > If there are two ways to fix this in glibc, and one way involves modifying > > any assembly that includes the header, and the other solves the problem > > once, then the changes to header are a better solution, for glibc, in my > > opinion. This doesn't take into account any user requirements to include > > the header in assembly files. > > What about arranging for the configured CFLAGS to be used when building .S > files, so that -O options are active and the warning is avoided that way? > A comment in Makeconfig notes how -std=* can't be used for .S files, but > the -O options ought to work for them. if we're going that route, what about adding -U_FORTIFY_SOURCE to ASFLAGS at configure time ? -mike
diff --git a/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S b/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S index d42a94a..78e997c 100644 --- a/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S +++ b/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S @@ -15,6 +15,9 @@ License along with the GNU C Library. If not, see <http://www.gnu.org/licenses/>. */ +/* When this file is built for inclusion in nscd, _FORTIFY_SOURCE is + defined but -O is not used, resulting in a warning from features.h. */ +#undef _FORTIFY_SOURCE #include <sysdep.h> /* Out-of-line syscall stub. We expect the system call number in ip