Re: [Tinycc-devel] MAJOR bug: tcc doesn't detect duplicate cases in switch statements

[Michael Matz]

Hi,

On Sat, 12 Mar 2016, Daniel Glöckner wrote:

The issue isn't just the lack of diagnostic - what kind of erroneous 
code is being generated in this case?


Let's see what the final C11 draft (N1570) says about this issue:

Section 6.8.4.2 paragraph 3:
The expression of each case label shall be an integer constant
expression and no two of the case constant expressions in the same
switch statement shall have the same value after conversion.

Section 4 paragraph 2:
If a "shall" or "shall not" requirement that appears outside of a
constraint or runtime-constraint is violated, the behavior is undefined.

There you read it, C11 allows us to generate whatever we want.


This is true.  But you missed the detail that all constraint violations 
require a diagnostic (5.1.1.3), and the above citation is a constraint on 
switches.  So, yes, TCC is not conforming :)



Ciao,
Michael.___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] MAJOR bug: tcc doesn't detect duplicate cases in switch statements

[Michael Matz]

Hi,

On Sat, 12 Mar 2016, arn...@skeeve.com wrote:

First, the bug can't be as MAJOR as the subject wants to make us 
believe.  After all it wasn't noticed since the last $MANY years.


The bug caused me to push bad code to gawk's repo.


I assumed something like that.  It still doesn't make the bug any more 
major than many of the other bugs that tcc has.


It's only luck that it wasn't noticed, but a bug is a bug. Geting the 
right answer a little more slowly is always better than getting the 
wrong answer super fast.


No, I don't agree with you that a missed diagnostics is in the same class 
like wrong-code bugs.  Especially if it's costly to implement a diagnostic 
it's quite reasonable to not implement it in a compiler that's supposed to 
be quick (at least as long as there's other missing diagnostics).  Unlike 
wrong-code bugs for instance (well, even there the world is not always 
black and white, only mostly so).


The issue isn't just the lack of diagnostic - what kind of erroneous 
code is being generated in this case?


Any we want.  As the behaviour of the input was undefined the generated 
code can't be erroneous.  In practice I can tell you: tcc translates a 
switch literally into a sequence of if-else tests, so the code with 
overlapping cases is simply:

  if (a == 1) ...
  else if (a == 1) ...

So, first one matching wins.

TCC is valuable for being both fast and correct.  Lose the correctness 
and you lose ALL the value.


Given those absolute terms, I don't know of any compiler that's correct.
TCC certainly isn't.  TCC isn't bad either in correctness, it's actually 
quite good, but not 100%.  I hope you still continue to find TCC valuable, 
your use of it for serious development is much welcome :)


And, of course, I'm not dead-set against giving a diagnostic.  An 
implementation that's not making eyes bleed and is fast would be nice. 
E.g. remember cases in an array of ranges while parsing, qsort them after 
switch is finished, generated diagnostics on overlap.



Ciao,
Michael.

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] tccgen.c: off by one in flexible array members

[Henry Kroll]

Thanks for looking into this. What made it seem like off-by one is it
happens whenever s[9] is declared as a multiple of [n*8+1].

|<---struct>|<---char s[0] gets overwritten
[b][u][g][s][ ][ ][ ][ ][n][o][ ][ ][ ][ ][ ][ ][ ]

It could be an internal alignment issue with s[9] though, probably
malloc() returning memory aligned in multiples of 8 bytes. Didn't think
of that.

Any number, say 5*8+1 also triggers the bug:

#include 
struct w {
char *data; char mem[];
};
int main (void) {
char s[5*8+1]="no"; struct w q = {"bugs"};
printf ("tcc has %s %s\n", s, q.data);
return !s[0];
}

The flexible array member, mem[], is not being initialized.
Only q.data is.

> One isn't allowed to
> initialize 
> flex array members, only via malloc and assignments.  (But the GCC 
> extension is probably quite prevalent as GCC doesn't even warn about
> it in 
> conformant mode.
> 
> 
> Ciao,
> Michael.
> 
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] tccgen.c: off by one in flexible array members

[Henry Kroll]

>This still isn't quite right.  The thing is, type->ref.c is -1 for
> an 
> unused flex array, so with your example ("char mem[]") your change 
> basically boils down to "size += -1 * 1 + 1" (i.e. +=0).  But with a 
> different type (say int) it's now "size += -1 * 4 + 1" (i.e.
> +=3).  So 
> this example still fails the same way:

Good catch and patch. Didn't realize type->ref.c could be -1. Seems to
be fixed now.

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] MAJOR bug: tcc doesn't detect duplicate cases in switch statements

[Daniel Glöckner]

On Sat, Mar 12, 2016 at 10:58:42AM -0700, arn...@skeeve.com wrote:
> The bug caused me to push bad code to gawk's repo. It's only luck that it
> wasn't noticed, but a bug is a bug.

So you have personal feelings towards that bug.

I'd never use TinyCC for serious development.
There are simply not enough testers.


> The issue isn't just the lack of diagnostic - what kind of erroneous
> code is being generated in this case?

Let's see what the final C11 draft (N1570) says about this issue:

Section 6.8.4.2 paragraph 3:
The expression of each case label shall be an integer constant
expression and no two of the case constant expressions in the same
switch statement shall have the same value after conversion.

Section 4 paragraph 2:
If a "shall" or "shall not" requirement that appears outside of a
constraint or runtime-constraint is violated, the behavior is undefined.

There you read it, C11 allows us to generate whatever we want.


Regarding the cost of your solution, a tree structure might be faster
and could also be implemented in a way that handles value ranges.

Best regards,

  Daniel

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] MAJOR bug: tcc doesn't detect duplicate cases in switch statements

[arnold]

Michael Matz  wrote:

> First, the bug can't be as MAJOR as 
> the subject wants to make us believe.  After all it wasn't noticed since 
> the last $MANY years.

The bug caused me to push bad code to gawk's repo. It's only luck that it
wasn't noticed, but a bug is a bug. Geting the right answer a little
more slowly is always better than getting the wrong answer super fast.

The issue isn't just the lack of diagnostic - what kind of erroneous
code is being generated in this case?

TCC is valuable for being both fast and correct.  Lose the correctness
and you lose ALL the value.

My two cents worth.

Arnold

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel