I'm trying to decipher the following RISC-V assembly code:
000003d0 <verify_crc>:
3d0: cd1ff2ef jal t0,a0 <__riscv_save_0>
3d4: 6405 lui s0,0x1
3d6: 87aa mv a5,a0
3d8: 1171 add sp,sp,-4
3da: 84ae mv s1,a1
3dc: 1471 add s0,s0,-4 # ffc <main+0x190>
3de: 557d li a0,-1
3e0: /-> 4581 li a1,0
3e2: | c03e sw a5,0(sp)
3e4: | 147d add s0,s0,-1
3e6: | 37e9 jal 3b0 <crc32_update>
3e8: | 4782 lw a5,0(sp)
3ea: \-- f87d bnez s0,3e0 <verify_crc+0x10>
3ec: fff54513 not a0,a0
3f0: /----- c399 beqz a5,3f6 <verify_crc+0x26>
3f2: | 0007a023 sw zero,0(a5)
3f6: \--/-X c091 beqz s1,3fa <verify_crc+0x2a>
3f8: | c088 sw a0,0(s1)
3fa: \-> 00153513 seqz a0,a0
3fe: 0111 add sp,sp,4
400: b16d j aa <__riscv_restore_0>
Further to my discovery in another thread (https://www.eevblog.com/forum/microcontrollers/generating-a-uuid/msg5185059/#msg5185059) of a technique whereby I can specify a static constant is assigned to its own custom output section, and then the contents of that section updated with that of an arbitrary external file post-build (using objcopy), I decided to employ this technique on another project where I previously had a 4KB blob included as an array in my C source.
I have some code that runs at startup which verifies that this blob's data is not corrupt, by checking a CRC32 checksum. That function's disassembled code is above, and the corresponding C code is as follows:
static const uint8_t data[4096] __attribute__((aligned(64), section(".my_data"), used, retain));
bool verify_crc(uint32_t *crc_expect, uint32_t *crc_calc) {
uint32_t crc_data, crc_new;
// Read the CRC embedded in the last 4 bytes of the data.
crc_data = *(uint32_t *)&data[(sizeof(data) / sizeof(data[0])) - sizeof(uint32_t)];
// Calculate CRC for all of the data except the last 4 bytes.
crc_new = crc32_init(); // Actually a macro that resolves to 0xFFFFFFFF
for(size_t i = 0; i < ((sizeof(data) / sizeof(data[0])) - sizeof(uint32_t)); i++) {
crc_new = crc32_update(crc_new, data[i]);
}
crc_new = crc32_final(crc_new); // Another macro that resolves to arg ^ 0xFFFFFFFF
// Output the expected and calculated CRCs if pointers given.
if(crc_expect != NULL) *crc_expect = crc_data;
if(crc_calc != NULL) *crc_calc = crc_new;
return (crc_new == crc_data);
}
The problem is that I am not sure the assembly code is actually doing what it's supposed to any more after this linker section change. It seems to me via my RISC-V-noob eyes that the assembly code will never calculate the CRC32 correctly, because it appears to just call crc32_update() with a fixed second argument of zero ("li a1,0") on every iteration of the loop! Is this correct? If this is true, I guess the compiler is assuming that the array is full of zeroes because it doesn't have any initialisation values.
How can I fix this? How can I tell the compiler not to assume that the data array contains any specific values? I guess I could slap volatile on the declaration, but that somehow seems wrong to me for static const...
static const uint8_t data[4096] __attribute__((aligned(64), section(".my_data"), used, retain));
The static instructs the compiler that the name data is not to be exported from this compilation unit, and this a different variable than any variable called data in some other compilation unit.
Therefore, yes, the compiler knows that it contains all 0s.
Nothing at all to do with RISC-V, that's just C.
Yes, I know that an un-initialised static variable will be initialised to zero. I had a feeling that if the assembly did indeed show that it was using a fixed value of zero, that's where it came from. I wanted to confirm that my interpretation of the assembly code was correct. I'm not especially familiar with RISC-V assembly code... yet. :) (And the operands for sw being backwards from how you expect - with destination last - still throws me for loop every time.)
Adding a volatile qualifier seems to solve the problem. I don't think I will make it extern (which is of course, saying this var is defined elsewhere - why that wasn't my first thought I don't know, duh!), because that'll involve messing about with, as suggested, either assembly files or diving into the linker script, neither of which I particularly feel like doing.
The compiled assembly code for the function is now:
000003d0 <verify_crc>:
3d0: cd1ff2ef jal t0,a0 <__riscv_save_0>
3d4: 6689 lui a3,0x2
3d6: 00068613 mv a2,a3
3da: 6785 lui a5,0x1
3dc: 97b2 add a5,a5,a2
3de: 6685 lui a3,0x1
3e0: ffc7a403 lw s0,-4(a5) # ffc <main+0x138>
3e4: 872a mv a4,a0
3e6: 1151 add sp,sp,-12
3e8: 84ae mv s1,a1
3ea: 4781 li a5,0
3ec: 557d li a0,-1
3ee: 16f1 add a3,a3,-4 # ffc <main+0x138>
3f0: /-> 00f605b3 add a1,a2,a5
3f4: | 0005c583 lbu a1,0(a1)
3f8: | c436 sw a3,8(sp)
3fa: | c23a sw a4,4(sp)
3fc: | c03e sw a5,0(sp)
3fe: | 3f4d jal 3b0 <crc32_update>
400: | 4782 lw a5,0(sp)
402: | 46a2 lw a3,8(sp)
404: | 6709 lui a4,0x2
406: | 0785 add a5,a5,1
408: | 00070613 mv a2,a4
40c: | 4712 lw a4,4(sp)
40e: \-- fed791e3 bne a5,a3,3f0 <verify_crc+0x20>
412: fff54793 not a5,a0
416: /-- c311 beqz a4,41a <verify_crc+0x4a>
418: | c300 sw s0,0(a4)
41a: /--\-X c091 beqz s1,41e <verify_crc+0x4e>
41c: | c09c sw a5,0(s1)
41e: \----> 40f40533 sub a0,s0,a5
422: 00153513 seqz a0,a0
426: 0131 add sp,sp,12
428: b149 j aa <__riscv_restore_0>
I think it's doing the right thing now. I think that "lui a3,0x2" is loading a value of 0x2000 into a3 ('loading upper' is shifting the immediate value 12 places into the higher bits, right?), which corresponds with the address where the linker map says the new section is located. And then it seems to grab the crc_data value by adding 0x1000 to that and loading from a -4 offset from that address.
Thanks all. :-+