(cc'ing Masahiro Yamada and Jason Cooper for objdiff)
On Fri, 2016-01-15 at 18:41 +0100, Peter Senna Tschudin wrote:
> The file drivers/usb/misc/sisusbvga/sisusb.c had many (192) coding style issues
> reported by checkpatch. This file also had a problematic error path in the
> probe function that could result in dereferencing a null pointer.
>
> This patch series fix coding style issues and a problematic error path which
> could result in a null pointer dereference.
>
> Patch 1 and 2 change whitespace only, patch 3 to 6 fix various coding style
> issues, and patch 7 fix a null pointer dereference bug.
>
> Joe Perches suggested me to include objtdiff output for patches that are not
> supposed to make semantic changes, but it is not working well for me with gcc
> (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2). Even compiling the same source code
> produces different output from objdump. The objdump command I'm using is from
> ./scripts/objdiff. See an example:
It seems gcc 5.3 isn't producing consistent output
for whitespace changes.
As far as I know, gcc has never made guarantees about
object output for the same input content.
> # A patch that should not make any semantic change
> $ cat /tmp/patch
> diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c
> index 8efbaba..a48b086d 100644
> --- a/drivers/usb/misc/sisusbvga/sisusb.c
> +++ b/drivers/usb/misc/sisusbvga/sisusb.c
> @@ -1353,7 +1353,7 @@ sisusb_testreadwrite(struct sisusb_usb_data *sisusb)
> static char srcbuffer[] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77 };
> char destbuffer[10];
> size_t dummy;
> - int i,j;
> + int i, j;
>
> sisusb_copy_memory(sisusb, srcbuffer, sisusb->vrambase, 7, &dummy);
>
> # Lets compile and collect a dump based on linux-next/master
> $ git checkout linux-next/master
> $ md5sum drivers/usb/misc/sisusbvga/sisusb.c
> d6ffbd44f3f1cf81fd55ce84441ab889 drivers/usb/misc/sisusbvga/sisusb.c
>
> $ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
> $ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
> sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump
>
> # Now let's apply the patch and collect other dump
> $ git apply /tmp/patch
> $ md5sum drivers/usb/misc/sisusbvga/sisusb.c
> 0b7d579c8ae2159f677c6a5c6efc4956 drivers/usb/misc/sisusbvga/sisusb.c
>
> $ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
> $ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
> sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/dump
>
> # I was expecting the diff to be empty
> $ diff /tmp/base_dump /tmp/dump
> 9135,9136c9135
> < : 8e 4d 31 mov 0x31(%rbp),%cs
> < : 46 00 00 rex.RX add %r8b,(%rax)
> ---
> > : 25 c4 31 46 00 and $0x4631c4,%eax
> 9139c9138
> < : 4b 00 00 rex.WXB add %al,(%r8)
> ---
> > : 00 4b 00 add %cl,0x0(%rbx)
>
> # But here is the interesting part. Even compiling the exact same source code
> # produces different results
> $ git checkout -- .
> $ md5sum drivers/usb/misc/sisusbvga/sisusb.c
> d6ffbd44f3f1cf81fd55ce84441ab889 drivers/usb/misc/sisusbvga/sisusb.c
>
> $ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
> $ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
> sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump_again
>
> $ diff /tmp/base_dump /tmp/base_dump_again
> 9135,9136c9135,9136
> < : 8e 4d 31 mov 0x31(%rbp),%cs
> < : 46 00 00 rex.RX add %r8b,(%rax)
> ---
> > : de 10 ficom (%rax)
> > : 33 46 00 xor 0x0(%rsi),%eax
> 9139c9139
> < : 4b 00 00 rex.WXB add %al,(%r8)
> ---
> > : 00 4b 00 add %cl,0x0(%rbx)
>
> Peter Senna Tschudin (7):
> usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes
> usb-misc: sisusbvga: Fix coding style: vertical whitespace changes
> usb-misc: sisusbvga: Fix coding style: braces, parenthesis, comment
> usb-misc: sisusbvga: Fix coding style: remove assignment from if tests
> usb-misc: sisusbvga: Remove null test before calls to kfree()
> usb-misc: sisusbvga: Remove memory allocation logs
> usb-misc: sisusbvga: fix error path
>
> drivers/usb/misc/sisusbvga/sisusb.c | 1543 +++++++++++++++++------------------
> 1 file changed, 752 insertions(+), 791 deletions(-)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
