Bug 862620 - "may be used uninitialized" warning at odds with assembly
Summary: "may be used uninitialized" warning at odds with assembly
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 17
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-10-03 11:54 UTC by Paul Bolle
Modified: 2012-10-03 19:15 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-10-03 13:12:24 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
test case (derived form Linux' drivers/base/regmap/regmap.c (1.63 KB, text/x-c)
2012-10-03 12:33 UTC, Paul Bolle
no flags Details
output of gdb disassemble /m regmap_raw_read (5.89 KB, text/plain)
2012-10-03 12:57 UTC, Paul Bolle
no flags Details

Description Paul Bolle 2012-10-03 11:54:12 UTC
Description of problem:
gcc warns that a variable "may be used uninitialized in" a function, though the generated code implies that this variable is always used initialized.

Version-Release number of selected component (if applicable):
gcc-4.7.2-2.fc17.x86_64

How reproducible:
Compile regmap.o (from drivers/base/regmap/regmap.c and related files, as shipped with linux kernel v3.6.0.

Steps to Reproduce:
1. Obvious
2.
3.
  
Actual results:
drivers/base/regmap/regmap.c: In function ‘regmap_raw_read’:
    drivers/base/regmap/regmap.c:1172:6: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Expected results:
No warning.

Additional info:
0) See discussion (starting) at https://lkml.org/lkml/2012/9/30/64 .

1) Filed here because I'm not sure how much Fedora's gcc differs from the upstream releases it's built from.

2) To substantiate my claim I hope to add a (single file) test case of sorts and some gdb disassembly output shortly.

Comment 1 Paul Bolle 2012-10-03 12:33:07 UTC
Created attachment 620821 [details]
test case (derived form Linux' drivers/base/regmap/regmap.c

0) This file contains:
    regmap_volatile_range(); and
    regmap_raw_read()

both copied verbatim from drivers/base/regmap/regmap.c (from Linux v3.6.0). Besides it contains the minimum necessary to compile those functions.

1) compile with:
    gcc -O2 -c -Wuninitialized regmap_raw_read.c -o regmap_raw_read.o

Comment 2 Paul Bolle 2012-10-03 12:57:29 UTC
Created attachment 620841 [details]
output of gdb disassemble /m regmap_raw_read

0) If one takes just the disassembler output from this file, and sorts it, one gets:
   0x0000000000000000 <+0>:	push   %r15
   0x0000000000000002 <+2>:	mov    %rcx,%rax
   0x0000000000000005 <+5>:	push   %r14
   0x0000000000000007 <+7>:	push   %r13
   0x0000000000000009 <+9>:	push   %r12
   0x000000000000000b <+11>:	push   %rbp
   0x000000000000000c <+12>:	push   %rbx
   0x000000000000000d <+13>:	mov    %rdi,%rbx
   0x0000000000000010 <+16>:	sub    $0x38,%rsp
   0x0000000000000014 <+20>:	mov    0x10(%rdi),%r14
   0x0000000000000018 <+24>:	mov    %rdx,0x18(%rsp)
   0x000000000000001d <+29>:	xor    %edx,%edx
   0x000000000000001f <+31>:	mov    %rcx,0x8(%rsp)
   0x0000000000000024 <+36>:	div    %r14
   0x0000000000000027 <+39>:	test   %rdx,%rdx
   0x000000000000002a <+42>:	mov    %rax,0x10(%rsp)
   0x000000000000002f <+47>:	jne    0x121 <regmap_raw_read+289>
   0x0000000000000035 <+53>:	xor    %edx,%edx
   0x0000000000000037 <+55>:	mov    %esi,%eax
   0x0000000000000039 <+57>:	mov    %esi,%ebp
   0x000000000000003b <+59>:	divl   0x20(%rdi)
   0x000000000000003e <+62>:	mov    $0xffffffea,%r13d
   0x0000000000000044 <+68>:	test   %edx,%edx
   0x0000000000000046 <+70>:	jne    0x10f <regmap_raw_read+271>
   [...]
   0x000000000000010f <+271>:	add    $0x38,%rsp
   0x0000000000000113 <+275>:	mov    %r13d,%eax
   0x0000000000000116 <+278>:	pop    %rbx
   0x0000000000000117 <+279>:	pop    %rbp
   0x0000000000000118 <+280>:	pop    %r12
   0x000000000000011a <+282>:	pop    %r13
   0x000000000000011c <+284>:	pop    %r14
   0x000000000000011e <+286>:	pop    %r15
   0x0000000000000120 <+288>:	retq   
   0x0000000000000121 <+289>:	mov    $0xffffffea,%r13d
   0x0000000000000127 <+295>:	jmp    0x10f <regmap_raw_read+271>

1) From the above (and the entire output gdb.txt) it follows that regmap_raw_read() puts its return value in %eax (unsurprising, I guess).

2) That return value is taken from %r13d. In the above code it is clear that %r13d will contain -EINVAL (-22 or 0xffffffea in two's compliment) at offset 289 if the first test fails:
    if (val_len % map->format.val_bytes)
        return -EINVAL;

But if that test succeeds %r13d will always be set to -EINVAL at offset 62 (that is, before the second test is actually run).

3) So it seems that in the code that is generated 'ret' will actually always be used initialized (to -EINVAL).

Of course, I know little about GCC, so it could be that the warning is issued while it is not yet clear what code will be generated.

Comment 3 Jakub Jelinek 2012-10-03 13:12:24 UTC
I don't consider this a bug.
If regmap_volatile_range isn't inlined, then the compiler can't know that
for val_len == 0 regmap_volatile_range will return true and thus ret can't be uninitialized (because then the for loop would loop 0 times and ret would be uninitialized).  Even when it is inlined, it isn't necessarily of the easy form the compiler can recognize for predicate aware uninitialized warnings (the problem is NP-complete, so only some most common cases like:
int var;
if (some_condition)
  var = something;
...
if (some_condition)
  use (var);
are recognized).

Comment 4 Jakub Jelinek 2012-10-03 19:01:43 UTC
BTW, important issue on your testcase is also that regmap_volatile_range
doesn't use the same type for the count (uses unsigned int) as the caller (unsigned long int), so even after jump threading the conditions are different.  If you use unsigned long int num argument of regmap_volatile_range, the warning goes away.

Comment 5 Paul Bolle 2012-10-03 19:15:32 UTC
(In reply to comment #4)
> If you use unsigned long int num argument of regmap_volatile_range, the warning
> goes away.

0) Correct. (Well, I used size_t, but that's basically the same.) Great find!

1) How would you like to be involved in a patch fixing this? Would you like to author a patch or should I draft a patch and mention you?


Note You need to log in before you can comment on or make changes to this bug.