[llvm-bugs] [Bug 39707] New: Wrong condition evaluation
via llvm-bugs
llvm-bugs at lists.llvm.org
Mon Nov 19 08:48:39 PST 2018
https://bugs.llvm.org/show_bug.cgi?id=39707
Bug ID: 39707
Summary: Wrong condition evaluation
Product: libraries
Version: 6.0
Hardware: PC
OS: All
Status: NEW
Severity: release blocker
Priority: P
Component: Backend: X86
Assignee: unassignedbugs at nondot.org
Reporter: royger at freebsd.org
CC: craig.topper at gmail.com, llvm-bugs at lists.llvm.org,
llvm-dev at redking.me.uk, spatel+llvm at rotateright.com
Hello,
While compiling Xen with clang version 6.0.1 (tags/RELEASE_601/final 335540)
(based on LLVM 6.0.1) on amd64 I came across the following bug. Given this C
code:
static void init_heap_pages(
struct page_info *pg, unsigned long nr_pages)
{
unsigned long i;
bool idle_scrub = false;
/*
* Some pages may not go through the boot allocator (e.g reserved
* memory at boot but released just after --- kernel, initramfs,
* etc.).
* Update first_valid_mfn to ensure those regions are covered.
*/
spin_lock(&heap_lock);
first_valid_mfn = mfn_min(page_to_mfn(pg), first_valid_mfn);
spin_unlock(&heap_lock);
if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
idle_scrub = true;
for ( i = 0; i < nr_pages; i++ )
{
unsigned int nid = phys_to_nid(page_to_maddr(pg+i));
if ( unlikely(!avail[nid]) )
{
unsigned long s = mfn_x(page_to_mfn(pg + i));
unsigned long e = mfn_x(mfn_add(page_to_mfn(pg + nr_pages - 1),
1));
bool_t use_tail = (nid == phys_to_nid(pfn_to_paddr(e - 1))) &&
!(s & ((1UL << MAX_ORDER) - 1)) &&
(find_first_set_bit(e) <= find_first_set_bit(s));
unsigned long n;
n = init_node_heap(nid, mfn_x(page_to_mfn(pg + i)), nr_pages - i,
&use_tail);
BUG_ON(i + n > nr_pages);
if ( n && !use_tail )
{
i += n - 1;
continue;
}
if ( i + n == nr_pages )
break;
nr_pages -= n;
}
free_heap_pages(pg + i, 0, scrub_debug || idle_scrub);
}
}
llvm generates the following assembly:
Dump of assembler code for function init_heap_pages:
0xffff82d080223900 <+0>: push %rbp
0xffff82d080223901 <+1>: mov %rsp,%rbp
0xffff82d080223904 <+4>: push %r15
0xffff82d080223906 <+6>: push %r14
0xffff82d080223908 <+8>: push %r13
0xffff82d08022390a <+10>: push %r12
0xffff82d08022390c <+12>: push %rbx
0xffff82d08022390d <+13>: sub $0x28,%rsp
0xffff82d080223911 <+17>: mov %rsi,%r15
0xffff82d080223914 <+20>: mov %rdi,%rbx
0xffff82d080223917 <+23>: movabs $0x7d2000000000,%r12
0xffff82d080223921 <+33>: lea 0x267d08(%rip),%r14 #
0xffff82d08048b630 <heap_lock>
0xffff82d080223928 <+40>: mov %r14,%rdi
0xffff82d08022392b <+43>: callq 0xffff82d08023a4c0 <_spin_lock>
0xffff82d080223930 <+48>: mov %rbx,-0x40(%rbp)
0xffff82d080223934 <+52>: lea (%rbx,%r12,1),%rdi
0xffff82d080223938 <+56>: sar $0x5,%rdi
0xffff82d08022393c <+60>: callq 0xffff82d0802249c0 <pdx_to_pfn>
0xffff82d080223941 <+65>: mov %rax,%rdi
0xffff82d080223944 <+68>: callq 0xffff82d080222340 <_mfn>
0xffff82d080223949 <+73>: mov 0x267cd8(%rip),%rsi #
0xffff82d08048b628 <first_valid_mfn>
0xffff82d080223950 <+80>: mov %rax,%rdi
0xffff82d080223953 <+83>: callq 0xffff82d080222310 <mfn_min>
0xffff82d080223958 <+88>: mov %rax,0x267cc9(%rip) #
0xffff82d08048b628 <first_valid_mfn>
0xffff82d08022395f <+95>: mov %r14,%rdi
0xffff82d080223962 <+98>: callq 0xffff82d08023a530 <_spin_unlock>
0xffff82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) #
0xffff82d08059e9a0 <system_state>
0xffff82d08022396e <+110>: setb -0x29(%rbp)
0xffff82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) #
0xffff82d08044c404 <opt_bootscrub>
0xffff82d080223979 <+121>: sete %al
0xffff82d08022397c <+124>: mov %r15,-0x38(%rbp)
0xffff82d080223980 <+128>: test %r15,%r15
0xffff82d080223983 <+131>: je 0xffff82d080223aee <init_heap_pages+494>
0xffff82d080223989 <+137>: and %al,-0x29(%rbp)
0xffff82d08022398c <+140>: xor %r12d,%r12d
0xffff82d08022398f <+143>: jmpq 0xffff82d080223a70 <init_heap_pages+368>
0xffff82d080223994 <+148>: mov -0x38(%rbp),%rax
0xffff82d080223998 <+152>: shl $0x5,%rax
0xffff82d08022399c <+156>: add -0x40(%rbp),%rax
0xffff82d0802239a0 <+160>: movabs $0x7d2000000000,%rcx
0xffff82d0802239aa <+170>: lea (%rcx,%rax,1),%rdi
0xffff82d0802239ae <+174>: add $0xffffffffffffffe0,%rdi
0xffff82d0802239b2 <+178>: sar $0x5,%rdi
0xffff82d0802239b6 <+182>: callq 0xffff82d0802249c0 <pdx_to_pfn>
0xffff82d0802239bb <+187>: mov %rax,%rdi
0xffff82d0802239be <+190>: callq 0xffff82d080222340 <_mfn>
0xffff82d0802239c3 <+195>: mov %rax,%rdi
0xffff82d0802239c6 <+198>: callq 0xffff82d080224ff0 <mfn_add>
0xffff82d0802239cb <+203>: mov %rax,%rdi
0xffff82d0802239ce <+206>: callq 0xffff82d080222dd0 <mfn_x>
0xffff82d0802239d3 <+211>: mov %rax,-0x48(%rbp)
0xffff82d0802239d7 <+215>: shl $0xc,%rax
0xffff82d0802239db <+219>: lea -0x1000(%rax),%rdi
0xffff82d0802239e2 <+226>: callq 0xffff82d0802238d0 <phys_to_nid>
0xffff82d0802239e7 <+231>: xor %ecx,%ecx
0xffff82d0802239e9 <+233>: test $0x3ffff,%r13d
0xffff82d0802239f0 <+240>: jne 0xffff82d080223a11 <init_heap_pages+273>
0xffff82d0802239f2 <+242>: cmp %al,%r14b
0xffff82d0802239f5 <+245>: jne 0xffff82d080223a11 <init_heap_pages+273>
0xffff82d0802239f7 <+247>: mov -0x48(%rbp),%rdi
0xffff82d0802239fb <+251>: callq 0xffff82d080225010 <find_first_set_bit>
0xffff82d080223a00 <+256>: mov %eax,%r14d
0xffff82d080223a03 <+259>: mov %r13,%rdi
0xffff82d080223a06 <+262>: callq 0xffff82d080225010 <find_first_set_bit>
0xffff82d080223a0b <+267>: cmp %eax,%r14d
0xffff82d080223a0e <+270>: setbe %cl
0xffff82d080223a11 <+273>: mov %cl,-0x2a(%rbp)
0xffff82d080223a14 <+276>: mov -0x38(%rbp),%r14
0xffff82d080223a18 <+280>: mov %r14,%rdx
0xffff82d080223a1b <+283>: sub %r12,%rdx
0xffff82d080223a1e <+286>: mov %r15d,%edi
0xffff82d080223a21 <+289>: mov %r13,%rsi
0xffff82d080223a24 <+292>: lea -0x2a(%rbp),%rcx
0xffff82d080223a28 <+296>: callq 0xffff82d080225030 <init_node_heap>
0xffff82d080223a2d <+301>: lea (%rax,%r12,1),%rcx
0xffff82d080223a31 <+305>: cmp %r14,%rcx
0xffff82d080223a34 <+308>: ja 0xffff82d080223afd <init_heap_pages+509>
0xffff82d080223a3a <+314>: test %rax,%rax
0xffff82d080223a3d <+317>: je 0xffff82d080223a53 <init_heap_pages+339>
0xffff82d080223a3f <+319>: movzbl -0x2a(%rbp),%edx
0xffff82d080223a43 <+323>: test %dl,%dl
0xffff82d080223a45 <+325>: jne 0xffff82d080223a53 <init_heap_pages+339>
0xffff82d080223a47 <+327>: add %rax,%r12
0xffff82d080223a4a <+330>: add $0xffffffffffffffff,%r12
0xffff82d080223a4e <+334>: jmpq 0xffff82d080223ae4 <init_heap_pages+484>
0xffff82d080223a53 <+339>: cmp -0x38(%rbp),%rcx
0xffff82d080223a57 <+343>: je 0xffff82d080223aee <init_heap_pages+494>
0xffff82d080223a5d <+349>: sub %rax,-0x38(%rbp)
0xffff82d080223a61 <+353>: jmp 0xffff82d080223aca <init_heap_pages+458>
0xffff82d080223a63 <+355>: data16 data16 data16 nopw %cs:0x0(%rax,%rax,1)
0xffff82d080223a70 <+368>: mov %r12,%rbx
0xffff82d080223a73 <+371>: shl $0x5,%rbx
0xffff82d080223a77 <+375>: add -0x40(%rbp),%rbx
0xffff82d080223a7b <+379>: movabs $0x7d2000000000,%rax
0xffff82d080223a85 <+389>: lea (%rbx,%rax,1),%rdi
0xffff82d080223a89 <+393>: sar $0x5,%rdi
0xffff82d080223a8d <+397>: callq 0xffff82d0802249c0 <pdx_to_pfn>
0xffff82d080223a92 <+402>: mov %rax,%rdi
0xffff82d080223a95 <+405>: callq 0xffff82d080222340 <_mfn>
0xffff82d080223a9a <+410>: mov %rax,%rdi
0xffff82d080223a9d <+413>: callq 0xffff82d080222dd0 <mfn_x>
0xffff82d080223aa2 <+418>: mov %rax,%r13
0xffff82d080223aa5 <+421>: mov %r13,%rdi
0xffff82d080223aa8 <+424>: shl $0xc,%rdi
0xffff82d080223aac <+428>: callq 0xffff82d0802238d0 <phys_to_nid>
0xffff82d080223ab1 <+433>: mov %eax,%r14d
0xffff82d080223ab4 <+436>: movzbl %r14b,%r15d
0xffff82d080223ab8 <+440>: lea 0x37cc81(%rip),%rax #
0xffff82d0805a0740 <avail>
0xffff82d080223abf <+447>: cmpq $0x0,(%rax,%r15,8)
0xffff82d080223ac4 <+452>: je 0xffff82d080223994 <init_heap_pages+148>
0xffff82d080223aca <+458>: cmpb $0x0,0x251aff(%rip) #
0xffff82d0804755d0 <scrub_debug>
0xffff82d080223ad1 <+465>: setne %al
0xffff82d080223ad4 <+468>: or -0x29(%rbp),%al
0xffff82d080223ad7 <+471>: movzbl %al,%edx
0xffff82d080223ada <+474>: xor %esi,%esi
0xffff82d080223adc <+476>: mov %rbx,%rdi
0xffff82d080223adf <+479>: callq 0xffff82d080223270 <free_heap_pages>
0xffff82d080223ae4 <+484>: add $0x1,%r12
0xffff82d080223ae8 <+488>: cmp %r12,-0x38(%rbp)
0xffff82d080223aec <+492>: ja 0xffff82d080223a70 <init_heap_pages+368>
0xffff82d080223aee <+494>: add $0x28,%rsp
0xffff82d080223af2 <+498>: pop %rbx
0xffff82d080223af3 <+499>: pop %r12
0xffff82d080223af5 <+501>: pop %r13
0xffff82d080223af7 <+503>: pop %r14
0xffff82d080223af9 <+505>: pop %r15
0xffff82d080223afb <+507>: pop %rbp
0xffff82d080223afc <+508>: retq
0xffff82d080223afd <+509>: ud2
End of assembler dump.
The code at <+114> is wrong, because llvm is evaluating the second part of the
if condition regardless of the result of the first part of the condition:
if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
Is translated into:
0xffff82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) #
0xffff82d08059e9a0 <system_state>
0xffff82d08022396e <+110>: setb -0x29(%rbp)
0xffff82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) #
0xffff82d08044c404 <opt_bootscrub>
0xffff82d080223979 <+121>: sete %al
This is wrong, the second part of the condition should never be evaluated if
the first part is already false.
--
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20181119/8348e9f6/attachment-0001.html>
More information about the llvm-bugs
mailing list