Making a function safe for use in an ISR

So during an interview for an embedded software position, I was asked to write any function I wanted in C. I chose an iterative factorial function like this:

int iterative_factorial(int i) { int f; f = 1;

while (i != 0) { f = i * f; i--; } return(f); }

I was then asked what I would have to worry about or fix to make the function safe to use in an ISR. I really could not think of anything wrong with this function as it seems reentrant and I don't see any concurrency issues. Can anyone think of anything that might make this function unsafe to use in an ISR?

Thanks,

Josh

Reply to
joshc
Loading thread data ...

Such a stupid request should've been answered with:

void any_function (void) { }

...then let them base any further questions on that function...

--
Mark McDougall, Engineer
Virtual Logic Pty Ltd, 
21-25 King St, Rockdale, 2216
Ph: +612-9599-3255 Fax: +612-9599-3266
Reply to
Mark McDougall

Exceptions of all types are usually handled with a totally separate stack from mainline code. It's not expected that vast amounts of data will be saved on the stack inside an ISR - you want to get in quickly, and get out quickly.

Also, in many architectures, entering an ISR disables interrupts automatically. Often, to simplify concurrency analysis, firmware will intentionally not re-enable interrupts inside an ISR. (It can be really hard to analyze nested interrupts and debug any problems you encounter). Calling this function under those circumstances could send the processor off for a long, long time and impact realtime foreground tasks.

Therefore I would not use an iterative function inside an ISR unless the entry conditions were constrained to prevent stack blowout, and the latency issues were very well understood.

Reply to
larwe

anythingExceptions of all types are usually handled with a totally separate

...

Are you confusing iterative and recursive when you talk of stack blowout? I don't see how entry conditions would affect the stack usage of this _iterative_ function.

Thanks for the other points though.

Reply to
joshc

You're thinking like a high-level language programmer. ;)

I'll not give you an answer, but some pointers to think about.

Consider what the most complex operation in that function is, and how you'd implement it on a CPU that lacks native support for that operation. Look at code-generation strategies for compilers on 'baby micros' and you'll probably start to see why this function will probably be reentrant on a 'big' CPU but (unless the compiler-writers have been competent and generous to you) not on some small ones.

I certainly know of (and have debugged customer code, and yelled at compiler vendors for failing to document things properly!) on C implementations where that code would break if reentered.

Also, from a determinism point of view, I'd bounce any code with O(N) runtime that someone tried to put in an ISR. Activate a task to do the computation and get the hell out of the ISR ASAP!

pete

--
pete@fenelon.com "it made about as much sense as a polythene sandwich"
Reply to
Pete Fenelon

As long as you perform an analysis to make sure that the time spent in that function does not exceed the limit for that interrupt, there's nothing intrinsically wrong. Since the time depends on the value of 'i', you'll need to determine and or limit its maximum value.

Reply to
Arlet

Le Wed, 24 Jan 2007 19:44:50 -0800, joshc a écrit :

One never should be let say anything obviously stupid ! So I would have answered simply that this kind of function should not never be called in an ISR. It is besides what occurs in the true life.

Habib.

Reply to
habib.bouaziz-viallet

  1. If you're going to call the function inside the ISR, I would hope that it's being done because the result is needed in the ISR. Can you then afford to delay the completion of the ISR while waiting for the task to run?
  2. Just assuming that you have an RTOS and tasks to activate may immediately take you out of the baby micro area.

Mark Borgerson

Reply to
Mark Borgerson

I've seen code destined for (cough... a very major American auto manufacturer...) written by (cough... a very major semiconductor manufacturer...) that basically ran an entire engine controller either in the idle task or in interrupt handlers. They were a little dismayed when it fell over at around 1800rpm. Which is about as much use as a chocolate teapot. Working out what needs to be in the idle task, what needs to be in interrupts and what needs to be in tasks. ;)

It may; then again it may not. I've been involved in putting RTOSes on devices as small as the PIC18 and Freescale HCS08; I know companies that have gone even lower (8051, PIC16...)

pete

--
pete@fenelon.com "it made about as much sense as a polythene sandwich"
Reply to
Pete Fenelon

Hello,

You did not say what kind of ISR. But I could think of the following things which should be considered:

  • Is the ISR a high level ISR or written in assembler? If it is in assembler you would have to make sure that when you call the C functions that all registers which are possibly modified by that ISR are stored on the stack. I had some of this issues with assembler written ISRs which did not push all modified registers on the stack. Since most ISRs are high level nowadays this might not be that common anymore.
  • If your compiler generates code for some instructions which make use of global variables. I don't see any of this in there but for example if you start using floating point and there is no hardware floating point implementation available this could be an issue. Anyhow using floats in an ISR is not a good idea.
  • As Arlet pointed out WCET is certainly an issue with this function and needs special consideration.

Kind regards, Christian Walter

--
\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\ \\\\\\ \\
email: wolti@sil.at      \ Friends don't let friends drink and su(1)
web: www.freemodbus.org  \               -- Kevin Harris
Reply to
Christian Walter

Right. I plead exhaustion. I was actually fixing an issue with an exploding recursive function at the time I wrote that posting, so it was uppermost in my mind.

Reply to
larwe

As long as all variables are of storage class automatic and there are no references to global variables or hardware, this function is already ISR-safe.

Basic assumptions: 1)the compiler you're using maintains stack frames and keeps everything on the stack, 2)your development tool chain doesn't have any other requirements ... such as special keywords that change which registers are saved/restored, etc.

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)
Reply to
David T. Ashley

As written the function is "unsafe" in many contexts within an embedded app (particularly so in an ISR where "get in and out quickly" is often important to maintain system timing) :

- i is signed, int may be 8,16,32 (maybe 64?) bits in size

- So, called with i=-1, the function goes completely wrong and executes for potentially many loops

- It may be safe, for example, if it is static and the whole module is reviewed to make sure this doesn't happen

- If not, I'd generally recommend checking for acceptable parameters

- I often code such things as __tError Function(int nPara, int *pnResult), returning an error code (e.g. PARAMETER_OUT_OF_RANGE) and "returning" the result by a pointer

Reply to
Gary Pace

This function can not be a stand-alone ISR (with register save preamble), since ISRs do not in general have input parameters or return any function results, so apparently this example function is called from some higher level interrupt service code.

If this is supposed to be standard C, then int can not be 8 bits.

Someone pointed out that the ISR could execute for a very long time. However, in this case this would not be a critical thing for legal values.

7! = 0x13B0 Legal 16 bit signed int 8! = 0x9D80 Legal 16 bit only as unsigned 9! = 0x58980 Does not fit into 16 bit 12! = 0x1C8CFC00 Legal 32 bit signed int 13! = 0x17328CC00 Does not fit into 32 bits

So for 16 bit int, at most 7 iterations would have to be performed, while the result f fits into int and with 32 bit int, 12 iterations are required and with 64 bit int, 20 iterations would be performed at most.

However, multiplications and divisions should be avoided in ISRs, especially if these are implemented in software or implemented as microcode with execution times several times larger than ordinary instructions.

Floating point instructions should be avoided even if the instruction set implements these instructions, since saving and restoring the floating point control/status and actual data registers would take a huge time.

While the 8087 style floating point processor (used in most x86 processors) contains an internal stack for internal values, this is practically useless for ISRs, since it contains only 8 slots and could be used to 7-8 slots already when the ISR occurs, so executing floating point operations in the ISR without first saving the stack could easily overflow the stack.

On later Pentium models, the FPU stack is used as MMX data registers and the processor might be in MMX mode when the interrupt occurs, switching it to floating point mode in the ISR takes a very long time as well as restoring the MMX state after the interrupt execution.

Paul

Reply to
Paul Keinanen

Paul Keinanen wrote: [snip]

These points are valid enough for X86 architectures. However the OP specified an "embedded" application, so it is unlikely a Pentium would be used (it might be, of course :) Many other CPUs have multiply and FP instructions that act directly on the CPU registers, so there would be no inherent bar to using them in interrupts. Of course, one would want to check the specs for just how long they take.

Reply to
David R Brooks

There is a quite large number of small form factor processor boards using mobile Pentiums from Intel. There are other processors from other manufacturers running Pentium code (of course these processors can not be called Pentium due to trade mark reasons) that fit into the PC/104 board. These small boards are used frequently in various embedded applications.

IMHO, the greatest problem in using x86 processors in embedded applications is the short production run, typically 1-2 years, while some products using embedded processors may be produced for years and may require support for 10-20 years.

Apart from bandwidth issues (e.g. double requires two 32 bit registers), one should also check the control register issues, for instance the rounding/truncate bit found in many processors. Are such bits readable from the hardware registers before modifying them ?

Paul

Reply to
Paul Keinanen

In article , joshc writes

The answers depend largely on the target architecture and the specific compiler you are using.

--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills  Staffs  England     /\/\/\/\/
/\/\/ chris@phaedsys.org      www.phaedsys.org \/\/\
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
Reply to
Chris Hills

No, this is for an embedded MCU so it can be 8 bits. Incidentally both signed and unsigned char are integer types. However the standard says an int is not less than 16 bits so the chars are usually used.

That is legal for that compiler on that target. ISO C has little relevance here.

That's true. However there is no reason to use floats in this function. Which instruction set do you mean? The compiler or the hardware? Some MCU have FPU most compilers have FP libs that either dit it internally or use the HW WFU

The only relevant Intel part here is likely to be the 386

--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills  Staffs  England     /\/\/\/\/
/\/\/ chris@phaedsys.org      www.phaedsys.org \/\/\
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
Reply to
Chris Hills

Not if you want to call the language C.

You can call it "C-like" if you want, but for the last 17 years there has been an international standard defining the language called C and one of its most fundamental features is that the type 'int' can hold all values between 32767 and -32767 inclusive. A language with a type 'int' that can not hold at least this range can not be called C.

The C standard _does_ allow for various extensions, and makes the distinction between "freestanding" and "hosted" implementations (most embedded platforms will be the former, which for instance doesn't need to provide all or even any of the standard library) but reducing the range of _the_ basic type in this way is just not on.

mlp

Reply to
Mark L Pappin

You are being a pedant and pointless.

I know I have been on that panel for about a decade.

This is not correct. Type in should be at least 16 bits. Ints can legally be any size above 16 bits. There are systems with 64 bit ints.

Also an int can be signed or unsigned. An unsigned int does not hold negative values...

Of course both signed and unsigned char are integer types though a char does not have to be 8 bits. Hence the communications industry having the type "octet" which we have suggested as a standard type on more than one occasion.

As I said you are a pedant and pointless. The difference between theory and practice....

There are many dialects of C Most break the rules somewhere. By your definition there are only , I think, 6 C compilers world wide but they only could for a minute fraction of the "c" programming in the world. The vast majority do not fully implement ISO 9899:1999 but rather something between C95 and C99 *with extensions*

There are some compilers that are as compliant as most of the other c compilers out there that for reasons of hardware architecture and efficiency my have 8 bit ints. It is not common but I would not stop them being called C compilers for just that reason alone.

As I said all "c" compilers have extensions and restrictions on ISO9899:1999. I think only 6 fully implement the standard, all the common ones don't, ALL of the C compilers have extensions.

C is like English, rarely ever used in it's pure form.

--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills  Staffs  England     /\/\/\/\/
/\/\/ chris@phaedsys.org      www.phaedsys.org \/\/\
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
Reply to
Chris Hills

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.