[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] missing check after calling type_size in classify_x86

From: Michael Matz
Subject: Re: [Tinycc-devel] missing check after calling type_size in classify_x86_64_arg
Date: Mon, 24 Jun 2019 05:01:00 +0200 (CEST)
User-agent: Alpine 2.21 (LSU 202 2017-01-01)


On Sun, 23 Jun 2019, Pascal Cuoq wrote:

The patch definitely goes into the right direction, though it seem more verbose 
than necessary.  I'd just test for functions or incomplete types (via 
type_size), and then you have the opportunity to retain the more precise error 
message for the former, ala

 if (func)
   tcc_error ...
 else if (type_size < 0)
   tcc_error ...
 okay ...

I considered this but:
- the single generic error message seemed consistent with a compiler which 
advertises its small size and its speed, and which was accepting these programs 
until recently,
- but even producing a generic error message, functions would have to remain as 
a separate case because type_size returns 1 for them,
- and also type_size returns 1 for void, so that would have to be another 
separate case if we wish to reject arrays of void.

Sure, but to avoid two cases you added five, and encoded the knowledge about the ->c field in still another place. What I meant is adding two lines like so:

diff --git a/tccgen.c b/tccgen.c
index c65cf76..84e0953 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -4507,6 +4507,8 @@ static int post_type(CType *type, AttributeDef *ad, int 
storage, int td)
         post_type(type, ad, storage, 0);
         if (type->t == VT_FUNC)
             tcc_error("declaration of an array of functions");
+        else if ((type->t & VT_BTYPE) == VT_VOID || type_size(type, &l) < 0)
+            tcc_error("wrong!");
         t1 |= type->t & VT_VLA;

         if (t1 & VT_VLA) {

Two separate cases mean we have to compute type->t & VT_BTYPE. If we call type_size it will recompute it, and store an alignment that we do not require to a variable that we will have to declare. It doesn't look like it's shorter or faster.

It's definitely shorter, see above :) If you consider compilation speed you'd have to compare with an optimizing compiler anyway, and then none of the above unoptimality matters. Even without an optimizing compiler (say, with TCC itself) the code generated for the switch will be larger than the above.

Thing is: we have the type_size helper routine, so it should be used.

In the end of course some of this is merely a question about style and that's subjective. Personally I like things being done in fewer lines of code (without breaking the formating style) first, and then optimize hot spots. This place definitely isn't one.

If you think that this much code should be put in a function, that might be useful elsewhere, I can do that,

Only if multiple users are added; the block (even your version) isn't complex enough to warrant a separate function with just a single caller IMHO.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]