`characters of assembly` count in disassembler output is incorrect in `printf` branch about selfie HOT 10 CLOSED

@Son-Typ Could you please have a look and see if you can fix this? Thanks!

from selfie.

A small test case would be useful. Is the number_of_written_characters wrong? I tried compiling using ./selfie -c selfie.c -o selfie.m and ./selfie -l selfie.m -m 42 -c selfie.c -o selfie2.m and those are my results:

For main:

• bootlevel 0: 185520 bytes generated with 42906 instructions and 13896 bytes of data
• bootlevel 1: 282792 bytes generated with 42906 instructions and 13896 bytes of data

For printf:

• bootlevel 0: 184936 bytes generated with 42580 instructions and 14616 bytes of data
• bootlevel 1: 184936 bytes generated with 42580 instructions and 14616 bytes of data

The printf branch generates the same number of bytes for both bootlevels which I would expect to make more sense.

What are the expected results?

from selfie.

@Son-Typ I can't reproduce the issue you are seeing for main. I am on commit 0de776c.

What I actually meant is the character count, not the byte count. But we may have to check both.

Try: ./selfie -c selfie.c -S selfie.s

Should show more than a million characters in the assembly file but does not on the printf branch. I am on commit 0cbbb30.

from selfie.

I was on the same commits while conducting both tests.
Regarding the problem: The reason is actually quite simple. At bootlevel 0 we use the bootstrapping compiler's dprintf to write into the file, while we use our own dprintf at the other bootlevels. Clang's/GCC's dprintf does not count the number of written characters in number_of_written_characters while the printf implementation in main and our dprintf implementation do count. We probably want to count the written characters inside direct_output as dprintf returns the number of written characters. In this case we would have to stop counting inside put_character, which is a nasty side effect to begin with.
I tried ./selfie -l selfie.m -c selfie.c -S selfie.s, i.e., disassembling at bootlevel 1, which resulted in the expected "selfie.m: 1901357 characters of assembly [...]".

from selfie.

We will work on it asap but since its family holidays it might take a little while. Will post here when we're done

from selfie.

Sure, that's fine. Thanks a lot for looking into this. I hope there is a clean solution.

from selfie.

We have suggested a solution to the problem (PR created already). It should be mentioned that this solution (counting using the return value of dprintf) is not thread-safe, however in order to make it thread-safe, due to the limitations of C* we would have to use local variables in every method that calls put_character and return those and continue to count. This would be tedious to implement as some of these methods already have return values that are used in other parts of the code (for example print format returning the index of the format string). Therefore this is our best suggestion.

from selfie.

@MartinGroesbacher I am actually not sure why you guys are concerned about thread safety. Are you worried about the concurrency assignments in the SE class?

from selfie.

We just wanted to point this out in case you eventually may want to use printf in a future concurrent project that uses selfie as a library. If this is ever the case, then one would have to adapt the implementation.

from selfie.

Got it! Thanks.

from selfie.