[dpdk-dev] Build is broken in dpdk-next-net
Gaëtan Rivet
gaetan.rivet at 6wind.com
Sat Mar 31 18:21:41 CEST 2018
On Sat, Mar 31, 2018 at 11:27:55AM -0400, Neil Horman wrote:
> On Sat, Mar 31, 2018 at 05:09:47PM +0200, Gaëtan Rivet wrote:
> > On Sat, Mar 31, 2018 at 09:33:43AM -0400, Neil Horman wrote:
> > > On Fri, Mar 30, 2018 at 10:47:09PM +0800, Tonghao Zhang wrote:
> > > > I rebuild it on ubuntu 17.10 and cash it. I use the 'RTE_SET_USED' to fix it.
> > > >
> > > >
> > > > diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c
> > > > index 771675718..f11803191 100644
> > > > --- a/lib/librte_vhost/fd_man.c
> > > > +++ b/lib/librte_vhost/fd_man.c
> > > > @@ -279,7 +279,8 @@ fdset_pipe_read_cb(int readfd, void *dat __rte_unused,
> > > > int *remove __rte_unused)
> > > > {
> > > > char charbuf[16];
> > > > - read(readfd, charbuf, sizeof(charbuf));
> > > > + int r = read(readfd, charbuf, sizeof(charbuf));
> > > > + RTE_SET_USED(r);
> > > > }
> > > >
> > > > void
> > > > @@ -319,5 +320,6 @@ fdset_pipe_init(struct fdset *fdset)
> > > > void
> > > > fdset_pipe_notify(struct fdset *fdset)
> > > > {
> > > > - write(fdset->u.writefd, "1", 1);
> > > > + int r = write(fdset->u.writefd, "1", 1);
> > > > + RTE_SET_USED(r);
> > > > }
> > > >
> > >
> > > A better option might be to use _Pragma
> > >
> > > Something like this perhaps
> > >
> > > #define ALLOW_UNUSED(x) \
> > > _Pragma(push) \
> > > _Pragma(diagnostic ignored "-Wunused-result") \
> > > #x;\
> > > _Pragma(pop)
> > >
> > > This is of course untested, so it probably needs some tweaking, but this method
> > > avoids the need to declare an additional stack variable, which i don't think can
> > > be eliminated due to the cast. I believe that this method should also work
> > > accross compilers (the gcc and clang compilers support this, and i think the
> > > intel compiler should as well)
> > >
> > > Neil
> > >
> >
> > It would be nice to avoid the definition of a useless variable.
> > An alternative could be
> >
> > if (read() < 0) {
> > /* Failure here is acceptable for such and such reason. */
> > }
> >
> > to ensure all-around compatibility, and the definition or another macro.
> > Just a suggestion.
> >
> That would be a good alternative, but I think its effectiveness is dependent on
> when the compiler does with the return value check. Without any code inside the
> conditional, the compiler may optimize the check out, meaning the warning will
> still be asserted. If it doesn't optimize the check out, then you have a
> useless compare and jump instruction left in the code path.
>
> Best
> Neil
>
I tested quickly, I see no difference with the three methods:
#include <unistd.h>
__attribute__((warn_unused_result))
int wur(void)
{
return read(0, NULL, 0);
}
void with_void(void)
{
int ret;
ret = wur();
(void) ret;
}
void with_pragma(void)
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wunused-result"
wur();
#pragma GCC diagnostic pop
}
void with_if(void)
{
if (wur() < 0) {
/* I do not care for errors. */
}
}
int main(void)
{
return 0;
}
----------------------------
gcc -D_FORTIFY_SOURCE=2 -Wunused-result -Werror -O3 _wur.c -o wur_O3.out
gcc -D_FORTIFY_SOURCE=2 -Wunused-result -Werror -O0 _wur.c -o wur_O0.out
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_void') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_void') || true
Dump of assembler code for function with_void: Dump of assembler code for function with_void:
0x00000000000006ca <+0>: push %rbp | 0x00000000000006e0 <+0>: xor %edx,%edx
0x00000000000006cb <+1>: mov %rsp,%rbp | 0x00000000000006e2 <+2>: xor %esi,%esi
0x00000000000006ce <+4>: sub $0x10,%rsp | 0x00000000000006e4 <+4>: xor %edi,%edi
0x00000000000006d2 <+8>: callq 0x6b0 <wur> | 0x00000000000006e6 <+6>: jmpq 0x560 <read at plt>
0x00000000000006d7 <+13>: mov %eax,-0x4(%rbp) <
0x00000000000006da <+16>: nop <
0x00000000000006db <+17>: leaveq <
0x00000000000006dc <+18>: retq <
End of assembler dump. End of assembler dump.
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_pragma') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_pragma') || true
Dump of assembler code for function with_pragma: Dump of assembler code for function with_pragma:
0x00000000000006dd <+0>: push %rbp | 0x00000000000006f0 <+0>: xor %edx,%edx
0x00000000000006de <+1>: mov %rsp,%rbp | 0x00000000000006f2 <+2>: xor %esi,%esi
0x00000000000006e1 <+4>: callq 0x6b0 <wur> | 0x00000000000006f4 <+4>: xor %edi,%edi
0x00000000000006e6 <+9>: nop | 0x00000000000006f6 <+6>: jmpq 0x560 <read at plt>
0x00000000000006e7 <+10>: pop %rbp <
0x00000000000006e8 <+11>: retq <
End of assembler dump. End of assembler dump.
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_if') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_if') || true
Dump of assembler code for function with_if: Dump of assembler code for function with_if:
0x00000000000006e9 <+0>: push %rbp | 0x0000000000000700 <+0>: xor %edx,%edx
0x00000000000006ea <+1>: mov %rsp,%rbp | 0x0000000000000702 <+2>: xor %esi,%esi
0x00000000000006ed <+4>: callq 0x6b0 <wur> | 0x0000000000000704 <+4>: xor %edi,%edi
0x00000000000006f2 <+9>: nop | 0x0000000000000706 <+6>: jmpq 0x560 <read at plt>
0x00000000000006f3 <+10>: pop %rbp <
0x00000000000006f4 <+11>: retq <
End of assembler dump. End of assembler dump.
And then with
__attribute__((warn_unused_result))
int wur(void)
{
return -1;
}
-----------------------------------
gcc -D_FORTIFY_SOURCE=2 -Wunused-result -Werror -O3 _wur.c -o wur_O3.out
gcc -D_FORTIFY_SOURCE=2 -Wunused-result -Werror -O0 _wur.c -o wur_O0.out
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_void') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_void') || true
Dump of assembler code for function with_void: Dump of assembler code for function with_void:
0x000000000000066b <+0>: push %rbp | 0x0000000000000680 <+0>: repz retq
0x000000000000066c <+1>: mov %rsp,%rbp <
0x000000000000066f <+4>: sub $0x10,%rsp <
0x0000000000000673 <+8>: callq 0x660 <wur> <
0x0000000000000678 <+13>: mov %eax,-0x4(%rbp) <
0x000000000000067b <+16>: nop <
0x000000000000067c <+17>: leaveq <
0x000000000000067d <+18>: retq <
End of assembler dump. End of assembler dump.
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_pragma') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_pragma') || true
Dump of assembler code for function with_pragma: Dump of assembler code for function with_pragma:
0x000000000000067e <+0>: push %rbp | 0x0000000000000690 <+0>: repz retq
0x000000000000067f <+1>: mov %rsp,%rbp <
0x0000000000000682 <+4>: callq 0x660 <wur> <
0x0000000000000687 <+9>: nop <
0x0000000000000688 <+10>: pop %rbp <
0x0000000000000689 <+11>: retq <
End of assembler dump. End of assembler dump.
diff -Napy <(gdb -batch -ex 'file ./wur_O0.out' -ex 'disassemble with_if') <(gdb -batch -ex 'file ./wur_O3.out' -ex 'disassemble with_if') || true
Dump of assembler code for function with_if: Dump of assembler code for function with_if:
0x000000000000068a <+0>: push %rbp | 0x00000000000006a0 <+0>: repz retq
0x000000000000068b <+1>: mov %rsp,%rbp <
0x000000000000068e <+4>: callq 0x660 <wur> <
0x0000000000000693 <+9>: nop <
0x0000000000000694 <+10>: pop %rbp <
0x0000000000000695 <+11>: retq <
End of assembler dump. End of assembler dump.
There is a slight difference with the unoptimized version, otherwise any
solution should work. We can also note that in -O3, having the
additional variable on the stack makes no difference.
--
Gaëtan Rivet
6WIND
More information about the dev
mailing list