3
\$\begingroup\$

When implementing interrupt-driven applications I usually create a bitfield to keep track of different interrupts. For example:

volatile struct {
    unsigned char ISR0: 1;
    unsigned char ISR1: 1;
    ...
    unsigned char ISR7: 1;
} ISRstatus;

The rest of the application might have the following structure:

ISR(ISR0) {
    // set status flag
    ISRstatus.ISR0 = 1;
}

void main() {
    while(1) {
        if (ISRstatus.ISR0){
            // serve interrupt
            /* ... */
            // clear status flag    
            ISRstatus.ISR0 = 0;
        } /* ... */
    }
}

Recently I came across a few articles that suggested avoiding bitfields due to their unexpected behavior across different compilers and architectures.

Assuming my compiler is GCC, is it a bad idea to use this approach?

\$\endgroup\$
5
  • 3
    \$\begingroup\$ What happens when the interrupt is raised a second time immediately before the line that clears ISR0? \$\endgroup\$
    – CL.
    Aug 29, 2015 at 19:01
  • 1
    \$\begingroup\$ Good point. Status flag should be cleared at the beginning of the if-statement. \$\endgroup\$
    – Ashton H.
    Aug 29, 2015 at 22:49
  • 2
    \$\begingroup\$ It's still possible for the interrupt to happen between reading and clearing the flag. You must disable interrupts during the test-and-clear operation. \$\endgroup\$
    – CL.
    Aug 30, 2015 at 6:05
  • 2
    \$\begingroup\$ I'll have to disagree. In time-critical application the situation you described would certainly lead to data loss. Disabling and re-enabling interrupts will only introduce more overhead, but won't help preserving data. \$\endgroup\$
    – Ashton H.
    Aug 30, 2015 at 7:28
  • \$\begingroup\$ It all depends on the nature of the interrupt... how to handle an interrupt is hardware-specific. \$\endgroup\$
    – Lundin
    Aug 31, 2015 at 11:53

7 Answers 7

1
\$\begingroup\$

CORRECTION: As Ben (and other commenters) have pointed out, clearing the status flag in the main code is a problem. Writes to bit fields are normally implemented as a read-modify-write, where (in your case) the full byte is read, then one bit is set or cleared, then the modified byte is written back. In pseudo-code, ISRstatus.ISR0 = 0 would become:

char temp = ISRstatus;
temp &= ~0x01;
ISRstatus = temp;

The problem here is that an interrupt can come in the middle of this sequence. For example, let's say that the ISR0 flag is set and interrupt 5 comes in. What happens is:

<interrupt 0>
    ISRstatus |= 0x01;  //Not really atomic, but it doesn't matter here
<exit interrupt 0>

if (ISRstatus.ISR0)
{
    char temp = ISRstatus;
    temp &= ~0x01;
    <interrupt 5>
        ISRstatus |= 0x20;  //Not really atomic, but it doesn't matter here
    <exit interrupt 5>
    ISRstatus = temp;
}

In this example, ISRstatus should be equal to 0x20 after the if statement, but instead it's equal to 0x00. The ISR5 flag got lost.

The way to fix this is to disable interrupts when writing to the global variable in your main code. (Reads are safe as long as the entire structure is loaded at once, which it should be for an 8-bit structure.)

The C standard does not guarantee any particular ordering or packing of bit fields. This means that using bit fields to access data stored in a specific format (like register or packet header fields) is not portable. If there's only one compiler for your CPU, portability won't be a problem, so you can get away with it.

My reading of the standard is that bit fields are intended to be used in exactly the way you're using them. Just keep the limitations in mind.

EDIT v2: The compiler probably won't let a single bit field cross a storage unit boundary. Your compiler manual should have more information, but it might take some trial and error to figure out the edge cases. Since all you care about is the data in the individual fields and not their arrangement within the storage unit, this shouldn't matter.

All that being said, portability is usually not a huge concern for interrupt code, and it's unlikely for a compiler to change the way it handles bit fields in a newer version.

\$\endgroup\$
10
  • \$\begingroup\$ Out of curiosity: what happens if I use more than 8 bits? I assume the compiler will automatically occupy another byte of memory and align the 'excessive' bits. Would that cause any problems? \$\endgroup\$
    – Ashton H.
    Aug 29, 2015 at 17:11
  • \$\begingroup\$ I updated my answer. \$\endgroup\$
    – Adam Haun
    Aug 29, 2015 at 17:24
  • \$\begingroup\$ Thank you. I didn't specify the architecture since I use this approach with different processors (8 and 32 bit). I'm not sure about the limit you are referring to. According to the standard implementation may allocate any addressable storage unit large enough to hold a bit-field. If insufficient space remains, whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined[source]. I assume there is no size limit for the bit field. \$\endgroup\$
    – Ashton H.
    Aug 29, 2015 at 17:36
  • 2
    \$\begingroup\$ @AshtonH. Almost all aspects of bit fields is implementation defined as the standard goes, so the implementation must specify the details, otherwise cannot claim to be conforming. So, as long as you have the docs of the compiler and you don't mind change some code (maybe with #if directives) to cope with portability across different architectures, you'll be fine. Note however that you might write better (more maintainable code) using unsigned ints. See also this questions on SO. \$\endgroup\$ Aug 29, 2015 at 18:12
  • \$\begingroup\$ I stand corrected. Even C89 allows bit field structures to be larger than a storage unit. I could've sworn I remembered seeing compiler error for that once, but maybe it was for misalignment. Regardless, your code shouldn't cause a problem. I fixed my answer. \$\endgroup\$
    – Adam Haun
    Aug 29, 2015 at 19:00
1
\$\begingroup\$

Indeed, bit fields are very poorly specified and the only purpose you can safely use them for is for "chunks of boolean flags" (see this for examples why you shouldn't use bit fields).

Even still, there is no point in using bit fields for that purpose either, because there are better ways. In your case:

typedef uint8_t ISRstatus;

volatile ISRstatus status;

if(status & (1 << isr_n))
{
  // flag is set
}
else
{
  // flag is not set
}

With optimizations on, this should boil down to the very same machine code (a bit check/bit set). The advantages of the above is:

  • 100% portable between compilers, microcontrollers and systems. The bit order is guaranteed, there's no padding, no nonsense, the code turns endianess-independent.
  • 1 << n is the embedded industry de facto standard for accessing a bit in C.
  • Allows more complex things such as if(status & ISR_MASK_1_4) where ISR_MASK_1_4 would be 0x0F.
\$\endgroup\$
1
  • \$\begingroup\$ Unfortunately, this still isn't a safe way to share flags between main program and ISR. Only atomic operations can be safely used on shared variables, and volatile does not make arbitrary operations atomic. \$\endgroup\$
    – Ben Voigt
    Jul 25, 2017 at 20:43
1
\$\begingroup\$

Whether you set and clear bits manually or with a bitfield, this could fail if used on a microcontroller which doesn't have atomic instructions for setting and clearing individual bits.

Under these circumstances, setting and clearing bits would require a read-modify-write, which in the main code would provide an opportunity for an interrupt to occur between the read and write, clearing a bit which has just been set before it can be checked.

So using the above code as a basis, the following sequence could occur:

  1. Interrupt 0 fires, and sets ISR0.
  2. Main code checks ISR0, and starts to execute relevant code.
  3. Main code reaches line which clears ISR0, and reads ISRstatus into register.
  4. Interrupt 1 fires, and sets ISR1.
  5. Main code continues, clearing ISR0 in register. ISR1 is already zero from the earlier read.
  6. Value in register is written back to ISRstatus, causing ISR1 (and ISR0) to be cleared.

If interrupts can interrupt each other, this could also happen when an ISR bit is set.

So, unless instructions exist for atomically setting and clearing bits (and the compiler uses them), it would be safer to use separate storage for these values.


As mentioned in a comment to the original question, it would be safer to clear the bit at the start of the code, not the end.

Even safer is not to have the same variable written by both an interrupt and the main code. For example, a common way of dealing with data transmission is to have a circular buffer, with a write pointer incremented in the main code and a read pointer incremented in the interrupt (and vice versa for data reception).

\$\endgroup\$
0
\$\begingroup\$

a better implementation would be a active counter for each interrupt source.

The interrupt handler would increment the appropriate active counter.

The main() would save an initial counter value. then during each loop, if the saved counter does not match the active counter, then perform the operation, then increment the saved counter..

That way no interrupt event would be lost

Note:

If the interrupt is occurring faster than the interrupt handler can execute and return, then nothing could stop the interrupt from being missed.

\$\endgroup\$
0
\$\begingroup\$

With avr-gcc (i.e. GCC for 8bit AVR microcontrollers) I would expect the bitfield to be stored in an int. An int is 16 bit by default in avr-gcc, while the word size the processor can read and store atomically is 8 bit.

With this case in mind, using a uint8_t value with #defined constants and the usual bit manipulations would appear to be more portable.

\$\endgroup\$
0
\$\begingroup\$

Assuming my compiler is GCC, is it a bad idea to use this approach?

Any man made approaches will have its pluses and minuses. In this particular case you should think about cases where the value of isr0 may change in an I consistent way. I would assert that you don't have such a case here.

\$\endgroup\$
0
\$\begingroup\$

Many platforms have ways of atomically updating individual bits within a byte, but they accomplish that in different ways. Although the Standard allows bitfields to be used within volatile-qualified storage, the only context in which that would be useful would be when all writes to the struct containing the bitfield are done within the same "thread" (or interrupt context). Outside of that limited usage pattern, the only way to get known semantics on most platforms is to use manual bit manipulation along with platforms' specific ways of achieving the required semantics.

For example, the Cortex-M3 supports LDREX/STREX instructions, which are mapped to compiler intrinsics. If code uses LDREX to read a word, compute a new value, and then STREX to attempt to write the new value to the same word, the STREX will succeed (and report success) if no interrupt had occurred after the LDREX and nothing could have disturbed the word in question, or it will fail (and report failure) without writing anything. If code wants to set the lower bit in a uint32_t volatile foo, it could do something like:

void set_bit_in_foo(void)
{
  uint32_t old_value;
  do
    old_value = __LDREXW(&foo);
  while(!__STREXW(old_value | 1, &foo));
}

If nothing accesses foo between the __LDREXW and the __STREXW, and no interrupts occur between them, the new value will be written to foo and __STREXW will return 1. Otherwise, __STREXW will return 1 and code will try again using an updated value of foo.

Other platforms have other ways of allowing atomic updates. C11 attempts to add features for atomic updates for platforms that support them, but is poorly specified in some key ways (an implementation that claims to offer any support for atomic operations cannot reject programs that require functions which the implementation can't support in useful fashion, but must instead provide broken implementations for those functions).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.