Paul Eggert wrote:
> > Copying and then discarding an uninitialized value of integer
> > or pointer type (i.e. not a signalling NaN (*)) is not undefined behaviour.
>
> Although that's how typical implementations behave, the C standard says
> that copying from a source variable has undefined behavior if the source
> could have been declared with 'register' (i.e., it's auto and never has
> its address taken). See C23 §6.3.2.1 paragraph 2. Since the code in
> question is copying from such a source, its behavior is technically
> undefined.
I disagree. The essential sentence of this paragraph is:
"If the lvalue designates an object of automatic storage duration
that could have been declared with the register storage class
(never had its address taken),
and that object is uninitialized (not declared with an initializer
and no assignment to it has been performed prior to use),
the behavior is undefined."
When this sentence is applied to the entire struct, it makes no sense.
Which you can see
1) by considering this example:
typedef struct { int x; int y; } vector;
vector func (...)
{
vector result;
result.x = 17;
result.y = 42;
return result;
}
The object 'result' is not declared with an initializer,
and no assignment to it exists. Therefore it is "uninitialized"
here, and the behaviour ought to be undefined.
2) because this paragraph only has the concept of "uninitialized",
not of "partially initialized" or "partially uninitialized".
And when this paragraph is applied to one of the members of the struct,
it talks about expressions that occur in the program's code. But
result.i, result.j, result.count do NOT occur in the program's code —
they are unused.
In summary, this paragraph makes no sense for structs.
> Proposed patch attached.
I would agree with such a patch if
* it was the programmer's intent to initialize all members, and
* the function gets inlined (because that's the experience you made
with the 'mcel' thing: that 'return (struct ...) { initializers }'
produces particularly good code with modern compilers).
But here, the struct has different uses for different implementations
of gl_list_t.
- It is not my intent to initialize all members, since the iterator
for lists of ARRAY type is only used with lists of this type, and
so on.
- The function does not get inlined, because its address gets stored
in a function pointer table and only ever gets used through this
table.
So, no, this patch is not an improvement.
Out of curiosity, I looked at the code that compilers produce for
the function 'gl_tree_iterator' (in the current state, without the
patch). I created a testdir of module 'avltree-list', then did
$ gcc -I. -I.. -Wall -O2 -S -fno-dwarf2-cfi-asm -fverbose-asm
gl_avltree_list.c
Find the detailed results below. The summary is:
- gcc initializes the "uninitialized" members anyway, at least since gcc 4.2.
- In gcc 4.2, they are initialized with random data; since gcc 4.3, they
are zero-initialized.
- gcc 4.2 to 4.4 nevertheless gives warnings "is used uninitialized",
although it fills in a value to initialize the members with.
- clang does not do so; it leaves the members uninitialized.
Bruno
================================================================================
gcc version 4.2.4
gl_anytree_list2.h: In function ‘gl_tree_iterator’:
gl_anytree_list2.h:603: warning: ‘result.j’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.i’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.count’ is used uninitialized in this
function
.L57:
movl %eax, 24(%eax) # result$j, <result>.j
movl %eax, 20(%eax) # result$i, <result>.i
movl %eax, 8(%eax) # result$count, <result>.count
movl $0, 16(%eax) #, <result>.q
movl %ecx, 12(%eax) # node, <result>.p
movl %ebx, 4(%eax) # list, <result>.list
movl %esi, (%eax) # result$vtable, <result>.vtable
popl %ebx #
popl %esi #
popl %ebp #
ret $4 #
gcc version 4.3.6
gl_anytree_list2.h: In function ‘gl_tree_iterator’:
gl_anytree_list2.h:603: warning: ‘result.j’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.i’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.count’ is used uninitialized in this
function
.L43:
movl $0, 24(%eax) #, <result>.j
movl $0, 20(%eax) #, <result>.i
movl $0, 8(%eax) #, <result>.count
movl $0, 16(%eax) #, <result>.q
movl %ecx, 12(%eax) # node, <result>.p
movl %ebx, 4(%eax) # list, <result>.list
movl %esi, (%eax) # D.3061, <result>.vtable
popl %ebx #
popl %esi #
popl %ebp #
ret $4 #
gcc version 4.4.7
gl_anytree_list2.h: In function ‘gl_tree_iterator’:
gl_anytree_list2.h:603: warning: ‘result.j’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.i’ is used uninitialized in this
function
gl_anytree_list2.h:603: warning: ‘result.count’ is used uninitialized in this
function
.L45:
movl $0, 24(%eax) #, <result>.j
movl $0, 20(%eax) #, <result>.i
movl $0, 8(%eax) #, <result>.count
movl $0, 16(%eax) #, <result>.q
movl %ecx, 12(%eax) # node, <result>.p
movl %ebx, 4(%eax) # list, <result>.list
movl %esi, (%eax) # D.3444, <result>.vtable
popl %ebx #
popl %esi #
popl %ebp #
ret $4 #
gcc version 4.5.4
.L50:
movl %esi, (%eax) # D.3899, <retval>.vtable
movl %ebx, 4(%eax) # list, <retval>.list
movl $0, 8(%eax) #, <retval>.count
movl %ecx, 12(%eax) # node, <retval>.p
movl $0, 16(%eax) #, <retval>.q
movl $0, 20(%eax) #, <retval>.i
movl $0, 24(%eax) #, <retval>.j
popl %ebx #
popl %esi #
popl %ebp #
ret $4 #
gcc version 4.6.4
movl %esi, (%eax) # D.2978, <retval>.vtable
movl %ebx, 4(%eax) # list, <retval>.list
movl $0, 8(%eax) #, MEM[(struct *)&<retval>].count
movl %ecx, 12(%eax) # node, <retval>.p
movl $0, 16(%eax) #, <retval>.q
movl $0, 20(%eax) #, MEM[(struct *)&<retval>].i
movl $0, 24(%eax) #, MEM[(struct *)&<retval>].j
popl %ebx #
popl %esi #
ret $4 #
gcc version 4.7.3
.L28:
movl %esi, (%eax) # D.3123, <retval>.vtable
movl %ebx, 4(%eax) # list, <retval>.list
movl $0, 8(%eax) #, MEM[(struct *)&<retval>].count
movl %ecx, 12(%eax) # node, <retval>.p
movl $0, 16(%eax) #, <retval>.q
movl $0, 20(%eax) #, MEM[(struct *)&<retval>].i
movl $0, 24(%eax) #, MEM[(struct *)&<retval>].j
popl %ebx #
popl %esi #
ret $4 #
gcc version 4.8.5
.L28:
movl %esi, (%eax) # D.3866, MEM[(struct *)&<retval>]
movl %ebx, 4(%eax) # list, MEM[(struct *)&<retval> + 4B]
movl $0, 8(%eax) #, MEM[(struct *)&<retval> + 8B]
movl %ecx, 12(%eax) # node, MEM[(struct *)&<retval> + 12B]
movl $0, 16(%eax) #, MEM[(struct *)&<retval> + 16B]
movl $0, 20(%eax) #, MEM[(struct *)&<retval> + 20B]
movl $0, 24(%eax) #, MEM[(struct *)&<retval> + 24B]
popl %ebx #
popl %esi #
ret $4 #
gcc version 4.9.4
.L24:
movq %rdi, (%rax) # D.4237, MEM[(struct *)&<retval>]
movq %rsi, 8(%rax) # list, MEM[(struct *)&<retval> + 8B]
movq $0, 16(%rax) #, MEM[(struct *)&<retval> + 16B]
movq %rcx, 24(%rax) # node, MEM[(struct *)&<retval> + 24B]
movq $0, 32(%rax) #, MEM[(struct *)&<retval> + 32B]
movq $0, 40(%rax) #, MEM[(struct *)&<retval> + 40B]
movq $0, 48(%rax) #, MEM[(struct *)&<retval> + 48B]
ret
gcc version 5.5, 6.5
.L24:
movq %rdi, (%rax) # _5, MEM[(struct *)&<retval>]
movq %rsi, 8(%rax) # list, MEM[(struct *)&<retval> + 8B]
movq $0, 16(%rax) #, MEM[(struct *)&<retval> + 16B]
movq %rcx, 24(%rax) # node, MEM[(struct *)&<retval> + 24B]
movq $0, 32(%rax) #, MEM[(struct *)&<retval> + 32B]
movq $0, 40(%rax) #, MEM[(struct *)&<retval> + 40B]
movq $0, 48(%rax) #, MEM[(struct *)&<retval> + 48B]
ret
gcc version 7.5, 8.5, 9.5
# gl_anytree_list2.h:603: return result;
movq %rdi, (%rax) # _1, MEM[(struct *)&<retval>]
movq %rsi, 8(%rax) # list, MEM[(struct *)&<retval> + 8B]
movq $0, 16(%rax) #, MEM[(struct *)&<retval> + 16B]
movq %rcx, 24(%rax) # node, MEM[(struct *)&<retval> + 24B]
movq $0, 32(%rax) #, MEM[(struct *)&<retval> + 32B]
movq $0, 40(%rax) #, MEM[(struct *)&<retval> + 40B]
movq $0, 48(%rax) #, MEM[(struct *)&<retval> + 48B]
# gl_anytree_list2.h:604: }
ret
gcc version 10.5
# gl_anytree_list2.h:603: return result;
movq %rsi, 8(%r8) # list, <retval>.list
movq $0, 16(%r8) #, <retval>.count
movq %rdx, 24(%r8) # node, <retval>.p
movq $0, 32(%r8) #, <retval>.q
movq $0, 40(%r8) #, <retval>.i
movq $0, 48(%r8) #, <retval>.j
# gl_anytree_list2.h:604: }
ret
gcc version 11.5
# gl_anytree_list2.h:603: return result;
movq %rdi, (%rax) # _1, <retval>.vtable
movq %rsi, 8(%rax) # list, <retval>.list
movq $0, 16(%rax) #, <retval>.count
movq %rcx, 24(%rax) # node, <retval>.p
movq $0, 32(%rax) #, <retval>.q
movq $0, 40(%rax) #, <retval>.i
movq $0, 48(%rax) #, <retval>.j
# gl_anytree_list2.h:604: }
ret
gcc version 12.4, 13.3, 14.2
# gl_anytree_list2.h:603: return result;
movq $0, 16(%rax) #, <retval>.count
movq %rcx, 24(%rax) # node, <retval>.p
movq $0, 32(%rax) #, <retval>.q
movq $0, 40(%rax) #, <retval>.i
movq $0, 48(%rax) #, <retval>.j
movups %xmm0, (%rax) # _21, MEM <vector(2) long unsigned int> [(void
*)&<retval>]
# gl_anytree_list2.h:604: }
ret
clang 15, 16
.LBB23_3:
movq %rcx, 24(%rax)
movq $0, 32(%rax)
retq
clang 17, 18
.LBB23_2:
movq %rcx, 24(%rax)
movq $0, 32(%rax)
retq
