Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1590139 > unrolled thread

Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

Started byArnd Bergmann <arnd@arndb.de>
First post2017-03-01 10:40 +0100
Last post2017-03-01 16:30 +0100
Articles 20 on this page of 22 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-01 10:40 +0100
    Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-01 11:00 +0100
      Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-01 15:50 +0100
        Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-01 18:30 +0100
          Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-01 23:10 +0100
          Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-01 23:50 +0100
            Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-02 02:20 +0100
              Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Ingo Molnar <mingo@kernel.org> - 2017-03-02 07:40 +0100
                Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Ingo Molnar <mingo@kernel.org> - 2017-03-02 15:00 +0100
                  Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-02 15:30 +0100
                    Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Ingo Molnar <mingo@kernel.org> - 2017-03-02 15:50 +0100
                Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-02 15:10 +0100
              Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-03 00:20 +0100
                Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-03 01:00 +0100
                  Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-03 10:30 +0100
                    Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-03 16:40 +0100
    Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-01 16:30 +0100
      Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-02 19:40 +0100
        Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Arnd Bergmann <arnd@arndb.de> - 2017-03-03 00:00 +0100
          Re: [PATCH] objtool: fix another gcc jump table detection issue Arnd Bergmann <arnd@arndb.de> - 2017-03-03 00:10 +0100
          [PATCH] objtool: fix another gcc jump table detection issue Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-03 04:20 +0100
    Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-01 16:30 +0100

Page 1 of 2  [1] 2  Next page →


#1590139 — Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-01 10:40 +0100
SubjectRe: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings
Message-ID<tg8TM-179-29@gated-at.bofh.it>
On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote:
>>
>> 3) 0xFC244C03-config:
>> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event()
>> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section
>>
>> These look like another bad gcc bug which is truncating functions:
>
> Same bug for both of them?

I ran into this one again today, after updating to the latest gcc-7.0.1:

drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool:
rxe_responder()+0xfe: sibling call from callable instruction with
changed frame pointer

Josh, did you get around to updating objtool the last time I reported it, or
is it still the same problem? If this is a new variation, I can provide more
details about the failure, otherwise I'll just ignore it for now.

    Arnd

[toc] | [next] | [standalone]


#1590153

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-01 11:00 +0100
Message-ID<tg9d8-1fv-21@gated-at.bofh.it>
In reply to#1590139
On Wed, Mar 1, 2017 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote:
>>>
>>> 3) 0xFC244C03-config:
>>> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event()
>>> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section
>>>
>>> These look like another bad gcc bug which is truncating functions:
>>
>> Same bug for both of them?
>
> I ran into this one again today, after updating to the latest gcc-7.0.1:
>
> drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool:
> rxe_responder()+0xfe: sibling call from callable instruction with
> changed frame pointer
>
> Josh, did you get around to updating objtool the last time I reported it, or
> is it still the same problem? If this is a new variation, I can provide more
> details about the failure, otherwise I'll just ignore it for now.

Actually, something must have changed in gcc since last month, I also
just got a report in another file:

drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe()
falls through to next function img_i2c_read_fifo()

See below for the relevant snippet from the assembler output.

      Arnd

# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176:           if
(i2c->bitrate <= timings[i].max_bitrate) {
        movl    1648(%rbx), %edx        # MEM[(struct img_i2c
*)_29].bitrate, _99
        cmpl    timings+8(%rip), %edx   # timings[0].max_bitrate, _99
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1171:
i2c->need_wr_rd_fence = true;
        movb    $1, 1652(%rbx)  #, MEM[(struct img_i2c *)_29].need_wr_rd_fence
        movl    timings+48(%rip), %ecx  # timings[1].max_bitrate, pretmp_260
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176:           if
(i2c->bitrate <= timings[i].max_bitrate) {
        jbe     .L59    #,
        cmpl    %ecx, %edx      # pretmp_260, _99
        jbe     .L60    #,
.L61:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1182:
dev_warn(i2c->adap.dev.parent,
        movq    240(%rbx), %rdi # MEM[(struct img_i2c
*)_29].adap.dev.parent, MEM[(struct img_i2c *)_29].adap.dev.parent
        movq    $.LC12, %rsi    #,
        call    dev_warn        #
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187:
i2c->bitrate = timing.max_bitrate;
        movl    timings+48(%rip), %eax  # MEM[(struct img_i2c_timings
*)&timings + 48B], MEM[(struct img_i2c_timings *)&timings + 48B]
        movl    %eax, 1648(%rbx)        # MEM[(struct img_i2c_timings
*)&timings + 48B], MEM[(struct img_i2c *)_29].bitrate
.L60:
        ud2
.L66:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1360:
dev_err(&pdev->dev, "can't request irq %d\n", irq);
        movl    %r13d, %edx     # <retval>,
        movq    $.LC6, %rsi     #,
        movq    %r14, %rdi      # _1,
        call    dev_err #
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1361:           return ret;
        movl    -52(%rbp), %eax # %sfp, _62
        movl    %eax, %r13d     # _62, <retval>
        jmp     .L52    #
.L65:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1341:
dev_err(&pdev->dev, "can't get irq number\n");
        movq    $.LC5, %rsi     #,
        movq    %r14, %rdi      # _1,
        call    dev_err #
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1342:           return irq;
        jmp     .L52    #
.L67:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1162:
dev_info(i2c->adap.dev.parent,
        movzbl  %ah, %ecx       # ret, tmp150
        movq    240(%rbx), %rdi # MEM[(struct img_i2c
*)_29].adap.dev.parent, MEM[(struct img_i2c *)_29].adap.dev.parent
        movl    %eax, %edx      # ret, tmp153
        movl    %ecx, %r8d      # tmp150, tmp150
        movl    %eax, %ecx      # ret, tmp151
        movzbl  %al, %r9d       # ret,
        shrl    $16, %ecx       #, tmp151
        shrl    $24, %edx       #, tmp153
        movq    $.LC11, %rsi    #,
        movzbl  %cl, %ecx       # tmp151, tmp152
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1404:   return ret;
        movl    $-22, %r13d     #, <retval>
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1162:
dev_info(i2c->adap.dev.parent,
        call    _dev_info       #
# /git/arm-soc/include/linux/clk.h:210:         might_sleep();
        xorl    %edx, %edx      #
        movl    $210, %esi      #,
        movq    $.LC0, %rdi     #,
        call    __might_sleep   #
        xorl    %edx, %edx      #
        movl    $210, %esi      #,
        movq    $.LC0, %rdi     #,
        call    __might_sleep   #
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1404:   return ret;
        jmp     .L52    #
.L62:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1332:           return -ENOMEM;
        movl    $-12, %r13d     #, <retval>
        jmp     .L52    #
.L59:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1181:   if
(i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
        cmpl    %ecx, %edx      # pretmp_260, _99
        ja      .L61    #,
        ud2
        .size   img_i2c_probe, .-img_i2c_probe


        .p2align 4,,15
        .type   img_i2c_read_fifo, @function
img_i2c_read_fifo:
1:      call    __fentry__
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:545:    while (i2c->msg.len) {
        cmpw    $0, 1828(%rdi)  #, i2c_10(D)->msg.len
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:544: {
        pushq   %rbp    #
        movq    %rsp, %rbp      #,
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:545:    while (i2c->msg.len) {
        je      .L68    #,
# /git/arm-soc/arch/x86/include/asm/io.h:66: build_mmio_write(writel,
"l", unsigned int, "r", :"memory")
        xorl    %esi, %esi      # tmp118
        movl    $255, %ecx      #, tmp119
        jmp     .L71    #

[toc] | [prev] | [next] | [standalone]


#1590365

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-01 15:50 +0100
Message-ID<tgdJM-4r7-19@gated-at.bofh.it>
In reply to#1590153
On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote:
> >>>
> >>> 3) 0xFC244C03-config:
> >>> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event()
> >>> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section
> >>>
> >>> These look like another bad gcc bug which is truncating functions:
> >>
> >> Same bug for both of them?
> >
> > I ran into this one again today, after updating to the latest gcc-7.0.1:
> >
> > drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool:
> > rxe_responder()+0xfe: sibling call from callable instruction with
> > changed frame pointer
> >
> > Josh, did you get around to updating objtool the last time I reported it, or
> > is it still the same problem? If this is a new variation, I can provide more
> > details about the failure, otherwise I'll just ignore it for now.
> 
> Actually, something must have changed in gcc since last month, I also
> just got a report in another file:
> 
> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe()
> falls through to next function img_i2c_read_fifo()

This one looks like it could be related to some recent objtool changes
which affect how it interprets 'ud2'.  Which commit were you testing
with?  Can you provide the .config file, and the object file if it's not
too big?

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1590524

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-01 18:30 +0100
Message-ID<tggeC-6gA-7@gated-at.bofh.it>
In reply to#1590365
On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 3:40 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote:
> 
> >> Actually, something must have changed in gcc since last month, I also
> >> just got a report in another file:
> >>
> >> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe()
> >> falls through to next function img_i2c_read_fifo()
> >
> > This one looks like it could be related to some recent objtool changes
> > which affect how it interprets 'ud2'.  Which commit were you testing
> > with?  Can you provide the .config file, and the object file if it's not
> > too big?
> 
> This is with my randconfig test series on top of latest linux-next.
> I see it with the latest gcc-7.0.1 snapshot as well as an earlier gcc-7.0.0
> build (20161201), but not with gcc-6.3.1

I wonder if this is another gcc bug.  gcc inserted two ud2 instructions
in img_i2c_probe() for no apparent reason.  Here's one of them:

     5c3:       e8 00 00 00 00          callq  5c8 <img_i2c_probe+0x298>
                        5c4: R_X86_64_PC32      dev_warn-0x4
     5c8:       8b 05 00 00 00 00       mov    0x0(%rip),%eax        # 5ce <img_i2c_probe+0x29e>
                        5ca: R_X86_64_PC32      .data+0xec
     5ce:       89 83 70 06 00 00       mov    %eax,0x670(%rbx)
     5d4:       0f 0b                   ud2

Which corresponds to the following code block:

	if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
		dev_warn(i2c->adap.dev.parent,
			 "requested bitrate (%u) is higher than the max bitrate supported (%u)\n",
			 i2c->bitrate,
			 timings[ARRAY_SIZE(timings) - 1].max_bitrate);
		timing = timings[ARRAY_SIZE(timings) - 1];
		i2c->bitrate = timing.max_bitrate;
	}

I see no apparent reason for the ud2.

Can you rebuild the object with CONFIG_DEBUG_INFO and use addr2line to
see what code lines are associated with the ud2's?

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1590655

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-01 23:10 +0100
Message-ID<tgkBA-12L-21@gated-at.bofh.it>
In reply to#1590524
On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
>> On Wed, Mar 1, 2017 at 3:40 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> > On Wed, Mar 01, 2017 at 10:45:03AM +0100, Arnd Bergmann wrote:
>>
>> >> Actually, something must have changed in gcc since last month, I also
>> >> just got a report in another file:
>> >>
>> >> drivers/i2c/busses/i2c-img-scb.o: warning: objtool: img_i2c_probe()
>> >> falls through to next function img_i2c_read_fifo()
>> >
>> > This one looks like it could be related to some recent objtool changes
>> > which affect how it interprets 'ud2'.  Which commit were you testing
>> > with?  Can you provide the .config file, and the object file if it's not
>> > too big?
>>
>> This is with my randconfig test series on top of latest linux-next.
>> I see it with the latest gcc-7.0.1 snapshot as well as an earlier gcc-7.0.0
>> build (20161201), but not with gcc-6.3.1
>
> I wonder if this is another gcc bug.  gcc inserted two ud2 instructions
> in img_i2c_probe() for no apparent reason.  Here's one of them:
>
>      5c3:       e8 00 00 00 00          callq  5c8 <img_i2c_probe+0x298>
>                         5c4: R_X86_64_PC32      dev_warn-0x4
>      5c8:       8b 05 00 00 00 00       mov    0x0(%rip),%eax        # 5ce <img_i2c_probe+0x29e>
>                         5ca: R_X86_64_PC32      .data+0xec
>      5ce:       89 83 70 06 00 00       mov    %eax,0x670(%rbx)
>      5d4:       0f 0b                   ud2
>
> Which corresponds to the following code block:
>
>         if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
>                 dev_warn(i2c->adap.dev.parent,
>                          "requested bitrate (%u) is higher than the max bitrate supported (%u)\n",
>                          i2c->bitrate,
>                          timings[ARRAY_SIZE(timings) - 1].max_bitrate);
>                 timing = timings[ARRAY_SIZE(timings) - 1];
>                 i2c->bitrate = timing.max_bitrate;
>         }
>
> I see no apparent reason for the ud2.
>
> Can you rebuild the object with CONFIG_DEBUG_INFO and use addr2line to
> see what code lines are associated with the ud2's?

$ addr2line -e drivers/i2c/busses/i2c-img-scb.o
0x5bc
/git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187: i2c->bitrate =
timing.max_bitrate;
0x65d
/git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1181: if (i2c->bitrate >
timings[ARRAY_SIZE(timings) - 1].max_bitrate) {

and from the .s file with line numbers:

.type img_i2c_probe, @function
img_i2c_probe:
.LFB1968:
.loc 1 1323 0
.cfi_startproc
.LVL40:
1: call __fentry__
pushq %rbp #
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
.LBB757:
.LBB758:
# /git/arm-soc/include/linux/device.h:668: return devm_kmalloc(dev,
size, gfp | __GFP_ZERO);
.file 5 "/git/arm-soc/include/linux/device.h"
.loc 5 668 0
movl $21004480, %edx #,
movl $1992, %esi #,
.LBE758:
.LBE757:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1323: {
.loc 1 1323 0
movq %rsp, %rbp #,
.cfi_def_cfa_register 6
pushq %r15 #
pushq %r14 #
.cfi_offset 15, -24
.cfi_offset 14, -32
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1330: i2c =
devm_kzalloc(&pdev->dev, sizeof(struct img_i2c), GFP_KERNEL);
.loc 1 1330 0
leaq 16(%rdi), %r14 #, _1
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1323: {
.loc 1 1323 0
pushq %r13 #
pushq %r12 #
pushq %rbx #
.cfi_offset 13, -40
.cfi_offset 12, -48
.cfi_offset 3, -56
movq %rdi, %r12 # pdev, pdev
subq $24, %rsp #,
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1324: struct
device_node *node = pdev->dev.of_node;
.loc 1 1324 0
movq 864(%rdi), %r15 # pdev_19(D)->dev.of_node, node
.LVL41:
.LBB760:
.LBB759:
# /git/arm-soc/include/linux/device.h:668: return devm_kmalloc(dev,
size, gfp | __GFP_ZERO);
.loc 5 668 0
movq %r14, %rdi # _1,
.LVL42:
call devm_kmalloc #
.LVL43:
.LBE759:
.LBE760:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1331: if (!i2c)
.loc 1 1331 0
testq %rax, %rax # _29
je .L62 #,
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1334: res =
platform_get_resource(pdev, IORESOURCE_MEM, 0);
.loc 1 1334 0
xorl %edx, %edx #
movl $512, %esi #,
movq %r12, %rdi # pdev,
movq %rax, %rbx #, _29
call platform_get_resource #
.LVL44:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base =
devm_ioremap_resource(&pdev->dev, res);
.loc 1 1335 0
movq %r14, %rdi # _1,
.LVL45:
movq %rax, %rsi # res,
call devm_ioremap_resource #
.LVL46:
.LBB761:
.LBB762:
# /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr);
.file 6 "/git/arm-soc/include/linux/err.h"
.loc 6 35 0
xorl %esi, %esi # tmp128
cmpq $-4096, %rax #, _2
.LBE762:
.LBE761:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base =
devm_ioremap_resource(&pdev->dev, res);
.loc 1 1335 0
movq %rax, %r13 #, _2
.LBB766:
.LBB764:
# /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr);
.loc 6 35 0
seta %sil #, tmp128
.LBE764:
.LBE766:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1335: i2c->base =
devm_ioremap_resource(&pdev->dev, res);
.loc 1 1335 0
movq %rax, 1624(%rbx) # _2, MEM[(struct img_i2c *)_29].base
.LBB767:
.LBB765:
.LBB763:
# /git/arm-soc/include/linux/err.h:35: return IS_ERR_VALUE((unsigned long)ptr);
.loc 6 35 0
xorl %edx, %edx #
movq $______f.2078, %rdi #,
call ftrace_likely_update #
.LVL47:
.LBE763:
.LBE765:
.LBE767:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1336: if (IS_ERR(i2c->base))
.loc 1 1336 0
cmpq $-4096, %r13 #, _2
jbe .L54 #,
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1337: return PTR_ERR(i2c->base);
.loc 1 1337 0
movl 1624(%rbx), %r13d # MEM[(struct img_i2c *)_29].base, <retval>
.LVL48:
.L52:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1405: }
.loc 1 1405 0
addq $24, %rsp #,
movl %r13d, %eax # <retval>,
popq %rbx #
.cfi_remember_state
.cfi_restore 3
popq %r12 #
.cfi_restore 12
.LVL49:
popq %r13 #
.cfi_restore 13
popq %r14 #
.cfi_restore 14
popq %r15 #
.cfi_restore 15
.LVL50:
popq %rbp #
.cfi_restore 6
.cfi_def_cfa 7, 8
ret
.LVL51:
.L54:
.cfi_restore_state
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1339: irq =
platform_get_irq(pdev, 0);
.loc 1 1339 0
xorl %esi, %esi #
movq %r12, %rdi # pdev,
call platform_get_irq #
.LVL52:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1340: if (irq < 0) {
.loc 1 1340 0
testl %eax, %eax # <retval>
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1339: irq =
platform_get_irq(pdev, 0);
.loc 1 1339 0
movl %eax, %r13d #, <retval>
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1340: if (irq < 0) {
.loc 1 1340 0
js .L65 #,
.LBB768:
.LBB769:
# /git/arm-soc/include/linux/interrupt.h:173: return
devm_request_threaded_irq(dev, irq, handler, NULL, irqflags,
.file 7 "/git/arm-soc/include/linux/interrupt.h"
.loc 7 173 0
movq (%r12), %r9 # pdev_19(D)->name,
.LBE769:
.LBE768:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1345: i2c->sys_clk =
devm_clk_get(&pdev->dev, "sys");
.loc 1 1345 0
movq $0, 1640(%rbx) #, MEM[(struct img_i2c *)_29].sys_clk
.LBB772:
.LBB770:
# /git/arm-soc/include/linux/interrupt.h:173: return
devm_request_threaded_irq(dev, irq, handler, NULL, irqflags,
.loc 7 173 0
xorl %r8d, %r8d #
.LBE770:
.LBE772:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1351: i2c->scb_clk =
devm_clk_get(&pdev->dev, "scb");
.loc 1 1351 0
movq $0, 1632(%rbx) #, MEM[(struct img_i2c *)_29].scb_clk
.LBB773:
.LBB771:
# /git/arm-soc/include/linux/interrupt.h:173: return
devm_request_threaded_irq(dev, irq, handler, NULL, irqflags,
.loc 7 173 0
xorl %ecx, %ecx #
movq %rbx, (%rsp) # _29,
movq $img_i2c_isr, %rdx #,
movl %eax, %esi # <retval>,
movq %r14, %rdi # _1,
call devm_request_threaded_irq #
.LVL53:
.LBE771:
.LBE773:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1359: if (ret) {
.loc 1 1359 0
testl %eax, %eax # _62
movl %eax, -52(%rbp) # _62, %sfp
jne .L66 #,
.LBB774:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1365:
init_timer(&i2c->check_timer);
.loc 1 1365 0
leaq 1864(%rbx), %rdi #, tmp131
xorl %esi, %esi #
movq $__key.25244, %rcx #,
movq $.LC7, %rdx #,
call init_timer_key #
.LVL54:
.LBE774:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1369: i2c->bitrate =
timings[0].max_bitrate;
.loc 1 1369 0
movl timings+8(%rip), %eax # timings[0].max_bitrate, timings[0].max_bitrate
.LBB775:
.LBB776:
.LBB777:
# /git/arm-soc/include/linux/of.h:458: int ret =
of_property_read_variable_u32_array(np, propname, out_values,
.file 8 "/git/arm-soc/include/linux/of.h"
.loc 8 458 0
leaq -44(%rbp), %rdx #, tmp161
xorl %r8d, %r8d #
.LBE777:
.LBE776:
.LBE775:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1366:
i2c->check_timer.function = img_i2c_check_timer;
.loc 1 1366 0
movq $img_i2c_check_timer, 1888(%rbx) #, MEM[(struct img_i2c
*)_29].check_timer.function
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1367:
i2c->check_timer.data = (unsigned long)i2c;
.loc 1 1367 0
movq %rbx, 1896(%rbx) # _29, MEM[(struct img_i2c *)_29].check_timer.data
.LBB782:
.LBB780:
.LBB778:
# /git/arm-soc/include/linux/of.h:458: int ret =
of_property_read_variable_u32_array(np, propname, out_values,
.loc 8 458 0
movl $1, %ecx #,
movq $.LC8, %rsi #,
movq %r15, %rdi # node,
.LBE778:
.LBE780:
.LBE782:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1369: i2c->bitrate =
timings[0].max_bitrate;
.loc 1 1369 0
movl %eax, 1648(%rbx) # timings[0].max_bitrate, MEM[(struct img_i2c
*)_29].bitrate
.LBB783:
.LBB781:
.LBB779:
# /git/arm-soc/include/linux/of.h:458: int ret =
of_property_read_variable_u32_array(np, propname, out_values,
.loc 8 458 0
call of_property_read_variable_u32_array #
.LVL55:
# /git/arm-soc/include/linux/of.h:460: if (ret >= 0)
.loc 8 460 0
testl %eax, %eax # ret
js .L57 #,
.LVL56:
.LBE779:
.LBE781:
.LBE783:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1371: i2c->bitrate = val;
.loc 1 1371 0
movl -44(%rbp), %eax # val, val
.LVL57:
movl %eax, 1648(%rbx) # val, MEM[(struct img_i2c *)_29].bitrate
.LVL58:
.L57:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1379: i2c->adap.nr = pdev->id;
.loc 1 1379 0
movl 8(%r12), %eax # pdev_19(D)->id, pdev_19(D)->id
.LVL59:
.LBB784:
.LBB785:
# /git/arm-soc/include/linux/spinlock.h:288: return &lock->rlock;
.loc 4 288 0
leaq 1752(%rbx), %rdi #, tmp142
.LBE785:
.LBE784:
.LBB786:
.LBB787:
.LBB788:
# /git/arm-soc/include/linux/device.h:1033: dev->driver_data = data;
.loc 5 1033 0
movq %rbx, 512(%rbx) # _29, MEM[(struct device *)_29 + 240B].driver_data
.LBE788:
.LBE787:
.LBE786:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1374:
i2c->adap.dev.parent = &pdev->dev;
.loc 1 1374 0
movq %r14, 240(%rbx) # _1, MEM[(struct img_i2c *)_29].adap.dev.parent
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1375:
i2c->adap.dev.of_node = node;
.loc 1 1375 0
movq %r15, 1088(%rbx) # node, MEM[(struct img_i2c *)_29].adap.dev.of_node
.LBB789:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383:
spin_lock_init(&i2c->lock);
.loc 1 1383 0
movq $__key.25245, %rdx #,
.LBE789:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1376: i2c->adap.owner
= THIS_MODULE;
.loc 1 1376 0
movq $__this_module, (%rbx) #, MEM[(struct img_i2c *)_29].adap.owner
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1377: i2c->adap.algo =
&img_i2c_algo;
.loc 1 1377 0
movq $img_i2c_algo, 16(%rbx) #, MEM[(struct img_i2c *)_29].adap.algo
.LBB790:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383:
spin_lock_init(&i2c->lock);
.loc 1 1383 0
movq $.LC9, %rsi #,
.LBE790:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1379: i2c->adap.nr = pdev->id;
.loc 1 1379 0
movl %eax, 1280(%rbx) # pdev_19(D)->id, MEM[(struct img_i2c *)_29].adap.nr
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1380:
snprintf(i2c->adap.name, sizeof(i2c->adap.name), "IMG SCB I2C");
.loc 1 1380 0
movabsq $2324494381979487561, %rax #, tmp162
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1378: i2c->adap.retries = 5;
.loc 1 1378 0
movl $5, 236(%rbx) #, MEM[(struct img_i2c *)_29].adap.retries
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1380:
snprintf(i2c->adap.name, sizeof(i2c->adap.name), "IMG SCB I2C");
.loc 1 1380 0
movq %rax, 1284(%rbx) # tmp162, MEM[(void *)_29 + 1284B]
movl $4403785, 1292(%rbx) #, MEM[(void *)_29 + 1284B]
.LBB791:
.LBB792:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:443: i2c->mode = mode;
.loc 1 443 0
movl $0, 1848(%rbx) #, MEM[(struct img_i2c *)_29].mode
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:444: i2c->int_enable =
img_i2c_int_enable_by_mode[mode];
.loc 1 444 0
movq $0, 1852(%rbx) #, MEM[(unsigned int *)_29 + 1852B]
.LBE792:
.LBE791:
.LBB793:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1383:
spin_lock_init(&i2c->lock);
.loc 1 1383 0
call __raw_spin_lock_init #
.LVL60:
.LBE793:
.LBB794:
.LBB795:
.LBB796:
# /git/arm-soc/include/linux/completion.h:79: init_waitqueue_head(&x->wait);
.file 9 "/git/arm-soc/include/linux/completion.h"
.loc 9 79 0
leaq 1664(%rbx), %rdi #, tmp144
.LBE796:
# /git/arm-soc/include/linux/completion.h:78: x->done = 0;
.loc 9 78 0
movl $0, 1656(%rbx) #, MEM[(struct completion *)_29 + 1656B].done
.LBB797:
# /git/arm-soc/include/linux/completion.h:79: init_waitqueue_head(&x->wait);
.loc 9 79 0
movq $__key.8818, %rdx #,
movq $.LC10, %rsi #,
call __init_waitqueue_head #
.LVL61:
.LBE797:
.LBE795:
.LBE794:
.LBB798:
.LBB799:
.LBB800:
.LBB801:
# /git/arm-soc/include/linux/clk.h:191: might_sleep();
.loc 2 191 0
xorl %edx, %edx #
.LBE801:
.LBE800:
.LBE799:
.LBE798:
.LBB805:
.LBB806:
.LBB807:
# /git/arm-soc/include/linux/device.h:1033: dev->driver_data = data;
.loc 5 1033 0
movq %rbx, 288(%r12) # _29, MEM[(struct device *)pdev_19(D) + 16B].driver_data
.LBE807:
.LBE806:
.LBE805:
.LBB808:
.LBB804:
.LBB803:
.LBB802:
# /git/arm-soc/include/linux/clk.h:191: might_sleep();
.loc 2 191 0
movl $191, %esi #,
movq $.LC0, %rdi #,
call __might_sleep #
.LVL62:
.LBE802:
.LBE803:
.LBE804:
.LBE808:
.LBB809:
.LBB810:
.LBB811:
.LBB812:
.LBB813:
.LBB814:
xorl %edx, %edx #
movl $191, %esi #,
movq $.LC0, %rdi #,
call __might_sleep #
.LVL63:
.LBE814:
.LBE813:
.LBE812:
.LBE811:
.LBB815:
.LBB816:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:418: return
readl(i2c->base + offset);
.loc 1 418 0
movq 1624(%rbx), %rax # MEM[(struct img_i2c *)_29].base, MEM[(struct
img_i2c *)_29].base
.LBB817:
.LBB818:
# /git/arm-soc/arch/x86/include/asm/io.h:58: build_mmio_read(readl,
"l", unsigned int, "=r", :"memory")
.loc 3 58 0
#APP
# 58 "/git/arm-soc/arch/x86/include/asm/io.h" 1
movl 128(%rax),%eax # MEM[(volatile unsigned int *)_81], ret
# 0 "" 2
.LVL64:
#NO_APP
.LBE818:
.LBE817:
.LBE816:
.LBE815:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1161: if ((rev &
0x00ffffff) < 0x00020200) {
.loc 1 1161 0
movl %eax, %edx # ret, tmp147
andl $16777215, %edx #, tmp147
cmpl $131583, %edx #, tmp147
jbe .L67 #,
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate
<= timings[i].max_bitrate) {
.loc 1 1176 0
movl 1648(%rbx), %edx # MEM[(struct img_i2c *)_29].bitrate, _99
cmpl timings+8(%rip), %edx # timings[0].max_bitrate, _99
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1171:
i2c->need_wr_rd_fence = true;
.loc 1 1171 0
movb $1, 1652(%rbx) #, MEM[(struct img_i2c *)_29].need_wr_rd_fence
movl timings+48(%rip), %ecx # timings[1].max_bitrate, pretmp_260
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1176: if (i2c->bitrate
<= timings[i].max_bitrate) {
.loc 1 1176 0
jbe .L59 #,
cmpl %ecx, %edx # pretmp_260, _99
jbe .L60 #,
.L61:
.LBB819:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1182:
dev_warn(i2c->adap.dev.parent,
.loc 1 1182 0
movq 240(%rbx), %rdi # MEM[(struct img_i2c *)_29].adap.dev.parent,
MEM[(struct img_i2c *)_29].adap.dev.parent
movq $.LC12, %rsi #,
call dev_warn #
.LVL65:
# /git/arm-soc/drivers/i2c/busses/i2c-img-scb.c:1187: i2c->bitrate =
timing.max_bitrate;
.loc 1 1187 0
movl timings+48(%rip), %eax # MEM[(struct img_i2c_timings *)&timings +
48B], MEM[(struct img_i2c_timings *)&timings + 48B]
movl %eax, 1648(%rbx) # MEM[(struct img_i2c_timings *)&timings + 48B],
MEM[(struct img_i2c *)_29].bitrate
.LVL66:
.L60:
ud2
.LVL67:
.L66:
.LBE819:
.LBE810:
.LBE809:

[toc] | [prev] | [next] | [standalone]


#1590671

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-01 23:50 +0100
Message-ID<tgleh-1ix-9@gated-at.bofh.it>
In reply to#1590524
On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:

> I see no apparent reason for the ud2.

It's the possible division by zero. This change would avoid the ud2:

diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index db8e8b40569d..a2b09c518225 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
        clk_khz /= prescale;

        /* Setup the clock increment value */
+       if (clk_khz < 1)
+               clk_khz = 1;
        inc = (256 * 16 * bitrate_khz) / clk_khz;

        /*


     Arnd

[toc] | [prev] | [next] | [standalone]


#1590762

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-02 02:20 +0100
Message-ID<tgnzr-34a-7@gated-at.bofh.it>
In reply to#1590671
On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> 
> > I see no apparent reason for the ud2.
> 
> It's the possible division by zero. This change would avoid the ud2:
> 
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> index db8e8b40569d..a2b09c518225 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
>         clk_khz /= prescale;
> 
>         /* Setup the clock increment value */
> +       if (clk_khz < 1)
> +               clk_khz = 1;
>         inc = (256 * 16 * bitrate_khz) / clk_khz;
> 
>         /*

Ok, I see what gcc is doing.

	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
	...
	inc = (256 * 16 * bitrate_khz) / clk_khz;

Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
clk_khz is always zero, so the last statement *always* results in a
divide-by-zero.  So that looks like a bug in the code.

However, I'm baffled by how gcc handles it.  Instead of:

  a) reporting a compile-time warning/error; or

  b) letting the #DE (divide error) exception happen;

it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1590863

FromIngo Molnar <mingo@kernel.org>
Date2017-03-02 07:40 +0100
Message-ID<tgsz7-6IB-1@gated-at.bofh.it>
In reply to#1590762
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> > 
> > > I see no apparent reason for the ud2.
> > 
> > It's the possible division by zero. This change would avoid the ud2:
> > 
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> > index db8e8b40569d..a2b09c518225 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> >         clk_khz /= prescale;
> > 
> >         /* Setup the clock increment value */
> > +       if (clk_khz < 1)
> > +               clk_khz = 1;
> >         inc = (256 * 16 * bitrate_khz) / clk_khz;
> > 
> >         /*
> 
> Ok, I see what gcc is doing.
> 
> 	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> 	...
> 	inc = (256 * 16 * bitrate_khz) / clk_khz;
> 
> Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> clk_khz is always zero, so the last statement *always* results in a
> divide-by-zero.  So that looks like a bug in the code.
> 
> However, I'm baffled by how gcc handles it.  Instead of:
> 
>   a) reporting a compile-time warning/error; or
> 
>   b) letting the #DE (divide error) exception happen;
> 
> it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?

Well, technically an invalid opcode is shorter code than generating an (integer) 
division by zero exception, right?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1591090

FromIngo Molnar <mingo@kernel.org>
Date2017-03-02 15:00 +0100
Message-ID<tgzqV-2ZE-1@gated-at.bofh.it>
In reply to#1590863
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote:
> > 
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> > > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> > > > 
> > > > > I see no apparent reason for the ud2.
> > > > 
> > > > It's the possible division by zero. This change would avoid the ud2:
> > > > 
> > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> > > > index db8e8b40569d..a2b09c518225 100644
> > > > --- a/drivers/i2c/busses/i2c-img-scb.c
> > > > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> > > >         clk_khz /= prescale;
> > > > 
> > > >         /* Setup the clock increment value */
> > > > +       if (clk_khz < 1)
> > > > +               clk_khz = 1;
> > > >         inc = (256 * 16 * bitrate_khz) / clk_khz;
> > > > 
> > > >         /*
> > > 
> > > Ok, I see what gcc is doing.
> > > 
> > > 	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> > > 	...
> > > 	inc = (256 * 16 * bitrate_khz) / clk_khz;
> > > 
> > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> > > clk_khz is always zero, so the last statement *always* results in a
> > > divide-by-zero.  So that looks like a bug in the code.
> > > 
> > > However, I'm baffled by how gcc handles it.  Instead of:
> > > 
> > >   a) reporting a compile-time warning/error; or
> > > 
> > >   b) letting the #DE (divide error) exception happen;
> > > 
> > > it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?
> > 
> > Well, technically an invalid opcode is shorter code than generating an (integer) 
> > division by zero exception, right?
> 
> What does that matter if it's the wrong behavior?

Well, both terminate the program, and it's obvious if you look at it with a 
debugger what happened, right?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1591130

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-02 15:30 +0100
Message-ID<tgzTY-3s2-23@gated-at.bofh.it>
In reply to#1591090
On Thu, Mar 02, 2017 at 02:46:29PM +0100, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote:
> > > 
> > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > 
> > > > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> > > > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> > > > > 
> > > > > > I see no apparent reason for the ud2.
> > > > > 
> > > > > It's the possible division by zero. This change would avoid the ud2:
> > > > > 
> > > > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> > > > > index db8e8b40569d..a2b09c518225 100644
> > > > > --- a/drivers/i2c/busses/i2c-img-scb.c
> > > > > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > > > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> > > > >         clk_khz /= prescale;
> > > > > 
> > > > >         /* Setup the clock increment value */
> > > > > +       if (clk_khz < 1)
> > > > > +               clk_khz = 1;
> > > > >         inc = (256 * 16 * bitrate_khz) / clk_khz;
> > > > > 
> > > > >         /*
> > > > 
> > > > Ok, I see what gcc is doing.
> > > > 
> > > > 	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> > > > 	...
> > > > 	inc = (256 * 16 * bitrate_khz) / clk_khz;
> > > > 
> > > > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> > > > clk_khz is always zero, so the last statement *always* results in a
> > > > divide-by-zero.  So that looks like a bug in the code.
> > > > 
> > > > However, I'm baffled by how gcc handles it.  Instead of:
> > > > 
> > > >   a) reporting a compile-time warning/error; or
> > > > 
> > > >   b) letting the #DE (divide error) exception happen;
> > > > 
> > > > it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?
> > > 
> > > Well, technically an invalid opcode is shorter code than generating an (integer) 
> > > division by zero exception, right?
> > 
> > What does that matter if it's the wrong behavior?
> 
> Well, both terminate the program, and it's obvious if you look at it with a 
> debugger what happened, right?

If it were obvious, we wouldn't be having this discussion :-)

The only thing obvious to me was that gcc mysteriously removed a bunch
of code and replaced it with a 'ud2' instruction in the middle of the
function for no apparent reason.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1591148

FromIngo Molnar <mingo@kernel.org>
Date2017-03-02 15:50 +0100
Message-ID<tgAdl-3As-27@gated-at.bofh.it>
In reply to#1591130
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> > > > Well, technically an invalid opcode is shorter code than generating an 
> > > > (integer) division by zero exception, right?
> > > 
> > > What does that matter if it's the wrong behavior?
> > 
> > Well, both terminate the program, and it's obvious if you look at it with a 
> > debugger what happened, right?
> 
> If it were obvious, we wouldn't be having this discussion :-)

Touche ;-)

> The only thing obvious to me was that gcc mysteriously removed a bunch of code 
> and replaced it with a 'ud2' instruction in the middle of the function for no 
> apparent reason.

I don't know what their motivation was, but if it's not a bug, if it was done 
intentionally, then I'd guess it's roughly the argument I made: in simple 
testcases it can be argued to be a code size improvement, plus it's probably 
allowed by the letter of the compiler standards (program termination behavior is 
notoriously platform dependent and thus vaguely specified) - but for real-life 
code I very much agree that it's a step backward in generated code quality...

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1591116

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-02 15:10 +0100
Message-ID<tgzqV-2ZE-3@gated-at.bofh.it>
In reply to#1590863
On Thu, Mar 02, 2017 at 07:31:39AM +0100, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> > > On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> > > 
> > > > I see no apparent reason for the ud2.
> > > 
> > > It's the possible division by zero. This change would avoid the ud2:
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> > > index db8e8b40569d..a2b09c518225 100644
> > > --- a/drivers/i2c/busses/i2c-img-scb.c
> > > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > > @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> > >         clk_khz /= prescale;
> > > 
> > >         /* Setup the clock increment value */
> > > +       if (clk_khz < 1)
> > > +               clk_khz = 1;
> > >         inc = (256 * 16 * bitrate_khz) / clk_khz;
> > > 
> > >         /*
> > 
> > Ok, I see what gcc is doing.
> > 
> > 	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> > 	...
> > 	inc = (256 * 16 * bitrate_khz) / clk_khz;
> > 
> > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> > clk_khz is always zero, so the last statement *always* results in a
> > divide-by-zero.  So that looks like a bug in the code.
> > 
> > However, I'm baffled by how gcc handles it.  Instead of:
> > 
> >   a) reporting a compile-time warning/error; or
> > 
> >   b) letting the #DE (divide error) exception happen;
> > 
> > it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?
> 
> Well, technically an invalid opcode is shorter code than generating an (integer) 
> division by zero exception, right?

What does that matter if it's the wrong behavior?

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1591545

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-03 00:20 +0100
Message-ID<tgIaR-Qo-9@gated-at.bofh.it>
In reply to#1590762
On Thu, Mar 2, 2017 at 2:03 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
>> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
>>
>> > I see no apparent reason for the ud2.
>>
>> It's the possible division by zero. This change would avoid the ud2:
>>
>> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
>> index db8e8b40569d..a2b09c518225 100644
>> --- a/drivers/i2c/busses/i2c-img-scb.c
>> +++ b/drivers/i2c/busses/i2c-img-scb.c
>> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
>>         clk_khz /= prescale;
>>
>>         /* Setup the clock increment value */
>> +       if (clk_khz < 1)
>> +               clk_khz = 1;
>>         inc = (256 * 16 * bitrate_khz) / clk_khz;
>>
>>         /*
>
> Ok, I see what gcc is doing.
>
>         clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
>         ...
>         inc = (256 * 16 * bitrate_khz) / clk_khz;
>
> Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> clk_khz is always zero, so the last statement *always* results in a
> divide-by-zero.  So that looks like a bug in the code.
>
> However, I'm baffled by how gcc handles it.  Instead of:
>
>   a) reporting a compile-time warning/error; or
>
>   b) letting the #DE (divide error) exception happen;
>
> it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?

Just FYI, I found another one like this:

0000000000000000 <hibvt_pwm_get_state>:
   0:   e8 00 00 00 00          callq  5 <hibvt_pwm_get_state+0x5>
                        1: R_X86_64_PC32        __fentry__-0x4
   5:   8b 46 10                mov    0x10(%rsi),%eax
   8:   55                      push   %rbp
   9:   48 89 e5                mov    %rsp,%rbp
   c:   c1 e0 05                shl    $0x5,%eax
   f:   48 03 47 48             add    0x48(%rdi),%rax
  13:   8b 00                   mov    (%rax),%eax
  15:   0f 0b                   ud2
  17:   66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
  1e:   00 00

static inline unsigned long clk_get_rate(struct clk *clk)
{
        return 0;
}

static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
                                struct pwm_state *state)
{
        struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
        void __iomem *base;
        u32 freq, value;

        freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000);
        base = hi_pwm_chip->base;

        value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm));
        state->period = div_u64(value * 1000, freq);

        value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm));
        state->duty_cycle = div_u64(value * 1000, freq);

        value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm));
        state->enabled = (PWM_ENABLE_MASK & value);
}


    Arnd

[toc] | [prev] | [next] | [standalone]


#1591568

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-03 01:00 +0100
Message-ID<tgINA-13L-19@gated-at.bofh.it>
In reply to#1591545
On Thu, Mar 02, 2017 at 11:49:49PM +0100, Arnd Bergmann wrote:
> On Thu, Mar 2, 2017 at 2:03 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Mar 01, 2017 at 11:42:54PM +0100, Arnd Bergmann wrote:
> >> On Wed, Mar 1, 2017 at 5:53 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> > On Wed, Mar 01, 2017 at 04:27:29PM +0100, Arnd Bergmann wrote:
> >>
> >> > I see no apparent reason for the ud2.
> >>
> >> It's the possible division by zero. This change would avoid the ud2:
> >>
> >> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> >> index db8e8b40569d..a2b09c518225 100644
> >> --- a/drivers/i2c/busses/i2c-img-scb.c
> >> +++ b/drivers/i2c/busses/i2c-img-scb.c
> >> @@ -1196,6 +1196,8 @@ static int img_i2c_init(struct img_i2c *i2c)
> >>         clk_khz /= prescale;
> >>
> >>         /* Setup the clock increment value */
> >> +       if (clk_khz < 1)
> >> +               clk_khz = 1;
> >>         inc = (256 * 16 * bitrate_khz) / clk_khz;
> >>
> >>         /*
> >
> > Ok, I see what gcc is doing.
> >
> >         clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> >         ...
> >         inc = (256 * 16 * bitrate_khz) / clk_khz;
> >
> > Because CONFIG_HAVE_CLK isn't set, clk_get_rate() returns 0, which means
> > clk_khz is always zero, so the last statement *always* results in a
> > divide-by-zero.  So that looks like a bug in the code.
> >
> > However, I'm baffled by how gcc handles it.  Instead of:
> >
> >   a) reporting a compile-time warning/error; or
> >
> >   b) letting the #DE (divide error) exception happen;
> >
> > it inserts a 'ud2', resulting in a #UD (invalid opcode).  Why?!?
> 
> Just FYI, I found another one like this:
> 
> 0000000000000000 <hibvt_pwm_get_state>:
>    0:   e8 00 00 00 00          callq  5 <hibvt_pwm_get_state+0x5>
>                         1: R_X86_64_PC32        __fentry__-0x4
>    5:   8b 46 10                mov    0x10(%rsi),%eax
>    8:   55                      push   %rbp
>    9:   48 89 e5                mov    %rsp,%rbp
>    c:   c1 e0 05                shl    $0x5,%eax
>    f:   48 03 47 48             add    0x48(%rdi),%rax
>   13:   8b 00                   mov    (%rax),%eax
>   15:   0f 0b                   ud2
>   17:   66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
>   1e:   00 00
> 
> static inline unsigned long clk_get_rate(struct clk *clk)
> {
>         return 0;
> }
> 
> static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>                                 struct pwm_state *state)
> {
>         struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
>         void __iomem *base;
>         u32 freq, value;
> 
>         freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000);
>         base = hi_pwm_chip->base;
> 
>         value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm));
>         state->period = div_u64(value * 1000, freq);
> 
>         value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm));
>         state->duty_cycle = div_u64(value * 1000, freq);
> 
>         value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm));
>         state->enabled = (PWM_ENABLE_MASK & value);
> }

I assume '-Wdiv-by-zero' is enabled and gcc isn't showing the "division
by zero" warning for either of these?  The 'ud2' is guaranteed to
trigger since the function has no branches.  Surely at least the missing
warning is a gcc bug.

The good news is objtool is flushing these out, albeit with a confusing
message.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1591798

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-03 10:30 +0100
Message-ID<tgRHb-7yR-5@gated-at.bofh.it>
In reply to#1591568
On Fri, Mar 3, 2017 at 12:05 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> I assume '-Wdiv-by-zero' is enabled and gcc isn't showing the "division
> by zero" warning for either of these?  The 'ud2' is guaranteed to
> trigger since the function has no branches.  Surely at least the missing
> warning is a gcc bug.
>
> The good news is objtool is flushing these out, albeit with a confusing
> message.

I'm still not sure if it's intentional or not. I've reduced the test case to the
simple

static inline int return0(void) { return 0; }
int provoke_div0_warning(void) { return 1 / return0(); }

which does not generate a compile-time warning, but will generate
an unconditional runtime warning if built with -fsanitze=integer-divide-by-zero.

     Arnd

[toc] | [prev] | [next] | [standalone]


#1592069

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-03 16:40 +0100
Message-ID<tgXtg-3ca-17@gated-at.bofh.it>
In reply to#1591798
I opened requests on both gcc and llvm, but it looks like there is no
easy way to get a warning here:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79828
https://bugs.llvm.org/show_bug.cgi?id=32126

     Arnd

[toc] | [prev] | [next] | [standalone]


#1590405

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-01 16:30 +0100
Message-ID<tgemt-4Vt-3@gated-at.bofh.it>
In reply to#1590139

[Multipart message — attachments visible in raw view] — view raw

On Wed, Mar 1, 2017 at 3:31 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Mar 01, 2017 at 10:34:42AM +0100, Arnd Bergmann wrote:
>> On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote:
>> >>
>> >> 3) 0xFC244C03-config:
>> >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event()
>> >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section
>> >>
>> >> These look like another bad gcc bug which is truncating functions:
>> >
>> > Same bug for both of them?
>>
>> I ran into this one again today, after updating to the latest gcc-7.0.1:
>>
>> drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool:
>> rxe_responder()+0xfe: sibling call from callable instruction with
>> changed frame pointer
>>
>> Josh, did you get around to updating objtool the last time I reported it, or
>> is it still the same problem? If this is a new variation, I can provide more
>> details about the failure, otherwise I'll just ignore it for now.
>
> This one should have been fixed with:
>
>   3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")

It was on the current linux-next, so that commit should certainly be included.

> Can you attach the object file?

done.

    Arnd

[toc] | [prev] | [next] | [standalone]


#1591389

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-02 19:40 +0100
Message-ID<tgDNT-6hJ-7@gated-at.bofh.it>
In reply to#1590405
On Wed, Mar 01, 2017 at 04:21:36PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 3:31 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Mar 01, 2017 at 10:34:42AM +0100, Arnd Bergmann wrote:
> >> On Tue, Oct 11, 2016 at 10:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote:
> >> >>
> >> >> 3) 0xFC244C03-config:
> >> >> drivers/scsi/fnic/fnic_main.o: warning: objtool: fnic_log_q_error() falls through to next function fnic_handle_link_event()
> >> >> drivers/scsi/snic/snic_res.o: warning: objtool: .text: unexpected end of section
> >> >>
> >> >> These look like another bad gcc bug which is truncating functions:
> >> >
> >> > Same bug for both of them?
> >>
> >> I ran into this one again today, after updating to the latest gcc-7.0.1:
> >>
> >> drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool:
> >> rxe_responder()+0xfe: sibling call from callable instruction with
> >> changed frame pointer
> >>
> >> Josh, did you get around to updating objtool the last time I reported it, or
> >> is it still the same problem? If this is a new variation, I can provide more
> >> details about the failure, otherwise I'll just ignore it for now.
> >
> > This one should have been fixed with:
> >
> >   3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
> 
> It was on the current linux-next, so that commit should certainly be included.
> 
> > Can you attach the object file?

Here's the preliminary fix for this one (still needs more testing):

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index bd12eb1..7b718bb 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -806,11 +806,20 @@ static struct rela *find_switch_table(struct objtool_file *file,
 		     insn->jump_dest->offset > orig_insn->offset))
 		    break;
 
+		/* look for a rela which references .rodata */
 		text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
 						    insn->len);
-		if (text_rela && text_rela->sym == file->rodata->sym)
-			return find_rela_by_dest(file->rodata,
-						 text_rela->addend);
+		if (!text_rela || text_rela->sym != file->rodata->sym)
+			continue;
+
+		/*
+		 * Make sure the .rodata address isn't associated with a
+		 * symbol.  gcc jump tables are anonymous data.
+		 */
+		if (find_symbol_containing(file->rodata, text_rela->addend))
+			continue;
+
+		return find_rela_by_dest(file->rodata, text_rela->addend);
 	}
 
 	return NULL;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 0d7983a..d897702 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -85,6 +85,18 @@ struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset)
 	return NULL;
 }
 
+struct symbol *find_symbol_containing(struct section *sec, unsigned long offset)
+{
+	struct symbol *sym;
+
+	list_for_each_entry(sym, &sec->symbol_list, list)
+		if (sym->type != STT_SECTION &&
+		    offset >= sym->offset && offset < sym->offset + sym->len)
+			return sym;
+
+	return NULL;
+}
+
 struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
 				     unsigned int len)
 {
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index aa1ff65..731973e 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -79,6 +79,7 @@ struct elf {
 struct elf *elf_open(const char *name);
 struct section *find_section_by_name(struct elf *elf, const char *name);
 struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
+struct symbol *find_symbol_containing(struct section *sec, unsigned long offset);
 struct rela *find_rela_by_dest(struct section *sec, unsigned long offset);
 struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
 				     unsigned int len);

[toc] | [prev] | [next] | [standalone]


#1591531

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-03 00:00 +0100
Message-ID<tgHRw-ro-17@gated-at.bofh.it>
In reply to#1591389
On Thu, Mar 2, 2017 at 7:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:

>> It was on the current linux-next, so that commit should certainly be included.
>>
>> > Can you attach the object file?
>
> Here's the preliminary fix for this one (still needs more testing):

It fixes the warning for me.

     Arnd

[toc] | [prev] | [next] | [standalone]


#1591537 — Re: [PATCH] objtool: fix another gcc jump table detection issue

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-03 00:10 +0100
SubjectRe: [PATCH] objtool: fix another gcc jump table detection issue
Message-ID<tgI1c-KT-21@gated-at.bofh.it>
In reply to#1591531
On Thu, Mar 2, 2017 at 11:57 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> Arnd Bergmann reported a (false positive) objtool warning:
>
>   drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0xfe: sibling call from callable instruction with changed frame pointer
>
> The issue is in find_switch_table().  It tries to find a switch
> statement's jump table by walking backwards from an indirect jump
> instruction, looking for a relocation to the .rodata section.  In this
> case it stopped walking prematurely: the first .rodata relocation it
> encountered was for a variable (resp_state_name) instead of a jump
> table, so it just assumed there wasn't a jump table.
>
> The fix is to ignore any .rodata relocation which refers to an ELF
> object symbol.  This works because the jump tables are anonymous and
> have no symbols associated with them.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Tested-by: Arnd Bergmann <arnd@arndb.de>

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web