Op 02-08-2025 om 16:22 schreef grischka:
On 02.08.2025 07:57, Herman ten Brugge via Tinycc-devel wrote:
I think I found the problem. The typedef type was used twice.
The first time ord2kw set the type to size 3.
Then ord3kw overflowed because the size was now 3.
(It also works when you remove the 2 extern lines)

So I unshared the type with attached patch.
I am not sure I did this at the rigth place ...

Hi Herman,

obviously that patch duplicates some code from elsewhere which means
that instead of one thing in the right place, there are good chances
that we now would have two times the thing but not in the right place.

Seems like related line was originally...
       while (1) { /* iterate thru each declaration */
           type = btype;
           ---> here
       http://repo.or.cz/w/tinycc.git/commitdiff/e034853b3

Alternatively a small change in patch_type() could work too:
       /* set array size if it was omitted in extern declaration */
-      sym->type.ref->c = type->ref->c;
+      sym->type.ref = type->ref;
where instead of changing the value we would just set the (already unshared)
reference.
This is a much better solutions. That is why I did not commit it.

Some other remarks I would have, unrelated:

arm-gen.c:
     int fc:
     ...
-    fc=-fc;
+    fc=-(unsigned)fc;
What's that ?!?
Silence a clang warning:

int main(void) {
  int fc = -2147483648;
  fc=-fc;
  fc=-(unsigned)fc;
}

clang -fsanitize=address,undefined,nullability tst.c; ./a.out results in:
b.c:3:6: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior b.c:5:6

So the first line gives error. The second line not.


arm-link.c
+                is_bl = ((unsigned) read32le(ptr)) >> 24 == 0xeb;
Why do you cast "uint32_t read32le()" to unsigned?
+                write16le(ptr, (uint16_t) ((hi & 0xf800) |
+                               (s << 10) | imm10));
why do you cast the 2nd arg of write16le(unsigned char *p, uint16_t x)?
I should have removed the casts. Sorry.
These cast here should also be removed:
                hi = (uint16_t) read16le(ptr);
                lo = (uint16_t) read16le(ptr+2);


In general, a cast means that you want it to behave differently
compared to how it would behave normally, without it. Otherwise
why add something if it makes no difference.

tccelf.c:
+    if (s->data == NULL) continue; /* bss */
single line different coding style ... doesn't look good ;)
This should be fixed.
+ if (nb_syms == 0) goto skip;
why not skip all of the rest, including
     old_to_new_syms = tcc_mallocz(nb_syms * sizeof(int));
above?  Or just return 'invalid object file', for that matter?
Clang reported a warning for empty file '/usr/lib/riscv64-linux-gnu/crti.o'
on riscv. I can remove this code.

tccpp.c:
typedef struct tal_header_t {
-    unsigned  size;
+    ALIGNED(PTR_SIZE) unsigned size;
+    unsigned adj_size = (size + PTR_SIZE - 1) & -PTR_SIZE;

'PTR_SIZE' is the size of a pointer in the compiled program.
Size of tcc's own pointers is 'sizeof (void*)' for example.

__declspec(align(sizeof (void*))) will not work, probably.

I agree this should be changed to sizeof(void *) to allow cross compiling.

These changes are needed to keep the returned pointer aligned to sizeof(void *). If you run with tcc compiled with 'clang -fsanitize=address,undefined,nullability'
you see that all pointers are aligned to int instead of sizeof(void *).

    Herman





_______________________________________________
Tinycc-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Reply via email to