4

Consider the following code: 

volatile int status;

status = process_package_header(&pack_header, PACK_INFO_CONST);

if ((((status) == (SUCCESS_CONST)) ? ((random_delay() && ((SUCCESS_CONST) == (status))) ? 0 : side_channel_sttack_detected()) : 1))
{
    ...
}

Which generates this machine code (produced with the toolchain's objdump):

  60:   f7ff fffe       bl      0 <process_package_header>
  64:   9000            str     r0, [sp, #0]     /* <- storing to memory as status is volatile */
  66:   42a0            cmp     r0, r4           /* <- where is the load before compare? status is volatile, it could have change between the last store instruction (above line) and now */
  68:   d164            bne.n   134 <func+0x134>
  6a:   f7ff fffe       bl      0 <random_delay>

Now, since status is volatile, it should have been read from memory when the if statement is reached. I would expect to see some load command before comparing it (cmp) to SUCCESS_CONST, regardless the fact that it was assigned with a return value from function process_package_header() and stored in memory, as status is volatile and could have been changed between str instruction and cmp instruction.

Please try to ignore the motivation for the if condition, it's purpose is to try detecting a physical attack on the CPU in which condition flags and registers can be altered externally by a pysical equipment.

Toolchain ARM DS-5_v5.27.0 arm compiler: ARMCompiler5.06u5 (armcc)

Target is ARM CortexM0+ CPU

izac89
  • 3,790
  • 7
  • 30
  • 46
  • 1
    `status` seem to be an automatic variable. Will behavior change if defined as static? I think there is no way of modifying such a variable in a way "not known to the implementation" which will not cause an undefined behavior (Well, maybe there is but I can't think of any). – Eugene Sh. Apr 01 '19 at 14:57
  • can you post more assembler , to see all the places where status is initialized and accessed ? – alinsoar Apr 01 '19 at 14:58
  • 1
    Even if you had a load command, it could have changed just after the load command. So perhaps it should have two load commands in case it changed just after the first one. Now it needs three load commands in case it changed after the second one. You cannot win. So just using the value that was stored by the previous instruction seems no worse than using a load command. – Ian Abbott Apr 01 '19 at 14:59
  • @IanAbbott No, think of it as a hardware register, that has a read value of say, status of a hardware, but write is performing some command. So the read and written values have a completely different meanings. – Eugene Sh. Apr 01 '19 at 15:01
  • @IanAbbott but there is a sequence point after the `status = process_package_header(&pack_header, PACK_INFO_CONST);` line and before the `if` statement, isn't it supposed to mean that volatile should be fetched again? – izac89 Apr 01 '19 at 15:02
  • Is `status` updated between the `cmp` and `op1` in `if (cmp) ? op1:op2 `? – alinsoar Apr 01 '19 at 15:02
  • 1
    you used too many unnecessary parentheses like `(status) == (SUCCESS_CONST)` – phuclv Apr 01 '19 at 15:03
  • 1
    @IanAbbott: `volatile` is not defined to mean the value contained in an object is updated as of the moment it is used but rather that each access in the C model of computation is implemented with an actual access. Therefore a single load suffices. – Eric Postpischil Apr 01 '19 at 15:06
  • @EugeneSh.: A way of modifying an automatic object in a way “not known to the implementation” is to use a debugger. – Eric Postpischil Apr 01 '19 at 15:07
  • @EricPostpischil Good point. I am actually using this compiler (well, some other minor version) and sometimes we do something like `volatile int x = 1; while (x==1);` to have a "break point" in some place in the code and be able to connect with debugger and continue. And yes, it is not always working.. – Eugene Sh. Apr 01 '19 at 15:12
  • @EricPostpischil yes, it is fetched once or twice, taking into account that it is accessed 2 times ? – alinsoar Apr 01 '19 at 15:16
  • 2
    Q: Does a volatile object with automatic storage duration need to have an actual location in memory, or is the compiler allowed to implement it in some non-addressable location such as a register? – Ian Abbott Apr 01 '19 at 15:41
  • @IanAbbott, I have been considering exactly that question. I have so far found nothing in the standard that requires volatile objects in general to reside in addressible storage locations. – John Bollinger Apr 01 '19 at 15:44
  • However, the OP's assembly seems to suggest that his particular volatile object does reside in addressible storage, since there appears to be a write to it. Can that write instead be interpreted as something else? – John Bollinger Apr 01 '19 at 15:46
  • @JohnBollinger, The OP's assembly also clearly shows that this particular volatile object does have automatic storage duration, so the compiler should be free to pretend that it is not stored in an addressable location (unless some other bit of code has a reference to it). – Ian Abbott Apr 01 '19 at 15:52
  • @IanAbbott: As defined in the C standard, an *object* is a ”region of data storage in the execution environment, the contents of which can represent values.” Also, objects are composed of bytes, and a *byte* is an “addressable unit of data storage…” So, in C’s abstract machine, an object must be in addressable memory. There is latitude in how implementations map the abstract machine to physical things, especially that “What constitutes an access to an object that has volatile-qualified type is implementation-defined.” – Eric Postpischil Apr 01 '19 at 19:43
  • Although this seems to be an nonconforming compiler implementation, perhaps creating a pointer to volatile (i.e. `volatile int *pstatus = &status;`) and then accessing the data by dereferencing the pointer would trick the compiler into actually doing the reads. On gcc, it produces the same code as simply reading `volatile int status` directly, but perhaps it would help. Or just switch to gcc/clang. – vgru Apr 01 '19 at 21:06

2 Answers2

4

The main rule governing volatile objects is this, from C11 6.7.3/7:

any expression referring to such an object shall be evaluated strictly according to the rules of the abstract machine, as described in 5.1.2.3. Furthermore, at every sequence point the value last stored in the object shall agree with that prescribed by the abstract machine, except as modified by the unknown factors mentioned previously.

And it goes on to say that

What constitutes an access to an object that has volatile-qualified type is implementation-defined.

, which applies to how other rules (e.g. in 5.1.2.3) are to be interpreted. Your compiler's Users' Guide discusses the details of volatile accesses, but there doesn't seem to be anything surprising there. Section 5.1.2.3 itself mainly talks about sequencing rules; the rules for evaluating expressions are elsewhere (but must still be followed as given with regard to accesses to your volatile object).

Here are the relevant details of the behavior of the abstract machine:

  1. the assignment operation has a side effect of storing a value in the object identified by status. There is a sequence point at the end of that statement, so

    • the side effect is applied before any evaluations appearing in subsequent statements are performed, and
    • because status is volatile, the assignment expressed by that line is the last write to status performed by the program before the sequence point.

  2. the conditional expression in the if statement is evaluated next, with

    • the sub-expression (status) == (SUCCESS_CONST) being evaluated first, before any of the other sub-expressions.
    • Evaluation of status happens before evaluation of the == operation, and
    • takes the form of converting that identifier to the value stored in the object it identifies (lvalue conversion, per paragraph 6.3.2.1/2).
    • In order to do anything with the value stored in status at that time, that value must first be read.

The standard does not require a volatile object to reside in addressable storage, so in principle, your volatile automatic variable could be assigned exclusively to a register. In that event, as long as machine instructions using that object either read its value directly from its register or make updates directly to its register, no separate loads or stores would be required to achieve proper volatile semantics. Your particular object does not appear to fall into this category, however, because the store instruction in your generated assembly seems to indicate that it is, indeed, associated with a location in memory.

Moreover, if the program correctly implemented volatile semantics for an object assigned to a register, then that register would have to be r0. I'm not familiar with the specifics of this assembly language and the processor on which the code runs, but it certainly does not look like r0 is a viable locus for such storage.

With that being the case I agree that status should have been read back from memory, and it should be read back from memory again if its second appearance in the conditional expression needs to be evaluated. This is the behavior of the abstract machine, which conforming implementations exhibit with respect to all volatile accesses. My analysis, then, is that your implementation is non-conforming in this regard, and I would be inclined to report that as a bug.

As for a workaround, I think your best bet as to write the important bits in assembly -- inline assembly if your implementation supports that, or as a complete function implemented in assembly if necessary.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • This may be a separate question, but what do you think should happen if the variable is declared as `register volatile int status;` but the compiler happens to place it in addressable storage rather than a register? Would it need to generate code to read the memory location every time that `status` is evaluated (as an rvalue)? – Ian Abbott Apr 02 '19 at 09:37
  • @IanAbbott, for a `volatile` object, I think it's all about where the implementation *actually* puts the object, regardless of where it *could* have put it. I think that's a direct consequence of following the rules set for the abstract machine. Use of the `register` keyword doesn't make a difference in that regard. – John Bollinger Apr 02 '19 at 12:10
1

The behavior described does not conform to the C standard, barring unconventional interpretations. If the compiler is supposed to conform in this regard, this should be reported as a bug.

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312