Deciphering some RISC-V assembly code

[HwAoRrDk]

I'm trying to decipher the following RISC-V assembly code:

Code: [Select]

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 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:

Code: [Select]

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...

[brucehoult]

Code: [Select]

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.

[SiliconWizard]

Yes, the only way is to qualify it volatile. There is no link between static and volatile, as Bruce explained.
There is no clash between const and volatile either, although that one may look less obvious. const only instructs the compiler that your code can't modify the content of the array. volatile instructs it not to assume anything about what could happen to it outside of your code, and thus will emit code that will access it everywhere you explicitely access it, no matter what.

[golden_labels]

To be even more specific: it’s not even “inaccessibility” of the variable, which makes it be “seen” as zeros. It is because that form of a declaration makes a request to fill it with zeros:⁽¹⁾

(…) If an object that has static or thread storage duration is not initialized explicitly, then:
— if it has pointer type, it is initialized to a null pointer;
— if it has arithmetic type, it is initialized to (positive or unsigned) zero;
— if it is an aggregate, every member is initialized (recursively) according to these rules, and any padding is initialized to zero bits;
— if it is a union, the first named member is initialized (recursively) according to these rules, and any padding is initialized to zero bits;

The language has no idea about the linker or any postprocessing you put the program through. The compiler assumes exactly what is being written. And from its perspective it’s written: generate an array of 4096 octets with unsigned 2’s complement representation, set them all to 0, then calculate CRC32 from them.

Aside from what SiliconWizard said, you can also make it non-static. After all, it is kind of visible to the outside, if you modify it during linking.

⁽¹⁾ 9899:2011 6.7.9§10, repeated in working draft of C2x.

[ejeffrey]

What I generally do here is to declare the symbol extern, then define it in an assembly file or just exporting a symbol directly from the linker script.  This ensures that C initialization rules do not apply.

[SiliconWizard]

What I generally do here is to declare the symbol extern, then define it in an assembly file or just exporting a symbol directly from the linker script.  This ensures that C initialization rules do not apply.

That works as well. The difference with 'static volatile' is a matter of style.
In your case, you make the object visible everywhere, in the case of 'static volatile', it's only visible to the compilation unit it's declared in.
Decide depending on the use case and style.

[HwAoRrDk]

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:

Code: [Select]

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.

[brucehoult]

And the operands for sw being backwards from how you expect - with destination last - still throws me for loop every time.)

RISC-V assembly language operand order for load and store is the same as for every other ISA I know of that has "load" and "store" mnemonics rather than "move". This includes but is not limited to all four Arm ISAs, MIPS, SPARC, PowerPC, AVR.

Where RISC-V differs from those is that the binary encoding consistently puts instruction "source" register always in the same bit field in the instruction if it exists (it doesn't for load) and "destination" register always in the same bit field (different to the "source" bit field) if it exists (it doesn't for store). Others tend to put the "source" register for store and the "destination" register for load in the same bit field, which complicates the hardware.  But this is not visible to the assembly language programmer.

[golden_labels]

In this code volatile is going to work. My gripe, with using it to solve such problems, are the limitations it adds. Most array-operating functions become useless. The program receives a bunch of restrictions, which neither have anything to do with the original intent nor fulfill any purpose. Less people can maintain the fragment of code, as it can no longer be given to less experienced programmers.

I’m not saying volatile is smelly or generally wrong. Just that — if other options are available — I would rather go for them.

[Noloader]

... My gripe, with using it to solve such problems, are the limitations it adds. Most array-operating functions become useless. The program receives a bunch of restrictions, which neither have anything to do with the original intent nor fulfill any purpose. Less people can maintain the fragment of code, as it can no longer be given to less experienced programmers...

I do a fair amount of work with cryptography and security libraries. Between the language specifications (C and C++) and the compiler behaviors (Clang and GCC), it makes it difficult to implement algorithms properly. Nowadays, I feel like it is a crap shot if we are going to get expected code generated by the compilers. It is getting especially bad when C++ is the language. And it is getting even worse with the linker due to LTO.

[golden_labels]

“I made a discovery today. I found a computer. Wait a second, this is cool. It does what I want it to. If it makes a mistake, it's because I screwed it up. Not because it doesn't like me... Or feels threatened by me.. Or thinks I'm a smart ass.. Or doesn't like teaching and shouldn't be here...” — from “The Conscience of a Hacker” by Loyd Blankenship

Compilers almost never generate wrong code. Bugtrackers of GCC and Clang indicate that there are exceptions, but they are very rare and happen primarily with major new feature sets. Almost exclusively it is the programmer having wrong expectations from the code.

With C and C++, as well as any parallelized code, in my experience this is usually caused by failure to understand program’s meaning. Code has meaning attributed to it, which was never in that code. Typically because the programmer never formally learned the tool. Instead the knowledge is replaced with unfounded guesses: by comparison to similar constructs from elsewhere (e.g. basic arithmetic) or incidental behavior observed in the past.

This is not limited to the tools mentioned above, but it is most visible there. In C and C++ it seems to me, that it’s caused by three factors playing together:

• The languages are particularly abstract and detached from hardware, and having very complex model at the same time. This bites hard, if the actual meaning is replaced with guesses. This is also the primary source of security problems attributed to language itself, leading even seemingly experienced programmers (like Linus Torvalds) to madness. The problem is greatly inflated by the lack of easily available sources to study the language, even if you want to devote years of your life to learning just a single tool. In microcontroller world made even worse by vendor’s proprietary tools, which claim to be “flavors of C”, but invent their own meanings.
• Undefined behavior can be found⁽¹⁾ in any Turing-complete imperative or functional language, but C and C++ are  infamous for experiencing it at the most basic level. Explicitly clarifying it in the specs is not really helping, if nobody knows them in the first place.⁽²⁾ The result is, that even a simple snippet of code may not have any defined behavior. In Java I must at least e.g. use java.util.HashSet, add something to it and then try to retrieve to get an UB. In C I can simply add two numbers together. Since it happens at a much more basic level, the consequences may also be much more severe.
• Both languages are very bare and boast high statement-level performance. Which leads to toolchains being focused on optimizing at this level.⁽³⁾ If you have a lot of code with no well-defined behavior at the basic level and optimization focused at the same area, what you get is a lot of behavior that differs from what the programmer imagined it would be. Unlike the previous two, this is not a shortcoming of the language. It’s just probability conspiring against us. While in other languages poorly constrained code has high chance of matching expectations (even if expectations are wrong) and errors being in general mild in consequences, in C and C++ these chances are particularly low and the consequences are often catastrophic.

⁽¹⁾ This is strong “can”, not mere “may”.
⁽²⁾ Even worse, it led to theories representing these clarifications as UB being intentionally added as a feature. From what I observed, usually by misinterpreting the lack of such clarification in other languages’ specs.
⁽³⁾ <snide>Because otherwise the only thing keeping them in business would be the lack of better alternatives and inertia of the industry.</snide>