#2817 Error on const pointer to pointer typedef parameter

[Philipp Klaus Krause]

Compiling

typedef int* ip_t;

void f(ip_t *const ip);

with SDCC 3.8.0 RC1 (the problem is reported to have existed already in SDCC 3.6.0, but not 3.5.0) gives an error though it shouldn't:

error 226: no type specifier for 'f parameter 1'

This error makes compilation of current FreeRTOS with SDCC fail:
https://www.freertos.org/FreeRTOS_Support_Forum_Archive/July_2018/freertos_Types_assignment_in_FreeRTOS_788db98dj.html

Philipp

[Philipp Klaus Krause]

I tried to look into this, but can't figure it out. I guess I'm just not familiar enough with the type and front-end stuff. Someone else should look into this bug.

Apparently, at addDecl(), we get a type of "unknown type unknown* const fixed" for ip. This also happens when ip is just a normal variable, not a parameter. But there it isn't a problem somehow.

Philipp

[Michael]

i tried to debug that today. I can see that the error comes from yyparse(); that is called from the main funktin in SDCCmain.c. But i can't debug into it.

Can someone give me a hint how i can debug this funktion and how is the sourcecode passed to it?

Michael

[Philipp Klaus Krause]

By "I can't debug into it" I guess you meant you have probems when using a standard debugger, such a gdb.

Often, it helps to reduce the optimization level in the Makefile from the default O2 down to O1 or O0 to get better results from gdb.

yyparse is in SDCCy.c, which is generated from SDCC.y.

Philipp

[Michael]

the Problem was that within Makefile.in y.tab.c is moved to SDCCy.c i changed that:

--- Makefile.in.orig 2019-04-27 18:05:10.377589256 +0200
+++ Makefile.in 2019-04-27 18:05:23.821638867 +0200
@@ -128,8 +128,8 @@ SDCCy.h: SDCCy.c

SDCCy.c: SDCC.y
$(YACC) -d $<

• mv y.tab.c $*.c
• mv y.tab.h $*.h
• cp y.tab.c $*.c
• cp y.tab.h $*.h

that way i can debug it within eclipse.

when a const pointer is passed to a function. etype->select->s->noun is set to 0 while when a usual pointer is passed etype->select->s->noun is set to 1 (V_INT)
i can see this in function addDecl() under sym->type->funcAttrs->args->etype->select->s.noun
addDecl seems to be called in SDCC.y in line 212 -> addDecl($2,0,p);

Unfortunally I'm totaly unfamiliar with bison.
How are these etypes generated ?

[Thomas]

I stumbled across this yesterday when trying to compile FreeRTOS 10.2.1, which made me start looking into this.

SDCC.y defines YYDEBUG, so we can tell the parser to log debug info at runtime. This is done by setting the global yydebug variable to 1 at runtime using a debugger. The parser then gives us a trace of all the grammar rules that it applies.

I started by comparing the output of 3.5.0 to 3.6.0, because 3.6.0 seems to be the version that introduced this issue. I ran SDCC on the above code snippet with the modification that I use a char in the typedef instead of an int. The output of the two runs is attached: log-3.5.0.txt and log-3.6.0.txt. They are identical other than line numbers, rule numbers, and state numbers.

I then made the problem go away by slightly modifying the code snippet in 3 different ways and observed the debug output:

1. Remove the const from the parameter declaration. That's log-3.6.0-no-const.txt. The difference this makes to the parser is that it now considers the pointer in the parameter declaration to be simply an unqualified_pointer. With the const present, the pointer is an unqualified_pointer followed by a type_specifier_list. So, a different part of the pointer grammar rule fires.

2. Turn the typedef from a char * into a char. (Remember that I'm using a char, not an int.) That's log-3.6.0-no-pointer.txt. In the debug output, the only discernible difference is for the typedef. The function declaration is parsed identically. And yet, the problem goes away.

3. Use a char instead of the typedef-ed type in the parameter declaration. That's log-3.6.0-no-typedef.txt. In the debug output, the only discernible difference is that the parser encounters a SD_CHAR terminal at the point where it previously encountered a TYPE_NAME terminal. Everything else remains the same. And yet, the problem goes away.

I then started looking into how the type of the function parameter is represented internally in SDCC. To that end, a symbol has a linked list of sym_link. The symbol's type field points to the head of this list, the symbol's etype field points to its tail.

A sym_link can be either a DECLARATOR or a SPECIFIER. In the case at hand it was enough for me to understand that a level of pointer indirection is represented by a DECLARATOR node, whereas an atomic type such as char is represented by a SPECIFIER node. Let's write DECL for a DECLARATOR node and SPEC(x) for a SPECIFIER node with atomic type x, e.g., SPEC(char).

The type char * would be represented as a 2-node list [DECL, SPEC(char)]. Similarly, char ** would be the 3-node list [DECL, DECL, SPEC(char)].

On top of that, SPECIFIER nodes have flags that specify things like const or volatile. Let's indicate these modifiers as additional items in a SPEC(), for example like this: SPEC(char, const).

Now, when I reproduce the error, here's what the parser does to run into trouble:

1. It parses the const into a SPEC(0, const). Note that the atomic type (the "noun") is not set, hence the 0 instead of an atomic type. the SPECIFIER node is just used to represent the const.

2. It parses the * into a DECLARATOR node.

3. The pointer : unqualified_pointer type_specifier_list grammar rule copies the const information from the SPECIFIER node to the DECLARATOR node. In addition, it makes the DCL_TSPEC member of the DECLARATOR node point to the SPECIFIER node. (Why? We just copied the pertinent information off the SPECIFIER onto the DECLARATOR.)

4. It invokes addDecl() with p pointing pointing to the DECLARATOR. The function parameter's symbol's type and etype are NULL, so the DECLARATOR becomes the only element on the symbol's linked list.

5. However, something else happens. That's the magic under the "if the type is an unknown pointer [...]" comment that I don't understand. p is not a SPECIFIER and it does have its DCL_TSPEC member set. Moreover, the last list element of the symbol's linked list is not a SPECIFIER. (We just made the DECLARATOR the one and only element on this list.) So, we append a new SPECIFIER to the symbol's list. So it looks like this code ensures that we always have a SPECIFIER at the end of the symbol's list, so that we can copy information from the DCL_TSPEC to it. We do so by copying a few members of the DCL_TSPEC into that new SPECIFIER. (In doing so, don't we lose the const, BTW?) All in all, we now have SPEC(0) at the end of the symbol's list. (Is the way the code appends to the list legal, BTW? = associates to the right, but it's not a sequence point, is it? Couldn't sym->etype be assigned before sym->etype->next? Just wondering.)

6. All in all, the addDecl() results in the symbol's list looking like this: [DECL, SPEC(0)]. (Note the lost const - is this another bug?)

Now parsing continues and addDecl() is called again for the function parameter, this time with the expanded ip_t. In this invocation, p is [DECL, SPEC(char)], i.e., the expansion of ip_t. (Keep in mind that my typedef uses char, not int.)

What addDecl() ends up doing in this case is append the list given by p to the symbol's existing list. Given the symbol's list of [DECL, SPEC(0)], appending [DECL, SPEC(char)] results in [DECL, SPEC(0), DECL, SPEC(char)].

**And that's what causes the trouble. We now have two SPECIFIER nodes in the symbol's list. One - SPEC(0) - without a "noun" - which triggers the error.
**

Let's now see how the above 3 modifications I made make the issue go away:

• Removing the const basically removes the SPEC(0, const) from the equation. So, DCL_TSPEC is always NULL. We don't run into the magic done under the "if the type is an unknown pointer [...]" comment.

• Turning the typedef from a char * into a char triggers a different code path in the second call to addDecl(). Now p points to a single-node list that only contains a SPECIFIER. No DECLARATOR involved. This triggers the code path that invokes mergeSpec(). So the SPEC(char) (on the list p) and the last element on the symbol's list SPEC(0) are coalesced into a single SPECIFIER.

• Same for directly using a char in the parameter declaration instead of the typedef-ed type. The mergeSpec() code path is triggered.

We can validate the findings by asking SDCC to dump its AST. It seems to do so only for function definitions, not for function declarations. But luckily, the issue at hand also shows for function definitions. When I add an empty function body to the declaration to turn it into a definition, I get the following types for the function with 3.5.0 and 3.6.0:

(void fixed) args (unsigned-char generic* int generic* const fixed)

(void fixed) args (char generic* int generic* const fixed)

I think that the int occurs, because a missing noun is filled in as int, so SPEC(0) becomes SPEC(int).

(Also, it looks like Ubuntu's pre-built 3.5.0 uses unsigned chars by default.)

This is all I have so far. In particular, I don't understand the data structures of SDCC (and, thus, all the different cases in addDecl()) well enough to devise a fix.

But maybe there's somebody out there who knows more about SDCC's internal data structures?

Questions I have:

• What's the point of DCL_TSPEC()? We already copied, say, the const information off the SPECIFIER to the DECLARATOR. Why do we still need the DECLARATOR to point to the SPECIFIER via DCL_TSPEC()?

• What do the different cases in addDecl() cover? Why do we have these specific cases?

I'll also poke around a little more. Maybe I'll learn enough to come up with a proposal for a fix.

[Thomas]

The AST generated for the function definition by the nightly build of 20200127 yields an identical type for the parameter as do 3.5.0 and 3.6.0:

(void fixed) args (unsigned-char generic* int generic* const fixed)

So, current nightly seems to have the same issue. I'm now switching to the current version to explore fixing this issue.

TLDR: It looks like addDecl() has issues splicing the sym_link list of a typedef-ed type into a symbol's existing sym_link list. It fails to coalesce the two SPECIFIERs. (See the spurious int in the above type. The SPECIFIER is noun-less, which then gets forced to an int.)

[Thomas]

I have attached a first draft of a proposed fix. Here's what it does:

• It adds a new case to addDecl(), which covers sym->type ending in a specifier and p being a list of one or more declarators followed by a specifier.
• In this case, the specifier of sym->type and p's specifier get merged. p's declarators then get spliced before the merged specifier.

It would be great, if somebody more knowledgeable could share an opinion on the proposal.

[Philipp Klaus Krause]

Unfortunately, I am not really familiar with that part of SDCC. But at least I didn't notice any regressions with the patch applied.

I'd recommend to use // for the single-line comments.

Looking at the svn log, the part you marked as "XXX - to be understood" was a fix for this bug : https://sourceforge.net/p/sdcc/bugs/1253/ - it looks dangerous, but apparently it works, as newLink() uses calloc and most C implementations apparently do not really use the freedom to overwrite padding bytes when writing other struct members (see section 6.2.6.1p6 of N2455). Still I probably should reopen the bug report.

[Thomas]

Thanks, Philipp, for the review, the additional guidance, and the pointer to #1253.

However, I just ran into another issue when mixing typedef-ed types with native / built-in types and modifiers. This made me think that it might be good, if I studied the parser some more, to better understand how types are parsed and represented in the AST. Maybe there's a way for dealing with typedefs that's fundamentally more robust than what the parser currently does.

Right now, my gut feeling is that my proposed fix only treats a symptom of a more deeply rooted problem with handling typedefed user types.

The issue that I just encountered: When I change the initial example given in this ticket to

ip_t const *ip

then this parses as

int const *const *ip

I.e., the const wrongly gets applied to both parts of the ip_t typedef, the pointer indirection (declarator) as well as the int (specifier).

For now, I'd like to retract my proposed fix. I think that I might be making things worse by not addressing a more general underlying issue at hand.

[Thomas]

Alright. Next attempt. I now understand the handling of typedef a little better and I think that the original proposed fix is OK. I have attached a slightly more polished version of the original fix:

• Used // for single-line comments.
• Took care of to-do items.
• Removed the assert(), as I now understand that the spec is always last in the type chain.
• Removed a redundant NULL assignment.
• Fixed a misleading comment.

I'll open a separate ticket for the other issue I observed an propose a separate patch for that.

[Philipp Klaus Krause]

Thanks. Applied in [r11556].

[Thomas]

And here's a regression test for this issue.