On 28.07.23 21:19, Jarkko Sakkinen wrote:
So, when exactly is it optimized away by the compiler? This is missing.
The problem is that declaring encl_buf as static, implies that it will
only be used in this file and the compiler is allowed to optimize away
any entries that are never used within this compilation unit (e.g., when
optimizing out the memcpy calls).
In reality, the tests outside test_encl.elf rely on both the size and
exact placement of encl_buf at the start of .data.
For example, clang -Os generates the following (legal) code when
encl_bug is declared as static:
0000000000001020 <do_encl_op_put_to_buf>:
mov 0x8(%rdi),%al
mov %al,0x1fd7(%rip) # 3000 <encl_buffer.0>
mov 0x9(%rdi),%al
mov %al,0x8fce(%rip) # a000 <encl_buffer.1.0>
mov 0xa(%rdi),%al
mov %al,0x8fd5(%rip) # a010 <encl_buffer.1.1>
mov 0xb(%rdi),%al
mov %al,0x8fce(%rip) # a012 <encl_buffer.1.2>
mov 0xc(%rdi),%al
mov %al,0x8fd3(%rip) # a020 <encl_buffer.1.3>
mov 0xd(%rdi),%al
mov %al,0x8fce(%rip) # a024 <encl_buffer.1.4>
mov 0xe(%rdi),%al
mov %al,0x8fd1(%rip) # a030 <encl_buffer.1.5>
mov 0xf(%rdi),%al
mov %al,0x8fca(%rip) # a032 <encl_buffer.1.6>
ret
Disassembly of section .data:
0000000000003000 <encl_buffer.0>:
3000: 01 00
...
0000000000004000 <encl_ssa_tcs1>:
Thus, this proposed patch fixes both the size and location:
1. removing the static keyword from the encl_bug declaration ensures
that the _entire_ buffer is preserved with expected size, as the
compiler cannot anymore assume encl_buf is only used in this file.
2. adding _attribute__((section(".data.encl_buffer"))) ensures that we
can control the expected location at the start of the .data section. I
think this is optional, as encl_buf always seems to be placed at the
start of .data in all my tests. But afaik this is not guaranteed as per
the C standard and such constraints on exact placement should better be
explicitly controlled in the linker script(?)
