Bug 862620 - "may be used uninitialized" warning at odds with assembly
"may be used uninitialized" warning at odds with assembly
Product: Fedora
Classification: Fedora
Component: gcc (Show other bugs)
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Jakub Jelinek
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2012-10-03 07:54 EDT by Paul Bolle
Modified: 2012-10-03 15:15 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2012-10-03 09:12:24 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

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

  None (edit)
Description Paul Bolle 2012-10-03 07:54:12 EDT
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):

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
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 08:33:07 EDT
Created attachment 620821 [details]
test case (derived form Linux' drivers/base/regmap/regmap.c

0) This file contains:
    regmap_volatile_range(); and

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 08:57:29 EDT
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 09:12:24 EDT
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 15:01:43 EDT
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 15:15:32 EDT
(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.