#2440 [stm8] - compiler emits invalid assembly for some function pointer operations

[Peter Dons Tychsen]

Hello SDCC,

Thanks for your superb compiler :-).

The following code generates bad assembly:

#include <string.h>
#include <stdint.h>

typedef void test_func_t(void);

typedef struct
{
  uint8_t var;
}test_struct1_t;

static test_struct1_t test_stuct1 = {{0}};
static test_func_t test_funcs[1];

void test_func(uint8_t port, test_func_t func_ptr);
void test_func(uint8_t port, test_func_t func_ptr)
{
  test_funcs[0] = func_ptr;
}

Options used: --fverbose-asm --opt-code-size -mstm8

The bad output looks like this:

;   ../platform_arm/common/src/i2c.c: 15: void test_func(uint8_t port, test_func_t func_ptr)
; genLabel
;   -----------------------------------------
;    function test_func
;   -----------------------------------------
;   Register assignment is optimal.
;   Stack space usage: 0 bytes.
_test_func:
;   ../platform_arm/common/src/i2c.c: 17: test_funcs[0] = func_ptr;
; genAddrOf
    ldw x, #_test_funcs+0
; genPointerSet
    ldw y, #
    ldw (x), y
; genLabel
; peephole j30 removed unused label 00101$.
; genEndFunction

Notice the bad code at "genPointerSet" (there is no value after #).

Generated error is:

Error: missing or improper operators, terminators, or delimiters

Thanks,

/pedro

[Philipp Klaus Krause]

Definitely looks like a bug. I opened bug report [#2438] for it.

Philipp

[Philipp Klaus Krause]

Forgot that I can move items. So the better approach is to move this to bugs.

Philipp

[Philipp Klaus Krause]

Ticket moved from /p/sdcc/support-requests/98/

Can't be converted:

• _milestone:

[Philipp Klaus Krause]

Shorter code for reproducing the issue.

Philipp

[Ben Shi]

I suggest treating it as a front end bug. The following code should be rejected,
typedef void test_func_t(void);
and it should be
typedef void ( * test_func_t) (void);

gcc-4.9 have an error
test.c:3:20: error: declaration of ‘test_funcs’ as array of functions
static test_func_t test_funcs[1];
^

clang's error:
test.c:3:30: error: 'test_funcs' declared as array of functions of type 'test_func_t' (aka 'void (void)')
static test_func_t test_funcs[1];
^

[Maarten Brock]

Unless someone reads in a C spec that this should be allowed, I agree to reject this in the front end.

[Peter Dons Tychsen]

Ben, you are right.

"typedef void (*test_func_t)(void);"

That is the correct way of typedefing a function. The one i sent was crappy.
It came from some of my other sources, were some other compiler incorrectly accepted that.

So yes, SDCC should probably throw an error on this incorrect format.

[Peter Dons Tychsen]

Hello again.

The reasoning in GCC and clang is sound.

typedef void test_func_t(void);
static test_func_t test_funcs[1];

This makes no sense, as it tries to make an array of functions, and not an array of pointers to functions.

The following is correct:

typedef void (*test_func_t)(void);
static test_func_t test_funcs[1];

or

typedef void test_func_t(void);
static test_func_t *test_funcs[1];

Thanks,

/pedro

[Peter Dons Tychsen]

So this bug, like my other bug is really a feature request so that the compiler will be more verbal.

In other words it should have told me i was doing something very wrong (like Ben proposed).

Once again i was fooled by code that another compiler accepted, but it was actually bad code which the other compiler should have rejected. My bad. I will try to be more thorough going forward :-).

[Philipp Klaus Krause]

Changing the category, since we now know we want to emit an error in the front-end.

Philipp

[Ben Shi]

Fixed in reversion #9417.

With the above test code, the following errors are generated,
test.c:4: error 220: type 'test_func_t' is function
test.c:4: error 220: type 'test_func_t' is function
test.c:4: error 220: type 'test_func_t' is function
test.c:17: warning 85: in function test_func unreferenced function argument : 'port'